[U-Boot] [RFC PATCH 00/10] sunxi: Allwinner A64 SPL support

[U-Boot] [RFC PATCH 00/10] sunxi: Allwinner A64 SPL support

[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,
but any addition will probably break it and exceed the 32KB limit that
the BROM imposes. Debug is the first obvious victim here.

So this series lets the SPL run entirely in AArch32, which is what the
BROM comes from anyway. With the help of patch 2/10 and 8/10 we detect
that the next binary should be run in AArch64 mode, so patch 9/10 adds
the required RMR switch to enter the 64-bit mode.
It compiles for me into less than 20 KB, so plenty of room for debug
and future code. Technically running the SPL in 32-bit or 64-bit does
not make any difference, actually having it in AArch32 helps FEL boot
(which works fine with this series).

The only real drawback of this approach is the build process, which is
now split between the SPL (AArch32) and the U-Boot proper (AArch64).
Patch 10/10 introduces a new defconfig, which defines CPU_V7 and thus
creates a 32-bit binary. This requires an ARM cross-compiler or a
native compile on an ARM board. The resulting sunxi-spl.bin can then
be either directly written to the SD card or later combined with the
AArch64 U-Boot proper into one image file.

What this series still lacks is proper Linux support, because we miss
the ARM Trusted Firmware (ATF) binary to be loaded and executed.
I have patches to extend the SPL FIT support to handle this very nicely
in a generic way, but these patches need some clean up. I didn't want
this series to be held back any longer, so I will send the FIT patches
later.

I appreciate any comments on this series!

Cheers,
Andre.

Andre Przywara (7):
  sun6i: Restrict some register initialization to Allwinner A31 SoC
  Makefile: use "arm64" architecture for U-Boot image files
  sunxi: provide default DRAM config for sun50i in Kconfig
  sunxi: H3/A64: fix non-ODT setting
  SPL: read and store arch property from U-Boot image
  sunxi: introduce RMR switch to enter payloads in 64-bit mode
  sunxi: A64: add 32-bit SPL support

Jens Kuske (3):
  sunxi: H3: add and rename some DRAM contoller registers
  sunxi: H3: add DRAM controller single bit delay support
  sunxi: A64: use H3 DRAM initialization code for A64

 Makefile                                        |   9 +-
 arch/arm/include/asm/arch-sunxi/clock_sun6i.h   |   1 +
 arch/arm/include/asm/arch-sunxi/dram.h          |   2 +-
 arch/arm/include/asm/arch-sunxi/dram_sun8i_h3.h |  51 +++---
 arch/arm/lib/spl.c                              |  14 ++
 arch/arm/mach-sunxi/Makefile                    |   2 +
 arch/arm/mach-sunxi/clock_sun6i.c               |   7 +-
 arch/arm/mach-sunxi/dram_sun8i_h3.c             | 213 +++++++++++++++++-------
 arch/arm/mach-sunxi/spl_switch.c                |  60 +++++++
 board/sunxi/Kconfig                             |  17 +-
 common/spl/spl.c                                |   1 +
 common/spl/spl_fit.c                            |   8 +
 configs/pine64_plus_defconfig                   |   4 +-
 configs/sun50i_spl32_defconfig                  |  11 ++
 include/spl.h                                   |   3 +-
 15 files changed, 308 insertions(+), 95 deletions(-)
 create mode 100644 arch/arm/mach-sunxi/spl_switch.c
 create mode 100644 configs/sun50i_spl32_defconfig

-- 
2.8.2

[U-Boot] [RFC PATCH 01/10] sun6i: Restrict some register initialization to Allwinner A31 SoC

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

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 arch/arm/mach-sunxi/clock_sun6i.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/mach-sunxi/clock_sun6i.c b/arch/arm/mach-sunxi/clock_sun6i.c
index ed8cd9b..382fa94 100644
--- a/arch/arm/mach-sunxi/clock_sun6i.c
+++ b/arch/arm/mach-sunxi/clock_sun6i.c
@@ -21,6 +21,8 @@ void clock_init_safe(void)
 {
 	struct sunxi_ccm_reg * const ccm =
 		(struct sunxi_ccm_reg *)SUNXI_CCM_BASE;
+
+#ifdef CONFIG_MACH_SUN6I
 	struct sunxi_prcm_reg * const prcm =
 		(struct sunxi_prcm_reg *)SUNXI_PRCM_BASE;
 
@@ -31,6 +33,7 @@ void clock_init_safe(void)
 		PRCM_PLL_CTRL_LDO_DIGITAL_EN | PRCM_PLL_CTRL_LDO_ANALOG_EN |
 		PRCM_PLL_CTRL_EXT_OSC_EN | PRCM_PLL_CTRL_LDO_OUT_L(1140));
 	clrbits_le32(&prcm->pll_ctrl1, PRCM_PLL_CTRL_LDO_KEY_MASK);
+#endif
 
 	clock_set_pll1(408000000);
 
@@ -41,7 +44,9 @@ void clock_init_safe(void)
 	writel(AHB1_ABP1_DIV_DEFAULT, &ccm->ahb1_apb1_div);
 
 	writel(MBUS_CLK_DEFAULT, &ccm->mbus0_clk_cfg);
+#ifdef CONFIG_MACH_SUN6I
 	writel(MBUS_CLK_DEFAULT, &ccm->mbus1_clk_cfg);
+#endif
 }
 #endif
 
-- 
2.8.2

[U-Boot] [RFC PATCH 02/10] Makefile: use "arm64" architecture for U-Boot image files

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

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 Makefile | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 37cbcb2..e76aeff 100644
--- a/Makefile
+++ b/Makefile
@@ -912,13 +912,18 @@ quiet_cmd_cpp_cfg = CFG     $@
 cmd_cpp_cfg = $(CPP) -Wp,-MD,$(depfile) $(cpp_flags) $(LDPPFLAGS) -ansi \
 	-DDO_DEPS_ONLY -D__ASSEMBLY__ -x assembler-with-cpp -P -dM -E -o $@ $<
 
+ifeq ($(CPU),armv8)
+IH_ARCH := arm64
+else
+IH_ARCH := $(ARCH)
+endif
 ifdef CONFIG_SPL_LOAD_FIT
