On 04/05/2020 14:45, Sylwester Nawrocki wrote:
> From: Marek Szyprowski <m.szyprow...@samsung.com>
>
> 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 <m.szyprow...@samsung.com>
> Signed-off-by: Sylwester Nawrocki <s.nawro...@samsung.com>
> Reviewed-by: Nicolas Saenz Julienne <nsaenzjulie...@suse.de>
> ---
> 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 <dm/device.h>
> #include <fdt_support.h>
>
> +#define BCM2711_RPI4_PCIE_XHCI_MMIO_PHYS 0x600000000UL
> +#define BCM2711_RPI4_PCIE_XHCI_MMIO_SIZE 0x800000UL
> +
> #ifdef CONFIG_ARM64
> #include <asm/armv8/mmu.h>
>
> -static struct mm_region bcm283x_mem_map[] = {
> +#define MAX_MAP_MAX_ENTRIES (4)
What stands the second 'MAX' for?
> +
> +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.
> + .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. 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).
Regards,
Matthias
> mem_map[i].virt = pd[i].virt;
> mem_map[i].phys = pd[i].phys;
> mem_map[i].size = pd[i].size;
>
