Definition for flash w25q128 is wrong

[Esben Haabendal]

"Michael Walle" <michael at walle.cc> writes:

> On Thu Jun 6, 2024 at 9:50 PM CEST, e9hack wrote:
>> I'm using a TP-LINK WDR3600 with a bigger flash. Since some time the
>> router hangs in an endless boot loop. I see the following message:
>>
>> ...
>> [    0.402716] spi-nor spi0.0: BFPT parsing failed. Please consider
>> using SPI_NOR_SKIP_SFDP when declaring the flash
>
> Looks like your flash doesn't support SFDP. Relevant threads are:
> https://lore.kernel.org/r/d99d87e7-47ba-d6fe-735f-16de2a2ec280@linaro.org/
> https://lore.kernel.org/r/20240603-macronix-mx25l3205d-fixups-v2-0-ff98da26835c@geanix.com/
>
>> [    0.413217] spi-nor: probe of spi0.0 failed with error -22
>> ...
>> [    0.926180] /dev/root: Can't open blockdev
>> [    0.930427] VFS: Cannot open root device "(null)" or
>> unknown-block(0,0): error -6
>> [    0.938037] Please append a correct "root=" boot option; here are
>> the available partitions:
>> [    0.946520] Kernel panic - not syncing: VFS: Unable to mount root
>> fs on unknown-block(0,0)
>> [    0.954914] Rebooting in 1 seconds..
>>
>> It looks like the definition for the flash is wrong:
>>
>> --- a/drivers/mtd/spi-nor/winbond.
>> c     2024-03-15 19:27:50.000000000 +0100
>> +++ b/drivers/mtd/spi-nor/winbond.c     2024-04-01 05:59:17.166780732 +0200
>> @@ -120,8 +120,8 @@ static const struct flash_info winbond_n
>>                 NO_SFDP_FLAGS(SECT_4K) },
>>         { "w25q80bl", INFO(0xef4014, 0, 64 * 1024,  16)
>>                 NO_SFDP_FLAGS(SECT_4K) },
>> -       { "w25q128", INFO(0xef4018, 0, 0, 0)
>> -               PARSE_SFDP
>> +       { "w25q128", INFO(0xef4018, 0, 64 * 1024, 256)
>> +               NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
>>                 FLAGS(SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB) },
>>         { "w25q256", INFO(0xef4019, 0, 64 * 1024, 512)
>>                 NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
>>
>> With these changes, the flash will be detected properly. The chip is
>> marked with:
>> The chip (SOIC8) is marked with:
>> winbond
>> 25Q128FVSG
>> 1327
>>
>> I use another TP-LINK router. This is an Archer C7 v2. It has the same
>> flash chip with date code 1528. This router doesn't have this issue.
>
> Looks like some of these flashes support SFDP and some don't :/

Which means that for those (old) flashes without SFDP was broken by
83e824a4a595 ("mtd: spi-nor: Correct flags for Winbond w25q128").

During patch review, the following was said:

>>> while here try, using INFO with INFO(0xef4018, 0, 0, 0), those
>>> parameters shall be discovered at run-time, so we prepare to get rid of
>>> explicitly setting them sooner or later.
>>
>> This is an entry matching various flash families from Winbond, see my
>> reply in v1. I'm not sure we should remove these as we could break the
>> older ones, which might or might not have SFDP tables. We don't know.
>
> I'd take the risk and break the older ones if there are some that don't
> define SFDP indeed, just to handle the conflict properly. We can't
> encourage code based on assumptions otherwise we'll get back to the
> knotted spi-nor code that we tried to untie in the last years.

I guess it is not an assumption anymore. There are flashes out there
with this jedec id which does not have SFDP tables.

So while parsing SFDP is good, keeping a fallback to some known "good"
values are needed. Something similar to what we are discussing in
https://lore.kernel.org/all/20240603-macronix-mx25l3205d-fixups-v2-0-ff98da26835c@geanix.com/

/Esben