[PATCH v1 0/3] Add spi boot to SPL on SUNIV

[PATCH v1 0/3] Add spi boot to SPL on SUNIV

[Jesse Taube]

This patch adds the ability to detect the
BROM's boot source as well as the ability to boot from spi.

Jesse Taube (3):
  mach-sunxi: Add boot device detection for SUNIV
  mach-sunxi: Add spi boot for SUNIV
  mach-sunxi: Enable spi boot for SUNIV

 arch/arm/include/asm/arch-sunxi/gpio.h |  1 +
 arch/arm/include/asm/arch-sunxi/spl.h  | 15 ++++++++
 arch/arm/mach-sunxi/Kconfig            |  2 +-
 arch/arm/mach-sunxi/board.c            | 50 ++++++++++++--------------
 arch/arm/mach-sunxi/spl_spi_sunxi.c    | 26 ++++++++++----
 configs/licheepi_nano_defconfig        |  1 +
 6 files changed, 60 insertions(+), 35 deletions(-)

-- 
2.34.1

[PATCH v1 1/3] mach-sunxi: Add boot device detection for SUNIV

[Jesse Taube]

Use Samuel's suggestion of looking at the BootRom's stack
to determine the boot device.

Signed-off-by: Jesse Taube <Mr.Bossman075@gmail.com>
Suggested-by: Samuel Holland <samuel@sholland.org>
---
 arch/arm/include/asm/arch-sunxi/spl.h | 15 ++++++++
 arch/arm/mach-sunxi/board.c           | 50 ++++++++++++---------------
 2 files changed, 38 insertions(+), 27 deletions(-)

diff --git a/arch/arm/include/asm/arch-sunxi/spl.h b/arch/arm/include/asm/arch-sunxi/spl.h
index 58cdf806d9..d069091297 100644
--- a/arch/arm/include/asm/arch-sunxi/spl.h
+++ b/arch/arm/include/asm/arch-sunxi/spl.h
@@ -19,8 +19,23 @@
 #define SUNXI_BOOTED_FROM_MMC0_HIGH	0x10
 #define SUNXI_BOOTED_FROM_MMC2_HIGH	0x12
 
+/*
+ * Values taken from the Bootrom's stack used
+ * to determine where we booted from.
+ * 0xffff40f8: mmc0
+ * 0xffff4114: spi0 NAND
+ * 0xffff4130: spi0 NOR
+ * 0xffff4150: mmc1
+ */
+
+#define SUNIV_BOOTED_FROM_MMC0	0xffff40f8
+#define SUNIV_BOOTED_FROM_NAND	0xffff4114
+#define SUNIV_BOOTED_FROM_SPI	0xffff4130
+#define SUNIV_BOOTED_FROM_MMC1	0xffff4150
+
 #define is_boot0_magic(addr)	(memcmp((void *)(addr), BOOT0_MAGIC, 8) == 0)
 
 uint32_t sunxi_get_boot_device(void);
+uint32_t suniv_get_boot_device(void);
 
 #endif
diff --git a/arch/arm/mach-sunxi/board.c b/arch/arm/mach-sunxi/board.c
index 57078f7a7b..b0658d583e 100644
--- a/arch/arm/mach-sunxi/board.c
+++ b/arch/arm/mach-sunxi/board.c
@@ -241,6 +241,25 @@ uint32_t sunxi_get_boot_device(void)
 	return -1;		/* Never reached */
 }
 
