Re: [PATCH 1/2] mtd: spi-nor: winbond: Add support for w25q01jv

[Pratyush Yadav <pratyush@kernel.org>]

On Mon, Jan 20 2025, Miquel Raynal wrote:

> Hi Pratyush,
>
>> Okay, I am confused because you said earlier that:
>>
>>> The bug that has been experienced followed this sequence:
>>> - send the write enable command (non-volatile)
>>> - wait for the ready/busy bit, ie. wait for the WEL bit to be set
>>>   because it is non-volatile write
>>> - active die is ready, (but idle die is not!)
>>> - enter 4-byte address mode, only the die that is ready processes the
>>>   command.
>>
>> Which says the WEL bit being set itself is racy. What I understand from
>> that is one die is ready to take writes and the other is not. Now when
>> you try to write the SR to enable 4B mode, it would only work on the die
>> that got the WEL set. The other one ignores it and stays in 3B mode. Do
>> I understand this correctly? To fix this you need to wait after the
>> write enable, before you initiate the write SR operation.
>
> Sorry for the confusion, I got myself confused as well. I double checked
> with Winbond and I think I have the correct explanation now:
>
> The WEL bit is volatile, there is no delay when setting it (well, about
> 10ns, but no specific deviation).
>
> On most chips, WEL enables all write operations:
>   * (single/dual/quad) page programs
>   * (sector/block/chip) erases
>   * status register write
>   * erase/program of other internal registers (like security registers)
>
> On Winbond, the above applies, but with the usual "Write Enable command
> (06h)", the status register bits are non-volatile, ie. they are stored
> in non-volatile cells (which takes time to program and are subject
> to deviations across dies). Hence, they added another command, called
> "Write Enable for Volatile Status Register (50h)" which is an addition
> to the usual "Write Enable command (06h)" which causes:
> - enabling writes on the status register only (if the WEL bit is not yet
>   set)
> - using volatile writes for the status register bits (ie. they are using
>   some kind of local cache which update almost immediately).
>
> So basically, if you do the following:
> - status register write
> - check the status bit with the standard helper
> - (and quickly after) do anything else on the idle die
> In this case you could experience a race, but that is not related to the
> Write Enable command.
>
> In general I believe enabling volatile status register writes would not
> be useful as long as we have the "read the status from all dies"
> workaround.
>
> Let me know if something is still unclear.

Ah, okay. That all makes sense now. With this explanation, your patches
should be doing the right thing. I will do a round of review on the v3
soon, but they looked mostly good when I last took a brief look.

-- 
Regards,
Pratyush Yadav