[PATCH 13/14] rockchip: rk3588-rock-5b: Enable boot from SPI NOR flash

Tue May 16 19:38:26 CEST 2023

Hi Eugen,

On 2023-05-16 17:06, Eugen Hristev wrote:
> Hi Jonas,
> 
> On 5/9/23 14:16, Kever Yang wrote:
>>
>> On 2023/4/22 09:23, Jonas Karlman wrote:
>>> Add sfc and flash node to device tree and config options to enable
>>> support for booting from SPI NOR flash on Radxa ROCK 5 Model B.
>>>
>>> Signed-off-by: Jonas Karlman <jonas at kwiboo.se>
>> Reviewed-by: Kever Yang <kever.yang at rock-chips.com>
>>
>> Thanks,
>> - Kever
>>> ---
>>>   arch/arm/dts/rk3588-rock-5b-u-boot.dtsi | 24 ++++++++++++++++++++++++
>>>   arch/arm/dts/rk3588s-u-boot.dtsi        | 20 ++++++++++++++++++++
>>>   arch/arm/mach-rockchip/rk3588/rk3588.c  |  1 +
>>>   configs/rock5b-rk3588_defconfig         | 10 ++++++++++
>>>   4 files changed, 55 insertions(+)
>>>
>>> diff --git a/arch/arm/dts/rk3588-rock-5b-u-boot.dtsi 
>>> b/arch/arm/dts/rk3588-rock-5b-u-boot.dtsi
>>> index 4bbc19058c90..b63dd40deb6d 100644
>>> --- a/arch/arm/dts/rk3588-rock-5b-u-boot.dtsi
>>> +++ b/arch/arm/dts/rk3588-rock-5b-u-boot.dtsi
>>> @@ -11,6 +11,7 @@
>>>   / {
>>>       aliases {
>>>           mmc1 = &sdmmc;
>>> +        spi0 = &sfc;
>>>       };
>>>       chosen {
>>> @@ -43,6 +44,25 @@
>>>       pinctrl-0 = <&emmc_bus8 &emmc_clk &emmc_cmd &emmc_data_strobe 
>>> &emmc_rstnout>;
>>>   };
>>> +&sfc {
>>> +    bootph-pre-ram;
> 
> Any reason why the sfc and flash are pre-ram and the pins bootph-all ?

This used to be u-boot,dm-spl and was later replaced with bootph-pre-ram.

For fspim2_pins I used the same bootph as the other pins. Not sure why
the pins use bootph-all to begin with, sdmmc and sdhci use bootph-pre-ram
in rk3588s-u-boot.dtsi.

> 
>>> +    u-boot,spl-sfc-no-dma;
>>> +    pinctrl-names = "default";
>>> +    pinctrl-0 = <&fspim2_pins>;
>>> +    #address-cells = <1>;
>>> +    #size-cells = <0>;
>>> +    status = "okay";
>>> +
>>> +    flash at 0 {
>>> +        bootph-pre-ram;
>>> +        compatible = "jedec,spi-nor";
>>> +        reg = <0>;
>>> +        spi-max-frequency = <24000000>;
>>> +        spi-rx-bus-width = <4>;
>>> +        spi-tx-bus-width = <1>;
>>> +    };
>>> +};
>>> +
>>>   &pinctrl {
>>>       bootph-all;
>>> @@ -69,6 +89,10 @@
>>>       bootph-all;
>>>   };
>>> +&fspim2_pins {
>>> +    bootph-all;
>>> +};
>>> +
>>>   &sdmmc_bus4 {
>>>       bootph-all;
>>>   };
>>> diff --git a/arch/arm/dts/rk3588s-u-boot.dtsi 
>>> b/arch/arm/dts/rk3588s-u-boot.dtsi
>>> index cd7e6cb50ee2..d8a471a37fd1 100644
>>> --- a/arch/arm/dts/rk3588s-u-boot.dtsi
>>> +++ b/arch/arm/dts/rk3588s-u-boot.dtsi
>>> @@ -104,6 +104,15 @@
>>>           };
>>>       };
>>> +    sfc: spi at fe2b0000 {
>>> +        compatible = "rockchip,sfc";
>>> +        reg = <0x0 0xfe2b0000 0x0 0x4000>;
>>> +        interrupts = <GIC_SPI 206 IRQ_TYPE_LEVEL_HIGH 0>;
>>> +        clocks = <&cru SCLK_SFC>, <&cru HCLK_SFC>;
>>> +        clock-names = "clk_sfc", "hclk_sfc";
>>> +        status = "disabled";
>>> +    };
>>> +
>>>       otp: nvmem at fecc0000 {
>>>           compatible = "rockchip,rk3588-otp";
>>>           reg = <0x0 0xfecc0000 0x0 0x400>;
>>> @@ -164,3 +173,14 @@
>>>   &ioc {
>>>       bootph-pre-ram;
>>>   };
>>> +
>>> +#ifdef CONFIG_ROCKCHIP_SPI_IMAGE
>>> +&binman {
>>> +    simple-bin-spi {
>>> +        mkimage {
>>> +            args = "-n", CONFIG_SYS_SOC, "-T", "rksd";
>>> +            offset = <0x8000>;
> 
> What is this offset referring to ?

This offset is referring to the normal mmc 32 KiB offset that idbloader
is normally written to. I used the offset prop so that the
u-boot-rockchip-spi.bin can be written to offset 0 of spi similar as is
currently done for rk3399.

> 
>>> +        };
>>> +    };
>>> +};
>>> +#endif
>>> diff --git a/arch/arm/mach-rockchip/rk3588/rk3588.c 
>>> b/arch/arm/mach-rockchip/rk3588/rk3588.c
>>> index 18e67b5ca9b2..0e85893e0096 100644
>>> --- a/arch/arm/mach-rockchip/rk3588/rk3588.c
>>> +++ b/arch/arm/mach-rockchip/rk3588/rk3588.c
>>> @@ -41,6 +41,7 @@ const char * const boot_devices[BROM_LAST_BOOTSOURCE 
>>> + 1] = {
>>>       [BROM_BOOTSOURCE_EMMC] = "/mmc at fe2e0000",
>>>       [BROM_BOOTSOURCE_SPINOR] = "/spi at fe2b0000/flash at 0",
>>>       [BROM_BOOTSOURCE_SD] = "/mmc at fe2c0000",
>>> +    [6] = "/spi at fe2b0000/flash at 0",
> 
> Is this '6' meaning something in particular ? or just the next number in 
> line ?

The bootrom on rk3588 use value 6 when booting from the spi flash on my
ROCK 5B, normally bootrom have used value 3 (BROM_BOOTSOURCE_SPINOR) on
rk356x and earlier socs.

I have no idea what we should call this BOOTSOURCE, could not find any
define in vendor u-boot to help give this a proper name.

Kever: Any insights into what this value should be called?

> 
>>>   };
>>>   static struct mm_region rk3588_mem_map[] = {
>>> diff --git a/configs/rock5b-rk3588_defconfig 
>>> b/configs/rock5b-rk3588_defconfig
>>> index 2f0a74ee5559..e6a903853fb7 100644
>>> --- a/configs/rock5b-rk3588_defconfig
>>> +++ b/configs/rock5b-rk3588_defconfig
>>> @@ -8,15 +8,20 @@ CONFIG_SPL_LIBGENERIC_SUPPORT=y
>>>   CONFIG_NR_DRAM_BANKS=2
>>>   CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
>>>   CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0xc00000
>>> +CONFIG_SF_DEFAULT_SPEED=24000000
>>> +CONFIG_SF_DEFAULT_MODE=0x2000
> 
> Any reason for changing the default mode ?

Default speed and mode is changed to match the speed and mode used in
the device tree. SPL use these Kconfig instead of reading the speed and
mode from fdt blob.

> 
>>>   CONFIG_DEFAULT_DEVICE_TREE="rk3588-rock-5b"
>>>   CONFIG_ROCKCHIP_RK3588=y
>>>   CONFIG_SPL_ROCKCHIP_COMMON_BOARD=y
>>> +CONFIG_ROCKCHIP_SPI_IMAGE=y
>>>   CONFIG_SPL_SERIAL=y
>>>   CONFIG_SPL_STACK_R_ADDR=0x600000
>>>   CONFIG_TARGET_ROCK5B_RK3588=y
>>>   CONFIG_SPL_STACK=0x400000
>>>   CONFIG_DEBUG_UART_BASE=0xFEB50000
>>>   CONFIG_DEBUG_UART_CLOCK=24000000
>>> +CONFIG_SPL_SPI_FLASH_SUPPORT=y
>>> +CONFIG_SPL_SPI=y
>>>   CONFIG_SYS_LOAD_ADDR=0xc00800
>>>   CONFIG_DEBUG_UART=y
>>>   CONFIG_FIT=y
>>> @@ -35,6 +40,8 @@ CONFIG_SPL_BSS_MAX_SIZE=0x4000
>>>   # CONFIG_SPL_RAW_IMAGE_SUPPORT is not set
>>>   # CONFIG_SPL_SHARES_INIT_SP_ADDR is not set
>>>   CONFIG_SPL_STACK_R=y
>>> +CONFIG_SPL_SPI_LOAD=y
>>> +CONFIG_SYS_SPI_U_BOOT_OFFS=0x60000
> 
> I have a feeling the default is 0x80000, do you have any reason for the 
> change to 0x60000 ?

The default is probably set for rk3399 that requires double space for
idbloader because bootrom only read first 2 KiB of every 4 KiB page.
To not waste usable space I used the same offset that was used by
multiple rk3399 configs:

  u-boot,spl-payload-offset = <0x60000>; /* @ 384KB */

SPI flash layout would look something like:

  idbloader: @ 32 KiB
  - TPL: current ddr init blob use around 54 KiB
  - SPL: max 256 KiB
  FIT payload: @ 384 KiB

Should mean we have at least 96 KiB for TPL.

Regards,
Jonas

> 
>>>   CONFIG_SPL_ATF=y
>>>   CONFIG_CMD_GPIO=y
>>>   CONFIG_CMD_GPT=y
>>> @@ -59,6 +66,8 @@ CONFIG_MMC_SDHCI=y
>>>   CONFIG_MMC_SDHCI_SDMA=y
>>>   # CONFIG_SPL_MMC_SDHCI_SDMA is not set
>>>   CONFIG_MMC_SDHCI_ROCKCHIP=y
>>> +CONFIG_SPI_FLASH_MACRONIX=y
>>> +CONFIG_SPI_FLASH_XTX=y
>>>   CONFIG_ETH_DESIGNWARE=y
>>>   CONFIG_GMAC_ROCKCHIP=y
>>>   CONFIG_PHY_ROCKCHIP_INNO_USB2=y
>>> @@ -69,6 +78,7 @@ CONFIG_SPL_RAM=y
>>>   CONFIG_BAUDRATE=1500000
>>>   CONFIG_DEBUG_UART_SHIFT=2
>>>   CONFIG_SYS_NS16550_MEM32=y
>>> +CONFIG_ROCKCHIP_SFC=y
>>>   CONFIG_SYSRESET=y
>>>   CONFIG_USB=y
>>>   CONFIG_USB_EHCI_HCD=y
> 