+uint32_t suniv_get_boot_device(void)
+{
+	/* Get the last function call from BootRom's stack. */
+	u32 brom_call = *(u32 *)(fel_stash.sp - 4);
+
+	switch (brom_call) {
+	case SUNIV_BOOTED_FROM_MMC0:
+		return BOOT_DEVICE_MMC1;
+	case SUNIV_BOOTED_FROM_NAND:
+	case SUNIV_BOOTED_FROM_SPI:
+		return BOOT_DEVICE_SPI;
+	case SUNIV_BOOTED_FROM_MMC1:
+		return BOOT_DEVICE_MMC2;
+	}
+
+	printf("Unknown boot source from BROM: 0x%x\n", brom_call);
+	return BOOT_DEVICE_MMC1;
+}
+
 #ifdef CONFIG_SPL_BUILD
 static u32 sunxi_get_spl_size(void)
 {
@@ -276,36 +295,13 @@ unsigned long spl_mmc_get_uboot_raw_sector(struct mmc *mmc,
 	return sector;
 }
 
-#ifdef CONFIG_MACH_SUNIV
-/*
- * The suniv BROM does not pass the boot media type to SPL, so we try with the
- * boot sequence in BROM: mmc0->spinor->fail.
- * TODO: This has the slight chance of being wrong (invalid SPL signature,
- * but valid U-Boot legacy image on the SD card), but this should be rare.
- * It looks like we can deduce from some BROM state upon entering the SPL
- * (registers, SP, or stack itself) where the BROM was coming from and use
- * that here.
- */
-void board_boot_order(u32 *spl_boot_list)
-{
-	/*
-	 * See the comments above in sunxi_get_boot_device() for information
-	 * about FEL boot.
-	 */
-	if (!is_boot0_magic(SPL_ADDR + 4)) {
-		spl_boot_list[0] = BOOT_DEVICE_BOARD;
-		return;
-	}
-
-	spl_boot_list[0] = BOOT_DEVICE_MMC1;
-	spl_boot_list[1] = BOOT_DEVICE_SPI;
-}
-#else
 u32 spl_boot_device(void)
 {
-	return sunxi_get_boot_device();
+	if (IS_ENABLED(CONFIG_MACH_SUNIV))
+		return suniv_get_boot_device();
+	else
+		return sunxi_get_boot_device();
 }
-#endif
 
 __weak void sunxi_sram_init(void)
 {
-- 
2.34.1

[PATCH v1 2/3] mach-sunxi: Add spi boot for SUNIV

[Jesse Taube]

Add support for the spi boot in spl on suniv architecture.

Signed-off-by: Jesse Taube <Mr.Bossman075@gmail.com>
---
 arch/arm/include/asm/arch-sunxi/gpio.h |  1 +
 arch/arm/mach-sunxi/spl_spi_sunxi.c    | 26 +++++++++++++++++++-------
 2 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/arch/arm/include/asm/arch-sunxi/gpio.h b/arch/arm/include/asm/arch-sunxi/gpio.h
index 7f7eb0517c..edd0fbf49f 100644
--- a/arch/arm/include/asm/arch-sunxi/gpio.h
+++ b/arch/arm/include/asm/arch-sunxi/gpio.h
@@ -160,6 +160,7 @@ enum sunxi_gpio_number {
 #define SUNXI_GPC_SDC2		3
 #define SUN6I_GPC_SDC3		4
 #define SUN50I_GPC_SPI0		4
+#define SUNIV_GPC_SPI0		2
 
 #define SUNXI_GPD_LCD0		2
 #define SUNXI_GPD_LVDS0		3
diff --git a/arch/arm/mach-sunxi/spl_spi_sunxi.c b/arch/arm/mach-sunxi/spl_spi_sunxi.c
index 910e805016..9a3666a2d7 100644
--- a/arch/arm/mach-sunxi/spl_spi_sunxi.c
+++ b/arch/arm/mach-sunxi/spl_spi_sunxi.c
@@ -90,6 +90,7 @@
 
 #define SPI0_CLK_DIV_BY_2           0x1000
 #define SPI0_CLK_DIV_BY_4           0x1001
+#define SPI0_CLK_DIV_BY_32          0x100f
 
 /*****************************************************************************/
 
@@ -132,7 +133,8 @@ static uintptr_t spi0_base_address(void)
 	if (IS_ENABLED(CONFIG_MACH_SUN50I_H6))
 		return 0x05010000;
 
-	if (!is_sun6i_gen_spi())
+	if (!is_sun6i_gen_spi() ||
+	    IS_ENABLED(CONFIG_MACH_SUNIV))
 		return 0x01C05000;
 
 	return 0x01C68000;
@@ -156,11 +158,17 @@ static void spi0_enable_clock(void)
 	if (!IS_ENABLED(CONFIG_MACH_SUN50I_H6))
 		setbits_le32(CCM_AHB_GATING0, (1 << AHB_GATE_OFFSET_SPI0));
 
-	/* Divide by 4 */
-	writel(SPI0_CLK_DIV_BY_4, base + (is_sun6i_gen_spi() ?
-				  SUN6I_SPI0_CCTL : SUN4I_SPI0_CCTL));
-	/* 24MHz from OSC24M */
-	writel((1 << 31), CCM_SPI0_CLK);
+	if (IS_ENABLED(CONFIG_MACH_SUNIV)) {
+		/* Divide by 32, clock source is AHB clock 200MHz */
+		writel(SPI0_CLK_DIV_BY_32, base + (IS_ENABLED(CONFIG_SUNXI_GEN_SUN6I) ?
+					   SUN6I_SPI0_CCTL : SUN4I_SPI0_CCTL));
+	} else {
+		/* Divide by 4 */
+		writel(SPI0_CLK_DIV_BY_4, base + (is_sun6i_gen_spi() ?
+					  SUN6I_SPI0_CCTL : SUN4I_SPI0_CCTL));
+		/* 24MHz from OSC24M */
+		writel((1 << 31), CCM_SPI0_CLK);
+	}
 
 	if (is_sun6i_gen_spi()) {
 		/* Enable SPI in the master mode and do a soft reset */
@@ -191,7 +199,8 @@ static void spi0_disable_clock(void)
 					     SUN4I_CTL_ENABLE);
 
 	/* Disable the SPI0 clock */
-	writel(0, CCM_SPI0_CLK);
+	if (!IS_ENABLED(CONFIG_MACH_SUNIV))
+		writel(0, CCM_SPI0_CLK);
 
 	/* Close the SPI0 gate */
 	if (!IS_ENABLED(CONFIG_MACH_SUN50I_H6))
@@ -213,6 +222,9 @@ static void spi0_init(void)
 	    IS_ENABLED(CONFIG_MACH_SUN50I_H6))
 		pin_function = SUN50I_GPC_SPI0;
 
+	if (IS_ENABLED(CONFIG_MACH_SUNIV))
+		pin_function = SUNIV_GPC_SPI0;
+
 	spi0_pinmux_setup(pin_function);
 	spi0_enable_clock();
 }
-- 
2.34.1

[PATCH v1 3/3] mach-sunxi: Enable spi boot for SUNIV

[Jesse Taube]

Enable spi boot in spl on suniv architecture.

Signed-off-by: Jesse Taube <Mr.Bossman075@gmail.com>
---
 arch/arm/mach-sunxi/Kconfig     | 2 +-
 configs/licheepi_nano_defconfig | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig
index 205fe3c9d3..d1c60d2408 100644
--- a/arch/arm/mach-sunxi/Kconfig
+++ b/arch/arm/mach-sunxi/Kconfig
@@ -1038,7 +1038,7 @@ config SPL_STACK_R_ADDR
 
 config SPL_SPI_SUNXI
 	bool "Support for SPI Flash on Allwinner SoCs in SPL"
-	depends on MACH_SUN4I || MACH_SUN5I || MACH_SUN7I || MACH_SUNXI_H3_H5 || MACH_SUN50I || MACH_SUN8I_R40 || MACH_SUN50I_H6
+	depends on MACH_SUN4I || MACH_SUN5I || MACH_SUN7I || MACH_SUNXI_H3_H5 || MACH_SUN50I || MACH_SUN8I_R40 || MACH_SUN50I_H6 || MACH_SUNIV
 	help
 	  Enable support for SPI Flash. This option allows SPL to read from
 	  sunxi SPI Flash. It uses the same method as the boot ROM, so does
diff --git a/configs/licheepi_nano_defconfig b/configs/licheepi_nano_defconfig
index 2ac0ef4285..9fd1dcc995 100644
--- a/configs/licheepi_nano_defconfig
+++ b/configs/licheepi_nano_defconfig
@@ -9,3 +9,4 @@ CONFIG_MACH_SUNIV=y
 CONFIG_DRAM_CLK=156
 CONFIG_DRAM_ZQ=0
 # CONFIG_VIDEO_SUNXI is not set
+CONFIG_SPL_SPI_SUNXI=y
-- 
2.34.1

Re: [PATCH v1 2/3] mach-sunxi: Add spi boot for SUNIV

[Andre Przywara]

On Wed,  9 Feb 2022 23:34:37 -0500
Jesse Taube <mr.bossman075@gmail.com> wrote:

Hi Jesse,

many thanks for sending this, I guess this makes those little boards much
more useful.

> Add support for the spi boot in spl on suniv architecture.

A more elaborate commit message would be welcomed.
Please mention the F1C100s, to give some more context. Also briefly
mention the differences, I think Icenowy summarised this quite well in
her version of the patch (06/27):

The suniv SoC come with a sun6i-style SPI controller at the base address
of sun4i SPI controller. The module clock of the SPI controller is also
missing.

You could add: "... is also missing, which leaves us running directly from
the AHB clock, set to 200 MHz."

> 
> Signed-off-by: Jesse Taube <Mr.Bossman075@gmail.com>
> ---
>  arch/arm/include/asm/arch-sunxi/gpio.h |  1 +
>  arch/arm/mach-sunxi/spl_spi_sunxi.c    | 26 +++++++++++++++++++-------
>  2 files changed, 20 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/arm/include/asm/arch-sunxi/gpio.h b/arch/arm/include/asm/arch-sunxi/gpio.h
> index 7f7eb0517c..edd0fbf49f 100644
> --- a/arch/arm/include/asm/arch-sunxi/gpio.h
> +++ b/arch/arm/include/asm/arch-sunxi/gpio.h
> @@ -160,6 +160,7 @@ enum sunxi_gpio_number {
>  #define SUNXI_GPC_SDC2		3
>  #define SUN6I_GPC_SDC3		4
>  #define SUN50I_GPC_SPI0		4
> +#define SUNIV_GPC_SPI0		2
>  
>  #define SUNXI_GPD_LCD0		2
>  #define SUNXI_GPD_LVDS0		3
> diff --git a/arch/arm/mach-sunxi/spl_spi_sunxi.c b/arch/arm/mach-sunxi/spl_spi_sunxi.c
> index 910e805016..9a3666a2d7 100644
> --- a/arch/arm/mach-sunxi/spl_spi_sunxi.c
> +++ b/arch/arm/mach-sunxi/spl_spi_sunxi.c
> @@ -90,6 +90,7 @@
>  
>  #define SPI0_CLK_DIV_BY_2           0x1000
>  #define SPI0_CLK_DIV_BY_4           0x1001
> +#define SPI0_CLK_DIV_BY_32          0x100f
>  
>  /*****************************************************************************/
>  
> @@ -132,7 +133,8 @@ static uintptr_t spi0_base_address(void)
>  	if (IS_ENABLED(CONFIG_MACH_SUN50I_H6))
>  		return 0x05010000;
>  
> -	if (!is_sun6i_gen_spi())
> +	if (!is_sun6i_gen_spi() ||
> +	    IS_ENABLED(CONFIG_MACH_SUNIV))
>  		return 0x01C05000;
>  
>  	return 0x01C68000;
> @@ -156,11 +158,17 @@ static void spi0_enable_clock(void)
>  	if (!IS_ENABLED(CONFIG_MACH_SUN50I_H6))
>  		setbits_le32(CCM_AHB_GATING0, (1 << AHB_GATE_OFFSET_SPI0));
>  
> -	/* Divide by 4 */
> -	writel(SPI0_CLK_DIV_BY_4, base + (is_sun6i_gen_spi() ?
> -				  SUN6I_SPI0_CCTL : SUN4I_SPI0_CCTL));
> -	/* 24MHz from OSC24M */
> -	writel((1 << 31), CCM_SPI0_CLK);
> +	if (IS_ENABLED(CONFIG_MACH_SUNIV)) {
> +		/* Divide by 32, clock source is AHB clock 200MHz */
> +		writel(SPI0_CLK_DIV_BY_32, base + (IS_ENABLED(CONFIG_SUNXI_GEN_SUN6I) ?

This seems pointless and redundant, since we exactly know the register when
MACH_SUNIV is selected.

> +					   SUN6I_SPI0_CCTL : SUN4I_SPI0_CCTL));
> +	} else {
> +		/* Divide by 4 */
> +		writel(SPI0_CLK_DIV_BY_4, base + (is_sun6i_gen_spi() ?
> +					  SUN6I_SPI0_CCTL : SUN4I_SPI0_CCTL));
> +		/* 24MHz from OSC24M */
> +		writel((1 << 31), CCM_SPI0_CLK);
> +	}
>  
>  	if (is_sun6i_gen_spi()) {
>  		/* Enable SPI in the master mode and do a soft reset */
> @@ -191,7 +199,8 @@ static void spi0_disable_clock(void)
>  					     SUN4I_CTL_ENABLE);
>  
>  	/* Disable the SPI0 clock */
> -	writel(0, CCM_SPI0_CLK);
> +	if (!IS_ENABLED(CONFIG_MACH_SUNIV))
> +		writel(0, CCM_SPI0_CLK);
>  
>  	/* Close the SPI0 gate */
>  	if (!IS_ENABLED(CONFIG_MACH_SUN50I_H6))
> @@ -213,6 +222,9 @@ static void spi0_init(void)
>  	    IS_ENABLED(CONFIG_MACH_SUN50I_H6))
>  		pin_function = SUN50I_GPC_SPI0;
>  
> +	if (IS_ENABLED(CONFIG_MACH_SUNIV))
> +		pin_function = SUNIV_GPC_SPI0;
> +

It looks a bit better to tie connect this with an "else if" to the
previous comparison, since there is only one choice.

Cheers,
Andre

>  	spi0_pinmux_setup(pin_function);
>  	spi0_enable_clock();
>  }

