diff mbox series

[v1,u-boot-marvell,4/5] arm64: mvebu: a37xx: add device-tree fixer for PCIe regions

Message ID 20200408172522.18941-5-marek.behun@nic.cz
State Accepted
Commit cb2ddb291ee6fcbddd6d8f4ff49089dfe580f5d7
Headers show
Series MVEBU ARM64 improvments + another Turris Mox patch | expand

Commit Message

Marek Behún April 8, 2020, 5:25 p.m. UTC
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 at nic.cz>
---
 arch/arm/mach-mvebu/armada3700/cpu.c   | 52 ++++++++++++++++++++++++++
 arch/arm/mach-mvebu/include/mach/cpu.h |  3 ++
 2 files changed, 55 insertions(+)

Comments

Stefan Roese April 9, 2020, 8:09 a.m. UTC | #1
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 at 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 at 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
Marek Behún April 9, 2020, 1:17 p.m. UTC | #2
> > +}
> > +
> > +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);
Marek Behún April 9, 2020, 1:29 p.m. UTC | #3
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 at 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; ?
Stefan Roese April 14, 2020, 8:03 a.m. UTC | #4
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 at nic.cz>

Reviewed-by: Stefan Roese <sr at 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 at 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 mbox series

Patch

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 at 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
  *