On 26.05.21 17:59, Pali Rohár wrote:
Current version of this function uses a lot of incorrect assumptions about
the `ranges` DT property:

  * parent(#address-cells) == 2
  * #size-cells == 2
  * number of entries == 2
  * address size of first entry == 0x1000000
  * second child address entry == base + 0x1000000

Trying to increase PCIe MEM space to more than 16 MiB leads to an overlap
with PCIe IO space, and trying to define additional MEM space (as a third
entry in the `ranges` DT property) causes U-Boot to crash when booting the
kernel.

   ## Flattened Device Tree blob at 04f00000
      Booting using the fdt blob at 0x4f00000
      Loading Device Tree to 000000001fb01000, end 000000001fb08f12 ... OK
   ERROR: board-specific fdt fixup failed: <unknown error>
    - must RESET the board to recover.

Fix a3700_fdt_fix_pcie_regions() to properly parse and update all addresses
in the `ranges` property according to
https://elinux.org/Device_Tree_Usage#PCI_Address_Translation

Now it is possible to increase PCIe MEM space from 16 MiB to maximal value
of 127 MiB.

Signed-off-by: Pali Rohár <p...@kernel.org>
Reviewed-by: Marek Behún <marek.be...@nic.cz>
Fixes: cb2ddb291ee6 ("arm64: mvebu: a37xx: add device-tree fixer for PCIe 
regions")

Reviewed-by: Stefan Roese <s...@denx.de>

Thanks,
Stefan

---
  arch/arm/mach-mvebu/armada3700/cpu.c | 74 ++++++++++++++++++++++------
  1 file changed, 60 insertions(+), 14 deletions(-)

diff --git a/arch/arm/mach-mvebu/armada3700/cpu.c 
b/arch/arm/mach-mvebu/armada3700/cpu.c
index 1abac7c9a47a..9aec0ce9a430 100644
--- a/arch/arm/mach-mvebu/armada3700/cpu.c
+++ b/arch/arm/mach-mvebu/armada3700/cpu.c
@@ -8,6 +8,7 @@
  #include <cpu_func.h>
  #include <dm.h>
  #include <fdtdec.h>
+#include <fdt_support.h>
  #include <init.h>
  #include <asm/global_data.h>
  #include <linux/bitops.h>
@@ -280,36 +281,81 @@ static u32 find_pcie_window_base(void)
        return -1;
  }
+static int fdt_setprop_inplace_u32_partial(void *blob, int node,
+                                          const char *name,
+                                          u32 idx, u32 val)
+{
+       val = cpu_to_fdt32(val);
+
+       return fdt_setprop_inplace_namelen_partial(blob, node, name,
+                                                  strlen(name),
+                                                  idx * sizeof(u32),
+                                                  &val, sizeof(u32));
+}
+
  int a3700_fdt_fix_pcie_regions(void *blob)
  {
-       u32 new_ranges[14], base;
+       int acells, pacells, scells;
+       u32 base, fix_offset;
        const u32 *ranges;
-       int node, len;
+       int node, pnode;
+       int ret, i, len;
+
+       base = find_pcie_window_base();
+       if (base == -1)
+               return -ENOENT;
node = fdt_node_offset_by_compatible(blob, -1, "marvell,armada-3700-pcie");
        if (node < 0)
                return node;
ranges = fdt_getprop(blob, node, "ranges", &len);
-       if (!ranges)
+       if (!ranges || len % sizeof(u32))
                return -ENOENT;
- if (len != sizeof(new_ranges))
-               return -EINVAL;
-
-       memcpy(new_ranges, ranges, len);
+       /*
+        * The "ranges" property is an array of
+        * { <child address> <parent address> <size in child address space> }
+        *
+        * All 3 elements can span a diffent number of cells. Fetch their sizes.
+        */
+       pnode = fdt_parent_offset(blob, node);
+       acells = fdt_address_cells(blob, node);
+       pacells = fdt_address_cells(blob, pnode);
+       scells = fdt_size_cells(blob, node);
- base = find_pcie_window_base();
-       if (base == -1)
+       /* Child PCI addresses always use 3 cells */
+       if (acells != 3)
                return -ENOENT;
- new_ranges[2] = cpu_to_fdt32(base);
-       new_ranges[4] = new_ranges[2];
+       /* Calculate fixup offset from first child address (in last cell) */
+       fix_offset = base - fdt32_to_cpu(ranges[2]);
- new_ranges[9] = cpu_to_fdt32(base + 0x1000000);
-       new_ranges[11] = new_ranges[9];
+       /*
+        * Fix address (last cell) of each child address and each parent
+        * address
+        */
+       for (i = 0; i < len / sizeof(u32); i += acells + pacells + scells) {
+               int idx;
+
+               /* fix child address */
+               idx = i + acells - 1;
+               ret = fdt_setprop_inplace_u32_partial(blob, node, "ranges", idx,
+                                                     fdt32_to_cpu(ranges[idx]) 
+
+                                                     fix_offset);
+               if (ret)
+                       return ret;
+
+               /* fix parent address */
+               idx = i + acells + pacells - 1;
+               ret = fdt_setprop_inplace_u32_partial(blob, node, "ranges", idx,
+                                                     fdt32_to_cpu(ranges[idx]) 
+
+                                                     fix_offset);
+               if (ret)
+                       return ret;
+       }
- return fdt_setprop_inplace(blob, node, "ranges", new_ranges, len);
+       return 0;
  }
void reset_cpu(void)



Viele Grüße,
Stefan

--
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: s...@denx.de

Reply via email to