[PATCH 2/2] arm: mvebu: clearfog: Add defconfig for SPI booting

[Tony Dinh]

Hi Stefan,

On Sun, Feb 26, 2023 at 11:40 PM Stefan Roese <sr at denx.de> wrote:
>
> Hi Tony,
>
> On 2/27/23 01:11, Tony Dinh wrote:
> > Hi Pali,
> >
> > On Sun, Feb 26, 2023 at 2:52 AM Pali Rohár <pali at kernel.org> wrote:
> >>
> >> On Saturday 25 February 2023 20:56:10 Tony Dinh wrote:
> >>> Hi Martin,
> >>>
> >>> On Sat, Feb 25, 2023 at 6:17 PM Martin Rowe <martin.p.rowe at gmail.com> wrote:
> >>>>
> >>>>>>> I'm not sure how to run proper timing tests on the process, but
> >>>>>>> stopwatch timing just between seeing "Trying to boot" and "U-Boot
> >>>>>>> 2023.04-rc2" showed the return to BootROM under 1 second, and the
> >>>>>>> direct from SPI around 4 seconds. I thought the goal of loading from
> >>>>>>> SPI directly was speed, but returning to BootROM is significantly
> >>>>>>> faster on this board.
> >>>>>>
> >>>>>> You should check SPI speed in DTS file and also in the defconfig.
> >>>>>
> >>>>> I think we have probably seen this slowdown before. There is a TODO in
> >>>>> the way the DTS nodes are parsed by DM uclass. So this config must be
> >>>>> set in defconfig to get around that bug:
> >>>>> CONFIG_SF_DEFAULT_SPEED=50000000
> >>>>
> >>>> That works and is much faster now. I'll submit it in a V2 for these
> >>>> two patches once Pali's mvebu changes are accepted. Only question I
> >>>> have is: should the faster speed be applied to all three defconfigs?
> >>>> CONFIG_SF_DEFAULT_SPEED=1000000 (50x less) is implicitly added to the
> >>>> MMC and SATA configs at the moment.
> >>>
> >>> This is only a workaround to get the SPI probe to work correctly. The
> >>> real fix should be in spi_flash_probe()
> >>> ./common/spl/spl_spi.c
> >>>          flash = spi_flash_probe(sf_bus, sf_cs,
> >>>                                  CONFIG_SF_DEFAULT_SPEED,
> >>>                                  CONFIG_SF_DEFAULT_MODE);
> >>> If spi_flash_probe() failed to get SPI max_hz from the DTS, the
> >>> fallback is CONFIG_SF_DEFAULT_SPEED.
> >>>
> >>> IMO, perhaps we could add CONFIG_SF_DEFAULT_SPEED and
> >>> CONFIG_SF_DEFAULT_MODE selection to arch/arm/mach-mvebu/Kconfig or
> >>> some other common place. And when we will have fixed the DTS parsing
> >>> problem, they can be removed.
> >>>
> >>> I'd like to hear from Stefan and Pali if this approach sounds good.
> >>>
> >>> All the best,
> >>> Tony
> >>
> >> Well, the maximal SPI speed depends on the wiring. You can have on the
> >> board some SPI device which does not support high frequency.
> >>
> >> But having some sane defaults in Kconfig for mvebu makes sense.
>
> Agreed.
>
> > The CONFIG_SF_DEFAULT_SPEED is set to 1_000_000 if the board defconfig
> > does not specify it. I think the sane default value is 10_000_000
> > (grepping the Armada* DTS files showing the slowest spi-max-frequency
> > is 10_000_000). While a big improvement, it is not fast enough for
> > many other boards. So we still need to define it in those board
> > defconfigs (before fixing that bug), or use return to BootROM.
>
> I'm fine with setting CONFIG_SF_DEFAULT_SPEED to 10.000.000 for MVEBU.
> Not sure if this should be done in drivers/mtd/spi/Kconfig, as this
> currently has no platform- / board- specific configurations. Perhaps
> it can be done in a mach-mvebu Kconfig file instead?

Yes, I think it should be done in mach-mvebu Kconfig, too. I will run
some tests.

Thanks,
Tony