Message ID | 20210526155940.26141-5-pali@kernel.org |
---|---|
State | Accepted |
Commit | 4a82fca8e330157081fc132a591ebd99ba02ee33 |
Delegated to: | Stefan Roese |
Headers | show |
Series | [v2,1/7] arm: a37xx: pci: Don't put link into LTSSM Recovery state during probe | expand |
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 <pali@kernel.org> > Reviewed-by: Marek Behún <marek.behun@nic.cz> > Fixes: cb2ddb291ee6 ("arm64: mvebu: a37xx: add device-tree fixer for PCIe regions") Reviewed-by: Stefan Roese <sr@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
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)