[PATCH v2 3/4] rockchip: rk3576: Add SPI Flash boot support

[Quentin Schulz]

Hi Jonas,

On 8/1/25 11:06 PM, Jonas Karlman wrote:
> The bootsource ids reported by BootROM of RK3576 for SPI NOR and USB
> differs slightly compared to prior SoCs:
> 
> - Booting from sfc0 (ROCK 4D) report the normal bootsource id 0x3.
> - Booting from sfc1 M1 (NanoPi M5) report a new bootsource id 0x23.
> - Booting from sfc1 M0 has not been tested (no board using this config).

I would refrain from adding partial support for it then.

> - Booting from USB report a new bootsource id 0x81.
> 
> Add a RK3576 specific read_brom_bootsource_id() function to help decode
> the new bootsource id values and the required boot_devices mapping of
> sfc0 and sfc1 to help support booting from SPI flash on RK3576.
> 
> Signed-off-by: Jonas Karlman <jonas at kwiboo.se>
> ---
> v2: No change
> ---
>   arch/arm/dts/rk3576-u-boot.dtsi        | 36 ++++++++++++++++++++++++++
>   arch/arm/mach-rockchip/rk3576/rk3576.c | 23 ++++++++++++++++
>   2 files changed, 59 insertions(+)
> 
> diff --git a/arch/arm/dts/rk3576-u-boot.dtsi b/arch/arm/dts/rk3576-u-boot.dtsi
> index fb5a107f47d9..c7ed09e03eec 100644
> --- a/arch/arm/dts/rk3576-u-boot.dtsi
> +++ b/arch/arm/dts/rk3576-u-boot.dtsi
> @@ -6,6 +6,11 @@
>   #include "rockchip-u-boot.dtsi"
>   
>   / {
> +	aliases {
> +		spi5 = &sfc0;
> +		spi6 = &sfc1;
> +	};
> +
>   	chosen {
>   		u-boot,spl-boot-order = "same-as-spl", &sdmmc, &sdhci;
>   	};
> @@ -16,6 +21,17 @@
>   	};
>   };
>   
> +#ifdef CONFIG_ROCKCHIP_SPI_IMAGE
> +&binman {
> +	simple-bin-spi {
> +		mkimage {
> +			args = "-n", CONFIG_SYS_SOC, "-T", "rksd";
> +			offset = <0x8000>;
> +		};
> +	};
> +};
> +#endif
> +
>   &cru {
>   	bootph-all;
>   };
> @@ -45,6 +61,16 @@
>   	bootph-some-ram;
>   };
>   
> +&fspi0_csn0 {
> +	bootph-pre-ram;
> +	bootph-some-ram;
> +};
> +
> +&fspi0_pins {
> +	bootph-pre-ram;
> +	bootph-some-ram;
> +};
> +
>   &ioc_grf {
>   	bootph-all;
>   };
> @@ -116,6 +142,16 @@
>   	bootph-some-ram;
>   };
>   
> +&sfc0 {
> +	bootph-some-ram;
> +	u-boot,spl-sfc-no-dma;
> +};
> +
> +&sfc1 {
> +	bootph-some-ram;
> +	u-boot,spl-sfc-no-dma;
> +};
> +
>   &sys_grf {
>   	bootph-all;
>   };
> diff --git a/arch/arm/mach-rockchip/rk3576/rk3576.c b/arch/arm/mach-rockchip/rk3576/rk3576.c
> index a6c2fbdc4840..3d5bdeeeb846 100644
> --- a/arch/arm/mach-rockchip/rk3576/rk3576.c
> +++ b/arch/arm/mach-rockchip/rk3576/rk3576.c
> @@ -36,8 +36,17 @@
>   #define USB_GRF_BASE		0x2601E000
>   #define USB3OTG0_CON1		0x0030
>   
> +enum {
> +	BROM_BOOTSOURCE_FSPI0 = 3,
> +	BROM_BOOTSOURCE_FSPI1_M0 = 4,
> +	BROM_BOOTSOURCE_FSPI1_M1 = 6,
> +};
> +
>   const char * const boot_devices[BROM_LAST_BOOTSOURCE + 1] = {
>   	[BROM_BOOTSOURCE_EMMC] = "/soc/mmc at 2a330000",
> +	[BROM_BOOTSOURCE_FSPI0] = "/soc/spi at 2a340000/flash at 0",
> +	[BROM_BOOTSOURCE_FSPI1_M0] = "/soc/spi at 2a300000/flash at 0",

This in addition to BROM_BOOTSOURCE_FSPI1_M0 in the enum may mislead the 
user into believing it is supported whereas we most likely need to do 
another mapping in read_brom_bootsource_id() below, so I would just not 
add either to this patch.

> +	[BROM_BOOTSOURCE_FSPI1_M1] = "/soc/spi at 2a300000/flash at 0",
>   	[BROM_BOOTSOURCE_SD] = "/soc/mmc at 2a310000",
>   };
>   
> @@ -85,6 +94,20 @@ void board_debug_uart_init(void)
>   {
>   }
>   
> +u32 read_brom_bootsource_id(void)
> +{
> +	u32 bootsource_id = readl(BROM_BOOTSOURCE_ID_ADDR);
> +
> +	if (bootsource_id == 0x23)
> +		return BROM_BOOTSOURCE_FSPI1_M1;
> +	else if (bootsource_id == 0x81)
> +		return BROM_BOOTSOURCE_USB;

I believe this differs from what we've done for all other SoCs where we 
have an entry at the offset bootsource_id in the boot_devices array.

On RK3588 it seems we're doing something similar of defining new 
BROM_BOOTSOURCE_ values in another enum and using them in the 
boot_devices array as indices. However, it seems we're using them 
verbatim instead of doing the mapping like we do here in rk3576.c. The 
only reason I could think of that is that 0x81 would create a pretty big 
array for no specific reason. Considering we already have an entry for 
the exact same scenario with a different number, I feel like it's fair 
to do the mapping.

It would be nice at least to provide some information on why it's done 
this way since it differs from what we've done until now. Especially 
that the enum doesn't match the raw value from the register anymore.

Cheers,
Quentin