Re: [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

["Michal Suchánek" <msuchanek@suse.de>]

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://source.denx.de/u-boot/u-boot/-/commit/2243d19e5618122d9d7aba23eb51f63f2719dba5
> > 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@fe330000",
        [BROM_BOOTSOURCE_SPINOR] = "/spi@ff1d0000/flash@0",
        [BROM_BOOTSOURCE_SD] = "/mmc@fe320000",
};

        } spl_boot_devices_tbl[] = {
                { BOOT_DEVICE_MMC1, "/mmc@fe320000" },
                { BOOT_DEVICE_MMC2, "/sdhci@fe330000" },
                { BOOT_DEVICE_SPI, "/spi@ff1d0000" },
        };

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

We then have this in rk3399.dtsi:

        sdio0: mmc@fe310000 {
                compatible = "rockchip,rk3399-dw-mshc",

        sdmmc: mmc@fe320000 {
                compatible = "rockchip,rk3399-dw-mshc",

        sdhci: mmc@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@fe310000: 3
mmc@fe320000: 1 (SD)
mmc@fe330000: 0 (eMMC)

This is not consistent with any of the above.

Also on upstream kernel eMMC is mmc0 and SD mmc1 (somewhat consistently
with the above), while on downstream kernel it seems these are mmc1 and
mmc2, respectively.

Thanks

Michal