sunxi: Allwinner A64 SPL support

These days many Allwinner SoCs use clock_sun6i.c, although out of them only the (original sun6i) A31 has a second MBUS clock register. Also setting up the PRCM PLL_CTLR1 register to provide the proper voltage seems to be an A31-only feature as well.
So restrict the initialization to this SoC only to avoid writing bogus values to (undefined) registers in other chips.

Signed-off-by Andre Przywara <[email protected]>

Quoted 8 lines · Andre Przywara

These days many Allwinner SoCs use clock_sun6i.c, although out of them only the (original sun6i) A31 has a second MBUS clock register. Also setting up the PRCM PLL_CTLR1 register to provide the proper voltage seems to be an A31-only feature as well.
So restrict the initialization to this SoC only to avoid writing bogus values to (undefined) registers in other chips.

Quoted 1 line

Signed-off-by Andre Przywara <[email protected]>

Reviewed-by Alexander Graf <[email protected]>

(However I haven't counter-checked with specs)

At the moment we use the arch/arm directory for arm64 boards as well, so the Makefile will pick up the "arm" name for the architecture to use for tagging binaries in U-Boot image files.
Differentiate between the two by looking at the CPU variable being defined to "armv8", and use the arm64 architecture name on creating the image file if that matches.

Signed-off-by Andre Przywara <[email protected]>

Quoted 8 lines · Andre Przywara

At the moment we use the arch/arm directory for arm64 boards as well, so the Makefile will pick up the "arm" name for the architecture to use for tagging binaries in U-Boot image files.
Differentiate between the two by looking at the CPU variable being defined
to "armv8", and use the arm64 architecture name on creating the image file if that matches.

Quoted 1 line

Signed-off-by Andre Przywara <[email protected]>

Why is this important? To know the state you have to be in for SPL->U-Boot transition later?

Why didn't anyone else stumble over this yet? Because nobody's using SPL?

Hi,

Quoted 9 lines · Alexander Graf

Quoted 8 lines · Andre Przywara

At the moment we use the arch/arm directory for arm64 boards as well, so the Makefile will pick up the "arm" name for the architecture to use for tagging binaries in U-Boot image files.
Differentiate between the two by looking at the CPU variable being defined
to "armv8", and use the arm64 architecture name on creating the image file if that matches.

Quoted 3 lines

Quoted 1 line

Signed-off-by Andre Przywara <[email protected]>

Why is this important? To know the state you have to be in for SPL->U-Boot transition later?

Yes.

Quoted 1 line

Why didn't anyone else stumble over this yet? Because nobody's using SPL?

Given the warnings and bugs I found when I compiled the SPL for 64 bit I'd assume the latter.

But I was asking this question myself already. Apparently everyone just hacked their firmware chain to live with "arm" in there, APM being a prominent example.

So given this I am a bit wary about the implication of this patch, I hope that people holler if this breaks their platform (and then fix that instead of hacking U-Boot again).

Quoted 19 lines · Andre Przywara

Hi,

Quoted 9 lines · Alexander Graf

Quoted 8 lines · Andre Przywara

At the moment we use the arch/arm directory for arm64 boards as well, so the Makefile will pick up the "arm" name for the architecture to use for tagging binaries in U-Boot image files.
Differentiate between the two by looking at the CPU variable being defined
to "armv8", and use the arm64 architecture name on creating the image file if that matches.

Quoted 3 lines

Quoted 1 line

Signed-off-by Andre Przywara <[email protected]>

Why is this important? To know the state you have to be in for SPL->U-Boot transition later?

Yes.

Quoted 2 lines

Why didn't anyone else stumble over this yet? Because nobody's using SPL?

Quoted 6 lines

Given the warnings and bugs I found when I compiled the SPL for 64 bit I'd assume the latter.

But I was asking this question myself already. Apparently everyone just hacked their firmware chain to live with "arm" in there, APM being a prominent example.

APM is "special". They even use the "arm" marker for kernels.

Quoted 3 lines

So given this I am a bit wary about the implication of this patch, I hope that people holler if this breaks their platform (and then fix that instead of hacking U-Boot again).

Well, I guess it's a step into the right direction. I'm still not a huge fan of having both 32bit and 64bit binaries on the same platform, but indicating which one we are is a good idea :).

At the moment we use the arch/arm directory for arm64 boards as well, so the Makefile will pick up the "arm" name for the architecture to use for tagging binaries in U-Boot image files.
Differentiate between the two by looking at the CPU variable being defined to "armv8", and use the arm64 architecture name on creating the image file if that matches.

Signed-off-by Andre Przywara <[email protected]>

---
 Makefile | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

To avoid enumerating the very same DRAM values in defconfig files for each and every Allwinner A64 board out there, let's put some sane default values in the Kconfig file.
Boards with different needs can override them at any time.

Signed-off-by Andre Przywara <[email protected]>

---
  board/sunxi/Kconfig           | 3 +++
  configs/pine64_plus_defconfig | 2 --
  2 files changed, 3 insertions(+), 2 deletions(-)

board/sunxi/Kconfig +2 -0

index e1d4ab1..d281a65 100644
@@ -145,6 +145,7 @@ config DRAM_CLK
  	default 792 if MACH_SUN9I
  	default 312 if MACH_SUN6I || MACH_SUN8I
  	default 360 if MACH_SUN4I || MACH_SUN5I || MACH_SUN7I
+	default 672 if MACH_SUN50I
  	---help---
  	Set the dram clock speed, valid range 240 - 480 (prior to sun9i),
  	must be a multiple of 24. For the sun9i (A80), the tested values

@@ -164,6 +165,7 @@ config DRAM_ZQ
  	default 123 if MACH_SUN4I || MACH_SUN5I || MACH_SUN6I || MACH_SUN8I
  	default 127 if MACH_SUN7I
  	default 4145117 if MACH_SUN9I
+	default 3881915 if MACH_SUN50I
  	---help---
  	Set the dram zq value.
 

board/sunxi/Kconfig +0 -0

  	---help---
  	Select this to enable dram odt (on die termination).
 

configs/pine64_plus_defconfig +0 -2

index 6d0198f..a53e968 100644
@@ -2,8 +2,6 @@ CONFIG_ARM=y
  CONFIG_ENABLE_ARM_SOC_BOOT0_HOOK=y
  CONFIG_ARCH_SUNXI=y
  CONFIG_MACH_SUN50I=y
-CONFIG_DRAM_CLK=672
-CONFIG_DRAM_ZQ=3881915
  CONFIG_DEFAULT_DEVICE_TREE="sun50i-a64-pine64-plus"
  # CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set
  CONFIG_CONSOLE_MUX=y

Quoted 30 lines · Andre Przywara

To avoid enumerating the very same DRAM values in defconfig files for each and every Allwinner A64 board out there, let's put some sane default values in the Kconfig file.
Boards with different needs can override them at any time.

Signed-off-by Andre Przywara <[email protected]>

