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

2020-04-14 Thread Stefan Roese

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 


Reviewed-by: Stefan Roese 

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@d007"

+
  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, , , 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", );
+   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 + 0x100);
+   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

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


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

2020-04-09 Thread Marek Behun
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  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; ?


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

2020-04-09 Thread Marek Behun
> > +}
> > +
> > +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", );
> > +   if (!ranges)
> > +   return -ENOENT;
> > +
> > +   if (len != sizeof(new_ranges))
> > +   return -EINVAL;
> > +
> > +   memcpy(new_ranges, ranges, len);


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

2020-04-09 Thread Stefan Roese

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 
---
  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@d007"

+
  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, , , 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", );
+   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 + 0x100);
+   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


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

2020-04-08 Thread Marek Behún
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 
---
 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@d007"
+
 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, , , 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", );
+   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 + 0x100);
+   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
  *
-- 
2.24.1