-MKIMAGEFLAGS_u-boot.img = -f auto -A $(ARCH) -T firmware -C none -O u-boot \
+MKIMAGEFLAGS_u-boot.img = -f auto -A $(IH_ARCH) -T firmware -C none -O u-boot \
 	-a $(CONFIG_SYS_TEXT_BASE) -e $(CONFIG_SYS_UBOOT_START) \
 	-n "U-Boot $(UBOOTRELEASE) for $(BOARD) board" -E \
 	$(patsubst %,-b arch/$(ARCH)/dts/%.dtb,$(subst ",,$(CONFIG_OF_LIST)))
 else
-MKIMAGEFLAGS_u-boot.img = -A $(ARCH) -T firmware -C none -O u-boot \
+MKIMAGEFLAGS_u-boot.img = -A $(IH_ARCH) -T firmware -C none -O u-boot \
 	-a $(CONFIG_SYS_TEXT_BASE) -e $(CONFIG_SYS_UBOOT_START) \
 	-n "U-Boot $(UBOOTRELEASE) for $(BOARD) board"
 endif
-- 
2.8.2

[U-Boot] [RFC PATCH 03/10] sunxi: provide default DRAM config for sun50i in Kconfig

[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 <andre.przywara@arm.com>
---
 board/sunxi/Kconfig           | 3 +++
 configs/pine64_plus_defconfig | 2 --
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig
index e1d4ab1..d281a65 100644
--- a/board/sunxi/Kconfig
+++ b/board/sunxi/Kconfig
@@ -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.
 
@@ -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
 	---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
--- a/configs/pine64_plus_defconfig
+++ b/configs/pine64_plus_defconfig
@@ -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
-- 
2.8.2

[U-Boot] [RFC PATCH 04/10] sunxi: H3: add and rename some DRAM contoller registers

[Andre Przywara]

From: Jens Kuske <jenskuske@gmail.com>

The IOCR registers got renamed to BDLR to match the public
documentation of similar controllers.

Signed-off-by: Jens Kuske <jenskuske@gmail.com>
Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 arch/arm/include/asm/arch-sunxi/dram_sun8i_h3.h | 43 ++++++++++++++-----------
 arch/arm/mach-sunxi/dram_sun8i_h3.c             | 34 +++++++++----------
 2 files changed, 41 insertions(+), 36 deletions(-)

diff --git a/arch/arm/include/asm/arch-sunxi/dram_sun8i_h3.h b/arch/arm/include/asm/arch-sunxi/dram_sun8i_h3.h
index d0f2b8a..867fd12 100644
--- a/arch/arm/include/asm/arch-sunxi/dram_sun8i_h3.h
+++ b/arch/arm/include/asm/arch-sunxi/dram_sun8i_h3.h
@@ -81,7 +81,7 @@ struct sunxi_mctl_ctl_reg {
 	u32 rfshtmg;		/* 0x90 refresh timing */
 	u32 rfshctl1;		/* 0x94 */
 	u32 pwrtmg;		/* 0x98 */
-	u8  res3[0x20];		/* 0x9c */
+	u8 res3[0x20];		/* 0x9c */
 	u32 dqsgmr;		/* 0xbc */
 	u32 dtcr;		/* 0xc0 */
 	u32 dtar[4];		/* 0xc4 */
@@ -106,20 +106,23 @@ struct sunxi_mctl_ctl_reg {
 	u32 perfhpr[2];		/* 0x1c4 */
 	u32 perflpr[2];		/* 0x1cc */
 	u32 perfwr[2];		/* 0x1d4 */
-	u8 res8[0x2c];		/* 0x1dc */
-	u32 aciocr;		/* 0x208 */
-	u8 res9[0xf4];		/* 0x20c */
+	u8 res8[0x24];		/* 0x1dc */
+	u32 acmdlr;		/* 0x200 AC master delay line register */
+	u32 aclcdlr;		/* 0x204 AC local calibrated delay line register */
+	u32 aciocr;		/* 0x208 AC I/O configuration register */
+	u8 res9[0x4];		/* 0x20c */
+	u32 acbdlr[31];		/* 0x210 AC bit delay line registers */
+	u8 res10[0x74];		/* 0x28c */
 	struct {		/* 0x300 DATX8 modules*/
-		u32 mdlr;		/* 0x00 */
-		u32 lcdlr[3];		/* 0x04 */
-		u32 iocr[11];		/* 0x10 IO configuration register */
-		u32 bdlr6;		/* 0x3c */
-		u32 gtr;		/* 0x40 */
-		u32 gcr;		/* 0x44 */
-		u32 gsr[3];		/* 0x48 */
+		u32 mdlr;		/* 0x00 master delay line register */
+		u32 lcdlr[3];		/* 0x04 local calibrated delay line registers */
+		u32 bdlr[12];		/* 0x10 bit delay line registers */
+		u32 gtr;		/* 0x40 general timing register */
+		u32 gcr;		/* 0x44 general configuration register */
+		u32 gsr[3];		/* 0x48 general status registers */
 		u8 res0[0x2c];		/* 0x54 */
-	} datx[4];
-	u8 res10[0x388];	/* 0x500 */
+	} dx[4];
+	u8 res11[0x388];	/* 0x500 */
 	u32 upd2;		/* 0x888 */
 };
 
@@ -174,12 +177,14 @@ struct sunxi_mctl_ctl_reg {
 
 #define ZQCR_PWRDOWN	(0x1 << 31)	/* ZQ power down */
 
-#define DATX_IOCR_DQ(x)	(x)		/* DQ0-7 IOCR index */
-#define DATX_IOCR_DM	(8)		/* DM IOCR index */
-#define DATX_IOCR_DQS	(9)		/* DQS IOCR index */
-#define DATX_IOCR_DQSN	(10)		/* DQSN IOCR index */
+#define ACBDLR_WRITE_DELAY(x)	((x) << 8)
 
-#define DATX_IOCR_WRITE_DELAY(x)	((x) << 8)
-#define DATX_IOCR_READ_DELAY(x)		((x) << 0)
+#define DXBDLR_DQ(x)	(x)		/* DQ0-7 BDLR index */
+#define DXBDLR_DM	(8)		/* DM BDLR index */
+#define DXBDLR_DQS	(9)		/* DQS BDLR index */
+#define DXBDLR_DQSN	(10)		/* DQSN BDLR index */
+
+#define DXBDLR_WRITE_DELAY(x)	((x) << 8)
+#define DXBDLR_READ_DELAY(x)	((x) << 0)
 
 #endif /* _SUNXI_DRAM_SUN8I_H3_H */
diff --git a/arch/arm/mach-sunxi/dram_sun8i_h3.c b/arch/arm/mach-sunxi/dram_sun8i_h3.c
index b08b8e6..3dd6803 100644
--- a/arch/arm/mach-sunxi/dram_sun8i_h3.c
+++ b/arch/arm/mach-sunxi/dram_sun8i_h3.c
@@ -72,21 +72,21 @@ static void mctl_dq_delay(u32 read, u32 write)
 	u32 val;
 
 	for (i = 0; i < 4; i++) {
-		val = DATX_IOCR_WRITE_DELAY((write >> (i * 4)) & 0xf) |
-		      DATX_IOCR_READ_DELAY(((read >> (i * 4)) & 0xf) * 2);
+		val = DXBDLR_WRITE_DELAY((write >> (i * 4)) & 0xf) |
+		      DXBDLR_READ_DELAY(((read >> (i * 4)) & 0xf) * 2);
 
-		for (j = DATX_IOCR_DQ(0); j <= DATX_IOCR_DM; j++)
-			writel(val, &mctl_ctl->datx[i].iocr[j]);
+		for (j = DXBDLR_DQ(0); j <= DXBDLR_DM; j++)
+			writel(val, &mctl_ctl->dx[i].bdlr[j]);
 	}
 
 	clrbits_le32(&mctl_ctl->pgcr[0], 1 << 26);
 
 	for (i = 0; i < 4; i++) {
-		val = DATX_IOCR_WRITE_DELAY((write >> (16 + i * 4)) & 0xf) |
-		      DATX_IOCR_READ_DELAY((read >> (16 + i * 4)) & 0xf);
+		val = DXBDLR_WRITE_DELAY((write >> (16 + i * 4)) & 0xf) |
+		      DXBDLR_READ_DELAY((read >> (16 + i * 4)) & 0xf);
 
-		writel(val, &mctl_ctl->datx[i].iocr[DATX_IOCR_DQS]);
-		writel(val, &mctl_ctl->datx[i].iocr[DATX_IOCR_DQSN]);
+		writel(val, &mctl_ctl->dx[i].bdlr[DXBDLR_DQS]);
+		writel(val, &mctl_ctl->dx[i].bdlr[DXBDLR_DQSN]);
 	}
 
 	setbits_le32(&mctl_ctl->pgcr[0], 1 << 26);
@@ -344,7 +344,7 @@ static int mctl_channel_init(struct dram_para *para)
 
 	/* set dramc odt */
 	for (i = 0; i < 4; i++)
-		clrsetbits_le32(&mctl_ctl->datx[i].gcr, (0x3 << 4) |
+		clrsetbits_le32(&mctl_ctl->dx[i].gcr, (0x3 << 4) |
 				(0x1 << 1) | (0x3 << 2) | (0x3 << 12) |
 				(0x3 << 14),
 				IS_ENABLED(CONFIG_DRAM_ODT_EN) ? 0x0 : 0x2);
@@ -364,8 +364,8 @@ static int mctl_channel_init(struct dram_para *para)
 
 	/* set half DQ */
 	if (para->bus_width != 32) {
-		writel(0x0, &mctl_ctl->datx[2].gcr);
-		writel(0x0, &mctl_ctl->datx[3].gcr);
+		writel(0x0, &mctl_ctl->dx[2].gcr);
+		writel(0x0, &mctl_ctl->dx[3].gcr);
 	}
 
 	/* data training configuration */
@@ -386,17 +386,17 @@ static int mctl_channel_init(struct dram_para *para)
 	/* detect ranks and bus width */
 	if (readl(&mctl_ctl->pgsr[0]) & (0xfe << 20)) {
 		/* only one rank */
-		if (((readl(&mctl_ctl->datx[0].gsr[0]) >> 24) & 0x2) ||
-		    ((readl(&mctl_ctl->datx[1].gsr[0]) >> 24) & 0x2)) {
+		if (((readl(&mctl_ctl->dx[0].gsr[0]) >> 24) & 0x2) ||
+		    ((readl(&mctl_ctl->dx[1].gsr[0]) >> 24) & 0x2)) {
 			clrsetbits_le32(&mctl_ctl->dtcr, 0xf << 24, 0x1 << 24);
 			para->dual_rank = 0;
 		}
 
 		/* only half DQ width */
-		if (((readl(&mctl_ctl->datx[2].gsr[0]) >> 24) & 0x1) ||
-		    ((readl(&mctl_ctl->datx[3].gsr[0]) >> 24) & 0x1)) {
-			writel(0x0, &mctl_ctl->datx[2].gcr);
-			writel(0x0, &mctl_ctl->datx[3].gcr);
+		if (((readl(&mctl_ctl->dx[2].gsr[0]) >> 24) & 0x1) ||
+		    ((readl(&mctl_ctl->dx[3].gsr[0]) >> 24) & 0x1)) {
+			writel(0x0, &mctl_ctl->dx[2].gcr);
+			writel(0x0, &mctl_ctl->dx[3].gcr);
 			para->bus_width = 16;
 		}
 
-- 
2.8.2

[U-Boot] [RFC PATCH 05/10] sunxi: H3: add DRAM controller single bit delay support

[Andre Przywara]

From: Jens Kuske <jenskuske@gmail.com>

Instead of setting the delay for whole bytes allow setting
it for each individual bit. Also add support for
address/command lane delays.

Signed-off-by: Jens Kuske <jenskuske@gmail.com>
Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 arch/arm/mach-sunxi/dram_sun8i_h3.c | 54 ++++++++++++++++++-------------------
 1 file changed, 27 insertions(+), 27 deletions(-)

diff --git a/arch/arm/mach-sunxi/dram_sun8i_h3.c b/arch/arm/mach-sunxi/dram_sun8i_h3.c
index 3dd6803..1647d76 100644
--- a/arch/arm/mach-sunxi/dram_sun8i_h3.c
+++ b/arch/arm/mach-sunxi/dram_sun8i_h3.c
@@ -16,12 +16,13 @@
 #include <linux/kconfig.h>
 
 struct dram_para {
-	u32 read_delays;
-	u32 write_delays;
 	u16 page_size;
 	u8 bus_width;
 	u8 dual_rank;
 	u8 row_bits;
+	const u8 dx_read_delays[4][11];
+	const u8 dx_write_delays[4][11];
+	const u8 ac_delays[31];
 };
 
 static inline int ns_to_t(int nanoseconds)
@@ -64,34 +65,25 @@ static void mctl_phy_init(u32 val)
 	mctl_await_completion(&mctl_ctl->pgsr[0], PGSR_INIT_DONE, 0x1);
 }
 
-static void mctl_dq_delay(u32 read, u32 write)
+static void mctl_set_bit_delays(struct dram_para *para)
 {
 	struct sunxi_mctl_ctl_reg * const mctl_ctl =
 			(struct sunxi_mctl_ctl_reg *)SUNXI_DRAM_CTL0_BASE;
 	int i, j;
-	u32 val;
-
-	for (i = 0; i < 4; i++) {
-		val = DXBDLR_WRITE_DELAY((write >> (i * 4)) & 0xf) |
-		      DXBDLR_READ_DELAY(((read >> (i * 4)) & 0xf) * 2);
-
-		for (j = DXBDLR_DQ(0); j <= DXBDLR_DM; j++)
-			writel(val, &mctl_ctl->dx[i].bdlr[j]);
-	}
 
 	clrbits_le32(&mctl_ctl->pgcr[0], 1 << 26);
 
-	for (i = 0; i < 4; i++) {
-		val = DXBDLR_WRITE_DELAY((write >> (16 + i * 4)) & 0xf) |
-		      DXBDLR_READ_DELAY((read >> (16 + i * 4)) & 0xf);
+	for (i = 0; i < 4; i++)
+		for (j = 0; j < 11; j++)
+			writel(DXBDLR_WRITE_DELAY(para->dx_write_delays[i][j]) |
+			       DXBDLR_READ_DELAY(para->dx_read_delays[i][j]),
+			       &mctl_ctl->dx[i].bdlr[j]);
 
-		writel(val, &mctl_ctl->dx[i].bdlr[DXBDLR_DQS]);
-		writel(val, &mctl_ctl->dx[i].bdlr[DXBDLR_DQSN]);
-	}
+	for (i = 0; i < 31; i++)
+		writel(ACBDLR_WRITE_DELAY(para->ac_delays[i]),
+		       &mctl_ctl->acbdlr[i]);
 
 	setbits_le32(&mctl_ctl->pgcr[0], 1 << 26);
-
-	udelay(1);
 }
 
 static void mctl_set_master_priority(void)
@@ -372,11 +364,8 @@ static int mctl_channel_init(struct dram_para *para)
 	clrsetbits_le32(&mctl_ctl->dtcr, 0xf << 24,
 			(para->dual_rank ? 0x3 : 0x1) << 24);
 
-
-	if (para->read_delays || para->write_delays) {
-		mctl_dq_delay(para->read_delays, para->write_delays);
-		udelay(50);
-	}
+	mctl_set_bit_delays(para);
+	udelay(50);
 
 	mctl_zq_calibration(para);
 
@@ -458,12 +447,23 @@ unsigned long sunxi_dram_init(void)
 			(struct sunxi_mctl_ctl_reg *)SUNXI_DRAM_CTL0_BASE;
 
 	struct dram_para para = {
-		.read_delays = 0x00007979,	/* dram_tpr12 */
-		.write_delays = 0x6aaa0000,	/* dram_tpr11 */
 		.dual_rank = 0,
 		.bus_width = 32,
 		.row_bits = 15,
 		.page_size = 4096,
+
+		.dx_read_delays =  {{ 18, 18, 18, 18, 18, 18, 18, 18, 18,  0,  0 },
+		                    { 14, 14, 14, 14, 14, 14, 14, 14, 14,  0,  0 },
+		                    { 18, 18, 18, 18, 18, 18, 18, 18, 18,  0,  0 },
+		                    { 14, 14, 14, 14, 14, 14, 14, 14, 14,  0,  0 }},
+		.dx_write_delays = {{  0,  0,  0,  0,  0,  0,  0,  0,  0, 10, 10 },
+		                    {  0,  0,  0,  0,  0,  0,  0,  0,  0, 10, 10 },
+		                    {  0,  0,  0,  0,  0,  0,  0,  0,  0, 10, 10 },
+		                    {  0,  0,  0,  0,  0,  0,  0,  0,  0,  6,  6 }},
+		.ac_delays = {  0,  0,  0,  0,  0,  0,  0,  0,
+		                0,  0,  0,  0,  0,  0,  0,  0,
+		                0,  0,  0,  0,  0,  0,  0,  0,
+		                0,  0,  0,  0,  0,  0,  0      },
 	};
 
 	mctl_sys_init(&para);
-- 
2.8.2

[U-Boot] [RFC PATCH 06/10] sunxi: A64: use H3 DRAM initialization code for A64

[Andre Przywara]

From: Jens Kuske <jenskuske@gmail.com>

The A64 DRAM controller is very similar to the H3 one,
so the code can be reused with some small changes.
[Andre: fixed up typo, merged in fixes from Jens]

Signed-off-by: Jens Kuske <jenskuske@gmail.com>
Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 arch/arm/include/asm/arch-sunxi/clock_sun6i.h   |   1 +
 arch/arm/include/asm/arch-sunxi/dram.h          |   2 +-
 arch/arm/include/asm/arch-sunxi/dram_sun8i_h3.h |  10 +-
 arch/arm/mach-sunxi/Makefile                    |   1 +
 arch/arm/mach-sunxi/clock_sun6i.c               |   2 +-
 arch/arm/mach-sunxi/dram_sun8i_h3.c             | 139 +++++++++++++++++++-----
 6 files changed, 123 insertions(+), 32 deletions(-)

diff --git a/arch/arm/include/asm/arch-sunxi/clock_sun6i.h b/arch/arm/include/asm/arch-sunxi/clock_sun6i.h
index be9fcfd..3f87672 100644
--- a/arch/arm/include/asm/arch-sunxi/clock_sun6i.h
+++ b/arch/arm/include/asm/arch-sunxi/clock_sun6i.h
@@ -322,6 +322,7 @@ struct sunxi_ccm_reg {
 #define CCM_DRAMCLK_CFG_DIV0_MASK	(0xf << 8)
 #define CCM_DRAMCLK_CFG_SRC_PLL5	(0x0 << 20)
 #define CCM_DRAMCLK_CFG_SRC_PLL6x2	(0x1 << 20)
+#define CCM_DRAMCLK_CFG_SRC_PLL11	(0x1 << 20) /* A64 only */
 #define CCM_DRAMCLK_CFG_SRC_MASK	(0x3 << 20)
 #define CCM_DRAMCLK_CFG_UPD		(0x1 << 16)
 #define CCM_DRAMCLK_CFG_RST		(0x1 << 31)
diff --git a/arch/arm/include/asm/arch-sunxi/dram.h b/arch/arm/include/asm/arch-sunxi/dram.h
index e0be744..53e6d47 100644
--- a/arch/arm/include/asm/arch-sunxi/dram.h
+++ b/arch/arm/include/asm/arch-sunxi/dram.h
@@ -24,7 +24,7 @@
 #include <asm/arch/dram_sun8i_a33.h>
 #elif defined(CONFIG_MACH_SUN8I_A83T)
 #include <asm/arch/dram_sun8i_a83t.h>
-#elif defined(CONFIG_MACH_SUN8I_H3)
+#elif defined(CONFIG_MACH_SUN8I_H3) || defined(CONFIG_MACH_SUN50I)
 #include <asm/arch/dram_sun8i_h3.h>
 #elif defined(CONFIG_MACH_SUN9I)
 #include <asm/arch/dram_sun9i.h>
diff --git a/arch/arm/include/asm/arch-sunxi/dram_sun8i_h3.h b/arch/arm/include/asm/arch-sunxi/dram_sun8i_h3.h
index 867fd12..b0e5d93 100644
--- a/arch/arm/include/asm/arch-sunxi/dram_sun8i_h3.h
+++ b/arch/arm/include/asm/arch-sunxi/dram_sun8i_h3.h
@@ -15,7 +15,8 @@
 
 struct sunxi_mctl_com_reg {
 	u32 cr;			/* 0x00 control register */
-	u8 res0[0xc];		/* 0x04 */
+	u8 res0[0x8];		/* 0x04 */
+	u32 tmr;		/* 0x0c (A64 only) */
 	u32 mcr[16][2];		/* 0x10 */
 	u32 bwcr;		/* 0x90 bandwidth control register */
 	u32 maer;		/* 0x94 master enable register */
@@ -32,7 +33,9 @@ struct sunxi_mctl_com_reg {
 	u32 swoffr;		/* 0xc4 */
 	u8 res2[0x8];		/* 0xc8 */
 	u32 cccr;		/* 0xd0 */
-	u8 res3[0x72c];		/* 0xd4 */
+	u8 res3[0x54];		/* 0xd4 */
+	u32 mdfs_bwlr[3];	/* 0x128 (A64 only) */
+	u8 res4[0x6cc];		/* 0x134 */
 	u32 protect;		/* 0x800 */
 };
 
@@ -81,7 +84,8 @@ struct sunxi_mctl_ctl_reg {
 	u32 rfshtmg;		/* 0x90 refresh timing */
 	u32 rfshctl1;		/* 0x94 */
 	u32 pwrtmg;		/* 0x98 */
-	u8 res3[0x20];		/* 0x9c */
+	u8 res3[0x1c];		/* 0x9c */
+	u32 vtfcr;		/* 0xb8 (A64 only) */
 	u32 dqsgmr;		/* 0xbc */
 	u32 dtcr;		/* 0xc0 */
 	u32 dtar[4];		/* 0xc4 */
diff --git a/arch/arm/mach-sunxi/Makefile b/arch/arm/mach-sunxi/Makefile
index e73114e..7daba11 100644
--- a/arch/arm/mach-sunxi/Makefile
+++ b/arch/arm/mach-sunxi/Makefile
@@ -50,4 +50,5 @@ obj-$(CONFIG_MACH_SUN8I_A33)	+= dram_sun8i_a33.o
 obj-$(CONFIG_MACH_SUN8I_A83T)	+= dram_sun8i_a83t.o
 obj-$(CONFIG_MACH_SUN8I_H3)	+= dram_sun8i_h3.o
 obj-$(CONFIG_MACH_SUN9I)	+= dram_sun9i.o
+obj-$(CONFIG_MACH_SUN50I)	+= dram_sun8i_h3.o
 endif
diff --git a/arch/arm/mach-sunxi/clock_sun6i.c b/arch/arm/mach-sunxi/clock_sun6i.c
index 382fa94..4570060 100644
--- a/arch/arm/mach-sunxi/clock_sun6i.c
+++ b/arch/arm/mach-sunxi/clock_sun6i.c
@@ -218,7 +218,7 @@ done:
 }
 #endif
 
-#ifdef CONFIG_MACH_SUN8I_A33
+#if defined(CONFIG_MACH_SUN8I_A33) || defined(CONFIG_MACH_SUN50I)
 void clock_set_pll11(unsigned int clk, bool sigma_delta_enable)
 {
 	struct sunxi_ccm_reg * const ccm =
diff --git a/arch/arm/mach-sunxi/dram_sun8i_h3.c b/arch/arm/mach-sunxi/dram_sun8i_h3.c
index 1647d76..2dc2071 100644
--- a/arch/arm/mach-sunxi/dram_sun8i_h3.c
+++ b/arch/arm/mach-sunxi/dram_sun8i_h3.c
@@ -32,30 +32,6 @@ static inline int ns_to_t(int nanoseconds)
 	return DIV_ROUND_UP(ctrl_freq * nanoseconds, 1000);
 }
 
-static u32 bin_to_mgray(int val)
-{
-	static const u8 lookup_table[32] = {
-		0x00, 0x01, 0x02, 0x03, 0x06, 0x07, 0x04, 0x05,
-		0x0c, 0x0d, 0x0e, 0x0f, 0x0a, 0x0b, 0x08, 0x09,
-		0x18, 0x19, 0x1a, 0x1b, 0x1e, 0x1f, 0x1c, 0x1d,
-		0x14, 0x15, 0x16, 0x17, 0x12, 0x13, 0x10, 0x11,
-	};
-
-	return lookup_table[clamp(val, 0, 31)];
-}
-
-static int mgray_to_bin(u32 val)
-{
-	static const u8 lookup_table[32] = {
-		0x00, 0x01, 0x02, 0x03, 0x06, 0x07, 0x04, 0x05,
-		0x0e, 0x0f, 0x0c, 0x0d, 0x08, 0x09, 0x0a, 0x0b,
-		0x1e, 0x1f, 0x1c, 0x1d, 0x18, 0x19, 0x1a, 0x1b,
-		0x10, 0x11, 0x12, 0x13, 0x16, 0x17, 0x14, 0x15,
-	};
-
-	return lookup_table[val & 0x1f];
-}
-
 static void mctl_phy_init(u32 val)
 {
 	struct sunxi_mctl_ctl_reg * const mctl_ctl =
@@ -91,8 +67,9 @@ static void mctl_set_master_priority(void)
 	struct sunxi_mctl_com_reg * const mctl_com =
 			(struct sunxi_mctl_com_reg *)SUNXI_DRAM_COM_BASE;
 
+#if defined(CONFIG_MACH_SUN8I_H3)
 	/* enable bandwidth limit windows and set windows size 1us */
-	writel(0x00010190, &mctl_com->bwcr);
+	writel((1 << 16) | (400 << 0), &mctl_com->bwcr);
 
 	/* set cpu high priority */
 	writel(0x00000001, &mctl_com->mapr);
@@ -121,6 +98,38 @@ static void mctl_set_master_priority(void)
 	writel(0x04001800, &mctl_com->mcr[10][1]);
 	writel(0x04000009, &mctl_com->mcr[11][0]);
 	writel(0x00400120, &mctl_com->mcr[11][1]);
+#elif defined(CONFIG_MACH_SUN50I)
+	/* enable bandwidth limit windows and set windows size 1us */
+	writel(399, &mctl_com->tmr);
+	writel((1 << 16), &mctl_com->bwcr);
+
+	writel(0x00a0000d, &mctl_com->mcr[0][0]);
+	writel(0x00500064, &mctl_com->mcr[0][1]);
+	writel(0x06000009, &mctl_com->mcr[1][0]);
+	writel(0x01000578, &mctl_com->mcr[1][1]);
+	writel(0x0200000d, &mctl_com->mcr[2][0]);
+	writel(0x00600100, &mctl_com->mcr[2][1]);
+	writel(0x01000009, &mctl_com->mcr[3][0]);
+	writel(0x00500064, &mctl_com->mcr[3][1]);
+	writel(0x07000009, &mctl_com->mcr[4][0]);
+	writel(0x01000640, &mctl_com->mcr[4][1]);
+	writel(0x01000009, &mctl_com->mcr[5][0]);
+	writel(0x00000080, &mctl_com->mcr[5][1]);
+	writel(0x01000009, &mctl_com->mcr[6][0]);
+	writel(0x00400080, &mctl_com->mcr[6][1]);
+	writel(0x0100000d, &mctl_com->mcr[7][0]);
+	writel(0x00400080, &mctl_com->mcr[7][1]);
+	writel(0x0100000d, &mctl_com->mcr[8][0]);
+	writel(0x00400080, &mctl_com->mcr[8][1]);
+	writel(0x04000009, &mctl_com->mcr[9][0]);
+	writel(0x00400100, &mctl_com->mcr[9][1]);
+	writel(0x20000209, &mctl_com->mcr[10][0]);
+	writel(0x08001800, &mctl_com->mcr[10][1]);
+	writel(0x05000009, &mctl_com->mcr[11][0]);
+	writel(0x00400090, &mctl_com->mcr[11][1]);
+
+	writel(0x81000004, &mctl_com->mdfs_bwlr[2]);
+#endif
 }
 
 static void mctl_set_timing_params(struct dram_para *para)
@@ -204,7 +213,32 @@ static void mctl_set_timing_params(struct dram_para *para)
 	writel(RFSHTMG_TREFI(trefi) | RFSHTMG_TRFC(trfc), &mctl_ctl->rfshtmg);
 }
 
-static void mctl_zq_calibration(struct dram_para *para)
+#ifdef CONFIG_MACH_SUN8I_H3
+static u32 bin_to_mgray(int val)
+{
+	static const u8 lookup_table[32] = {
+		0x00, 0x01, 0x02, 0x03, 0x06, 0x07, 0x04, 0x05,
+		0x0c, 0x0d, 0x0e, 0x0f, 0x0a, 0x0b, 0x08, 0x09,
+		0x18, 0x19, 0x1a, 0x1b, 0x1e, 0x1f, 0x1c, 0x1d,
+		0x14, 0x15, 0x16, 0x17, 0x12, 0x13, 0x10, 0x11,
+	};
+
+	return lookup_table[clamp(val, 0, 31)];
+}
+
+static int mgray_to_bin(u32 val)
+{
+	static const u8 lookup_table[32] = {
+		0x00, 0x01, 0x02, 0x03, 0x06, 0x07, 0x04, 0x05,
+		0x0e, 0x0f, 0x0c, 0x0d, 0x08, 0x09, 0x0a, 0x0b,
+		0x1e, 0x1f, 0x1c, 0x1d, 0x18, 0x19, 0x1a, 0x1b,
+		0x10, 0x11, 0x12, 0x13, 0x16, 0x17, 0x14, 0x15,
+	};
+
+	return lookup_table[val & 0x1f];
+}
+
+static void mctl_h3_zq_calibration_quirk(struct dram_para *para)
 {
 	struct sunxi_mctl_ctl_reg * const mctl_ctl =
 			(struct sunxi_mctl_ctl_reg *)SUNXI_DRAM_CTL0_BASE;
@@ -261,6 +295,7 @@ static void mctl_zq_calibration(struct dram_para *para)
 		writel((zq_val[5] << 16) | zq_val[4], &mctl_ctl->zqdr[2]);
 	}
 }
+#endif
 
 static void mctl_set_cr(struct dram_para *para)
 {
@@ -286,16 +321,27 @@ static void mctl_sys_init(struct dram_para *para)
 	clrbits_le32(&ccm->ahb_gate0, 1 << AHB_GATE_OFFSET_MCTL);
 	clrbits_le32(&ccm->ahb_reset0_cfg, 1 << AHB_RESET_OFFSET_MCTL);
 	clrbits_le32(&ccm->pll5_cfg, CCM_PLL5_CTRL_EN);
+#ifdef CONFIG_MACH_SUN50I
+	clrbits_le32(&ccm->pll11_cfg, CCM_PLL11_CTRL_EN);
+#endif
 	udelay(10);
 
 	clrbits_le32(&ccm->dram_clk_cfg, CCM_DRAMCLK_CFG_RST);
 	udelay(1000);
 
+#ifdef CONFIG_MACH_SUN50I
+	clock_set_pll11(CONFIG_DRAM_CLK * 2 * 1000000, false);
+	clrsetbits_le32(&ccm->dram_clk_cfg,
+			CCM_DRAMCLK_CFG_DIV_MASK | CCM_DRAMCLK_CFG_SRC_MASK,
+			CCM_DRAMCLK_CFG_DIV(1) | CCM_DRAMCLK_CFG_SRC_PLL11 |
+			CCM_DRAMCLK_CFG_UPD);
+#else
 	clock_set_pll5(CONFIG_DRAM_CLK * 2 * 1000000, false);
 	clrsetbits_le32(&ccm->dram_clk_cfg,
 			CCM_DRAMCLK_CFG_DIV_MASK | CCM_DRAMCLK_CFG_SRC_MASK,
 			CCM_DRAMCLK_CFG_DIV(1) | CCM_DRAMCLK_CFG_SRC_PLL5 |
 			CCM_DRAMCLK_CFG_UPD);
+#endif
 	mctl_await_completion(&ccm->dram_clk_cfg, CCM_DRAMCLK_CFG_UPD, 0);
 
 	setbits_le32(&ccm->ahb_reset0_cfg, 1 << AHB_RESET_OFFSET_MCTL);
@@ -347,12 +393,18 @@ static int mctl_channel_init(struct dram_para *para)
 	/* set DQS auto gating PD mode */
 	setbits_le32(&mctl_ctl->pgcr[2], 0x3 << 6);
 
+#if defined(CONFIG_MACH_SUN8I_H3)
 	/* dx ddr_clk & hdr_clk dynamic mode */
 	clrbits_le32(&mctl_ctl->pgcr[0], (0x3 << 14) | (0x3 << 12));
 
 	/* dphy & aphy phase select 270 degree */
 	clrsetbits_le32(&mctl_ctl->pgcr[2], (0x3 << 10) | (0x3 << 8),
 			(0x1 << 10) | (0x2 << 8));
+#elif defined(CONFIG_MACH_SUN50I)
+	/* dphy & aphy phase select ? */
+	clrsetbits_le32(&mctl_ctl->pgcr[2], (0x3 << 10) | (0x3 << 8),
+			(0x0 << 10) | (0x3 << 8));
+#endif
 
 	/* set half DQ */
 	if (para->bus_width != 32) {
@@ -367,10 +419,17 @@ static int mctl_channel_init(struct dram_para *para)
 	mctl_set_bit_delays(para);
 	udelay(50);
 
-	mctl_zq_calibration(para);
+#ifdef CONFIG_MACH_SUN8I_H3
+	mctl_h3_zq_calibration_quirk(para);
 
 	mctl_phy_init(PIR_PLLINIT | PIR_DCAL | PIR_PHYRST | PIR_DRAMRST |
 		      PIR_DRAMINIT | PIR_QSGATE);
+#else
+	clrsetbits_le32(&mctl_ctl->zqcr, 0xffffff, CONFIG_DRAM_ZQ);
+
+	mctl_phy_init(PIR_ZCAL | PIR_PLLINIT | PIR_DCAL | PIR_PHYRST |
+		      PIR_DRAMRST | PIR_DRAMINIT | PIR_QSGATE);
+#endif
 
 	/* detect ranks and bus width */
 	if (readl(&mctl_ctl->pgsr[0]) & (0xfe << 20)) {
@@ -408,7 +467,11 @@ static int mctl_channel_init(struct dram_para *para)
 	udelay(10);
 
 	/* set PGCR3, CKE polarity */
+#ifdef CONFIG_MACH_SUN50I
+	writel(0xc0aa0060, &mctl_ctl->pgcr[3]);
+#else
 	writel(0x00aa0060, &mctl_ctl->pgcr[3]);
+#endif
 
 	/* power down zq calibration module for power save */
 	setbits_le32(&mctl_ctl->zqcr, ZQCR_PWRDOWN);
@@ -452,6 +515,7 @@ unsigned long sunxi_dram_init(void)
 		.row_bits = 15,
 		.page_size = 4096,
 
+#if defined(CONFIG_MACH_SUN8I_H3)
 		.dx_read_delays =  {{ 18, 18, 18, 18, 18, 18, 18, 18, 18,  0,  0 },
 		                    { 14, 14, 14, 14, 14, 14, 14, 14, 14,  0,  0 },
 		                    { 18, 18, 18, 18, 18, 18, 18, 18, 18,  0,  0 },
@@ -464,6 +528,20 @@ unsigned long sunxi_dram_init(void)
 		                0,  0,  0,  0,  0,  0,  0,  0,
 		                0,  0,  0,  0,  0,  0,  0,  0,
 		                0,  0,  0,  0,  0,  0,  0      },
+#elif defined(CONFIG_MACH_SUN50I)
+		.dx_read_delays =  {{ 16, 16, 16, 16, 17, 16, 16, 17, 16,  1,  0 },
+		                    { 17, 17, 17, 17, 17, 17, 17, 17, 17,  1,  0 },
+		                    { 16, 17, 17, 16, 16, 16, 16, 16, 16,  0,  0 },
+		                    { 17, 17, 17, 17, 17, 17, 17, 17, 17,  1,  0 }},
+		.dx_write_delays = {{  0,  0,  0,  0,  0,  0,  0,  0,  0, 15, 15 },
+		                    {  0,  0,  0,  0,  1,  1,  1,  1,  0, 10, 10 },
+		                    {  1,  0,  1,  1,  1,  1,  1,  1,  0, 11, 11 },
+		                    {  1,  0,  0,  1,  1,  1,  1,  1,  0, 12, 12 }},
+		.ac_delays = {  5,  5, 13, 10,  2,  5,  3,  3,
+		                0,  3,  3,  3,  1,  0,  0,  0,
+		                3,  4,  0,  3,  4,  1,  4,  0,
+		                1,  1,  0,  1, 13,  5,  4      },
+#endif
 	};
 
 	mctl_sys_init(&para);
@@ -476,8 +554,15 @@ unsigned long sunxi_dram_init(void)
 		writel(0x00000201, &mctl_ctl->odtmap);
 	udelay(1);
 
+#ifdef CONFIG_MACH_SUN8I_H3
 	/* odt delay */
 	writel(0x0c000400, &mctl_ctl->odtcfg);
+#endif
+
+#ifdef CONFIG_MACH_SUN50I
+	setbits_le32(&mctl_ctl->vtfcr, (1 << 9));
+	clrbits_le32(&mctl_ctl->pgcr[2], (1 << 13));
+#endif
 
 	/* clear credit value */
 	setbits_le32(&mctl_com->cccr, 1 << 31);
-- 
2.8.2

[U-Boot] [RFC PATCH 07/10] sunxi: H3/A64: fix non-ODT setting

[Andre Przywara]

According to Jens disabling the on-die-termination should set bit 5,
not bit 1 in the respective register. Fix this.

Reported-by: Jens Kuske <jenskuske@gmail.com>
Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 arch/arm/mach-sunxi/dram_sun8i_h3.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mach-sunxi/dram_sun8i_h3.c b/arch/arm/mach-sunxi/dram_sun8i_h3.c
index 2dc2071..3d569fc 100644
--- a/arch/arm/mach-sunxi/dram_sun8i_h3.c
+++ b/arch/arm/mach-sunxi/dram_sun8i_h3.c
@@ -385,7 +385,7 @@ static int mctl_channel_init(struct dram_para *para)
 		clrsetbits_le32(&mctl_ctl->dx[i].gcr, (0x3 << 4) |
 				(0x1 << 1) | (0x3 << 2) | (0x3 << 12) |
 				(0x3 << 14),
-				IS_ENABLED(CONFIG_DRAM_ODT_EN) ? 0x0 : 0x2);
+				IS_ENABLED(CONFIG_DRAM_ODT_EN) ? 0x0 : 0x20);
 
 	/* AC PDR should always ON */
 	setbits_le32(&mctl_ctl->aciocr, 0x1 << 1);
-- 
2.8.2

[U-Boot] [RFC PATCH 08/10] SPL: read and store arch property from U-Boot image

[Andre Przywara]

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 <andre.przywara@arm.com>
---
 common/spl/spl.c     | 1 +
 common/spl/spl_fit.c | 8 ++++++++
 include/spl.h        | 3 ++-
 3 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/common/spl/spl.c b/common/spl/spl.c
index bdb165a..f76ddd2 100644
--- a/common/spl/spl.c
+++ b/common/spl/spl.c
@@ -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,
diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
index aae556f..a5d903b 100644
--- a/common/spl/spl_fit.c
+++ b/common/spl/spl_fit.c
@@ -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;
 }
 
+__weak u8 spl_genimg_get_arch_id(const char *arch_str)
+{
+	return IH_ARCH_DEFAULT;
+}
+
 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
diff --git a/include/spl.h b/include/spl.h
index e080a82..6a9d2fb 100644
--- a/include/spl.h
+++ b/include/spl.h
@@ -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;
 };
 
 /*
-- 
2.8.2

[U-Boot] [RFC PATCH 09/10] sunxi: introduce RMR switch to enter payloads in 64-bit mode

[Andre Przywara]

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 <andre.przywara@arm.com>
---
 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

diff --git a/arch/arm/lib/spl.c b/arch/arm/lib/spl.c
index e606d47..ae3d81c 100644
--- a/arch/arm/lib/spl.c
+++ b/arch/arm/lib/spl.c
@@ -63,3 +63,17 @@ void __noreturn jump_to_image_linux(struct spl_image_info *spl_image, void *arg)
 	image_entry(0, machid, arg);
 }
 #endif
+
+u8 spl_genimg_get_arch_id(const char *arch_str)
+{
+	if (!arch_str)
+		return IH_ARCH_DEFAULT;
+
+	if (!strcmp(arch_str, "arm"))
+		return IH_ARCH_ARM;
+
+	if (!strcmp(arch_str, "arm64"))
+		return IH_ARCH_ARM64;
+
+	return IH_ARCH_DEFAULT;
+}
diff --git a/arch/arm/mach-sunxi/Makefile b/arch/arm/mach-sunxi/Makefile
index 7daba11..128091e 100644
--- a/arch/arm/mach-sunxi/Makefile
+++ b/arch/arm/mach-sunxi/Makefile
@@ -51,4 +51,5 @@ obj-$(CONFIG_MACH_SUN8I_A83T)	+= dram_sun8i_a83t.o
 obj-$(CONFIG_MACH_SUN8I_H3)	+= dram_sun8i_h3.o
 obj-$(CONFIG_MACH_SUN9I)	+= dram_sun9i.o
 obj-$(CONFIG_MACH_SUN50I)	+= dram_sun8i_h3.o
+obj-$(CONFIG_MACH_SUN50I)	+= spl_switch.o
 endif
diff --git a/arch/arm/mach-sunxi/spl_switch.c b/arch/arm/mach-sunxi/spl_switch.c
new file mode 100644
index 0000000..20f21b1
--- /dev/null
+++ b/arch/arm/mach-sunxi/spl_switch.c
@@ -0,0 +1,60 @@
+/*
+ * (C) Copyright 2016 ARM Ltd.
+ *
+ * SPDX-License-Identifier:     GPL-2.0+
+ */
+
+#include <common.h>
+#include <spl.h>
+
+#include <asm/io.h>
+#include <asm/barriers.h>
+
+static void __noreturn jump_to_image_native(struct spl_image_info *spl_image)
+{
+	typedef void __noreturn (*image_entry_noargs_t)(void);
+
+	image_entry_noargs_t image_entry =
+				(image_entry_noargs_t)spl_image->entry_point;
+
+	image_entry();
+}
+
+static void __noreturn reset_rmr_switch(void)
+{
+#ifdef CONFIG_ARM64
+	__asm__ volatile ( "mrs	 x0, RMR_EL3\n\t"
+			   "bic  x0, x0, #1\n\t"   /* Clear enter-in-64 bit */
+			   "orr  x0, x0, #2\n\t"   /* set reset request bit */
+			   "msr  RMR_EL3, x0\n\t"
+			   "isb  sy\n\t"
+			   "nop\n\t"
+			   "wfi\n\t"
+			   "b    .\n"
+			   ::: "x0");
+#else
+	__asm__ volatile ( "mrc  15, 0, r0, cr12, cr0, 2\n\t"
+			   "orr  r0, r0, #3\n\t"   /* request reset in 64 bit */
+			   "mcr  15, 0, r0, cr12, cr0, 2\n\t"
+			   "isb\n\t"
+			   "nop\n\t"
+			   "wfi\n\t"
+			   "b    .\n"
+			   ::: "r0");
+#endif
+	while (1);	/* to avoid a compiler warning about __noreturn */
+}
+
+void __noreturn jump_to_image_no_args(struct spl_image_info *spl_image)
+{
+	if (spl_image->arch == IH_ARCH_DEFAULT) {
+		debug("entering by branch\n");
+		jump_to_image_native(spl_image);
+	} else {
+		debug("entering by RMR switch\n");
+		writel(spl_image->entry_point, 0x17000a0);
+		DSB;
+		ISB;
+		reset_rmr_switch();
+	}
+}
-- 
2.8.2

[U-Boot] [RFC PATCH 10/10] sunxi: A64: add 32-bit SPL support

[Andre Przywara]

When compiling the SPL for the Allwinner A64 in AArch64 mode, we can't
use the more compact Thumb2 encoding, which only exists for AArch32
code. This makes the SPL rather big, up to a point where any code
additions or even a different compiler may easily exceed the 32KB limit
that the Allwinner BROM imposes.
Introduce a separate, mostly generic sun50i-a64 configuration, which
defines the CPU_V7 symbol and thus will create a 32-bit binary using
the memory-saving Thumb2 encoding.
This should only be used for the SPL, the U-Boot proper should still be
using the existing 64-bit configuration. The SPL code can switch to
AArch64 if needed, so a 32-bit SPL can be combined with a 64-bit U-Boot
proper to eventually launch arm64 kernels.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 board/sunxi/Kconfig            | 14 ++++++++++++--
 configs/pine64_plus_defconfig  |  2 +-
 configs/sun50i_spl32_defconfig | 11 +++++++++++
 3 files changed, 24 insertions(+), 3 deletions(-)
 create mode 100644 configs/sun50i_spl32_defconfig

diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig
index d281a65..da8785a 100644
--- a/board/sunxi/Kconfig
+++ b/board/sunxi/Kconfig
@@ -43,6 +43,10 @@ config SUNXI_GEN_SUN6I
 	watchdog, etc.
 
 
+config MACH_SUN50I
+	bool
+	select SUNXI_GEN_SUN6I
+
 choice
 	prompt "Sunxi SoC Variant"
 	optional
@@ -121,10 +125,16 @@ config MACH_SUN9I
 	select SUNXI_GEN_SUN6I
 	select SUPPORT_SPL
 
-config MACH_SUN50I
+config MACH_SUN50I_64
 	bool "sun50i (Allwinner A64)"
+	select MACH_SUN50I
 	select ARM64
-	select SUNXI_GEN_SUN6I
+
+config MACH_SUN50I_32
+	bool "sun50i (Allwinner A64) SPL-32bit"
+	select MACH_SUN50I
+	select CPU_V7
+	select SUPPORT_SPL
 
 endchoice
 
diff --git a/configs/pine64_plus_defconfig b/configs/pine64_plus_defconfig
index a53e968..634652e 100644
--- a/configs/pine64_plus_defconfig
+++ b/configs/pine64_plus_defconfig
@@ -1,7 +1,7 @@
 CONFIG_ARM=y
 CONFIG_ENABLE_ARM_SOC_BOOT0_HOOK=y
 CONFIG_ARCH_SUNXI=y
-CONFIG_MACH_SUN50I=y
+CONFIG_MACH_SUN50I_64=y
 CONFIG_DEFAULT_DEVICE_TREE="sun50i-a64-pine64-plus"
 # CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set
 CONFIG_CONSOLE_MUX=y
diff --git a/configs/sun50i_spl32_defconfig b/configs/sun50i_spl32_defconfig
new file mode 100644
index 0000000..12d102d
--- /dev/null
+++ b/configs/sun50i_spl32_defconfig
@@ -0,0 +1,11 @@
+CONFIG_ARM=y
+CONFIG_ARCH_SUNXI=y
+CONFIG_MACH_SUN50I_32=y
+CONFIG_DRAM_CLK=672
+CONFIG_SPL=y
+CONFIG_DEFAULT_DEVICE_TREE="sun50i-a64-pine64-plus"
+CONFIG_OF_LIST="sun50i-a64-pine64 sun50i-a64-pine64-plus"
+# CONFIG_CMD_IMLS is not set
+# CONFIG_CMD_FLASH is not set
+# CONFIG_CMD_FPGA is not set
+CONFIG_MMC_SUNXI_SLOT_EXTRA=2
-- 
2.8.2

[U-Boot] [RFC PATCH 00/10] sunxi: Allwinner A64 SPL support

[Alexander Graf]

On 11/03/2016 02:36 AM, Andre Przywara wrote:
> 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.

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


Alex

[U-Boot] [RFC PATCH 01/10] sun6i: Restrict some register initialization to Allwinner A31 SoC

[Alexander Graf]

On 11/03/2016 02:36 AM, Andre Przywara wrote:
> 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 <andre.przywara@arm.com>

Reviewed-by: Alexander Graf <agraf@suse.de>

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


Alex

[U-Boot] [RFC PATCH 02/10] Makefile: use "arm64" architecture for U-Boot image files

[Alexander Graf]

On 11/03/2016 02:36 AM, Andre Przywara wrote:
> 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 <andre.przywara@arm.com>

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?


Alex

[U-Boot] [RFC PATCH 03/10] sunxi: provide default DRAM config for sun50i in Kconfig

[Alexander Graf]

On 11/03/2016 02:36 AM, Andre Przywara wrote:
> 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 <andre.przywara@arm.com>
> ---
>   board/sunxi/Kconfig           | 3 +++
>   configs/pine64_plus_defconfig | 2 --
>   2 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig
> index e1d4ab1..d281a65 100644
> --- a/board/sunxi/Kconfig
> +++ b/board/sunxi/Kconfig
> @@ -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.
>   
> @@ -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.


Alex

>   	---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
> --- a/configs/pine64_plus_defconfig
> +++ b/configs/pine64_plus_defconfig
> @@ -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

[U-Boot] [RFC PATCH 02/10] Makefile: use "arm64" architecture for U-Boot image files

[Andre Przywara]

Hi,

On 03/11/16 08:54, Alexander Graf wrote:
> On 11/03/2016 02:36 AM, Andre Przywara wrote:
>> 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 <andre.przywara@arm.com>
> 
> Why is this important? To know the state you have to be in for
> SPL->U-Boot transition later?

Yes.

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

Cheers,
Andre.

[U-Boot] [RFC PATCH 02/10] Makefile: use "arm64" architecture for U-Boot image files

[Alexander Graf]

On 11/03/2016 10:08 AM, Andre Przywara wrote:
> Hi,
>
> On 03/11/16 08:54, Alexander Graf wrote:
>> On 11/03/2016 02:36 AM, Andre Przywara wrote:
>>> 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 <andre.przywara@arm.com>
>> Why is this important? To know the state you have to be in for
>> SPL->U-Boot transition later?
> Yes.
>
>> 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.

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

> 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 :).


Alex

[U-Boot] [RFC PATCH 03/10] sunxi: provide default DRAM config for sun50i in Kconfig

[Andre Przywara]

Hi,

On 03/11/16 08:54, Alexander Graf wrote:
> On 11/03/2016 02:36 AM, Andre Przywara wrote:
>> 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 <andre.przywara@arm.com>
>> ---
>>   board/sunxi/Kconfig           | 3 +++
>>   configs/pine64_plus_defconfig | 2 --
>>   2 files changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig
>> index e1d4ab1..d281a65 100644
>> --- a/board/sunxi/Kconfig
>> +++ b/board/sunxi/Kconfig
>> @@ -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.
>>   @@ -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.

>>       ---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
>> --- a/configs/pine64_plus_defconfig
>> +++ b/configs/pine64_plus_defconfig
>> @@ -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
> 

[U-Boot] [RFC PATCH 03/10] sunxi: provide default DRAM config for sun50i in Kconfig

[Hans de Goede]

Hi,

On 03-11-16 10:10, Andre Przywara wrote:
> Hi,
>
> On 03/11/16 08:54, Alexander Graf wrote:
>> On 11/03/2016 02:36 AM, Andre Przywara wrote:
>>> 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 <andre.przywara@arm.com>
>>> ---
>>>   board/sunxi/Kconfig           | 3 +++
>>>   configs/pine64_plus_defconfig | 2 --
>>>   2 files changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig
>>> index e1d4ab1..d281a65 100644
>>> --- a/board/sunxi/Kconfig
>>> +++ b/board/sunxi/Kconfig
>>> @@ -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.
>>>   @@ -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.

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.

Regards,

Hans



>
> Cheers,
> Andre.
>
>>>       ---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
>>> --- a/configs/pine64_plus_defconfig
>>> +++ b/configs/pine64_plus_defconfig
>>> @@ -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
>>

[U-Boot] [RFC PATCH 02/10] Makefile: use "arm64" architecture for U-Boot image files

[Andre Przywara]

Hi,

On 03/11/16 09:10, Alexander Graf wrote:
> On 11/03/2016 10:08 AM, Andre Przywara wrote:
>> Hi,
>>
>> On 03/11/16 08:54, Alexander Graf wrote:
>>> On 11/03/2016 02:36 AM, Andre Przywara wrote:
>>>> 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 <andre.przywara@arm.com>
>>> Why is this important? To know the state you have to be in for
>>> SPL->U-Boot transition later?
>> Yes.
>>
>>> 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.
> 
> APM is "special". They even use the "arm" marker for kernels.

Yeah, I remember this ;-)

>> 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 :).

I was thinking the same. Even if we eventually scratch that idea this
patch shouldn't hurt, and makes U-Boot more versatile.

Cheers,
Andre.

[U-Boot] [RFC PATCH 03/10] sunxi: provide default DRAM config for sun50i in Kconfig

[Andre Przywara]

Hi Hans,

thanks for having a look.

On 03/11/16 09:13, Hans de Goede wrote:
> Hi,
> 
> On 03-11-16 10:10, Andre Przywara wrote:
>> Hi,
>>
>> On 03/11/16 08:54, Alexander Graf wrote:
>>> On 11/03/2016 02:36 AM, Andre Przywara wrote:
>>>> 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 <andre.przywara@arm.com>
>>>> ---
>>>>   board/sunxi/Kconfig           | 3 +++
>>>>   configs/pine64_plus_defconfig | 2 --
>>>>   2 files changed, 3 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig
>>>> index e1d4ab1..d281a65 100644
>>>> --- a/board/sunxi/Kconfig
>>>> +++ b/board/sunxi/Kconfig
>>>> @@ -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.
>>>>   @@ -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.
> 
> 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?   ;-)             ^^^^

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

Like the hunk below?

Cheers,
Andre.

>>>>       ---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
>>>> --- a/configs/pine64_plus_defconfig
>>>> +++ b/configs/pine64_plus_defconfig
>>>> @@ -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
>>>

[U-Boot] [RFC PATCH 00/10] sunxi: Allwinner A64 SPL support

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

On 03-11-16 09:49, Alexander Graf wrote:
> On 11/03/2016 02:36 AM, Andre Przywara wrote:
>> 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.
>
>> 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

[U-Boot] [RFC PATCH 03/10] sunxi: provide default DRAM config for sun50i in Kconfig

[Hans de Goede]

Hi,

On 03-11-16 10:17, Andre Przywara wrote:
> Hi Hans,
>
> thanks for having a look.
>
> On 03/11/16 09:13, Hans de Goede wrote:
>> Hi,
>>
>> On 03-11-16 10:10, Andre Przywara wrote:
>>> Hi,
>>>
>>> On 03/11/16 08:54, Alexander Graf wrote:
>>>> On 11/03/2016 02:36 AM, Andre Przywara wrote:
>>>>> 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 <andre.przywara@arm.com>
>>>>> ---
>>>>>   board/sunxi/Kconfig           | 3 +++
>>>>>   configs/pine64_plus_defconfig | 2 --
>>>>>   2 files changed, 3 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig
>>>>> index e1d4ab1..d281a65 100644
>>>>> --- a/board/sunxi/Kconfig
>>>>> +++ b/board/sunxi/Kconfig
>>>>> @@ -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.
>>>>>   @@ -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.
>>
>> 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?   ;-)             ^^^^
>
>> DRAM_CLK and DRAM_ZQ values from configs/pine64_plus_defconfig as
>> part of this patch.
>
> Like the hunk below?

Ugh, yes like that one, so from my pov this patch is good to go :)