---
  board/sunxi/Kconfig           | 3 +++
  configs/pine64_plus_defconfig | 2 --
  2 files changed, 3 insertions(+), 2 deletions(-)

board/sunxi/Kconfig +2 -0

index e1d4ab1..d281a65 100644
@@ -145,6 +145,7 @@ config DRAM_CLK
      default 792 if MACH_SUN9I
      default 312 if MACH_SUN6I || MACH_SUN8I
      default 360 if MACH_SUN4I || MACH_SUN5I || MACH_SUN7I
+    default 672 if MACH_SUN50I
      ---help---
      Set the dram clock speed, valid range 240 - 480 (prior to sun9i),
      must be a multiple of 24. For the sun9i (A80), the tested values

@@ -164,6 +165,7 @@ config DRAM_ZQ
      default 123 if MACH_SUN4I || MACH_SUN5I || MACH_SUN6I || MACH_SUN8I
      default 127 if MACH_SUN7I
      default 4145117 if MACH_SUN9I
+    default 3881915 if MACH_SUN50I
      ---help---
      Set the dram zq value.

Quoted 5 lines

board/sunxi/Kconfig +0 -0

  @@ -171,6 +173,7 @@ config DRAM_ODT_EN

bool "sunxi dram odt enable"
default n if !MACH_SUN8I_A23
default y if MACH_SUN8I_A23
+ default y if MACH_SUN50I

This change isn't obvious. It's not mentioned in the patch description or removed from the defconfig.

Quoted 16 lines

---help---
Select this to enable dram odt (on die termination).
diff --git a/configs/pine64_plus_defconfig
b/configs/pine64_plus_defconfig
index 6d0198f..a53e968 100644

configs/pine64_plus_defconfig +0 -2

@@ -2,8 +2,6 @@ CONFIG_ARM=y
  CONFIG_ENABLE_ARM_SOC_BOOT0_HOOK=y
  CONFIG_ARCH_SUNXI=y
  CONFIG_MACH_SUN50I=y
-CONFIG_DRAM_CLK=672
-CONFIG_DRAM_ZQ=3881915
  CONFIG_DEFAULT_DEVICE_TREE="sun50i-a64-pine64-plus"
  # CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set
  CONFIG_CONSOLE_MUX=y

Hi,

Quoted 31 lines · Alexander Graf

Quoted 30 lines · Andre Przywara

To avoid enumerating the very same DRAM values in defconfig files for each and every Allwinner A64 board out there, let's put some sane default values in the Kconfig file.
Boards with different needs can override them at any time.

Signed-off-by Andre Przywara <[email protected]>

---
  board/sunxi/Kconfig           | 3 +++
  configs/pine64_plus_defconfig | 2 --
  2 files changed, 3 insertions(+), 2 deletions(-)

board/sunxi/Kconfig +2 -0

index e1d4ab1..d281a65 100644
@@ -145,6 +145,7 @@ config DRAM_CLK
      default 792 if MACH_SUN9I
      default 312 if MACH_SUN6I || MACH_SUN8I
      default 360 if MACH_SUN4I || MACH_SUN5I || MACH_SUN7I
+    default 672 if MACH_SUN50I
      ---help---
      Set the dram clock speed, valid range 240 - 480 (prior to sun9i),
      must be a multiple of 24. For the sun9i (A80), the tested values

@@ -164,6 +165,7 @@ config DRAM_ZQ
      default 123 if MACH_SUN4I || MACH_SUN5I || MACH_SUN6I || MACH_SUN8I
      default 127 if MACH_SUN7I
      default 4145117 if MACH_SUN9I
+    default 3881915 if MACH_SUN50I
      ---help---
      Set the dram zq value.

Quoted 8 lines

Quoted 5 lines

board/sunxi/Kconfig +0 -0

  @@ -171,6 +173,7 @@ config DRAM_ODT_EN

bool "sunxi dram odt enable"
default n if !MACH_SUN8I_A23
default y if MACH_SUN8I_A23
+ default y if MACH_SUN50I

This change isn't obvious. It's not mentioned in the patch description or removed from the defconfig.

Oh, right, looks like a rebase artifact, since I juggled the patches around quite a bit. It belongs to patch 10/10, I guess.

Cheers,
Andre.

Quoted 17 lines

Quoted 16 lines

---help---
Select this to enable dram odt (on die termination).
diff --git a/configs/pine64_plus_defconfig
b/configs/pine64_plus_defconfig
index 6d0198f..a53e968 100644

configs/pine64_plus_defconfig +0 -2

@@ -2,8 +2,6 @@ CONFIG_ARM=y
  CONFIG_ENABLE_ARM_SOC_BOOT0_HOOK=y
  CONFIG_ARCH_SUNXI=y
  CONFIG_MACH_SUN50I=y
-CONFIG_DRAM_CLK=672
-CONFIG_DRAM_ZQ=3881915
  CONFIG_DEFAULT_DEVICE_TREE="sun50i-a64-pine64-plus"
  # CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set
  CONFIG_CONSOLE_MUX=y

Hi,

Quoted 44 lines · Andre Przywara

Hi,

Quoted 40 lines · Alexander Graf

Quoted 28 lines · Andre Przywara

To avoid enumerating the very same DRAM values in defconfig files for each and every Allwinner A64 board out there, let's put some sane default values in the Kconfig file.
Boards with different needs can override them at any time.

Signed-off-by Andre Przywara <[email protected]>

---
  board/sunxi/Kconfig           | 3 +++
  configs/pine64_plus_defconfig | 2 --
  2 files changed, 3 insertions(+), 2 deletions(-)

board/sunxi/Kconfig +1 -0

index e1d4ab1..d281a65 100644
@@ -145,6 +145,7 @@ config DRAM_CLK
      default 792 if MACH_SUN9I
      default 312 if MACH_SUN6I || MACH_SUN8I
      default 360 if MACH_SUN4I || MACH_SUN5I || MACH_SUN7I
+    default 672 if MACH_SUN50I
      ---help---
      Set the dram clock speed, valid range 240 - 480 (prior to sun9i),
      must be a multiple of 24. For the sun9i (A80), the tested values

@@ -164,6 +165,7 @@ config DRAM_ZQ
      default 123 if MACH_SUN4I || MACH_SUN5I || MACH_SUN6I ||

MACH_SUN8I
default 127 if MACH_SUN7I
default 4145117 if MACH_SUN9I

board/sunxi/Kconfig config DRAM_ODT_EN · line 176

Quoted 8 lines

board/sunxi/Kconfig +2 -0

+    default 3881915 if MACH_SUN50I
      ---help---
      Set the dram zq value.
  @@ -171,6 +173,7 @@ config DRAM_ODT_EN
      bool "sunxi dram odt enable"
      default n if !MACH_SUN8I_A23
      default y if MACH_SUN8I_A23
+    default y if MACH_SUN50I

This change isn't obvious. It's not mentioned in the patch description or removed from the defconfig.