Re: [PATCH v1 1/3] mach-sunxi: Add boot device detection for SUNIV

[Andre Przywara]

On Wed,  9 Feb 2022 23:34:36 -0500
Jesse Taube <mr.bossman075@gmail.com> wrote:

Hi Jesse,

many thanks for sending this, much appreciated!

> Use Samuel's suggestion of looking at the BootRom's stack
> to determine the boot device.

Can you please elaborate here what's going on, for future reference? Like:
=============
In contrast to other Allwinner SoCs the F1C100s BROM does not store a boot
source indicator in the eGON header in SRAM. This leaves us guessing where
we were exactly booted from, and for instance trying the SD card first,
even though we booted from SPI flash.
By inspecting the BROM code and by experimentation, Samuel found that the
top of the BROM stack contains unique pointers for each of the boot
sources, which we can use as a boot source indicator.

Remove the existing board_boot_order kludge and replace it with a proper
boot source indication function.
=============


> 
> Signed-off-by: Jesse Taube <Mr.Bossman075@gmail.com>
> Suggested-by: Samuel Holland <samuel@sholland.org>
> ---
>  arch/arm/include/asm/arch-sunxi/spl.h | 15 ++++++++
>  arch/arm/mach-sunxi/board.c           | 50 ++++++++++++---------------
>  2 files changed, 38 insertions(+), 27 deletions(-)
> 
> diff --git a/arch/arm/include/asm/arch-sunxi/spl.h b/arch/arm/include/asm/arch-sunxi/spl.h
> index 58cdf806d9..d069091297 100644
> --- a/arch/arm/include/asm/arch-sunxi/spl.h
> +++ b/arch/arm/include/asm/arch-sunxi/spl.h
> @@ -19,8 +19,23 @@
>  #define SUNXI_BOOTED_FROM_MMC0_HIGH	0x10
>  #define SUNXI_BOOTED_FROM_MMC2_HIGH	0x12
>  
> +/*
> + * Values taken from the Bootrom's stack used
> + * to determine where we booted from.
> + * 0xffff40f8: mmc0
> + * 0xffff4114: spi0 NAND
> + * 0xffff4130: spi0 NOR
> + * 0xffff4150: mmc1

Those last four lines are redundant, as you say exactly that, in code,
down here again. Comments are good, speaking code is better.

> + */
> +
> +#define SUNIV_BOOTED_FROM_MMC0	0xffff40f8
> +#define SUNIV_BOOTED_FROM_NAND	0xffff4114
> +#define SUNIV_BOOTED_FROM_SPI	0xffff4130
> +#define SUNIV_BOOTED_FROM_MMC1	0xffff4150
> +
>  #define is_boot0_magic(addr)	(memcmp((void *)(addr), BOOT0_MAGIC, 8) == 0)
>  
>  uint32_t sunxi_get_boot_device(void);
> +uint32_t suniv_get_boot_device(void);
>  
>  #endif
> diff --git a/arch/arm/mach-sunxi/board.c b/arch/arm/mach-sunxi/board.c
> index 57078f7a7b..b0658d583e 100644
> --- a/arch/arm/mach-sunxi/board.c
> +++ b/arch/arm/mach-sunxi/board.c
> @@ -241,6 +241,25 @@ uint32_t sunxi_get_boot_device(void)
>  	return -1;		/* Never reached */
>  }
>  
> +uint32_t suniv_get_boot_device(void)