Regards.

Hans

>
> Cheers,
> Andre.
>
>>>>>       ---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
>>>>> --- a/configs/pine64_plus_defconfig
>>>>> +++ b/configs/pine64_plus_defconfig
>>>>> @@ -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
>>>>

[U-Boot] [RFC PATCH 00/10] sunxi: Allwinner A64 SPL support

[Andre Przywara]

Hi,

On 03/11/16 08:49, Alexander Graf wrote:
> On 11/03/2016 02:36 AM, Andre Przywara wrote:
>> 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?

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

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

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

Cheers,
Andre.

[U-Boot] [RFC PATCH 00/10] sunxi: Allwinner A64 SPL support

[Hans de Goede]

Hi,

On 03-11-16 10:38, Andre Przywara wrote:
> Hi,
>
> On 03/11/16 08:49, Alexander Graf wrote:
>> On 11/03/2016 02:36 AM, Andre Przywara wrote:
>>> 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?
>
> 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!)
>
>> 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).
>
>>> 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).

IIRC we build the SPL as thumb2, and u-boot proper as regular armv7,
which may explain why the impact on the SPL for going 64 bit is much
larger.

Anyways I'm a fan of just going with 64 bits for now + trying to
squeeze some bytes of the size left and right too.

Have you looked at the size(s) of the C-string sections? In the past
we've had linker bugs where unused C-strings where kept in the final
linked binary, and even if you're not hitting that bug then strings
may make up for a significant chunk of the size and we could try
to make various (error) messages less verbose (or add some CONFIG
option to do so) to make things smaller.

