[RFC PATCH v3 3/3] rpi4: add a mapping for the PCIe XHCI controller MMIO registers (ARM 32bit)

[Marek Szyprowski]

Hi Simon,

On 19.05.2020 18:47, Simon Glass wrote:
> On Tue, 19 May 2020 at 06:00, Marek Szyprowski <m.szyprowski at samsung.com> wrote:
>> On 19.05.2020 00:38, Simon Glass wrote:
>>> On Mon, 18 May 2020 at 07:18, Marek Szyprowski <m.szyprowski at samsung.com> wrote:
>>>> Create a non-cacheable mapping for the 0x600000000 physical memory region,
>>>> where MMIO registers for the PCIe XHCI controller are instantiated by the
>>>> PCIe bridge. Due to 32bit limit in the CPU virtual address space in ARM
>>>> 32bit mode, this region is mapped at 0xff800000 CPU virtual address.
>>>>
>>>> Signed-off-by: Marek Szyprowski <m.szyprowski at samsung.com>
>>>> ---
>>>>    arch/arm/mach-bcm283x/Kconfig             |  1 +
>>>>    arch/arm/mach-bcm283x/include/mach/base.h |  8 ++++++++
>>>>    arch/arm/mach-bcm283x/init.c              | 20 ++++++++++++++++++++
>>>>    include/configs/rpi.h                     |  7 +++++++
>>>>    4 files changed, 36 insertions(+)
>>>>
>>>> diff --git a/arch/arm/mach-bcm283x/Kconfig b/arch/arm/mach-bcm283x/Kconfig
>>>> index 00419bf..bcb7f1d 100644
>>>> --- a/arch/arm/mach-bcm283x/Kconfig
>>>> +++ b/arch/arm/mach-bcm283x/Kconfig
>>>> @@ -36,6 +36,7 @@ config BCM2711_32B
>>>>           select BCM2711
>>>>           select ARMV7_LPAE
>>>>           select CPU_V7A
>>>> +       select PHYS_64BIT
>>>>
>>>>    config BCM2711_64B
>>>>           bool "Broadcom BCM2711 SoC 64-bit support"
>>>> diff --git a/arch/arm/mach-bcm283x/include/mach/base.h b/arch/arm/mach-bcm283x/include/mach/base.h
>>>> index c4ae398..4ccaf69 100644
>>>> --- a/arch/arm/mach-bcm283x/include/mach/base.h
>>>> +++ b/arch/arm/mach-bcm283x/include/mach/base.h
>>>> @@ -8,4 +8,12 @@
>>>>
>>>>    extern unsigned long rpi_bcm283x_base;
>>>>
>>>> +#ifdef CONFIG_ARMV7_LPAE
>>>> +#ifdef CONFIG_TARGET_RPI_4_32B
>>>> +#include <addr_map.h>
>>>> +#define phys_to_virt addrmap_phys_to_virt
>>>> +#define virt_to_phys addrmap_virt_to_phys
>>>> +#endif
>>>> +#endif
>>>> +
>>>>    #endif
>>>> diff --git a/arch/arm/mach-bcm283x/init.c b/arch/arm/mach-bcm283x/init.c
>>>> index 9f5bca3..008b312 100644
>>>> --- a/arch/arm/mach-bcm283x/init.c
>>>> +++ b/arch/arm/mach-bcm283x/init.c
>>>> @@ -145,6 +145,26 @@ int mach_cpu_init(void)
>>>>    }
>>>>
>>>>    #ifdef CONFIG_ARMV7_LPAE
>>>> +#ifdef CONFIG_TARGET_RPI_4_32B
>>>> +#define BCM2711_RPI4_PCIE_XHCI_MMIO_VIRT       0xff800000UL
>>>> +#include <addr_map.h>
>>>> +
>>>> +void init_addr_map(void)
>>>> +{
>>>> +       mmu_set_region_dcache_behaviour_phys(BCM2711_RPI4_PCIE_XHCI_MMIO_VIRT,
>>>> +                                            BCM2711_RPI4_PCIE_XHCI_MMIO_PHYS,
>>>> +                                            BCM2711_RPI4_PCIE_XHCI_MMIO_SIZE,
>>>> +                                            DCACHE_OFF);
>>>> +
>>>> +       /* identity mapping for 0..BCM2711_RPI4_PCIE_XHCI_MMIO_VIRT */
>>>> +       addrmap_set_entry(0, 0, BCM2711_RPI4_PCIE_XHCI_MMIO_VIRT, 0);
>>>> +       /* XHCI MMIO on PCIe at BCM2711_RPI4_PCIE_XHCI_MMIO_VIRT */
>>>> +       addrmap_set_entry(BCM2711_RPI4_PCIE_XHCI_MMIO_VIRT,
>>>> +                         BCM2711_RPI4_PCIE_XHCI_MMIO_PHYS,
>>>> +                         BCM2711_RPI4_PCIE_XHCI_MMIO_SIZE, 1);
>>>> +}
>>>> +#endif
>>>> +
>>>>    void enable_caches(void)
>>>>    {
>>>>           dcache_enable();
>>>> diff --git a/include/configs/rpi.h b/include/configs/rpi.h
>>>> index b53a4b6..296e8ee 100644
>>>> --- a/include/configs/rpi.h
>>>> +++ b/include/configs/rpi.h
>>>> @@ -63,6 +63,13 @@
>>>>    #define CONFIG_SYS_BOOTM_LEN           SZ_64M
>>>>    #endif
>>>>
>>>> +#ifdef CONFIG_ARMV7_LPAE
>>>> +#ifdef CONFIG_TARGET_RPI_4_32B
>>>> +#define CONFIG_ADDR_MAP 1
>>>> +#define CONFIG_SYS_NUM_ADDR_MAP 2
>>>> +#endif
>>>> +#endif
>>> We should be removing things from the config files. Can you move this
>>> to devicetree or Kconfig?
>> I can move them to Kconfig, no problem. However I would like to get some
>> comments if the approach I presented in this patchset is fine.
> Yes, no problem.
>
> I suspect we may need to expand the DMA drivers, perhaps, or some
> other way to map memory on a per-device basis using driver model.

I'm not sure that we really need such a complex solution. Usually the 
board (or even SoC), if ever, needs one or two such non-identity 
mappings, which can be easily created by the respective init code. The 
PowerPC case mentioned here looks a bit different, because it simply 
copies the mappings already configured in the CPU registers by the 
earlier firmware. Anyway, I don't think that we would ever need to 
manage physical/virtual mapping dynamically in the u-boot. All that 
cover current (and most future?) cases would be a simple function to map 
an arbitrary physical address at the predefined virtual one.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland