diff mbox series

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

Message ID 20200518131801.465-4-m.szyprowski@samsung.com
State New
Headers show
Series ARM: arbitrary virtual-physical mappings for RPi4 XHCI support | expand

Commit Message

Marek Szyprowski May 18, 2020, 1:18 p.m. UTC
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(+)

Comments

Simon Glass May 18, 2020, 10:38 p.m. UTC | #1
Hi Marek,

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?

> +
>  /* Devices */
>  /* GPIO */
>  #define CONFIG_BCM2835_GPIO
> --
> 1.9.1
>

Regards,
Simon
Marek Szyprowski May 19, 2020, noon UTC | #2
Hi Simon,

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.

Best regards
Simon Glass May 19, 2020, 4:47 p.m. UTC | #3
Hi Marek,

On Tue, 19 May 2020 at 06:00, Marek Szyprowski <m.szyprowski at samsung.com> wrote:
>
> Hi Simon,
>
> 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.

Regards,
SImon
Marek Szyprowski May 22, 2020, 5:56 a.m. UTC | #4
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
Simon Glass May 22, 2020, 1:42 p.m. UTC | #5
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
Marek Szyprowski May 25, 2020, 10:48 a.m. UTC | #6
Hi Simon,

On 22.05.2020 15:42, Simon Glass wrote:
> On Thu, 21 May 2020 at 23:56, Marek Szyprowski <m.szyprowski at samsung.com> wrote:
>> 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.
In this case (CONFIG_ADDR_MAP) the values provided by the board's 
config.h (CONFIG_SYS_NUM_ADDR_MAP) only allows the code to use a simple 
static array instead of allocating things dynamically. It has no 
relation to the drivers nor device-tree.

Best regards
Simon Glass May 25, 2020, 2:57 p.m. UTC | #7
Hi Marek,

On Mon, 25 May 2020 at 04:48, Marek Szyprowski <m.szyprowski at samsung.com> wrote:
>
> Hi Simon,
>
> On 22.05.2020 15:42, Simon Glass wrote:
> > On Thu, 21 May 2020 at 23:56, Marek Szyprowski <m.szyprowski at samsung.com> wrote:
> >> 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.
> In this case (CONFIG_ADDR_MAP) the values provided by the board's
> config.h (CONFIG_SYS_NUM_ADDR_MAP) only allows the code to use a simple
> static array instead of allocating things dynamically. It has no
> relation to the drivers nor device-tree.

We are working to remove the board config.h files and move everything
to Kconfig.

If there really is no need for a driver here, then we could perhaps
use a weak function for the interface? (Tom, note :-)

Regards,
Simon
diff mbox series

Patch

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
+
 /* Devices */
 /* GPIO */
 #define CONFIG_BCM2835_GPIO