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

[Pratyush Yadav <pratyush@kernel.org>]

On Fri, Jan 10 2025, Miquel Raynal wrote:

> Add support for Winbond w25q01jv spi-nor chip.
>
> This chip is internally made of two dies with linear addressing
> capabilities to make it transparent to the user that two dies were
> used. There is one drawback however, the read status operation is racy
> as the status bit only gives the active die status and not the status of
> the other die. For commands affecting the two dies, it means if another
> command is sent too fast after the first die has returned a valid status
> (deviation can be up to 200us), the chip will get corrupted/in an
> unstable state.
>
> This chip hence requires a better status register read. There are three
> solutions here:
>
> 1- If we assume that the most common situation producing this problem is
> status register writes, maybe we could change the "non-volatile"
> status register write commands to become "volatile" status register
> writes. In practice, what takes time is the write operation of the bits
> themselves, and not the activation of the feature in the internal
> circuitry. Enabling "volatile" status register writes would make the
> writes nearly instant.
>
> This approach, besides probably being the less impacting one, could
> overlook other possible actions where both dies can be used at the same
> time like a chip erase (or any erase over die boundaries in general).
>
> 2- Wait about 200us after getting a first status ready feedback. This
> 200us is about the maximum possible deviation between dies and would
> cover all cases.
>
> 3- We iterate manually over all internal dies (which takes about 30us
> per die) until all are ready. This approach will always be faster than
> a blind delay which represents the maximum deviation, while also being
> totally safe.
>
> This third approach has been adopted. A flash-specific hook for the
> status register read had to be implemented. Testing with the flash_speed
> benchmark shown no difference with the existing performances (using the
> regular status read core function). In practice there are difference in
> the experimental results below, but they are part of the natural
> deviation of the benchmark:
>
> 	> Without the fixup
> 	$ flash_speed /dev/mtd0 -c100 -d
> 	eraseblock write speed is 442 KiB/s
> 	eraseblock read speed is 1606 KiB/s
> 	page write speed is 439 KiB/s
> 	page read speed is 1520 KiB/s
> 	2 page write speed is 441 KiB/s
> 	2 page read speed is 1562 KiB/s
> 	erase speed is 68 KiB/s
>
> 	> With the fixup
> 	$ flash_speed /dev/mtd0 -c100 -d
> 	eraseblock write speed is 428 KiB/s
> 	eraseblock read speed is 1626 KiB/s
> 	page write speed is 426 KiB/s
> 	page read speed is 1538 KiB/s
> 	2 page write speed is 426 KiB/s
> 	2 page read speed is 1574 KiB/s
> 	erase speed is 66 KiB/s
>
> However, the fixup, whatever which one we pick, must be applied on
> multi-die chips, which hence must be properly flagged. The SFDP tables
> implemented give a lot of information but the die details are part of an
> optional table that is not implemented, hence we use a post parsing
> fixup hook to set the params->n_dice value manually.
>
> Link: https://www.winbond.com/resource-files/W25Q01JV%20SPI%20RevE%2003042024%20Plus.pdf
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>

Reviewed-by: Pratyush Yadav <pratyush@kernel.org>

-- 
Regards,
Pratyush Yadav