Regards,

Hans


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

[U-Boot] [RFC PATCH 00/10] sunxi: Allwinner A64 SPL support

[Andre Przywara]

Hi,

On 03/11/16 09:34, Hans de Goede wrote:
> <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.
> 
> On 03-11-16 09:49, Alexander Graf wrote:
>> On 11/03/2016 02:36 AM, Andre Przywara wrote:
>>> 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.
>>
>>> 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'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?

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

Indeed providing the two options seems like a sensible approach, even
with the hacks needed to package it. I will try to bake a joint series
so that people can play with it.

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

I am afraid that the SPL build is already quite minimal, at least I
couldn't find any really useless function in the dump.
But lets keep the hope up ;-)

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

Don't forget ATF ;-)

I still wonder why _every_ distribution aims to build the firmware for
_every_ board, but I think we had this discussion already. And having an
automated way of building it which gets exercised regularly isn't a bad
thing, I guess.

Cheers,
Andre.

> 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

[U-Boot] [RFC PATCH 00/10] sunxi: Allwinner A64 SPL support

[Alexander Graf]

On 11/03/2016 10:51 AM, Andre Przywara wrote:
> Hi,
>
> On 03/11/16 09:34, Hans de Goede wrote:
>> <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.
>>
>> On 03-11-16 09:49, Alexander Graf wrote:
>>> On 11/03/2016 02:36 AM, Andre Przywara wrote:
>>>> 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.
>>>
>>>> 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'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.


