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

Tony Dinh
2023-02-28T01:17:53Z

Hi Pali,

On Mon, Feb 27, 2023 at 4:42 PM Tony Dinh <mibodhi@gmail.com> wrote:
>
> Hi Pali,
>
> On Mon, Feb 27, 2023 at 3:41 PM Tony Dinh <mibodhi@gmail.com> wrote:
> >
> > Hi Pali,
> >
> > On Mon, Feb 27, 2023 at 1:57 PM Tony Dinh <mibodhi@gmail.com> wrote:
> > >
> > > Hi Stefan,
> > >
> > > On Sun, Feb 26, 2023 at 11:40 PM Stefan Roese <sr@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@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@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?

Found the answer in the A38x Hardware Specs. I've never noticed this
before. This board has the Sample at Reset set to boot from NAND!

"Table 48: Boot Device Mode Options
0x9
BootROM Enabled, Boot from NAND: 8 bits width, with page size of 512B,
4 Address cycles support per page, using MPP multiplexing option of
NAND 8 bits
0x32
BootROM Enabled, Boot from SPI: Controller #0, 24 address bits, NOR
Flash type, using MPP multiplexing option of SPI on MPP[25:22]"

So what we actually see here is the fall back to BootROM. And BootROM
still loads the image from SPI, ignoring that strapping. Am I confused
or correct? :)

Thanks,
Tony