Quoted 2 lines

Oh, right, looks like a rebase artifact, since I juggled the patches around quite a bit. It belongs to patch 10/10, I guess.

It is setting a DRAM default, so it is fine, as for it not being removed from the defconfig, it is not yet present in the one defconfig we have for a64. OTOH it would be food to remove the

DRAM_CLK and DRAM_ZQ values from configs/pine64_plus_defconfig as part of this patch.

Quoted 17 lines

Quoted 17 lines

Quoted 16 lines

---help---
Select this to enable dram odt (on die termination).
diff --git a/configs/pine64_plus_defconfig
b/configs/pine64_plus_defconfig
index 6d0198f..a53e968 100644

configs/pine64_plus_defconfig +0 -2

@@ -2,8 +2,6 @@ CONFIG_ARM=y
  CONFIG_ENABLE_ARM_SOC_BOOT0_HOOK=y
  CONFIG_ARCH_SUNXI=y
  CONFIG_MACH_SUN50I=y
-CONFIG_DRAM_CLK=672
-CONFIG_DRAM_ZQ=3881915
  CONFIG_DEFAULT_DEVICE_TREE="sun50i-a64-pine64-plus"
  # CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set
  CONFIG_CONSOLE_MUX=y

Hi Hans,

thanks for having a look.

Quoted 47 lines · Hans de Goede

Hi,

Quoted 44 lines · Andre Przywara

Hi,

Quoted 40 lines · Alexander Graf

Quoted 28 lines · Andre Przywara

To avoid enumerating the very same DRAM values in defconfig files for each and every Allwinner A64 board out there, let's put some sane default values in the Kconfig file.
Boards with different needs can override them at any time.

Signed-off-by Andre Przywara <[email protected]>

---
  board/sunxi/Kconfig           | 3 +++
  configs/pine64_plus_defconfig | 2 --
  2 files changed, 3 insertions(+), 2 deletions(-)

board/sunxi/Kconfig +1 -0

index e1d4ab1..d281a65 100644
@@ -145,6 +145,7 @@ config DRAM_CLK
      default 792 if MACH_SUN9I
      default 312 if MACH_SUN6I || MACH_SUN8I
      default 360 if MACH_SUN4I || MACH_SUN5I || MACH_SUN7I
+    default 672 if MACH_SUN50I
      ---help---
      Set the dram clock speed, valid range 240 - 480 (prior to sun9i),
      must be a multiple of 24. For the sun9i (A80), the tested values

@@ -164,6 +165,7 @@ config DRAM_ZQ
      default 123 if MACH_SUN4I || MACH_SUN5I || MACH_SUN6I ||

MACH_SUN8I
default 127 if MACH_SUN7I
default 4145117 if MACH_SUN9I

board/sunxi/Kconfig config DRAM_ODT_EN · line 176

Quoted 8 lines

board/sunxi/Kconfig +2 -0

+    default 3881915 if MACH_SUN50I
      ---help---
      Set the dram zq value.
  @@ -171,6 +173,7 @@ config DRAM_ODT_EN
      bool "sunxi dram odt enable"
      default n if !MACH_SUN8I_A23
      default y if MACH_SUN8I_A23
+    default y if MACH_SUN50I

This change isn't obvious. It's not mentioned in the patch description or removed from the defconfig.

Quoted 6 lines

Quoted 2 lines

Oh, right, looks like a rebase artifact, since I juggled the patches around quite a bit. It belongs to patch 10/10, I guess.

It is setting a DRAM default, so it is fine, as for it not being removed from the defconfig, it is not yet present in the one defconfig we have for a64. OTOH it would be food to remove the

No breakfast yet? ;-) ^^^^

Quoted 2 lines

DRAM_CLK and DRAM_ZQ values from configs/pine64_plus_defconfig as part of this patch.

Like the hunk below?

Cheers,
Andre.

Quoted 17 lines

Quoted 17 lines

Quoted 17 lines

Quoted 16 lines

---help---
Select this to enable dram odt (on die termination).
diff --git a/configs/pine64_plus_defconfig
b/configs/pine64_plus_defconfig
index 6d0198f..a53e968 100644

configs/pine64_plus_defconfig +0 -2

@@ -2,8 +2,6 @@ CONFIG_ARM=y
  CONFIG_ENABLE_ARM_SOC_BOOT0_HOOK=y
  CONFIG_ARCH_SUNXI=y
  CONFIG_MACH_SUN50I=y
-CONFIG_DRAM_CLK=672
-CONFIG_DRAM_ZQ=3881915
  CONFIG_DEFAULT_DEVICE_TREE="sun50i-a64-pine64-plus"
  # CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set
  CONFIG_CONSOLE_MUX=y

Read the specified "arch" value from a legacy or FIT U-Boot image and store it in our SPL data structure.
This allows loaders to take the target architecture in account for custom loading procedures.
Having the complete string -> arch mapping for FIT based images in the SPL would be too big, so we leave it up to architectures (or boards) to overwrite the weak function that does the actual translation, possibly covering only the required subset there.

Signed-off-by Andre Przywara <[email protected]>

---
 common/spl/spl.c     | 1 +
 common/spl/spl_fit.c | 8 ++++++++
 include/spl.h        | 3 ++-
 3 files changed, 11 insertions(+), 1 deletion(-)

common/spl/spl.c +1 -0

index bdb165a..f76ddd2 100644
@@ -114,6 +114,7 @@ int spl_parse_image_header(struct spl_image_info *spl_image,
                                header_size;
                }
                spl_image->os = image_get_os(header);