Alex

[U-Boot] [RFC PATCH 00/10] sunxi: Allwinner A64 SPL support

[Hans de Goede]

Hi,

On 03-11-16 11:36, Alexander Graf wrote:
> On 11/03/2016 10:51 AM, Andre Przywara wrote:
>> Hi,
>>
>> On 03/11/16 09:34, Hans de Goede wrote:
>>> <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.
>>>
>>> On 03-11-16 09:49, Alexander Graf wrote:
>>>> On 11/03/2016 02:36 AM, Andre Przywara wrote:
>>>>> 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.
>>>>
>>>>> 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'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).

Regards,

Hans

[U-Boot] [RFC PATCH 00/10] sunxi: Allwinner A64 SPL support

[Alexander Graf]

On 11/03/2016 11:49 AM, Hans de Goede wrote:
> Hi,
>
> On 03-11-16 11:36, Alexander Graf wrote:
>> On 11/03/2016 10:51 AM, Andre Przywara wrote:
>>> Hi,
>>>
>>> On 03/11/16 09:34, Hans de Goede wrote:
>>>> <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.
>>>>
>>>> On 03-11-16 09:49, Alexander Graf wrote:
>>>>> On 11/03/2016 02:36 AM, Andre Przywara wrote:
>>>>>> 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.
>>>>>
>>>>>> 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'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).

