From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marek Szyprowski Date: Tue, 5 May 2020 16:10:52 +0200 Subject: [PATCH v2 05/10] rpi4: add a mapping for the PCIe XHCI controller MMIO registers (ARM 64bit) In-Reply-To: <6c5feb04-a0ba-e746-cc87-6604df6ee810@suse.com> References: <20200504124523.23484-1-s.nawrocki@samsung.com> <20200504124523.23484-6-s.nawrocki@samsung.com> <6c5feb04-a0ba-e746-cc87-6604df6ee810@suse.com> Message-ID: <6f6482f9-e804-89bf-c3b8-ba04eda74a14@samsung.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Matthias, On 05.05.2020 16:00, Matthias Brugger wrote: > On 04/05/2020 14:45, Sylwester Nawrocki wrote: >> From: Marek Szyprowski >> >> 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. >> >> Signed-off-by: Marek Szyprowski >> Signed-off-by: Sylwester Nawrocki >> Reviewed-by: Nicolas Saenz Julienne >> --- >> Changes since v1: >> - none. >> --- >> arch/arm/mach-bcm283x/init.c | 18 +++++++++++++++--- >> 1 file changed, 15 insertions(+), 3 deletions(-) >> >> diff --git a/arch/arm/mach-bcm283x/init.c b/arch/arm/mach-bcm283x/init.c >> index 4295356..6a748da 100644 >> --- a/arch/arm/mach-bcm283x/init.c >> +++ b/arch/arm/mach-bcm283x/init.c >> @@ -11,10 +11,15 @@ >> #include >> #include >> >> +#define BCM2711_RPI4_PCIE_XHCI_MMIO_PHYS 0x600000000UL >> +#define BCM2711_RPI4_PCIE_XHCI_MMIO_SIZE 0x800000UL >> + >> #ifdef CONFIG_ARM64 >> #include >> >> -static struct mm_region bcm283x_mem_map[] = { >> +#define MAX_MAP_MAX_ENTRIES (4) > What stands the second 'MAX' for? a simple copy/paste issue. I will fix it. >> + >> +static struct mm_region bcm283x_mem_map[MAX_MAP_MAX_ENTRIES] = { >> { >> .virt = 0x00000000UL, >> .phys = 0x00000000UL, >> @@ -34,7 +39,7 @@ static struct mm_region bcm283x_mem_map[] = { >> } >> }; >> >> -static struct mm_region bcm2711_mem_map[] = { >> +static struct mm_region bcm2711_mem_map[MAX_MAP_MAX_ENTRIES] = { >> { >> .virt = 0x00000000UL, >> .phys = 0x00000000UL, >> @@ -49,6 +54,13 @@ static struct mm_region bcm2711_mem_map[] = { >> PTE_BLOCK_NON_SHARE | >> PTE_BLOCK_PXN | PTE_BLOCK_UXN >> }, { > I'd prefer a comment instead of using the BCM2711_RPI4_PCIE_XHCI_MMIO_* defines. Those defines are also used in ARM 32bit code. >> + .virt = BCM2711_RPI4_PCIE_XHCI_MMIO_PHYS> + .phys = BCM2711_RPI4_PCIE_XHCI_MMIO_PHYS, >> + .size = BCM2711_RPI4_PCIE_XHCI_MMIO_SIZE, >> + .attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) | >> + PTE_BLOCK_NON_SHARE | >> + PTE_BLOCK_PXN | PTE_BLOCK_UXN >> + }, { >> /* List terminator */ >> 0, >> } >> @@ -71,7 +83,7 @@ static void _rpi_update_mem_map(struct mm_region *pd) >> { >> int i; >> >> - for (i = 0; i < 2; i++) { >> + for (i = 0; i < MAX_MAP_MAX_ENTRIES; i++) { > Variable mem_map points to bcm283x_mem_map which only holds two mm_region's > (plus list terminator). So we have an overflow here. Nope, I've changed the bcm283x_mem_map to be large enough, see the above diff. > I think we should just > define bcm2711_mem_map and bcm283x_mem_map with a fixed array size of 4 (see > comment on the define naming above). That's exactly what I did. Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland