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

[Stefan Roese]

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?

Thanks,
Stefan