Definition for flash w25q128 is wrong

[e9hack]

Am 07.06.2024 um 13:44 schrieb 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 :/

Related to the spec
	https://www.winbond.com/hq/support/documentation/levelOne.jsp?__locale=en&DocNo=DA00-W25Q128FV
chapter 8.2.30, older flashes don't support SFDP. Newer flashes may support SFDP. From my two versions, one doesn't support SFDP, the other does support it.

> 
> 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