This can be static, right?

> +{
> +	/* Get the last function call from BootRom's stack. */
> +	u32 brom_call = *(u32 *)(fel_stash.sp - 4);
> +
> +	switch (brom_call) {
> +	case SUNIV_BOOTED_FROM_MMC0:
> +		return BOOT_DEVICE_MMC1;
> +	case SUNIV_BOOTED_FROM_NAND:
> +	case SUNIV_BOOTED_FROM_SPI:
> +		return BOOT_DEVICE_SPI;
> +	case SUNIV_BOOTED_FROM_MMC1:
> +		return BOOT_DEVICE_MMC2;
> +	}

Don't you need to handle FEL boot here somehow?

Cheers,
Andre

> +
> +	printf("Unknown boot source from BROM: 0x%x\n", brom_call);
> +	return BOOT_DEVICE_MMC1;
> +}
> +
>  #ifdef CONFIG_SPL_BUILD
>  static u32 sunxi_get_spl_size(void)
>  {
> @@ -276,36 +295,13 @@ unsigned long spl_mmc_get_uboot_raw_sector(struct mmc *mmc,
>  	return sector;
>  }
>  
> -#ifdef CONFIG_MACH_SUNIV
> -/*
> - * The suniv BROM does not pass the boot media type to SPL, so we try with the
> - * boot sequence in BROM: mmc0->spinor->fail.
> - * TODO: This has the slight chance of being wrong (invalid SPL signature,
> - * but valid U-Boot legacy image on the SD card), but this should be rare.
> - * It looks like we can deduce from some BROM state upon entering the SPL
> - * (registers, SP, or stack itself) where the BROM was coming from and use
> - * that here.
> - */
> -void board_boot_order(u32 *spl_boot_list)
> -{
> -	/*
> -	 * See the comments above in sunxi_get_boot_device() for information
> -	 * about FEL boot.
> -	 */
> -	if (!is_boot0_magic(SPL_ADDR + 4)) {
> -		spl_boot_list[0] = BOOT_DEVICE_BOARD;
> -		return;
> -	}
> -
> -	spl_boot_list[0] = BOOT_DEVICE_MMC1;
> -	spl_boot_list[1] = BOOT_DEVICE_SPI;
> -}
> -#else
>  u32 spl_boot_device(void)
>  {
> -	return sunxi_get_boot_device();
> +	if (IS_ENABLED(CONFIG_MACH_SUNIV))
> +		return suniv_get_boot_device();
> +	else
> +		return sunxi_get_boot_device();
>  }
> -#endif
>  
>  __weak void sunxi_sram_init(void)
>  {

Re: [PATCH v1 3/3] mach-sunxi: Enable spi boot for SUNIV

[Andre Przywara]

On Wed,  9 Feb 2022 23:34:38 -0500
Jesse Taube <mr.bossman075@gmail.com> wrote:

Hi,

> Enable spi boot in spl on suniv architecture.

Please mention the F1C100s for context, and also that you are enabling it
for the LicheePi Nano board, which comes with SPI flash.

Otherwise:

> Signed-off-by: Jesse Taube <Mr.Bossman075@gmail.com>

Reviewed-by: Andre Przywara <andre.przywara@arm.com>

Cheers,
Andre

> ---
>  arch/arm/mach-sunxi/Kconfig     | 2 +-
>  configs/licheepi_nano_defconfig | 1 +
>  2 files changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig
> index 205fe3c9d3..d1c60d2408 100644
> --- a/arch/arm/mach-sunxi/Kconfig
> +++ b/arch/arm/mach-sunxi/Kconfig
> @@ -1038,7 +1038,7 @@ config SPL_STACK_R_ADDR
>  
>  config SPL_SPI_SUNXI
>  	bool "Support for SPI Flash on Allwinner SoCs in SPL"
> -	depends on MACH_SUN4I || MACH_SUN5I || MACH_SUN7I || MACH_SUNXI_H3_H5 || MACH_SUN50I || MACH_SUN8I_R40 || MACH_SUN50I_H6
> +	depends on MACH_SUN4I || MACH_SUN5I || MACH_SUN7I || MACH_SUNXI_H3_H5 || MACH_SUN50I || MACH_SUN8I_R40 || MACH_SUN50I_H6 || MACH_SUNIV
>  	help
>  	  Enable support for SPI Flash. This option allows SPL to read from
>  	  sunxi SPI Flash. It uses the same method as the boot ROM, so does
> diff --git a/configs/licheepi_nano_defconfig b/configs/licheepi_nano_defconfig
> index 2ac0ef4285..9fd1dcc995 100644
> --- a/configs/licheepi_nano_defconfig
> +++ b/configs/licheepi_nano_defconfig
> @@ -9,3 +9,4 @@ CONFIG_MACH_SUNIV=y
>  CONFIG_DRAM_CLK=156
>  CONFIG_DRAM_ZQ=0
>  # CONFIG_VIDEO_SUNXI is not set
> +CONFIG_SPL_SPI_SUNXI=y

Re: [PATCH v1 1/3] mach-sunxi: Add boot device detection for SUNIV

[Jesse Taube]

On 2/10/22 05:57, Andre Przywara wrote:
> On Wed,  9 Feb 2022 23:34:36 -0500
> Jesse Taube <mr.bossman075@gmail.com> wrote:
> 
> Hi Jesse,
> 
> many thanks for sending this, much appreciated!
> 
>> Use Samuel's suggestion of looking at the BootRom's stack
>> to determine the boot device.
> 
> Can you please elaborate here what's going on, for future reference? Like:
> =============
> In contrast to other Allwinner SoCs the F1C100s BROM does not store a boot
> source indicator in the eGON header in SRAM. This leaves us guessing where
> we were exactly booted from, and for instance trying the SD card first,
> even though we booted from SPI flash.
> By inspecting the BROM code and by experimentation, Samuel found that the
> top of the BROM stack contains unique pointers for each of the boot
> sources, which we can use as a boot source indicator.
> 
> Remove the existing board_boot_order kludge and replace it with a proper
> boot source indication function.
> =============
> 
> 
>>
>> Signed-off-by: Jesse Taube <Mr.Bossman075@gmail.com>
>> Suggested-by: Samuel Holland <samuel@sholland.org>
>> ---
>>   arch/arm/include/asm/arch-sunxi/spl.h | 15 ++++++++
>>   arch/arm/mach-sunxi/board.c           | 50 ++++++++++++---------------
>>   2 files changed, 38 insertions(+), 27 deletions(-)
>>
>> diff --git a/arch/arm/include/asm/arch-sunxi/spl.h b/arch/arm/include/asm/arch-sunxi/spl.h
>> index 58cdf806d9..d069091297 100644
>> --- a/arch/arm/include/asm/arch-sunxi/spl.h
>> +++ b/arch/arm/include/asm/arch-sunxi/spl.h
>> @@ -19,8 +19,23 @@
>>   #define SUNXI_BOOTED_FROM_MMC0_HIGH	0x10
>>   #define SUNXI_BOOTED_FROM_MMC2_HIGH	0x12
>>   
>> +/*
>> + * Values taken from the Bootrom's stack used
>> + * to determine where we booted from.
>> + * 0xffff40f8: mmc0
>> + * 0xffff4114: spi0 NAND
>> + * 0xffff4130: spi0 NOR
>> + * 0xffff4150: mmc1
> 
> Those last four lines are redundant, as you say exactly that, in code,
> down here again. Comments are good, speaking code is better.
> 
>> + */
>> +
>> +#define SUNIV_BOOTED_FROM_MMC0	0xffff40f8
>> +#define SUNIV_BOOTED_FROM_NAND	0xffff4114
>> +#define SUNIV_BOOTED_FROM_SPI	0xffff4130
>> +#define SUNIV_BOOTED_FROM_MMC1	0xffff4150
>> +
>>   #define is_boot0_magic(addr)	(memcmp((void *)(addr), BOOT0_MAGIC, 8) == 0)
>>   
>>   uint32_t sunxi_get_boot_device(void);
>> +uint32_t suniv_get_boot_device(void);
>>   
>>   #endif
>> diff --git a/arch/arm/mach-sunxi/board.c b/arch/arm/mach-sunxi/board.c
>> index 57078f7a7b..b0658d583e 100644
>> --- a/arch/arm/mach-sunxi/board.c
>> +++ b/arch/arm/mach-sunxi/board.c
>> @@ -241,6 +241,25 @@ uint32_t sunxi_get_boot_device(void)
>>   	return -1;		/* Never reached */
>>   }
>>   
>> +uint32_t suniv_get_boot_device(void)
> 
> This can be static, right?
Yes it can it is not because sunxi_get_boot_device isn'tm,
but that is only not static because board/sunxi/board.c:mmc_get_env_dev.
I will make it static.
> 
>> +{
>> +	/* Get the last function call from BootRom's stack. */
>> +	u32 brom_call = *(u32 *)(fel_stash.sp - 4);
>> +
>> +	switch (brom_call) {
>> +	case SUNIV_BOOTED_FROM_MMC0:
>> +		return BOOT_DEVICE_MMC1;
>> +	case SUNIV_BOOTED_FROM_NAND:
>> +	case SUNIV_BOOTED_FROM_SPI:
>> +		return BOOT_DEVICE_SPI;
>> +	case SUNIV_BOOTED_FROM_MMC1:
>> +		return BOOT_DEVICE_MMC2;
>> +	}
> 
> Don't you need to handle FEL boot here somehow?
> 
> Cheers,
> Andre
> 
>> +
>> +	printf("Unknown boot source from BROM: 0x%x\n", brom_call);
>> +	return BOOT_DEVICE_MMC1;
>> +}
>> +
>>   #ifdef CONFIG_SPL_BUILD
>>   static u32 sunxi_get_spl_size(void)
>>   {
>> @@ -276,36 +295,13 @@ unsigned long spl_mmc_get_uboot_raw_sector(struct mmc *mmc,
>>   	return sector;
>>   }
>>   
>> -#ifdef CONFIG_MACH_SUNIV
>> -/*
>> - * The suniv BROM does not pass the boot media type to SPL, so we try with the
>> - * boot sequence in BROM: mmc0->spinor->fail.
>> - * TODO: This has the slight chance of being wrong (invalid SPL signature,
>> - * but valid U-Boot legacy image on the SD card), but this should be rare.
>> - * It looks like we can deduce from some BROM state upon entering the SPL
>> - * (registers, SP, or stack itself) where the BROM was coming from and use
>> - * that here.
>> - */
>> -void board_boot_order(u32 *spl_boot_list)
>> -{
>> -	/*
>> -	 * See the comments above in sunxi_get_boot_device() for information
>> -	 * about FEL boot.
>> -	 */
>> -	if (!is_boot0_magic(SPL_ADDR + 4)) {
>> -		spl_boot_list[0] = BOOT_DEVICE_BOARD;
>> -		return;
>> -	}
>> -
>> -	spl_boot_list[0] = BOOT_DEVICE_MMC1;
>> -	spl_boot_list[1] = BOOT_DEVICE_SPI;
>> -}
>> -#else
>>   u32 spl_boot_device(void)
>>   {
>> -	return sunxi_get_boot_device();
>> +	if (IS_ENABLED(CONFIG_MACH_SUNIV))
>> +		return suniv_get_boot_device();
>> +	else
>> +		return sunxi_get_boot_device();
>>   }
>> -#endif
>>   
>>   __weak void sunxi_sram_init(void)
>>   {
> 

Re: [PATCH v1 1/3] mach-sunxi: Add boot device detection for SUNIV

[Jesse Taube]

On 2/10/22 05:57, Andre Przywara wrote:
> On Wed,  9 Feb 2022 23:34:36 -0500
> Jesse Taube <mr.bossman075@gmail.com> wrote:
> 
> Hi Jesse,
> 
> many thanks for sending this, much appreciated!
> 
>> Use Samuel's suggestion of looking at the BootRom's stack
>> to determine the boot device.
> 
> Can you please elaborate here what's going on, for future reference? Like:
> =============
> In contrast to other Allwinner SoCs the F1C100s BROM does not store a boot
> source indicator in the eGON header in SRAM. This leaves us guessing where
> we were exactly booted from, and for instance trying the SD card first,
> even though we booted from SPI flash.
> By inspecting the BROM code and by experimentation, Samuel found that the
> top of the BROM stack contains unique pointers for each of the boot
> sources, which we can use as a boot source indicator.
> 
> Remove the existing board_boot_order kludge and replace it with a proper
> boot source indication function.
> =============
> 
> 
>>
>> Signed-off-by: Jesse Taube <Mr.Bossman075@gmail.com>
>> Suggested-by: Samuel Holland <samuel@sholland.org>
>> ---
>>   arch/arm/include/asm/arch-sunxi/spl.h | 15 ++++++++
>>   arch/arm/mach-sunxi/board.c           | 50 ++++++++++++---------------
>>   2 files changed, 38 insertions(+), 27 deletions(-)
>>
>> diff --git a/arch/arm/include/asm/arch-sunxi/spl.h b/arch/arm/include/asm/arch-sunxi/spl.h
>> index 58cdf806d9..d069091297 100644
>> --- a/arch/arm/include/asm/arch-sunxi/spl.h
>> +++ b/arch/arm/include/asm/arch-sunxi/spl.h
>> @@ -19,8 +19,23 @@
>>   #define SUNXI_BOOTED_FROM_MMC0_HIGH	0x10
>>   #define SUNXI_BOOTED_FROM_MMC2_HIGH	0x12
>>   
>> +/*
>> + * Values taken from the Bootrom's stack used
>> + * to determine where we booted from.
>> + * 0xffff40f8: mmc0
>> + * 0xffff4114: spi0 NAND
>> + * 0xffff4130: spi0 NOR
>> + * 0xffff4150: mmc1
> 
> Those last four lines are redundant, as you say exactly that, in code,
> down here again. Comments are good, speaking code is better.
> 
>> + */
>> +
>> +#define SUNIV_BOOTED_FROM_MMC0	0xffff40f8
>> +#define SUNIV_BOOTED_FROM_NAND	0xffff4114
>> +#define SUNIV_BOOTED_FROM_SPI	0xffff4130
>> +#define SUNIV_BOOTED_FROM_MMC1	0xffff4150
>> +
>>   #define is_boot0_magic(addr)	(memcmp((void *)(addr), BOOT0_MAGIC, 8) == 0)
>>   
>>   uint32_t sunxi_get_boot_device(void);
>> +uint32_t suniv_get_boot_device(void);
>>   
>>   #endif
>> diff --git a/arch/arm/mach-sunxi/board.c b/arch/arm/mach-sunxi/board.c
>> index 57078f7a7b..b0658d583e 100644
>> --- a/arch/arm/mach-sunxi/board.c
>> +++ b/arch/arm/mach-sunxi/board.c
>> @@ -241,6 +241,25 @@ uint32_t sunxi_get_boot_device(void)
>>   	return -1;		/* Never reached */
>>   }
>>   
>> +uint32_t suniv_get_boot_device(void)
> 
> This can be static, right?
> 
>> +{
>> +	/* Get the last function call from BootRom's stack. */
>> +	u32 brom_call = *(u32 *)(fel_stash.sp - 4);You are okay with this I was expecting you to explain a better way that 
i don't know about.
>> +
>> +	switch (brom_call) {
>> +	case SUNIV_BOOTED_FROM_MMC0:
>> +		return BOOT_DEVICE_MMC1;
>> +	case SUNIV_BOOTED_FROM_NAND:
>> +	case SUNIV_BOOTED_FROM_SPI:
>> +		return BOOT_DEVICE_SPI;
>> +	case SUNIV_BOOTED_FROM_MMC1:
>> +		return BOOT_DEVICE_MMC2;
>> +	}
> 
> Don't you need to handle FEL boot here somehow?
Yes but I have no clue what the SP is also wouldn't we have it hang 
anyway. The other changes requested i have fixed, I'm sorry about the 
subpar commit descriptions.

Thanks,
	Jesse
> Cheers,
> Andre
> 
>> +
>> +	printf("Unknown boot source from BROM: 0x%x\n", brom_call);
>> +	return BOOT_DEVICE_MMC1;
>> +}
>> +
>>   #ifdef CONFIG_SPL_BUILD
>>   static u32 sunxi_get_spl_size(void)
>>   {
>> @@ -276,36 +295,13 @@ unsigned long spl_mmc_get_uboot_raw_sector(struct mmc *mmc,
>>   	return sector;
>>   }
>>   
>> -#ifdef CONFIG_MACH_SUNIV
>> -/*
>> - * The suniv BROM does not pass the boot media type to SPL, so we try with the
>> - * boot sequence in BROM: mmc0->spinor->fail.
>> - * TODO: This has the slight chance of being wrong (invalid SPL signature,
>> - * but valid U-Boot legacy image on the SD card), but this should be rare.
>> - * It looks like we can deduce from some BROM state upon entering the SPL
>> - * (registers, SP, or stack itself) where the BROM was coming from and use
>> - * that here.
>> - */
>> -void board_boot_order(u32 *spl_boot_list)
>> -{
>> -	/*
>> -	 * See the comments above in sunxi_get_boot_device() for information
>> -	 * about FEL boot.
>> -	 */
>> -	if (!is_boot0_magic(SPL_ADDR + 4)) {
>> -		spl_boot_list[0] = BOOT_DEVICE_BOARD;
>> -		return;
>> -	}
>> -
>> -	spl_boot_list[0] = BOOT_DEVICE_MMC1;
>> -	spl_boot_list[1] = BOOT_DEVICE_SPI;
>> -}
>> -#else
>>   u32 spl_boot_device(void)
>>   {
>> -	return sunxi_get_boot_device();
>> +	if (IS_ENABLED(CONFIG_MACH_SUNIV))
>> +		return suniv_get_boot_device();
>> +	else
>> +		return sunxi_get_boot_device();
>>   }
>> -#endif
>>   
>>   __weak void sunxi_sram_init(void)
>>   {
> 

Re: [PATCH v1 1/3] mach-sunxi: Add boot device detection for SUNIV

[Jesse Taube]

On 2/10/22 14:38, Siarhei Siamashka wrote:
> On Thu, Feb 10, 2022 at 6:35 AM Jesse Taube <mr.bossman075@gmail.com> wrote:
> [...]
>> +       case SUNIV_BOOTED_FROM_NAND:
>> +       case SUNIV_BOOTED_FROM_SPI:
>> +               return BOOT_DEVICE_SPI;
> 
> Is it really okay to lump SPI and NAND together and return
> BOOT_DEVICE_SPI for both?
Booting from SPI NAND.
I can change it to BOOT_DEVICE_NAND.
> What's the NAND support story on this device?
I thought BOOT_DEVICE_NAND was for parallel NAND but I may be wrong 
could come one clarify please?

Thanks,
	Jesse Taube

Re: [PATCH v1 1/3] mach-sunxi: Add boot device detection for SUNIV

[Andre Przywara]

On Thu, 10 Feb 2022 14:20:26 -0500
Jesse Taube <mr.bossman075@gmail.com> wrote:

Hi,

> On 2/10/22 05:57, Andre Przywara wrote:
> > On Wed,  9 Feb 2022 23:34:36 -0500
> > Jesse Taube <mr.bossman075@gmail.com> wrote:
> > 
> > Hi Jesse,
> > 
> > many thanks for sending this, much appreciated!
> >   
> >> Use Samuel's suggestion of looking at the BootRom's stack
> >> to determine the boot device.  
> > 
> > Can you please elaborate here what's going on, for future reference? Like:
> > =============
> > In contrast to other Allwinner SoCs the F1C100s BROM does not store a boot
> > source indicator in the eGON header in SRAM. This leaves us guessing where
> > we were exactly booted from, and for instance trying the SD card first,
> > even though we booted from SPI flash.
> > By inspecting the BROM code and by experimentation, Samuel found that the
> > top of the BROM stack contains unique pointers for each of the boot
> > sources, which we can use as a boot source indicator.
> > 
> > Remove the existing board_boot_order kludge and replace it with a proper
> > boot source indication function.
> > =============
> > 
> >   
> >>
> >> Signed-off-by: Jesse Taube <Mr.Bossman075@gmail.com>
> >> Suggested-by: Samuel Holland <samuel@sholland.org>
> >> ---
> >>   arch/arm/include/asm/arch-sunxi/spl.h | 15 ++++++++
> >>   arch/arm/mach-sunxi/board.c           | 50 ++++++++++++---------------
> >>   2 files changed, 38 insertions(+), 27 deletions(-)
> >>
> >> diff --git a/arch/arm/include/asm/arch-sunxi/spl.h b/arch/arm/include/asm/arch-sunxi/spl.h
> >> index 58cdf806d9..d069091297 100644
> >> --- a/arch/arm/include/asm/arch-sunxi/spl.h
> >> +++ b/arch/arm/include/asm/arch-sunxi/spl.h
> >> @@ -19,8 +19,23 @@
> >>   #define SUNXI_BOOTED_FROM_MMC0_HIGH	0x10
> >>   #define SUNXI_BOOTED_FROM_MMC2_HIGH	0x12
> >>   
> >> +/*
> >> + * Values taken from the Bootrom's stack used
> >> + * to determine where we booted from.
> >> + * 0xffff40f8: mmc0
> >> + * 0xffff4114: spi0 NAND
> >> + * 0xffff4130: spi0 NOR
> >> + * 0xffff4150: mmc1  
> > 
> > Those last four lines are redundant, as you say exactly that, in code,
> > down here again. Comments are good, speaking code is better.
> >   
> >> + */
> >> +
> >> +#define SUNIV_BOOTED_FROM_MMC0	0xffff40f8
> >> +#define SUNIV_BOOTED_FROM_NAND	0xffff4114
> >> +#define SUNIV_BOOTED_FROM_SPI	0xffff4130
> >> +#define SUNIV_BOOTED_FROM_MMC1	0xffff4150
> >> +
> >>   #define is_boot0_magic(addr)	(memcmp((void *)(addr), BOOT0_MAGIC, 8) == 0)
> >>   
> >>   uint32_t sunxi_get_boot_device(void);
> >> +uint32_t suniv_get_boot_device(void);
> >>   
> >>   #endif
> >> diff --git a/arch/arm/mach-sunxi/board.c b/arch/arm/mach-sunxi/board.c
> >> index 57078f7a7b..b0658d583e 100644
> >> --- a/arch/arm/mach-sunxi/board.c
> >> +++ b/arch/arm/mach-sunxi/board.c
> >> @@ -241,6 +241,25 @@ uint32_t sunxi_get_boot_device(void)
> >>   	return -1;		/* Never reached */
> >>   }
> >>   
> >> +uint32_t suniv_get_boot_device(void)  
> > 
> > This can be static, right?

Actually, looking closer, I think this should be restructured:
You keep this function, as a static, but let it return the sunxi
internal boot source codes (that the BROM would write on other SoCs):
	case SUNIV_BOOTED_FROM_MMC0:
		return SUNXI_BOOTED_FROM_MMC0;
	...
Possibly rename it to suniv_get_boot_source.
Then you change sunxi_get_boot_source() to:
	...
	if (IS_ENABLED(CONFIG_MACH_SUNIV))
		return suniv_get_boot_source();
	else
		return readb(SPL_ADDR + 0x28);

This way we fix this one level deeper, and can call
sunxi_get_boot_source() from elsewhere as well.

Hope that makes sense.

> >   
> >> +{
> >> +	/* Get the last function call from BootRom's stack. */
> >> +	u32 brom_call = *(u32 *)(fel_stash.sp - 4);You are okay with this I was expecting you to explain a better way that   
> i don't know about.

That looks alright, I think we explained how it works by now (commit
message plus two comments), plus the code makes that clear.

> >> +
> >> +	switch (brom_call) {
> >> +	case SUNIV_BOOTED_FROM_MMC0:
> >> +		return BOOT_DEVICE_MMC1;
> >> +	case SUNIV_BOOTED_FROM_NAND:
> >> +	case SUNIV_BOOTED_FROM_SPI:
> >> +		return BOOT_DEVICE_SPI;
> >> +	case SUNIV_BOOTED_FROM_MMC1:
> >> +		return BOOT_DEVICE_MMC2;
> >> +	}  
> > 
> > Don't you need to handle FEL boot here somehow?  
> Yes but I have no clue what the SP is also wouldn't we have it hang 
> anyway.

The stack would be from some SPL thunk code, which is not necessarily
stable (in fact we are still working on that). However we support FEL
booting already, because our FEL thunk code writes a signature into
SRAM - even when the BROM does not do that.
So we just keep that check from the current code, see the first part
of board_boot_order().

> The other changes requested i have fixed, I'm sorry about the 
> subpar commit descriptions.

No worries, it's just from experience that we want good commit
messages, so that others (or even yourself!) can make sense of code in
the future.

Cheers,
Andre

> >> +
> >> +	printf("Unknown boot source from BROM: 0x%x\n", brom_call);
> >> +	return BOOT_DEVICE_MMC1;
> >> +}
> >> +
> >>   #ifdef CONFIG_SPL_BUILD
> >>   static u32 sunxi_get_spl_size(void)
> >>   {
> >> @@ -276,36 +295,13 @@ unsigned long spl_mmc_get_uboot_raw_sector(struct mmc *mmc,
> >>   	return sector;
> >>   }
> >>   
> >> -#ifdef CONFIG_MACH_SUNIV
> >> -/*
> >> - * The suniv BROM does not pass the boot media type to SPL, so we try with the
> >> - * boot sequence in BROM: mmc0->spinor->fail.
> >> - * TODO: This has the slight chance of being wrong (invalid SPL signature,
> >> - * but valid U-Boot legacy image on the SD card), but this should be rare.
> >> - * It looks like we can deduce from some BROM state upon entering the SPL
> >> - * (registers, SP, or stack itself) where the BROM was coming from and use
> >> - * that here.
> >> - */
> >> -void board_boot_order(u32 *spl_boot_list)
> >> -{
> >> -	/*
> >> -	 * See the comments above in sunxi_get_boot_device() for information
> >> -	 * about FEL boot.
> >> -	 */
> >> -	if (!is_boot0_magic(SPL_ADDR + 4)) {
> >> -		spl_boot_list[0] = BOOT_DEVICE_BOARD;
> >> -		return;
> >> -	}
> >> -
> >> -	spl_boot_list[0] = BOOT_DEVICE_MMC1;
> >> -	spl_boot_list[1] = BOOT_DEVICE_SPI;
> >> -}
> >> -#else
> >>   u32 spl_boot_device(void)
> >>   {
> >> -	return sunxi_get_boot_device();
> >> +	if (IS_ENABLED(CONFIG_MACH_SUNIV))
> >> +		return suniv_get_boot_device();
> >> +	else
> >> +		return sunxi_get_boot_device();
> >>   }
> >> -#endif
> >>   
> >>   __weak void sunxi_sram_init(void)
> >>   {  
> >   

Re: [PATCH v1 1/3] mach-sunxi: Add boot device detection for SUNIV

[Andre Przywara]

On Thu, 10 Feb 2022 14:51:11 -0500
Jesse Taube <mr.bossman075@gmail.com> wrote:

> On 2/10/22 14:38, Siarhei Siamashka wrote:
> > On Thu, Feb 10, 2022 at 6:35 AM Jesse Taube <mr.bossman075@gmail.com> wrote:
> > [...]  
> >> +       case SUNIV_BOOTED_FROM_NAND:
> >> +       case SUNIV_BOOTED_FROM_SPI:
> >> +               return BOOT_DEVICE_SPI;  
> > 
> > Is it really okay to lump SPI and NAND together and return
> > BOOT_DEVICE_SPI for both?  
> Booting from SPI NAND.
> I can change it to BOOT_DEVICE_NAND.

Ah, sorry, I missed that. So SPI NAND is different from both SPI NOR and
parallel NAND, and we just don't support it (yet).
So just bail out in this case, ideally with a message.

Cheers,
Andre


> > What's the NAND support story on this device?  
> I thought BOOT_DEVICE_NAND was for parallel NAND but I may be wrong 
> could come one clarify please?
> 
> Thanks,
> 	Jesse Taube