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

[Simon Glass]

Hi Marek,

On Thu, 21 May 2020 at 23:56, Marek Szyprowski <m.szyprowski at samsung.com> wrote:
>
> 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.

OK, well see how you go. When I see CONFIG options that specify values
I often think there should be a driver and device-tree node. We can
see how things go in the future as to whether we need a driver.

Regards,
Simon