Yes, it's possible, but means that we need to build armv7l and aarch64 
in the same repository. It also means that we actually *have* to build 
armv7l at all to compile a pine64 SPL, which I'd rather not want to 
limit myself to ;).


Alex

[U-Boot] [RFC PATCH 01/10] sun6i: Restrict some register initialization to Allwinner A31 SoC

[Chen-Yu Tsai]

On Thu, Nov 3, 2016 at 4:52 PM, Alexander Graf <agraf@suse.de> wrote:
> On 11/03/2016 02:36 AM, Andre Przywara wrote:
>>
>> 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 <andre.przywara@arm.com>
>
>
> Reviewed-by: Alexander Graf <agraf@suse.de>
>
> (However I haven't counter-checked with specs)

Reviewed-by: Chen-Yu Tsai <wens@csie.org>

[U-Boot] [RFC PATCH 08/10] SPL: read and store arch property from U-Boot image

[Simon Glass]

Hi Andre,

On 2 November 2016 at 19:36, Andre Przywara <andre.przywara@arm.com> wrote:
> 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 <andre.przywara@arm.com>
> ---
>  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 <sjg@chromium.org>

> diff --git a/common/spl/spl.c b/common/spl/spl.c
> index bdb165a..f76ddd2 100644
> --- a/common/spl/spl.c
> +++ b/common/spl/spl.c
> @@ -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,
> diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
> index aae556f..a5d903b 100644
> --- a/common/spl/spl_fit.c
> +++ b/common/spl/spl_fit.c
> @@ -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;
>  }
>
> +__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

