soc: arm: xilinx_zynq7000: add MMU PTEs for all AXI GPIO instances

[ibirnbaum]

Add the MMU page table entries for all instances of the Xilinx AXI GPIO controller IP core. Other than any Zynq-7000 peripheral supported so far, the existance of 1..n instances of the IP core within the FPGA part of the SoC is optional. Therefore, other than addressing instances of supported peripherals using their DT nodelabel as has always been the case so far, the data for the MMU page table is added using the DT_FOREACH_STATUS_OKAY macro.

Signed-off-by: Immo Birnbaum [email protected]

[henrikbrixandersen]

The driver should be refactored to use DEVICE_MMIO_ROM_INIT() instead.

[henrikbrixandersen]

See #46477 for further discussions related to this.

[carlocaione]

Yeah, as @henrikbrixandersen said this should be really fixed at driver level by adding support for the MMIO APIs. The discussion in #46477 is really for ARM64 but the point is the same here.

[ibirnbaum]

One point already mentioned in the discussion in #46477 is that the driver for which the [0..n] MMU entries are being added here is for a peripheral which exists both in the Zynq-7000, where the memory has to be mapped via the MMU, and in the UltraScale, which has the MPU. At the time being, all of the UltraScale's peripherals and the On-Chip Memory are covered by a single 128 MB R/W/nocache MPU region if the MPU is active, so there's nothing to do with regards to the UltraScale. If the driver was to be modified to use MMIO rather than having the MMU set up based on code outside of the driver, that would be SoC family-specific.

This issue basically affects all supported Xilinx peripherals: the AXI GPIO, the PS GPIO, the UART, the GEM, the CAN controller for which a driver has been submitted for review, and some others still in my pipeline (I2C, SPI, etc.).

[carlocaione]

If the driver was to be modified to use MMIO rather than having the MMU set up based on code outside of the driver, that would be SoC family-specific.

Why? The MMIO API is MMU mapping the addresses only when an MMU is present, see

zephyr/include/zephyr/sys/device_mmio.h

Lines 36 to 38 in 1578dc6

	#if defined(CONFIG_MMU) || defined(CONFIG_PCIE)
	#define DEVICE_MMIO_IS_IN_RAM
	#endif

In case of MPU, the API is simply falling back to access the usual address without mapping anything:

zephyr/include/zephyr/sys/device_mmio.h

Lines 261 to 269 in 1578dc6

	#ifdef DEVICE_MMIO_IS_IN_RAM
	#define DEVICE_MMIO_MAP(dev, flags) \
	device_map(DEVICE_MMIO_RAM_PTR(dev), \
	DEVICE_MMIO_ROM_PTR(dev)->phys_addr, \
	DEVICE_MMIO_ROM_PTR(dev)->size, \
	(flags))
	#else
	#define DEVICE_MMIO_MAP(dev, flags) do { } while (0)
	#endif

This issue basically affects all supported Xilinx peripherals: the AXI GPIO, the PS GPIO, the UART, the GEM, the CAN controller for which a driver has been submitted for review, and some others still in my pipeline (I2C, SPI, etc.).

Yeah this sucks. I'm trying to push for all the new drivers to support MMIO API as soon as they are introduced but I cannot push people to fix all the existing drivers, especially when there are a lot of them. So, I can approve this as workaround for an existing driver but if you are going to introduce new drivers these must support the MMIO API out-of-the-box.

[ibirnbaum]

This issue basically affects all supported Xilinx peripherals: the AXI GPIO, the PS GPIO, the UART, the GEM, the CAN controller for which a driver has been submitted for review, and some others still in my pipeline (I2C, SPI, etc.).

Yeah this sucks. I'm trying to push for all the new drivers to support MMIO API as soon as they are introduced but I cannot push people to fix all the existing drivers, especially when there are a lot of them. So, I can approve this as workaround for an existing driver but if you are going to introduce new drivers these must support the MMIO API out-of-the-box.

I don't mind converting all the existing Xilinx drivers, that's something that shouldn't take too long, most of them should be fairly simple to switch over, and if memory areas like the SLCR and the MPCore registers can still be mapped via the mmu_regions table in future releases and DMA can still be set up as it currently is being set up in the GEM driver in future releases, there shouldn't be any major conflicts. What I was trying to point out was that if the memory mapping API wasn't compatible with non-MMU systems, some sort of SoC-specific adaptation would have been required for all the Xilinx drivers as the Zynq and the UltraScale support all those peripherals equally. As the physical address is returned by the API in the non-MMU scenario, that assumed issue doesn't exist. So I guess I'll get to it.

[ibirnbaum]

@henrikbrixandersen @carlocaione I've had a go at refactoring the Xilinx AXI GPIO and PS GPIO drivers to use the MMIO API instead, but at the time being, it's just not possible to let GPIO drivers use the MMIO API due to conflicting demands - I opened #48243 to address the issue, as I don't think that having a hand full of particular device drivers or one class of device drivers use a workaround all the other drivers don't need is a good idea. Therefore, could we for now go ahead as suggested herein for now, until a clean solution is found? I'd really like to add support for the Zedboard, where unfortunately, most of the useful GPIOs (slider switches, buttons, LEDs) can only be connected to instances of the AXI GPIO IP core.

stale

[carlocaione]

@ibirnbaum I'm fine to put aside the MMIO story but please fix the CI to be all green so I can take again another look.

[ibirnbaum]

@carlocaione I'll force-push in order to run the CI again. During the last run, the completely unrelated test case net.socket.tcp failed on the unrelated target mps2_an385. Looks to me like a glitch of the CI itself?

[ibirnbaum]

@carlocaione CI passed this time as intended.

[henrikbrixandersen]

I am okay with this work-around for now, but we really need to get this working for all kinds of peripherals (in-tree as well as out-of-tree).

If we re-organise the devicetree to closer match that of the Linux kernel (where all memory-mapped peripherals of the Zynq7000 are below an amba node) we should be able to iterate all these child devices and add corresponding MMU regions.