Hi Pali, On Mon, Feb 27, 2023 at 3:41 PM Tony Dinh <mibo...@gmail.com> wrote: > > Hi Pali, > > On Mon, Feb 27, 2023 at 1:57 PM Tony Dinh <mibo...@gmail.com> wrote: > > > > Hi Stefan, > > > > On Sun, Feb 26, 2023 at 11:40 PM Stefan Roese <s...@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 <p...@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.r...@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. > > It is not related to this patch series (I also tested without the > patch series to confirm). But it is strange that I can no longer get > the configuration to boot from SPI. The 1st device in the boot order > is alway BOOTROM. The spl_boot_list is printed out below. > > <BEGIN LOG> > High speed PHY - Ended Successfully > mv_ddr: 14.0.0 > DDR4 Training Sequence - Switching XBAR Window to FastPath Window > mv_ddr: completed successfully > board_boot_order spl_boot_list[0] = 15 > Trying to boot from BOOTROM > Returning to BootROM (return address 0xffff05c4)... > BootROM: Image checksum verification PASSED > <END LOG> > > The SPL SPI configs (board Thecus N2350) are: > # grep SPL .config| grep SPI > > CONFIG_MVEBU_SPL_BOOT_DEVICE_SPI=y > CONFIG_SPL_DM_SPI=y > CONFIG_SPL_SPI_FLASH_SUPPORT=y > CONFIG_SPL_SPI=y > CONFIG_SPL_DM_SPI_FLASH=y > CONFIG_SPL_SPI_FLASH_TINY=y > # CONFIG_SPL_SPI_FLASH_MTD is not set > CONFIG_SPL_SPI_LOAD=y > > Did I miss something new lately? > > Thanks, > Tony > > Trying to boot from BOOTROM > Returning to BootROM (return address 0xffff05c4)... > BootROM: Image checksum verification PASSED
It turns out that the board strapping register itself is the problem. boot_device=0x9 was printed out in arch/arm/mach-mvebu/cpu.c. It surely does not match what we expected for A38x (#define BOOT_FROM_SPI 0x32). Actually 0x9 is not defined in cpu.c at all. So it fell to the default case, which is BOOTROM. <BEGIN LOG> U-Boot SPL 2023.04-rc2-tld-1-00089-g3fe03f96fc-dirty (Feb 27 2023 - 16:24:01 -0800) High speed PHY - Version: 2.0 Detected Device ID 6820 board SerDes lanes topology details: | Lane # | Speed | Type | -------------------------------- | 0 | 0 | SGMII0 | | 1 | 3 | SATA0 | | 2 | 3 | SATA1 | | 4 | 5 | USB3 HOST0 | | 5 | 5 | USB3 HOST1 | -------------------------------- High speed PHY - Ended Successfully mv_ddr: 14.0.0 DDR4 Training Sequence - Switching XBAR Window to FastPath Window mv_ddr: completed successfully BOOTROM_REG=0x97001000 boot_device=0x9 spl_boot_device boot_device = 15 board_boot_order spl_boot_list[0] = 15 Trying to boot from BOOTROM Returning to BootROM (return address 0xffff05c4)... BootROM: Image checksum verification PASSED <END LOG> Is there a chance this value 0x9 means something that we have not come across? Thanks, Tony
