| Message ID | 20230122211531.23181-3-samuel@sholland.org |
|---|---|
| State | Superseded |
| Delegated to: | Andre Przywara |
| Headers | show |
| Series | sunxi: SPL FIT support for 32-bit sunxi SoCs | expand |
HI Samuel, On Sun, 22 Jan 2023 at 14:16, Samuel Holland <samuel@sholland.org> wrote: > > This is easier to read than the #ifdef staircase, provides better > visibility into the memory map (alongside the other Kconfig > definitions), and allows these addresses to be reused from code. > > Signed-off-by: Samuel Holland <samuel@sholland.org> > --- > > Changes in v2: > - New patch for v2, split from the .dtsi changes > > arch/arm/dts/sunxi-u-boot.dtsi | 24 +++++++----------------- > arch/arm/mach-sunxi/Kconfig | 17 +++++++++++++++++ > 2 files changed, 24 insertions(+), 17 deletions(-) Reviewed-by: Simon Glass <sjg@chromium.org> Is this info not in the device tree? Regards, Simon
Hi Simon, On 1/23/23 12:42, Simon Glass wrote: > HI Samuel, > > On Sun, 22 Jan 2023 at 14:16, Samuel Holland <samuel@sholland.org> wrote: >> >> This is easier to read than the #ifdef staircase, provides better >> visibility into the memory map (alongside the other Kconfig >> definitions), and allows these addresses to be reused from code. >> >> Signed-off-by: Samuel Holland <samuel@sholland.org> >> --- >> >> Changes in v2: >> - New patch for v2, split from the .dtsi changes >> >> arch/arm/dts/sunxi-u-boot.dtsi | 24 +++++++----------------- >> arch/arm/mach-sunxi/Kconfig | 17 +++++++++++++++++ >> 2 files changed, 24 insertions(+), 17 deletions(-) > > Reviewed-by: Simon Glass <sjg@chromium.org> > > Is this info not in the device tree? It is not. These values do not correspond to any hardware property. For example, on A64/H5/H6, BL31 and SCP share a single "SRAM A2" region. The division of the SRAM between them was chosen arbitrarily (though now it is ABI). How would you represent this in the devicetree? Regards, Samuel
Hi Samuel, On Mon, 23 Jan 2023 at 22:22, Samuel Holland <samuel@sholland.org> wrote: > > Hi Simon, > > On 1/23/23 12:42, Simon Glass wrote: > > HI Samuel, > > > > On Sun, 22 Jan 2023 at 14:16, Samuel Holland <samuel@sholland.org> wrote: > >> > >> This is easier to read than the #ifdef staircase, provides better > >> visibility into the memory map (alongside the other Kconfig > >> definitions), and allows these addresses to be reused from code. > >> > >> Signed-off-by: Samuel Holland <samuel@sholland.org> > >> --- > >> > >> Changes in v2: > >> - New patch for v2, split from the .dtsi changes > >> > >> arch/arm/dts/sunxi-u-boot.dtsi | 24 +++++++----------------- > >> arch/arm/mach-sunxi/Kconfig | 17 +++++++++++++++++ > >> 2 files changed, 24 insertions(+), 17 deletions(-) > > > > Reviewed-by: Simon Glass <sjg@chromium.org> > > > > Is this info not in the device tree? > > It is not. These values do not correspond to any hardware property. For > example, on A64/H5/H6, BL31 and SCP share a single "SRAM A2" region. The > division of the SRAM between them was chosen arbitrarily (though now it > is ABI). How would you represent this in the devicetree? I would probably add a new node for the SRAM, with partition information within that, as is done for MTD. Regards, Simon
On Tue, 24 Jan 2023 15:44:30 -0700 Simon Glass <sjg@chromium.org> wrote: > Hi Samuel, > > On Mon, 23 Jan 2023 at 22:22, Samuel Holland <samuel@sholland.org> wrote: > > > > Hi Simon, > > > > On 1/23/23 12:42, Simon Glass wrote: > > > HI Samuel, > > > > > > On Sun, 22 Jan 2023 at 14:16, Samuel Holland <samuel@sholland.org> wrote: > > >> > > >> This is easier to read than the #ifdef staircase, provides better > > >> visibility into the memory map (alongside the other Kconfig > > >> definitions), and allows these addresses to be reused from code. > > >> > > >> Signed-off-by: Samuel Holland <samuel@sholland.org> > > >> --- > > >> > > >> Changes in v2: > > >> - New patch for v2, split from the .dtsi changes > > >> > > >> arch/arm/dts/sunxi-u-boot.dtsi | 24 +++++++----------------- > > >> arch/arm/mach-sunxi/Kconfig | 17 +++++++++++++++++ > > >> 2 files changed, 24 insertions(+), 17 deletions(-) > > > > > > Reviewed-by: Simon Glass <sjg@chromium.org> > > > > > > Is this info not in the device tree? > > > > It is not. These values do not correspond to any hardware property. For > > example, on A64/H5/H6, BL31 and SCP share a single "SRAM A2" region. The > > division of the SRAM between them was chosen arbitrarily (though now it > > is ABI). How would you represent this in the devicetree? > > I would probably add a new node for the SRAM, with partition > information within that, as is done for MTD. I am probably missing something, but why would we want to do that? Looks a bit like a solution looking for a problem to me. The split and assignment of the SRAM regions is an agreement between BL31, crust and U-Boot, with U-Boot taking the lead here, because it's the initial loader of those components, and does the packaging. So what would putting those addresses in the generic DT bring us, aside from more complicated code to look them up? Cheers, Andre
Hi Andre, On Wed, 25 Jan 2023 at 05:24, Andre Przywara <andre.przywara@arm.com> wrote: > > On Tue, 24 Jan 2023 15:44:30 -0700 > Simon Glass <sjg@chromium.org> wrote: > > > Hi Samuel, > > > > On Mon, 23 Jan 2023 at 22:22, Samuel Holland <samuel@sholland.org> wrote: > > > > > > Hi Simon, > > > > > > On 1/23/23 12:42, Simon Glass wrote: > > > > HI Samuel, > > > > > > > > On Sun, 22 Jan 2023 at 14:16, Samuel Holland <samuel@sholland.org> wrote: > > > >> > > > >> This is easier to read than the #ifdef staircase, provides better > > > >> visibility into the memory map (alongside the other Kconfig > > > >> definitions), and allows these addresses to be reused from code. > > > >> > > > >> Signed-off-by: Samuel Holland <samuel@sholland.org> > > > >> --- > > > >> > > > >> Changes in v2: > > > >> - New patch for v2, split from the .dtsi changes > > > >> > > > >> arch/arm/dts/sunxi-u-boot.dtsi | 24 +++++++----------------- > > > >> arch/arm/mach-sunxi/Kconfig | 17 +++++++++++++++++ > > > >> 2 files changed, 24 insertions(+), 17 deletions(-) > > > > > > > > Reviewed-by: Simon Glass <sjg@chromium.org> > > > > > > > > Is this info not in the device tree? > > > > > > It is not. These values do not correspond to any hardware property. For > > > example, on A64/H5/H6, BL31 and SCP share a single "SRAM A2" region. The > > > division of the SRAM between them was chosen arbitrarily (though now it > > > is ABI). How would you represent this in the devicetree? > > > > I would probably add a new node for the SRAM, with partition > > information within that, as is done for MTD. > > I am probably missing something, but why would we want to do that? Looks a > bit like a solution looking for a problem to me. > The split and assignment of the SRAM regions is an agreement between BL31, > crust and U-Boot, with U-Boot taking the lead here, because it's the > initial loader of those components, and does the packaging. > So what would putting those addresses in the generic DT bring us, aside > from more complicated code to look them up? Well I just answered the question. I'm not necessarily advocating for it. We could perhaps use the new Transfer List to pass this info instead. But if separate projects have implicit hard-coded values it is going to lead to pain when they conflict. It is better to set this all up at packaging time. Regards, Simon
diff --git a/arch/arm/dts/sunxi-u-boot.dtsi b/arch/arm/dts/sunxi-u-boot.dtsi index 8a6c9e901a..f38359fd42 100644 --- a/arch/arm/dts/sunxi-u-boot.dtsi +++ b/arch/arm/dts/sunxi-u-boot.dtsi @@ -1,15 +1,5 @@ #include <config.h> -#ifdef CONFIG_MACH_SUN50I_H6 -#define BL31_ADDR 0x104000 -#define SCP_ADDR 0x114000 -#elif defined(CONFIG_MACH_SUN50I_H616) -#define BL31_ADDR 0x40000000 -#else -#define BL31_ADDR 0x44000 -#define SCP_ADDR 0x50000 -#endif - / { aliases { #ifndef CONFIG_MACH_SUNIV @@ -64,8 +54,8 @@ os = "arm-trusted-firmware"; arch = "arm64"; compression = "none"; - load = <BL31_ADDR>; - entry = <BL31_ADDR>; + load = <CONFIG_SUNXI_BL31_BASE>; + entry = <CONFIG_SUNXI_BL31_BASE>; atf-bl31 { filename = "bl31.bin"; @@ -73,13 +63,13 @@ }; }; -#ifdef SCP_ADDR +#if CONFIG_SUNXI_SCP_BASE scp { description = "SCP firmware"; type = "firmware"; arch = "or1k"; compression = "none"; - load = <SCP_ADDR>; + load = <CONFIG_SUNXI_SCP_BASE>; scp { filename = "scp.bin"; @@ -101,10 +91,10 @@ @config-SEQ { description = "NAME"; firmware = "atf"; -#ifndef SCP_ADDR - loadables = "uboot"; -#else +#if CONFIG_SUNXI_SCP_BASE loadables = "scp", "uboot"; +#else + loadables = "uboot"; #endif fdt = "fdt-SEQ"; }; diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig index dbe6005daa..8e67f7e91d 100644 --- a/arch/arm/mach-sunxi/Kconfig +++ b/arch/arm/mach-sunxi/Kconfig @@ -110,6 +110,23 @@ config SUNXI_SRAM_ADDRESS Some newer SoCs map the boot ROM at address 0 instead and move the SRAM to a different address. +config SUNXI_BL31_BASE + hex + default 0x00044000 if MACH_SUN50I || MACH_SUN50I_H5 + default 0x00104000 if MACH_SUN50I_H6 + default 0x40000000 if MACH_SUN50I_H616 + default 0x0 + help + Address where BL31 (TF-A) is loaded, or zero if BL31 is not used. + +config SUNXI_SCP_BASE + hex + default 0x00050000 if MACH_SUN50I || MACH_SUN50I_H5 + default 0x00114000 if MACH_SUN50I_H6 + default 0x0 + help + Address where SCP firmware is loaded, or zero if it is not used. + config SUNXI_A64_TIMER_ERRATUM bool
This is easier to read than the #ifdef staircase, provides better visibility into the memory map (alongside the other Kconfig definitions), and allows these addresses to be reused from code. Signed-off-by: Samuel Holland <samuel@sholland.org> --- Changes in v2: - New patch for v2, split from the .dtsi changes arch/arm/dts/sunxi-u-boot.dtsi | 24 +++++++----------------- arch/arm/mach-sunxi/Kconfig | 17 +++++++++++++++++ 2 files changed, 24 insertions(+), 17 deletions(-)