[PATCH v2 5/5] rockchip: rock-pi-4: dts: spi: Make the index of the spi flash the same in SPL and U-Boot proper

[Kever Yang]

Hi Michal, Quentin, Xavier,


On 2022/7/18 21:01, Quentin Schulz wrote:
> Hi Michal,
>
> On 7/18/22 13:00, Michal Suchánek wrote:
>> On Mon, Jul 18, 2022 at 11:09:56AM +0200, Xavier Drudis Ferran wrote:
>>> El Mon, Jul 18, 2022 at 10:33:18AM +0200, Quentin Schulz deia:
>>>> Hi Xavier,
>>>>
>>>> On 7/15/22 18:30, Xavier Drudis Ferran wrote:
>>>>> Spi0 is not needed in SPL and SPL could be a little smaller 
>>>>> without it,
>>>>> but then the SF_DEFAULT_BOOT would have to be 0 to refer to spi1, and
>>>>> that's confusing, because once U-Boot proper runs, it numbers the 
>>>>> bus 1.
>>>>>
>>>>> Add spi0 to the pre-reloc and spl trees so that the flash is always
>>>>> connected to bus 1.
>>>>>
>>>>
>>>> Mmmm... Could we instead make U-Boot use the bus number from the 
>>>> alias in
>>>> the aliases DT node? I think the mmc subsystem does this already 
>>>> and it
>>>> would mean we don't need to enable unnecessary devices. Also, 
>>>> relying on
>>>> boot order for the bus number is brittle in Linux, I don't know about
>>>> U-Boot, but if we can avoid this assumption, it'd be great :)
>>>>
>>>> See: 
>>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__source.denx.de_u-2Dboot_u-2Dboot_-2D_commit_2243d19e5618122d9d7aba23eb51f63f2719dba5&d=DwIBAg&c=_sEr5x9kUWhuk4_nFwjJtA&r=LYjLexDn7rXIzVmkNPvw5ymA1XTSqHGq8yBP6m6qZZ4njZguQhZhkI_-172IIy1t&m=G-LrZmlVXqwFFza02frCo5F8vUCol4vDYe6RpSOpezwdgWuNQZyIB2hF_SNO4Gz3&s=O9wEK10SUOQNv_9zcY0K2oSdD_soaQtgjga-pw9nAgY&e=
>>>> for how to do it today?
>>>>
>>>
>>> Maybe I should just drop this patch and try to define
>>> CONFIG_SPL_DM_SEQ_ALIAS in configs/rock-pi-4-rk3399 instead ?
>>> Let me test this a little...
>>>
>>> I have CONFIG_DM_SEQ_ALIAS=y but   CONFIG_SPL_DM_SEQ_ALIAS unset.
>>>
>>>>
>>>> Your patch series got sent with each commit in their individual thread
>>>
>>> I know. Sorry for the lapsus. I did it right in v1, wrong in v2, and 
>>> will do right in v3.
>>
>> What is actually the correct naming here?
>>
>> We have in arch/arm/mach-rockchip/rk3399/rk3399.c
>>
>> const char * const boot_devices[BROM_LAST_BOOTSOURCE + 1] = {
>>          [BROM_BOOTSOURCE_EMMC] = "/sdhci at fe330000",
>>          [BROM_BOOTSOURCE_SPINOR] = "/spi at ff1d0000/flash at 0",
>>          [BROM_BOOTSOURCE_SD] = "/mmc at fe320000",
>> };
>>
>>          } spl_boot_devices_tbl[] = {
>>                  { BOOT_DEVICE_MMC1, "/mmc at fe320000" },
>>                  { BOOT_DEVICE_MMC2, "/sdhci at fe330000" },
>>                  { BOOT_DEVICE_SPI, "/spi at ff1d0000" },
>>          };
>>
>
> This one was incorrect for boards not redefining the aliases nodes for 
> mmc devices from rk3399-u-boot.dtsi and I've sent a patch for it:
> https://lore.kernel.org/u-boot/20220715151552.953654-1-foss+uboot@0leil.net/ 
>
>
>> which then presumably gets converted in common/spl/spl_mmc.c
>>
>> SPL_LOAD_IMAGE_METHOD("MMC1", 0, BOOT_DEVICE_MMC1, spl_mmc_load_image);
>> SPL_LOAD_IMAGE_METHOD("MMC2", 0, BOOT_DEVICE_MMC2, spl_mmc_load_image);
>> SPL_LOAD_IMAGE_METHOD("MMC2_2", 0, BOOT_DEVICE_MMC2_2, 
>> spl_mmc_load_image);
>>
>
> I actually think from spl_mmc_get_device_index()?
>
>> We then have this in rk3399.dtsi:
>>
>>          sdio0: mmc at fe310000 {
>>                  compatible = "rockchip,rk3399-dw-mshc",
>>
>>          sdmmc: mmc at fe320000 {
>>                  compatible = "rockchip,rk3399-dw-mshc",
>>
>>          sdhci: mmc at fe330000 {
>>                  compatible = "rockchip,rk3399-sdhci-5.1", 
>> "arasan,sdhci-5.1";
>>
>> and this in rk3399-u-boot.dtsi
>>
>>                  mmc0 = &sdhci;
>>                  mmc1 = &sdmmc;
>>
>> and this in arch/arm/dts/rk3399-pinebook-pro.dts
>>
>>          aliases {
>>                  mmc0 = &sdio0;
>>                  mmc1 = &sdmmc;
>>                  mmc2 = &sdhci;
>>          };
>>
>>
>> mmc at fe310000: 3
>> mmc at fe320000: 1 (SD)
>> mmc at fe330000: 0 (eMMC)
>>
>> This is not consistent with any of the above.
>>
For the boot sequence in SPL and U-Boot proper, two issue here:

1. mmc index

     There is no alias for mmc few years ago when we add alias for mmc 
in U-Boot, we would like to make the boot order like this:

SDMMC -> eMMC

   And SDIO is not in consideration because it's not storage in most 
case, we add alias for mmc so that we can use the boot script in 
include/configs/rockchip-common.h to get the Fixed boot order:

/* First try to boot from SD (index 1), then eMMC (index 0) */

     The SD card is removable and easy to replace, and not easy to break 
the board system, it's better to boot from SD card if there is available 
firmware inside.

     And after this rule in U-Boot works with boot script and mmc alias 
in -u-boot.dtsi, the kernel also add the alias without consideration for 
the boot order, then we can see the difference in u-boot dts now.


2. boot order in SPL and U-Boot proper

     For SPL, it's recommend to follow the BootRom boot device, because 
the loader until U-Boot usually in the same storage media;

     For U-Boot proper, it's per board different:

- for most SBC and product, it only support eMMC and SD, we would like 
to boot from SD card first so that we can upgrade the firmware to eMMC;

- for device with SPI, system will boot from SPI in loader stage, and 
boot from eMMC or NvMe in U-Boot proper for OS;


Hope the information is enough for you.


Thanks,
- Kever