Re: [PATCH v3 08/23] mtd: spi-nor: winbond: Prepare introduction of W25QxxRV-Q/N parts

["Michael Walle" <mwalle@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 4157 bytes --]

On Thu Aug 13, 2026 at 5:19 PM CEST, Miquel Raynal wrote:
> There is an ID collision between the JV and RV families. Both chips are
> very similar in practice, it is mostly a matter of electrical
> differences (mostly power consumption being lower).
>
> As a significant difference, RV chips identify themselves as supporting
> the new SFDP (rev F) field which forces an alternate write SR2 opcode
> (0x31). They also do not require the multi-die fixups which must remain
> assigned to the JV chips.
>
> Finally, since they share the IDs but not the names, we must hide the
> names using a fixup.
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>

Reviewed-by: Michael Walle <mwalle@kernel.org>

With a comment below.

> ---
>  drivers/mtd/spi-nor/winbond.c | 56 ++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 53 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/winbond.c b/drivers/mtd/spi-nor/winbond.c
> index 583b1669270f..8c1cad9e21b4 100644
> --- a/drivers/mtd/spi-nor/winbond.c
> +++ b/drivers/mtd/spi-nor/winbond.c
> @@ -146,6 +146,51 @@ static const struct spi_nor_fixups winbond_nor_multi_die_fixups = {
>  	.post_sfdp = winbond_nor_multi_die_post_sfdp_fixups,
>  };
>  
> +static int winbond_nor_partname_post_sfdp_fixups(struct spi_nor *nor)
> +{
> +	/*
> +	 * W25QxxRV parts re-use the JEDEC IDs of the JV family. Their name
> +	 * being a legacy field, it is kept for the already established JV parts
> +	 * but must not be exposed by the newer RV ones.
> +	 */
> +	nor->partname = NULL;
> +
> +	return 0;
> +}
> +
> +static const struct spi_nor_fixups winbond_nor_partname_fixups = {
> +	.post_sfdp = winbond_nor_partname_post_sfdp_fixups,
> +};
> +
> +static bool is_w25qxxrv(const struct spi_nor *nor)
> +{
> +	struct sfdp_header *sfdp_h = (struct sfdp_header *)nor->sfdp->dwords;

nitpick, spi_nor_sfdp_get_header()?

> +
> +	/*
> +	 * W25QxxRV chips re-use the same ID as the W25QxxJV family.
> +	 *
> +	 * Chips are very similar, W25QxxRV brings mostly performance and power
> +	 * consumption improvements. The RV family does not require the multi
> +	 * die fixup.
> +	 *
> +	 * They can be distinguished based on their SFDP minor revision:
> +	 * W25QxxJV:        JESD216A, minor revision == 05h
> +	 * W25Q512/01/02JV: JESD216B, minor revision == 06h
> +	 * W25QxxRV:        JESD216F, minor revision >= 0Ah
> +	 */
> +	return sfdp_h->minor >= SFDP_JESD216F_MINOR;
> +}
> +
> +static bool winbond_jv_match(const struct spi_nor *nor)
> +{
> +	return !nor->sfdp || !is_w25qxxrv(nor);

So how do we know if nor->sfdp is already there for a given fixup.
Without having looked at the code, there could potentially be fixups
before SFDP is parsed (and the nor->sfdp is populated), right? Might
be worth to be mentioned somewhere.

-michael

> +}
> +
> +static bool winbond_rv_match(const struct spi_nor *nor)
> +{
> +	return nor->sfdp && is_w25qxxrv(nor);
> +}
> +
>  static const struct flash_info winbond_nor_parts[] = {
>  	{
>  		.id = SNOR_ID(0xef, 0x30, 0x10),
> @@ -552,9 +597,14 @@ static const struct spi_nor_fixup winbond_fixups[] = {
>  	{ .fixups = &winbond_nor_fixups },
>  	{ .id = SNOR_ID(0xef, 0x40, 0x18), .fixups = &w25q128_fixups },
>  	{ .id = SNOR_ID(0xef, 0x40, 0x19), .fixups = &w25q256_fixups },
> -	{ .id = SNOR_ID(0xef, 0x40, 0x21), .fixups = &winbond_nor_multi_die_fixups },
> -	{ .id = SNOR_ID(0xef, 0x70, 0x21), .fixups = &winbond_nor_multi_die_fixups },
> -	{ .id = SNOR_ID(0xef, 0x70, 0x22), .fixups = &winbond_nor_multi_die_fixups },
> +	{ .id = SNOR_ID(0xef, 0x40), .match = winbond_rv_match,
> +	  .fixups = &winbond_nor_partname_fixups },
> +	{ .id = SNOR_ID(0xef, 0x40, 0x21), .match = winbond_jv_match,
> +	  .fixups = &winbond_nor_multi_die_fixups },
> +	{ .id = SNOR_ID(0xef, 0x70, 0x21), .match = winbond_jv_match,
> +	  .fixups = &winbond_nor_multi_die_fixups },
> +	{ .id = SNOR_ID(0xef, 0x70, 0x22), .match = winbond_jv_match,
> +	  .fixups = &winbond_nor_multi_die_fixups },
>  };
>  
>  const struct spi_nor_manufacturer spi_nor_winbond = {


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 297 bytes --]