>  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
> diff --git a/include/spl.h b/include/spl.h
> index e080a82..6a9d2fb 100644
> --- a/include/spl.h
> +++ b/include/spl.h
> @@ -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?

>  };
>
>  /*
> --
> 2.8.2
>

Regards,
Simon

[U-Boot] [RFC PATCH 09/10] sunxi: introduce RMR switch to enter payloads in 64-bit mode

[Simon Glass]

Hi Andre,

On 2 November 2016 at 19:36, Andre Przywara <andre.przywara@arm.com> wrote:
> 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 <andre.przywara@arm.com>
> ---
>  arch/arm/lib/spl.c               | 14 ++++++++++

I think the changes to this file should go in a separate patch as they
are generic to ARM.

>  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
[...]

Regards,
Simon

[U-Boot] [RFC PATCH 02/10] Makefile: use "arm64" architecture for U-Boot image files

[Simon Glass]

On 2 November 2016 at 19:36, Andre Przywara <andre.przywara@arm.com> wrote:
> 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 <andre.przywara@arm.com>
> ---
>  Makefile | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [RFC PATCH 00/10] sunxi: Allwinner A64 SPL support

[Andre Przywara]

Hi,

On 03/11/16 09:38, Andre Przywara wrote:
> Hi,
> 
> On 03/11/16 08:49, Alexander Graf wrote:
>> On 11/03/2016 02:36 AM, Andre Przywara wrote:
>>> 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?
> 
> 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!)
> 
>> 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).
> 
>>> 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.

Just a quick update: The 64-bit SPL patches were in a worse state than I
originally thought and I failed to resurrect them quickly again.

After spending some hours with debugging (the old way via writing to SD
cards) I discovered that there is apparently a regression in 2016.11-rc3
over the 2016.09 release with respect to these patches.
So my old 64-bit SPL patches work in the branch which is on top of
2016.09, but break after rebasing them to 2016.11-rc3. I now managed to
squash these patches in between the two releases, so I should be able to
bisect them later tonight. I keep you posted.

Cheers,
Andre.

[U-Boot] [RFC PATCH 08/10] SPL: read and store arch property from U-Boot image

[André Przywara]

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

Hi Simon,

> On 2 November 2016 at 19:36, Andre Przywara <andre.przywara@arm.com> wrote:
>> 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 <andre.przywara@arm.com>
>> ---
>>  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 <sjg@chromium.org>

Thanks!

> 
>> diff --git a/common/spl/spl.c b/common/spl/spl.c
>> index bdb165a..f76ddd2 100644
>> --- a/common/spl/spl.c
>> +++ b/common/spl/spl.c
>> @@ -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,
>> diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
>> index aae556f..a5d903b 100644
>> --- a/common/spl/spl_fit.c
>> +++ b/common/spl/spl_fit.c
>> @@ -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;
>>  }
>>
>> +__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.

> 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

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?

> 
>>  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
>> diff --git a/include/spl.h b/include/spl.h
>> index e080a82..6a9d2fb 100644
>> --- a/include/spl.h
>> +++ b/include/spl.h
>> @@ -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?

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

Cheers,
Andre.

[U-Boot] [RFC PATCH 08/10] SPL: read and store arch property from U-Boot image

[Simon Glass]

Hi Andre,

On 17 November 2016 at 18:50, Andr? Przywara <andre.przywara@arm.com> wrote:
> On 05/11/16 16:10, Simon Glass wrote:
>
> Hi Simon,
>
>> On 2 November 2016 at 19:36, Andre Przywara <andre.przywara@arm.com> wrote:
>>> 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 <andre.przywara@arm.com>
>>> ---
>>>  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 <sjg@chromium.org>
>
> Thanks!
>
>>
>>> diff --git a/common/spl/spl.c b/common/spl/spl.c
>>> index bdb165a..f76ddd2 100644
>>> --- a/common/spl/spl.c
>>> +++ b/common/spl/spl.c
>>> @@ -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,
>>> diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
>>> index aae556f..a5d903b 100644
>>> --- a/common/spl/spl_fit.c
>>> +++ b/common/spl/spl_fit.c
>>> @@ -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;
>>>  }
>>>
>>> +__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.
>
>> 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
>
> So I found the weak function more elegant than #ifdef-ing architecture
> specific code into a generic file.

Fair enough.

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

>
>>
>>>  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
>>> diff --git a/include/spl.h b/include/spl.h
>>> index e080a82..6a9d2fb 100644
>>> --- a/include/spl.h
>>> +++ b/include/spl.h
>>> @@ -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?
>
> 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 :-)

Regards,
Simon

>

[U-Boot] [RFC PATCH 08/10] SPL: read and store arch property from U-Boot image

[André Przywara]

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

Hi Simon,

> On 17 November 2016 at 18:50, Andr? Przywara <andre.przywara@arm.com> wrote:
>> On 05/11/16 16:10, Simon Glass wrote:
>>
>> Hi Simon,
>>
>>> On 2 November 2016 at 19:36, Andre Przywara <andre.przywara@arm.com> wrote:
>>>> 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 <andre.przywara@arm.com>
>>>> ---
>>>>  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 <sjg@chromium.org>
>>
>> Thanks!
>>
>>>
>>>> diff --git a/common/spl/spl.c b/common/spl/spl.c
>>>> index bdb165a..f76ddd2 100644
>>>> --- a/common/spl/spl.c
>>>> +++ b/common/spl/spl.c
>>>> @@ -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,
>>>> diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
>>>> index aae556f..a5d903b 100644
>>>> --- a/common/spl/spl_fit.c
>>>> +++ b/common/spl/spl_fit.c
>>>> @@ -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;
>>>>  }
>>>>
>>>> +__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.
>>
>>> 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
>>
>> So I found the weak function more elegant than #ifdef-ing architecture
>> specific code into a generic file.
> 
> Fair enough.
> 
>> 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.

>>>
>>>>  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
>>>> diff --git a/include/spl.h b/include/spl.h
>>>> index e080a82..6a9d2fb 100644
>>>> --- a/include/spl.h
>>>> +++ b/include/spl.h
>>>> @@ -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?
>>
>> 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 ;-)

Cheers,
Andre.

[U-Boot] [RFC PATCH 08/10] SPL: read and store arch property from U-Boot image

[Simon Glass]

Hi Andre,

On 19 November 2016 at 09:35, Andr? Przywara <andre.przywara@arm.com> wrote:
> On 19/11/16 13:49, Simon Glass wrote:
>
> Hi Simon,
>
>> On 17 November 2016 at 18:50, Andr? Przywara <andre.przywara@arm.com> wrote:
>>> On 05/11/16 16:10, Simon Glass wrote:
>>>
>>> Hi Simon,
>>>
>>>> On 2 November 2016 at 19:36, Andre Przywara <andre.przywara@arm.com> wrote:
>>>>> 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 <andre.przywara@arm.com>
>>>>> ---
>>>>>  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 <sjg@chromium.org>
>>>
>>> Thanks!
>>>
>>>>
>>>>> diff --git a/common/spl/spl.c b/common/spl/spl.c
>>>>> index bdb165a..f76ddd2 100644
>>>>> --- a/common/spl/spl.c
>>>>> +++ b/common/spl/spl.c
>>>>> @@ -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,
>>>>> diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
>>>>> index aae556f..a5d903b 100644
>>>>> --- a/common/spl/spl_fit.c
>>>>> +++ b/common/spl/spl_fit.c
>>>>> @@ -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;
>>>>>  }
>>>>>
>>>>> +__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.
>>>
>>>> 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
>>>
>>> So I found the weak function more elegant than #ifdef-ing architecture
>>> specific code into a generic file.
>>
>> Fair enough.
>>
>>> 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.

OK, well more comments help.

>
>>>>
>>>>>  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
>>>>> diff --git a/include/spl.h b/include/spl.h
>>>>> index e080a82..6a9d2fb 100644
>>>>> --- a/include/spl.h
>>>>> +++ b/include/spl.h
>>>>> @@ -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?
>>>
>>> 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 ;-)

Yes that's what I mean, thanks.

Regards,
Simon