Hi Stefan, On Sun, Feb 26, 2023 at 11:40 PM Stefan Roese <[email protected]> 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 <[email protected]> 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 <[email protected]> > >>> 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