+               spl_image->arch = image_get_arch(header);
                spl_image->name = image_get_name(header);
                debug("spl: payload image: %.*s load addr: 0x%x size: %d\n",
                        (int)sizeof(spl_image->name), spl_image->name,

common/spl/spl_fit.c +0 -0

index aae556f..a5d903b 100644

common/spl/spl_fit.c +3 -0

 int spl_load_simple_fit(struct spl_image_info *spl_image,
                        struct spl_load_info *info, ulong sector, void *fit)
 {

@@ -136,6 +141,7 @@ int spl_load_simple_fit(struct spl_image_info *spl_image,
        int base_offset, align_len = ARCH_DMA_MINALIGN - 1;
        int src_sector;
        void *dst, *src;
+       const char *arch_str;
 
        /*
         * Figure out where the external images start. This is the base for the

@@ -184,10 +190,12 @@ int spl_load_simple_fit(struct spl_image_info *spl_image,
        data_offset = fdt_getprop_u32(fit, node, "data-offset");
        data_size = fdt_getprop_u32(fit, node, "data-size");
        load = fdt_getprop_u32(fit, node, "load");
+       arch_str = fdt_getprop(fit, node, "arch", NULL);
        debug("data_offset=%x, data_size=%x\n", data_offset, data_size);
        spl_image->load_addr = load;
        spl_image->entry_point = load;
        spl_image->os = IH_OS_U_BOOT;
+       spl_image->arch = spl_genimg_get_arch_id(arch_str);
 
        /*
         * Work out where to place the image. We read it so that the first

include/spl.h +0 -0

index e080a82..6a9d2fb 100644

include/spl.h +0 -1

 };
 
 /*
--

2.8.2

Quoted 9 lines

Read the specified "arch" value from a legacy or FIT U-Boot image and store it in our SPL data structure.
This allows loaders to take the target architecture in account for custom loading procedures.
Having the complete string -> arch mapping for FIT based images in the SPL would be too big, so we leave it up to architectures (or boards) to overwrite the weak function that does the actual translation, possibly covering only the required subset there.

Quoted 7 lines

Signed-off-by Andre Przywara <[email protected]>

---
 common/spl/spl.c     | 1 +
 common/spl/spl_fit.c | 8 ++++++++
 include/spl.h        | 3 ++-
 3 files changed, 11 insertions(+), 1 deletion(-)

Reviewed-by Simon Glass <[email protected]>

Quoted 21 lines

common/spl/spl.c +1 -0

index bdb165a..f76ddd2 100644
@@ -114,6 +114,7 @@ int spl_parse_image_header(struct spl_image_info *spl_image,
                                header_size;
                }
                spl_image->os = image_get_os(header);
+               spl_image->arch = image_get_arch(header);
                spl_image->name = image_get_name(header);
                debug("spl: payload image: %.*s load addr: 0x%x size: %d\n",
                        (int)sizeof(spl_image->name), spl_image->name,

common/spl/spl_fit.c +0 -0

index aae556f..a5d903b 100644
@@ -123,6 +123,11 @@ static int get_aligned_image_size(struct spl_load_info *info, int data_size,
        return (data_size + info->bl_len - 1) / info->bl_len;
 }
 

common/spl/spl_fit.c static int get_aligned_image_size(struct spl_load_info *info, int data_size, · line 130

Quoted 5 lines

common/spl/spl_fit.c +5 -0

+__weak u8 spl_genimg_get_arch_id(const char *arch_str)
+{
+       return IH_ARCH_DEFAULT;
+}
+

Do we need this weak function, or could we just add a normal function in the ARM code somewhere?

If you don't think we need much error checking you could do something like:

#ifdef CONFIG_ARM
... possible strcmp() here
return IH_ARCH_ARM;
#endif

Quoted 31 lines

common/spl/spl_fit.c +3 -0

 int spl_load_simple_fit(struct spl_image_info *spl_image,
                        struct spl_load_info *info, ulong sector, void *fit)
 {

@@ -136,6 +141,7 @@ int spl_load_simple_fit(struct spl_image_info *spl_image,
        int base_offset, align_len = ARCH_DMA_MINALIGN - 1;
        int src_sector;
        void *dst, *src;
+       const char *arch_str;
 
        /*
         * Figure out where the external images start. This is the base for the

@@ -184,10 +190,12 @@ int spl_load_simple_fit(struct spl_image_info *spl_image,
        data_offset = fdt_getprop_u32(fit, node, "data-offset");
        data_size = fdt_getprop_u32(fit, node, "data-size");
        load = fdt_getprop_u32(fit, node, "load");
+       arch_str = fdt_getprop(fit, node, "arch", NULL);
        debug("data_offset=%x, data_size=%x\n", data_offset, data_size);
        spl_image->load_addr = load;
        spl_image->entry_point = load;
        spl_image->os = IH_OS_U_BOOT;
+       spl_image->arch = spl_genimg_get_arch_id(arch_str);
 
        /*
         * Work out where to place the image. We read it so that the first

include/spl.h +0 -0

index e080a82..6a9d2fb 100644
@@ -22,11 +22,12 @@
 

include/spl.h line 30

Quoted 9 lines

include/spl.h +2 -1

 struct spl_image_info {
        const char *name;
-       u8 os;
        u32 load_addr;
        u32 entry_point;
        u32 size;
        u32 flags;
+       u8 os;
+       u8 arch;

Can you please add a struct comment?

On 05/11/16 16:10, Simon Glass wrote:

Hi Simon,

Quoted 18 lines · Andre Przywara

Quoted 9 lines

Read the specified "arch" value from a legacy or FIT U-Boot image and store it in our SPL data structure.
This allows loaders to take the target architecture in account for custom loading procedures.
Having the complete string -> arch mapping for FIT based images in the SPL would be too big, so we leave it up to architectures (or boards) to overwrite the weak function that does the actual translation, possibly covering only the required subset there.

Quoted 7 lines

Signed-off-by Andre Przywara <[email protected]>

---
 common/spl/spl.c     | 1 +
 common/spl/spl_fit.c | 8 ++++++++
 include/spl.h        | 3 ++-
 3 files changed, 11 insertions(+), 1 deletion(-)

Quoted 1 line

Reviewed-by Simon Glass <[email protected]>

Thanks!

Quoted 21 lines

Quoted 21 lines

common/spl/spl.c +1 -0

index bdb165a..f76ddd2 100644
@@ -114,6 +114,7 @@ int spl_parse_image_header(struct spl_image_info *spl_image,
                                header_size;
                }
                spl_image->os = image_get_os(header);
+               spl_image->arch = image_get_arch(header);
                spl_image->name = image_get_name(header);
                debug("spl: payload image: %.*s load addr: 0x%x size: %d\n",
                        (int)sizeof(spl_image->name), spl_image->name,

common/spl/spl_fit.c +0 -0

index aae556f..a5d903b 100644
@@ -123,6 +123,11 @@ static int get_aligned_image_size(struct spl_load_info *info, int data_size,
        return (data_size + info->bl_len - 1) / info->bl_len;
 }
 

Quoted 8 lines

common/spl/spl_fit.c static int get_aligned_image_size(struct spl_load_info *info, int data_size, · line 130

Quoted 5 lines

common/spl/spl_fit.c +5 -0

+__weak u8 spl_genimg_get_arch_id(const char *arch_str)
+{
+       return IH_ARCH_DEFAULT;
+}
+

Do we need this weak function, or could we just add a normal function in the ARM code somewhere?

Mmh, but this here is generic code. In a later patch I provide an ARM specific function under arch/arm, but so far this weak function just mimics the current behaviour: return the current architecture.

Quoted 2 lines

If you don't think we need much error checking you could do something like:

Quoted 4 lines

#ifdef CONFIG_ARM
... possible strcmp() here
return IH_ARCH_ARM;
#endif

So I found the weak function more elegant than #ifdef-ing architecture specific code into a generic file.

Is there any issue with weak functions, shall we avoid them?

Quoted 32 lines

Quoted 32 lines

common/spl/spl_fit.c +3 -0

 int spl_load_simple_fit(struct spl_image_info *spl_image,
                        struct spl_load_info *info, ulong sector, void *fit)
 {

@@ -136,6 +141,7 @@ int spl_load_simple_fit(struct spl_image_info *spl_image,
        int base_offset, align_len = ARCH_DMA_MINALIGN - 1;
        int src_sector;
        void *dst, *src;
+       const char *arch_str;
 
        /*
         * Figure out where the external images start. This is the base for the

@@ -184,10 +190,12 @@ int spl_load_simple_fit(struct spl_image_info *spl_image,
        data_offset = fdt_getprop_u32(fit, node, "data-offset");
        data_size = fdt_getprop_u32(fit, node, "data-size");
        load = fdt_getprop_u32(fit, node, "load");
+       arch_str = fdt_getprop(fit, node, "arch", NULL);
        debug("data_offset=%x, data_size=%x\n", data_offset, data_size);
        spl_image->load_addr = load;
        spl_image->entry_point = load;
        spl_image->os = IH_OS_U_BOOT;
+       spl_image->arch = spl_genimg_get_arch_id(arch_str);
 
        /*
         * Work out where to place the image. We read it so that the first

include/spl.h +0 -0

index e080a82..6a9d2fb 100644
@@ -22,11 +22,12 @@
 

Quoted 11 lines

include/spl.h line 30

Quoted 9 lines

include/spl.h +2 -1

 struct spl_image_info {
        const char *name;
-       u8 os;
        u32 load_addr;
        u32 entry_point;
        u32 size;
        u32 flags;
+       u8 os;
+       u8 arch;

Can you please add a struct comment?

Is that something specific / a special documentation format or do you mean just document the structure members?

Quoted 62 lines · Simon Glass

Hi Simon,

Quoted 18 lines · Andre Przywara

Quoted 9 lines

Read the specified "arch" value from a legacy or FIT U-Boot image and store it in our SPL data structure.
This allows loaders to take the target architecture in account for custom loading procedures.
Having the complete string -> arch mapping for FIT based images in the SPL would be too big, so we leave it up to architectures (or boards) to overwrite the weak function that does the actual translation, possibly covering only the required subset there.

Quoted 7 lines

Signed-off-by Andre Przywara <[email protected]>

---
 common/spl/spl.c     | 1 +
 common/spl/spl_fit.c | 8 ++++++++
 include/spl.h        | 3 ++-
 3 files changed, 11 insertions(+), 1 deletion(-)

Quoted 1 line

Reviewed-by Simon Glass <[email protected]>

Thanks!

Quoted 21 lines

Quoted 21 lines

common/spl/spl.c +1 -0

index bdb165a..f76ddd2 100644
@@ -114,6 +114,7 @@ int spl_parse_image_header(struct spl_image_info *spl_image,
                                header_size;
                }
                spl_image->os = image_get_os(header);
+               spl_image->arch = image_get_arch(header);
                spl_image->name = image_get_name(header);
                debug("spl: payload image: %.*s load addr: 0x%x size: %d\n",
                        (int)sizeof(spl_image->name), spl_image->name,

common/spl/spl_fit.c +0 -0

index aae556f..a5d903b 100644
@@ -123,6 +123,11 @@ static int get_aligned_image_size(struct spl_load_info *info, int data_size,
        return (data_size + info->bl_len - 1) / info->bl_len;
 }
 

Quoted 8 lines

common/spl/spl_fit.c static int get_aligned_image_size(struct spl_load_info *info, int data_size, · line 130

Quoted 5 lines

common/spl/spl_fit.c +5 -0

+__weak u8 spl_genimg_get_arch_id(const char *arch_str)
+{
+       return IH_ARCH_DEFAULT;
+}
+

Do we need this weak function, or could we just add a normal function in the ARM code somewhere?

Mmh, but this here is generic code. In a later patch I provide an ARM specific function under arch/arm, but so far this weak function just mimics the current behaviour: return the current architecture.

Quoted 2 lines

If you don't think we need much error checking you could do something like:

Quoted 7 lines

Quoted 4 lines

#ifdef CONFIG_ARM
... possible strcmp() here
return IH_ARCH_ARM;
#endif

So I found the weak function more elegant than #ifdef-ing architecture specific code into a generic file.

Fair enough.

Quoted 1 line

Is there any issue with weak functions, shall we avoid them?

Well they can be confusing since it's hard to know which function gets linked in. We have things like CONFIG_MISC_INIT_F, rather than make misc_init() weak.

Quoted 43 lines

Quoted 42 lines

Quoted 30 lines

common/spl/spl_fit.c +3 -0

 int spl_load_simple_fit(struct spl_image_info *spl_image,
                        struct spl_load_info *info, ulong sector, void *fit)
 {

@@ -136,6 +141,7 @@ int spl_load_simple_fit(struct spl_image_info *spl_image,
        int base_offset, align_len = ARCH_DMA_MINALIGN - 1;
        int src_sector;
        void *dst, *src;
+       const char *arch_str;
 
        /*
         * Figure out where the external images start. This is the base for the

@@ -184,10 +190,12 @@ int spl_load_simple_fit(struct spl_image_info *spl_image,
        data_offset = fdt_getprop_u32(fit, node, "data-offset");
        data_size = fdt_getprop_u32(fit, node, "data-size");
        load = fdt_getprop_u32(fit, node, "load");
+       arch_str = fdt_getprop(fit, node, "arch", NULL);
        debug("data_offset=%x, data_size=%x\n", data_offset, data_size);
        spl_image->load_addr = load;
        spl_image->entry_point = load;
        spl_image->os = IH_OS_U_BOOT;
+       spl_image->arch = spl_genimg_get_arch_id(arch_str);
 
        /*
         * Work out where to place the image. We read it so that the first

include/spl.h +0 -0

index e080a82..6a9d2fb 100644

include/spl.h line 30

Quoted 10 lines

include/spl.h +2 -1

@@ -22,11 +22,12 @@
 
 struct spl_image_info {
        const char *name;
-       u8 os;
        u32 load_addr;
        u32 entry_point;
        u32 size;
        u32 flags;
+       u8 os;
+       u8 arch;

Can you please add a struct comment?

Quoted 2 lines

Is that something specific / a special documentation format or do you mean just document the structure members?

The latter - see struct spl_load_info for an example. If we improve the code we change everyone wins :-)

On 19/11/16 13:49, Simon Glass wrote:

Hi Simon,

Quoted 73 lines · Andr? Przywara

Quoted 62 lines · Simon Glass

Hi Simon,

Quoted 18 lines · Andre Przywara

Quoted 9 lines

Read the specified "arch" value from a legacy or FIT U-Boot image and store it in our SPL data structure.
This allows loaders to take the target architecture in account for custom loading procedures.
Having the complete string -> arch mapping for FIT based images in the SPL would be too big, so we leave it up to architectures (or boards) to overwrite the weak function that does the actual translation, possibly covering only the required subset there.

Quoted 7 lines

Signed-off-by Andre Przywara <[email protected]>

---
 common/spl/spl.c     | 1 +
 common/spl/spl_fit.c | 8 ++++++++
 include/spl.h        | 3 ++-
 3 files changed, 11 insertions(+), 1 deletion(-)

Quoted 1 line

Reviewed-by Simon Glass <[email protected]>

Thanks!

Quoted 21 lines

Quoted 21 lines

common/spl/spl.c +1 -0

index bdb165a..f76ddd2 100644
@@ -114,6 +114,7 @@ int spl_parse_image_header(struct spl_image_info *spl_image,
                                header_size;
                }
                spl_image->os = image_get_os(header);
+               spl_image->arch = image_get_arch(header);
                spl_image->name = image_get_name(header);
                debug("spl: payload image: %.*s load addr: 0x%x size: %d\n",
                        (int)sizeof(spl_image->name), spl_image->name,

common/spl/spl_fit.c +0 -0

index aae556f..a5d903b 100644
@@ -123,6 +123,11 @@ static int get_aligned_image_size(struct spl_load_info *info, int data_size,
        return (data_size + info->bl_len - 1) / info->bl_len;
 }
 

Quoted 8 lines

common/spl/spl_fit.c static int get_aligned_image_size(struct spl_load_info *info, int data_size, · line 130

Quoted 5 lines

common/spl/spl_fit.c +5 -0

+__weak u8 spl_genimg_get_arch_id(const char *arch_str)
+{
+       return IH_ARCH_DEFAULT;
+}
+

Do we need this weak function, or could we just add a normal function in the ARM code somewhere?

Mmh, but this here is generic code. In a later patch I provide an ARM specific function under arch/arm, but so far this weak function just mimics the current behaviour: return the current architecture.

Quoted 2 lines

If you don't think we need much error checking you could do something like:

Quoted 7 lines

Quoted 4 lines

#ifdef CONFIG_ARM
... possible strcmp() here
return IH_ARCH_ARM;
#endif

So I found the weak function more elegant than #ifdef-ing architecture specific code into a generic file.

Fair enough.

Quoted 5 lines

Quoted 1 line

Is there any issue with weak functions, shall we avoid them?

Well they can be confusing since it's hard to know which function gets linked in. We have things like CONFIG_MISC_INIT_F, rather than make misc_init() weak.

I see. But I think in this case a weak function is neater. I will add a comment to the non-weak definition pointing out its nature, if that helps.

Quoted 44 lines

Quoted 44 lines

Quoted 43 lines

Quoted 31 lines

common/spl/spl_fit.c +3 -0

 int spl_load_simple_fit(struct spl_image_info *spl_image,
                        struct spl_load_info *info, ulong sector, void *fit)
 {

@@ -136,6 +141,7 @@ int spl_load_simple_fit(struct spl_image_info *spl_image,
        int base_offset, align_len = ARCH_DMA_MINALIGN - 1;
        int src_sector;
        void *dst, *src;
+       const char *arch_str;
 
        /*
         * Figure out where the external images start. This is the base for the

@@ -184,10 +190,12 @@ int spl_load_simple_fit(struct spl_image_info *spl_image,
        data_offset = fdt_getprop_u32(fit, node, "data-offset");
        data_size = fdt_getprop_u32(fit, node, "data-size");
        load = fdt_getprop_u32(fit, node, "load");
+       arch_str = fdt_getprop(fit, node, "arch", NULL);
        debug("data_offset=%x, data_size=%x\n", data_offset, data_size);
        spl_image->load_addr = load;
        spl_image->entry_point = load;
        spl_image->os = IH_OS_U_BOOT;
+       spl_image->arch = spl_genimg_get_arch_id(arch_str);
 
        /*
         * Work out where to place the image. We read it so that the first

include/spl.h +0 -0

index e080a82..6a9d2fb 100644

include/spl.h line 30

Quoted 10 lines

include/spl.h +2 -1

@@ -22,11 +22,12 @@
 
 struct spl_image_info {
        const char *name;
-       u8 os;
        u32 load_addr;
        u32 entry_point;
        u32 size;
        u32 flags;
+       u8 os;
+       u8 arch;

Can you please add a struct comment?

Quoted 5 lines

Quoted 2 lines

Is that something specific / a special documentation format or do you mean just document the structure members?

The latter - see struct spl_load_info for an example. If we improve the code we change everyone wins :-)

Yeah, sorry for that stupid question: When I just wanted to add something I saw what you meant in the struct next to it ;-)

The ARMv8 capable Allwinner A64 SoC comes out of reset in AArch32 mode. To run AArch64 code, we have to trigger a warm reset via the RMR register, which proceeds with code execution at the address stored in the RVBAR register.
If the bootable payload in the FIT image is using a different architecture than the SPL has been compiled for, enter it via this said RMR switch mechanism, by writing the entry point address into the MMIO mapped, writable version of the RVBAR register.
Then the warm reset is triggered via a system register write. If the payload architecture is the same as the SPL, we use the normal branch as usual.

Signed-off-by Andre Przywara <[email protected]>

---
 arch/arm/lib/spl.c               | 14 ++++++++++

 arch/arm/mach-sunxi/Makefile     |  1 +
 arch/arm/mach-sunxi/spl_switch.c | 60 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 75 insertions(+)
 create mode 100644 arch/arm/mach-sunxi/spl_switch.c

Hi,

this is my first take on the SPL support for the Allwinner A64 SoC. The actual meat - the DRAM initialization code - has been provided by Jens - many thanks for that!
The rest of the patches mostly deal with the 32-bit/64-bit switch.

While it is possible and seems natural to let the SPL also run in 64-bit, this creates a really large binary (32600 Bytes in my case). With some hacks (plus some fixes to make the SPL 64-bit safe) I got this to work,

but any addition will probably break it and exceed the 32KB limit that the BROM imposes. Debug is the first obvious victim here.

Quoted 10 lines · Andre Przywara

Hi,

this is my first take on the SPL support for the Allwinner A64 SoC. The actual meat - the DRAM initialization code - has been provided by Jens - many thanks for that!
The rest of the patches mostly deal with the 32-bit/64-bit switch.

While it is possible and seems natural to let the SPL also run in 64-bit, this creates a really large binary (32600 Bytes in my case). With some hacks (plus some fixes to make the SPL 64-bit safe) I got this to work,

So how about we merge the 64bit version first (since that's *way* easier to compile for everyone) and then consider the move to 32bit afterwards? I don't even want to start to imagine how to squeeze a 32bit SPL build into the build process for our U-Boot binaries.

Quoted 2 lines

but any addition will probably break it and exceed the 32KB limit that the BROM imposes. Debug is the first obvious victim here.

Do you have some section size comparisons between the two?

<Adding Peter Robinson to the Cc to see how much he will
object my packaging ideas>

Hi,

First of all cool stuff! Thank you Andre and all others involved for making this happen.

Quoted 18 lines · Alexander Graf

Quoted 11 lines · Andre Przywara

Hi,

this is my first take on the SPL support for the Allwinner A64 SoC. The actual meat - the DRAM initialization code - has been provided by Jens - many thanks for that!
The rest of the patches mostly deal with the 32-bit/64-bit switch.

While it is possible and seems natural to let the SPL also run in 64-bit,
this creates a really large binary (32600 Bytes in my case). With some hacks (plus some fixes to make the SPL 64-bit safe) I got this to work,

So how about we merge the 64bit version first (since that's *way* easier to compile for everyone) and then consider the move to 32bit afterwards? I don't even want to start to imagine how to squeeze a 32bit SPL build into the build process for our U-Boot binaries.

Quoted 4 lines

Quoted 2 lines

but any addition will probably break it and exceed the 32KB limit that the BROM imposes. Debug is the first obvious victim here.

Do you have some section size comparisons between the two?

Later down in the mail Andre says that in 32 bit (thumb) mode the size goes down to 20KB which gives us a lot more head-room then the 32600 out of 32768 bytes available for the 64 bit version.

With that said I agree with you (Alex) that having a 32 bit SPL + 64 bit u-boot proper is worry-some from a distro pov.

What I would like to see is:

1) Ensure we keep a pure 64 bit build working, which should lead to a fully functional u-boot-sunxi-with-spl.bin just like how things work for current 32 bit boards. This will give distros an easy way to deal with this and is also easier for users who built from source (like people who to the occasional contribution but are not really into u-boot).

2) Offer the 32 bit option in the do 2 builds, combine 32 bit SPL with 64 bit u-boot proper (and ATF) as Andre is suggesting as an option.

We may need to strip some features from "1" in the future, e.g. no NAND support.

For distros we could then still opt to use 2, to e.g. get NAND support. One solution I have in mind is:

a) Do a native 32 bit build for the 32 bit SPL, store the SPL somewhere (just like we store u-boot-sunxi-with-spl.bin for end users to dd for other 32 bit boards), on Fedora this build would generate uboot-images-armv7.noarch.rpm (we already put all the generated u-boot-sunxi-with-spl.bin files in this noarch rpm).

b) Do a native 64 bit build which stores both a 64 bit u-boot-sunxi-with-spl.bin as well as just a u-boot.bin (unless we can extract the latter easy enough), on fedora this build would generate uboot-images-armv8.noarch.rpm (as we already do).

c) Have a separate noarch (in rpm terms) package which depends on the 2 packages build from a. and b. this is This can then just combine the results from a. and b. into a mixed 32 + 64 bit u-boot-sunxi-with-spl.bin we could call this uboot-images-arm-mixed.noarch.rpm :)

So although not pretty from the Fedora pov I can see ways to work around things and even be able to use "2" in Fedora.

Regards,

Hans

Hi,

Quoted 34 lines · Hans de Goede

<Adding Peter Robinson to the Cc to see how much he will
object my packaging ideas>

Hi,

First of all cool stuff! Thank you Andre and all others involved for making this happen.

Quoted 12 lines · Alexander Graf

Quoted 11 lines · Andre Przywara

Hi,

this is my first take on the SPL support for the Allwinner A64 SoC. The actual meat - the DRAM initialization code - has been provided by Jens - many thanks for that!
The rest of the patches mostly deal with the 32-bit/64-bit switch.

While it is possible and seems natural to let the SPL also run in 64-bit,
this creates a really large binary (32600 Bytes in my case). With some hacks (plus some fixes to make the SPL 64-bit safe) I got this to work,

Quoted 8 lines

So how about we merge the 64bit version first (since that's *way* easier to compile for everyone) and then consider the move to 32bit afterwards? I don't even want to start to imagine how to squeeze a 32bit SPL build into the build process for our U-Boot binaries.

Quoted 2 lines

but any addition will probably break it and exceed the 32KB limit that the BROM imposes. Debug is the first obvious victim here.

Do you have some section size comparisons between the two?

Later down in the mail Andre says that in 32 bit (thumb) mode the size goes down to 20KB which gives us a lot more head-room then the 32600 out of 32768 bytes available for the 64 bit version.

Quoted 2 lines

With that said I agree with you (Alex) that having a 32 bit SPL + 64 bit u-boot proper is worry-some from a distro pov.

What's even nastier is the requirement of a cross compiler even for a native build. Do Fedora and Suse offer packaged cross-compilers for the other ARM bitness, respectively?

Quoted 37 lines · Andre Przywara

Hi,

Quoted 34 lines · Hans de Goede

<Adding Peter Robinson to the Cc to see how much he will
object my packaging ideas>

Hi,

First of all cool stuff! Thank you Andre and all others involved for making this happen.

Quoted 12 lines · Alexander Graf

Quoted 11 lines · Andre Przywara

Hi,

this is my first take on the SPL support for the Allwinner A64 SoC. The actual meat - the DRAM initialization code - has been provided by Jens - many thanks for that!
The rest of the patches mostly deal with the 32-bit/64-bit switch.

While it is possible and seems natural to let the SPL also run in 64-bit,
this creates a really large binary (32600 Bytes in my case). With some hacks (plus some fixes to make the SPL 64-bit safe) I got this to work,

Quoted 8 lines

So how about we merge the 64bit version first (since that's *way* easier to compile for everyone) and then consider the move to 32bit afterwards? I don't even want to start to imagine how to squeeze a 32bit SPL build into the build process for our U-Boot binaries.

Quoted 2 lines

but any addition will probably break it and exceed the 32KB limit that the BROM imposes. Debug is the first obvious victim here.

Do you have some section size comparisons between the two?

Later down in the mail Andre says that in 32 bit (thumb) mode the size goes down to 20KB which gives us a lot more head-room then the 32600 out of 32768 bytes available for the 64 bit version.

Quoted 5 lines

Quoted 2 lines

With that said I agree with you (Alex) that having a 32 bit SPL + 64 bit u-boot proper is worry-some from a distro pov.

What's even nastier is the requirement of a cross compiler even for a native build. Do Fedora and Suse offer packaged cross-compilers for the other ARM bitness, respectively?

Andreas Faerber was working on cross compilers in openSUSE, but I don't think they're part of the distribution yet.

Hi,

Quoted 41 lines · Alexander Graf

Quoted 40 lines · Andre Przywara

Hi,

Quoted 37 lines · Hans de Goede

<Adding Peter Robinson to the Cc to see how much he will
object my packaging ideas>

Hi,

First of all cool stuff! Thank you Andre and all others involved for making this happen.

Quoted 19 lines · Alexander Graf

Quoted 7 lines · Andre Przywara

Hi,

this is my first take on the SPL support for the Allwinner A64 SoC. The actual meat - the DRAM initialization code - has been provided by Jens - many thanks for that!
The rest of the patches mostly deal with the 32-bit/64-bit switch.

Quoted 6 lines

While it is possible and seems natural to let the SPL also run in 64-bit,
this creates a really large binary (32600 Bytes in my case). With some
hacks (plus some fixes to make the SPL 64-bit safe) I got this to work,

So how about we merge the 64bit version first (since that's *way* easier to compile for everyone) and then consider the move to 32bit afterwards? I don't even want to start to imagine how to squeeze a 32bit SPL build into the build process for our U-Boot binaries.

Quoted 4 lines

Quoted 3 lines

but any addition will probably break it and exceed the 32KB limit that
the BROM imposes. Debug is the first obvious victim here.

Do you have some section size comparisons between the two?

Later down in the mail Andre says that in 32 bit (thumb) mode the size goes down to 20KB which gives us a lot more head-room then the 32600 out of 32768 bytes available for the 64 bit version.

Quoted 8 lines

Quoted 5 lines

Quoted 2 lines

With that said I agree with you (Alex) that having a 32 bit SPL + 64 bit u-boot proper is worry-some from a distro pov.

What's even nastier is the requirement of a cross compiler even for a native build. Do Fedora and Suse offer packaged cross-compilers for the other ARM bitness, respectively?

Andreas Faerber was working on cross compilers in openSUSE, but I don't think they're part of the distribution yet.

Fedora does have cross-compilers (for kernel and u-boot use, no support for userland stuff). But as I outlined in my other mail, with modern rpm you can do a build on all archs (which you want to do anyways to get a native mkimage everywhere) and then use %ifarch armv7hl / %ifarch aarch64 to build u-boot binaries when doing a native build on armv7 / aarch64 and store all the build binaries (one binary per supported board) under /usr/share/u-boot and put /usr/share/u-boot in a noarch sub-pkg, and Fedora's compose tools will then make that noarch sub-pkg from that one specific arch build available in the repos for all archs.

Combine this with an extra noarch package which depends on the noarch u-boot-images rpms generated by both the 32 and 64 bit build and that extra package can generate a combined img ready for dd-ing to a sdcard. If you solve it like this you do not need cross compilation. Yes this is not pretty, but it will work (at least for Fedora, I do not know how Suse's tools deal with noarch sub-packages of a non noarch rpm, rpm itself can handle this, but something needs to copy the generated noarch sub-rpm to the other archs).

Quoted 7 lines · Andre Przywara

Hi,

this is my first take on the SPL support for the Allwinner A64 SoC. The actual meat - the DRAM initialization code - has been provided by Jens - many thanks for that!
The rest of the patches mostly deal with the 32-bit/64-bit switch.

Quoted 3 lines

While it is possible and seems natural to let the SPL also run in 64-bit, this creates a really large binary (32600 Bytes in my case). With some hacks (plus some fixes to make the SPL 64-bit safe) I got this to work,

So how about we merge the 64bit version first (since that's *way* easier to compile for everyone) and then consider the move to 32bit afterwards?

I don't even want to start to imagine how to squeeze a 32bit SPL build into the build process for our U-Boot binaries.

Quoted 2 lines

but any addition will probably break it and exceed the 32KB limit that the BROM imposes. Debug is the first obvious victim here.

Do you have some section size comparisons between the two?

Hi,

Quoted 8 lines · Alexander Graf

Quoted 7 lines · Andre Przywara

Hi,

this is my first take on the SPL support for the Allwinner A64 SoC. The actual meat - the DRAM initialization code - has been provided by Jens - many thanks for that!
The rest of the patches mostly deal with the 32-bit/64-bit switch.

Quoted 6 lines

Quoted 3 lines

While it is possible and seems natural to let the SPL also run in 64-bit, this creates a really large binary (32600 Bytes in my case). With some hacks (plus some fixes to make the SPL 64-bit safe) I got this to work,

So how about we merge the 64bit version first (since that's *way* easier to compile for everyone) and then consider the move to 32bit afterwards?

Mmmh, interesting idea, may be worth a try alone for the sake of the 64-bit fixes to the SPL code I have done in this process. But I am not sure it doesn't already break for people using just a different compiler. I even started to chop of some bytes here and there to bring the size down (I gained 200 Bytes at the ctype implementation, yeah!)

Quoted 2 lines

I don't even want to start to imagine how to squeeze a 32bit SPL build into the build process for our U-Boot binaries.

I totally agree, even for me it's quite a pain, because a "make clean" (which you need in between) removes the build result of that other bitness, so you always have to remember to copy the binaries first (and then up using stale copies afterwards).

Quoted 3 lines

Quoted 2 lines

but any addition will probably break it and exceed the 32KB limit that the BROM imposes. Debug is the first obvious victim here.

Quoted 1 line

Do you have some section size comparisons between the two?

I spent some time into looking at readelf dumps to find the reason for the bigger size, but nothing really stood out. Still it is a bit odd, since the size ratio for U-Boot proper is much better (like +20% for 64-bit).

A promising approach I then tried was to use -mabi=ilp32, which GCC itself supports for quite a while already. But that was running into ICEs, with no obviously bogus code. If someone more compiler-savvy could take a look later, I'd be grateful.

That being said I will try to revive the AArch64 port tonight and push this somewhere, so that people can have a look.

Cheers,
Andre.

Hi,

Quoted 8 lines · Alexander Graf

Quoted 7 lines · Andre Przywara

Hi,

this is my first take on the SPL support for the Allwinner A64 SoC. The actual meat - the DRAM initialization code - has been provided by Jens - many thanks for that!
The rest of the patches mostly deal with the 32-bit/64-bit switch.

Quoted 6 lines

Quoted 3 lines

While it is possible and seems natural to let the SPL also run in 64-bit, this creates a really large binary (32600 Bytes in my case). With some hacks (plus some fixes to make the SPL 64-bit safe) I got this to work,

So how about we merge the 64bit version first (since that's *way* easier to compile for everyone) and then consider the move to 32bit afterwards?

Mmmh, interesting idea, may be worth a try alone for the sake of the 64-bit fixes to the SPL code I have done in this process. But I am not sure it doesn't already break for people using just a different compiler. I even started to chop of some bytes here and there to bring the size down (I gained 200 Bytes at the ctype implementation, yeah!)

Quoted 2 lines

I don't even want to start to imagine how to squeeze a 32bit SPL build into the build process for our U-Boot binaries.

I totally agree, even for me it's quite a pain, because a "make clean" (which you need in between) removes the build result of that other bitness, so you always have to remember to copy the binaries first (and then up using stale copies afterwards).

Quoted 4 lines

Quoted 2 lines

but any addition will probably break it and exceed the 32KB limit that the BROM imposes. Debug is the first obvious victim here.

Do you have some section size comparisons between the two?

I spent some time into looking at readelf dumps to find the reason for the bigger size, but nothing really stood out. Still it is a bit odd, since the size ratio for U-Boot proper is much better (like +20% for 64-bit).
A promising approach I then tried was to use -mabi=ilp32, which GCC itself supports for quite a while already. But that was running into ICEs, with no obviously bogus code. If someone more compiler-savvy could take a look later, I'd be grateful.

That being said I will try to revive the AArch64 port tonight and push this somewhere, so that people can have a look.