| Message ID | 20200408172522.18941-5-marek.behun@nic.cz |
|---|---|
| State | Accepted |
| Commit | cb2ddb291ee6fcbddd6d8f4ff49089dfe580f5d7 |
| Delegated to: | Stefan Roese |
| Headers | show |
| Series | MVEBU ARM64 improvments + another Turris Mox patch | expand |
On 08.04.20 19:25, Marek Behún wrote: > In case when ARM Trusted Firmware changes the default address of PCIe > regions (which can be done for devices with 4 GB RAM to maximize the > amount of RAM the device can use) we add code that looks at how ATF > changed the PCIe windows in the CPU Address Decoder and changes given > device-tree blob accordingly. > > Signed-off-by: Marek Behún <marek.behun@nic.cz> > --- > arch/arm/mach-mvebu/armada3700/cpu.c | 52 ++++++++++++++++++++++++++ > arch/arm/mach-mvebu/include/mach/cpu.h | 3 ++ > 2 files changed, 55 insertions(+) > > diff --git a/arch/arm/mach-mvebu/armada3700/cpu.c b/arch/arm/mach-mvebu/armada3700/cpu.c > index 959a909d8a..17d2d43bab 100644 > --- a/arch/arm/mach-mvebu/armada3700/cpu.c > +++ b/arch/arm/mach-mvebu/armada3700/cpu.c > @@ -50,6 +50,8 @@ > #define A3700_PTE_BLOCK_DEVICE \ > (PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) | PTE_BLOCK_NON_SHARE) > > +#define PCIE_PATH "/soc/pcie@d0070000" > + > DECLARE_GLOBAL_DATA_PTR; > > static struct mm_region mvebu_mem_map[MAX_MEM_MAP_REGIONS] = { > @@ -259,6 +261,56 @@ int a3700_dram_init_banksize(void) > return 0; > } > > +static u32 find_pcie_window_base(void) > +{ > + int win; > + > + for (win = 0; win < MVEBU_CPU_DEC_WINS; ++win) { > + u32 base, tgt; > + > + /* skip disabled windows */ > + if (get_cpu_dec_win(win, &tgt, &base, NULL)) > + continue; > + > + if (tgt == MVEBU_CPU_DEC_WIN_CTRL_TGT_PCIE) > + return base; > + } > + > + return -1; return -ENOENT; ? > +} > + > +int a3700_fdt_fix_pcie_regions(void *blob) > +{ > + u32 new_ranges[14], base; Where does this "14" come from? Is this a safe upper margin? > + const u32 *ranges; > + int node, len; > + > + node = fdt_path_offset(blob, PCIE_PATH); > + if (node < 0) > + return node; > + > + ranges = fdt_getprop(blob, node, "ranges", &len); > + if (!ranges) > + return -ENOENT; > + > + if (len != sizeof(new_ranges)) > + return -EINVAL; > + > + memcpy(new_ranges, ranges, len); > + > + base = find_pcie_window_base(); > + if (base == -1) > + return -ENOENT; if (base < 0) return base; ? > + > + new_ranges[2] = cpu_to_fdt32(base); > + new_ranges[4] = new_ranges[2]; > + > + new_ranges[9] = cpu_to_fdt32(base + 0x1000000); > + new_ranges[11] = new_ranges[9]; > + > + return fdt_setprop_inplace(blob, node, "ranges", new_ranges, len); > +} > + > void reset_cpu(ulong ignored) > { > /* > diff --git a/arch/arm/mach-mvebu/include/mach/cpu.h b/arch/arm/mach-mvebu/include/mach/cpu.h > index 2a53329420..1d619c4e49 100644 > --- a/arch/arm/mach-mvebu/include/mach/cpu.h > +++ b/arch/arm/mach-mvebu/include/mach/cpu.h > @@ -178,6 +178,9 @@ int a8k_dram_init_banksize(void); > int a3700_dram_init(void); > int a3700_dram_init_banksize(void); > > +/* A3700 PCIe regions fixer for device tree */ > +int a3700_fdt_fix_pcie_regions(void *blob); > + > /* > * get_ref_clk > * > Thanks, Stefan
> > +} > > + > > +int a3700_fdt_fix_pcie_regions(void *blob) > > +{ > > + u32 new_ranges[14], base; > > Where does this "14" come from? Is this a safe upper margin? Yes, the way how the code below works, it won't overflow or anything. I even test whether the "ranges" property from the dtc has the same size, and if not, new_ranges is not written at all. If the given device tree is changed somehow so that the ranges property structure is changed, the problem it would cause is that the PCIe driver won't work or it will cause segfaults or something (in U-Boot and in Linux). But such change in device-tree would be incompatible with Linux's driver anyway, so I don't think something like that will be done. > > + const u32 *ranges; > > + int node, len; > > + > > + node = fdt_path_offset(blob, PCIE_PATH); > > + if (node < 0) > > + return node; > > + > > + ranges = fdt_getprop(blob, node, "ranges", &len); > > + if (!ranges) > > + return -ENOENT; > > + > > + if (len != sizeof(new_ranges)) > > + return -EINVAL; > > + > > + memcpy(new_ranges, ranges, len);
Hi Stefan, sorry I overlooked the other two things you commented on the code. On Thu, 9 Apr 2020 10:09:52 +0200 Stefan Roese <sr@denx.de> wrote: > > + return -1; > > return -ENOENT; ? The function returns u32. The error is reported by returning (u32)-1. The check base < 0 won't work. I would specifically have to check for -ENOENT, or use something like ERR_PTR from Linux. > if (base < 0) > return base; ?
On 08.04.20 19:25, Marek Behún wrote: > In case when ARM Trusted Firmware changes the default address of PCIe > regions (which can be done for devices with 4 GB RAM to maximize the > amount of RAM the device can use) we add code that looks at how ATF > changed the PCIe windows in the CPU Address Decoder and changes given > device-tree blob accordingly. > > Signed-off-by: Marek Behún <marek.behun@nic.cz> Reviewed-by: Stefan Roese <sr@denx.de> Thanks, Stefan > --- > arch/arm/mach-mvebu/armada3700/cpu.c | 52 ++++++++++++++++++++++++++ > arch/arm/mach-mvebu/include/mach/cpu.h | 3 ++ > 2 files changed, 55 insertions(+) > > diff --git a/arch/arm/mach-mvebu/armada3700/cpu.c b/arch/arm/mach-mvebu/armada3700/cpu.c > index 959a909d8a..17d2d43bab 100644 > --- a/arch/arm/mach-mvebu/armada3700/cpu.c > +++ b/arch/arm/mach-mvebu/armada3700/cpu.c > @@ -50,6 +50,8 @@ > #define A3700_PTE_BLOCK_DEVICE \ > (PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) | PTE_BLOCK_NON_SHARE) > > +#define PCIE_PATH "/soc/pcie@d0070000" > + > DECLARE_GLOBAL_DATA_PTR; > > static struct mm_region mvebu_mem_map[MAX_MEM_MAP_REGIONS] = { > @@ -259,6 +261,56 @@ int a3700_dram_init_banksize(void) > return 0; > } > > +static u32 find_pcie_window_base(void) > +{ > + int win; > + > + for (win = 0; win < MVEBU_CPU_DEC_WINS; ++win) { > + u32 base, tgt; > + > + /* skip disabled windows */ > + if (get_cpu_dec_win(win, &tgt, &base, NULL)) > + continue; > + > + if (tgt == MVEBU_CPU_DEC_WIN_CTRL_TGT_PCIE) > + return base; > + } > + > + return -1; > +} > + > +int a3700_fdt_fix_pcie_regions(void *blob) > +{ > + u32 new_ranges[14], base; > + const u32 *ranges; > + int node, len; > + > + node = fdt_path_offset(blob, PCIE_PATH); > + if (node < 0) > + return node; > + > + ranges = fdt_getprop(blob, node, "ranges", &len); > + if (!ranges) > + return -ENOENT; > + > + if (len != sizeof(new_ranges)) > + return -EINVAL; > + > + memcpy(new_ranges, ranges, len); > + > + base = find_pcie_window_base(); > + if (base == -1) > + return -ENOENT; > + > + new_ranges[2] = cpu_to_fdt32(base); > + new_ranges[4] = new_ranges[2]; > + > + new_ranges[9] = cpu_to_fdt32(base + 0x1000000); > + new_ranges[11] = new_ranges[9]; > + > + return fdt_setprop_inplace(blob, node, "ranges", new_ranges, len); > +} > + > void reset_cpu(ulong ignored) > { > /* > diff --git a/arch/arm/mach-mvebu/include/mach/cpu.h b/arch/arm/mach-mvebu/include/mach/cpu.h > index 2a53329420..1d619c4e49 100644 > --- a/arch/arm/mach-mvebu/include/mach/cpu.h > +++ b/arch/arm/mach-mvebu/include/mach/cpu.h > @@ -178,6 +178,9 @@ int a8k_dram_init_banksize(void); > int a3700_dram_init(void); > int a3700_dram_init_banksize(void); > > +/* A3700 PCIe regions fixer for device tree */ > +int a3700_fdt_fix_pcie_regions(void *blob); > + > /* > * get_ref_clk > * > Viele Grüße, Stefan
diff --git a/arch/arm/mach-mvebu/armada3700/cpu.c b/arch/arm/mach-mvebu/armada3700/cpu.c index 959a909d8a..17d2d43bab 100644 --- a/arch/arm/mach-mvebu/armada3700/cpu.c +++ b/arch/arm/mach-mvebu/armada3700/cpu.c @@ -50,6 +50,8 @@ #define A3700_PTE_BLOCK_DEVICE \ (PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) | PTE_BLOCK_NON_SHARE) +#define PCIE_PATH "/soc/pcie@d0070000" + DECLARE_GLOBAL_DATA_PTR; static struct mm_region mvebu_mem_map[MAX_MEM_MAP_REGIONS] = { @@ -259,6 +261,56 @@ int a3700_dram_init_banksize(void) return 0; } +static u32 find_pcie_window_base(void) +{ + int win; + + for (win = 0; win < MVEBU_CPU_DEC_WINS; ++win) { + u32 base, tgt; + + /* skip disabled windows */ + if (get_cpu_dec_win(win, &tgt, &base, NULL)) + continue; + + if (tgt == MVEBU_CPU_DEC_WIN_CTRL_TGT_PCIE) + return base; + } + + return -1; +} + +int a3700_fdt_fix_pcie_regions(void *blob) +{ + u32 new_ranges[14], base; + const u32 *ranges; + int node, len; + + node = fdt_path_offset(blob, PCIE_PATH); + if (node < 0) + return node; + + ranges = fdt_getprop(blob, node, "ranges", &len); + if (!ranges) + return -ENOENT; + + if (len != sizeof(new_ranges)) + return -EINVAL; + + memcpy(new_ranges, ranges, len); + + base = find_pcie_window_base(); + if (base == -1) + return -ENOENT; + + new_ranges[2] = cpu_to_fdt32(base); + new_ranges[4] = new_ranges[2]; + + new_ranges[9] = cpu_to_fdt32(base + 0x1000000); + new_ranges[11] = new_ranges[9]; + + return fdt_setprop_inplace(blob, node, "ranges", new_ranges, len); +} + void reset_cpu(ulong ignored) { /* diff --git a/arch/arm/mach-mvebu/include/mach/cpu.h b/arch/arm/mach-mvebu/include/mach/cpu.h index 2a53329420..1d619c4e49 100644 --- a/arch/arm/mach-mvebu/include/mach/cpu.h +++ b/arch/arm/mach-mvebu/include/mach/cpu.h @@ -178,6 +178,9 @@ int a8k_dram_init_banksize(void); int a3700_dram_init(void); int a3700_dram_init_banksize(void); +/* A3700 PCIe regions fixer for device tree */ +int a3700_fdt_fix_pcie_regions(void *blob); + /* * get_ref_clk *
In case when ARM Trusted Firmware changes the default address of PCIe regions (which can be done for devices with 4 GB RAM to maximize the amount of RAM the device can use) we add code that looks at how ATF changed the PCIe windows in the CPU Address Decoder and changes given device-tree blob accordingly. Signed-off-by: Marek Behún <marek.behun@nic.cz> --- arch/arm/mach-mvebu/armada3700/cpu.c | 52 ++++++++++++++++++++++++++ arch/arm/mach-mvebu/include/mach/cpu.h | 3 ++ 2 files changed, 55 insertions(+)