Re: [PATCH 16/25] mtd: spi-nor: winbond: Prepare introduction of W25QxxPW-Q/N parts

[Michael Walle]

On Wed Sep 9, 2026 at 5:25 PM CEST, Miquel Raynal wrote:
> There is an ID collision between chips of same density from the JW
> family (which it self conflicted with yet another family) with new PW
> parts. Chips are very similar in practice, it is mostly a matter of
> electrical differences (mostly power consumption being lower) as well as
> the addition of inline ECC capability for the higher devices (> 32Mb).
>
> Chips with ECC capability protect 16 bytes chunks are against single
> errors. In case a non-aligned write happens, ECC is locally disabled
> until the next erase.
>
> Another significant difference is that PW chips identify themselves as
> supporting the new SFDP (rev F) QER field which forces an alternate
> write SR2 opcode (0x31).
>
> Signed-off-by: Miquel Raynal <miquel.raynal@xxxxxxxxxxx>
> ---
>  drivers/mtd/spi-nor/winbond.c | 55 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 55 insertions(+)
>
> diff --git a/drivers/mtd/spi-nor/winbond.c b/drivers/mtd/spi-nor/winbond.c
> index fbad9e408d7a..efa9de1f0f66 100644
> --- a/drivers/mtd/spi-nor/winbond.c
> +++ b/drivers/mtd/spi-nor/winbond.c
> @@ -71,6 +71,41 @@ static bool winbond_rv_match(const struct spi_nor *nor)
>  	return nor->sfdp && is_w25qxxrv(nor);
>  }
>  
> +static bool is_w25qxxpw(const struct spi_nor *nor)
> +{
> +	struct sfdp_header *sfdp_h = spi_nor_sfdp_get_header(nor);
> +
> +	/*
> +	 * W25QxxPW chips re-use the same ID as the W25QxxJW/NW family.
> +	 *
> +	 * Chips are very similar, W25QxxPW brings mostly performance and power
> +	 * consumption improvements. One key difference in behaviour is the
> +	 * automatic 16-byte based error correction.
> +	 *
> +	 * They can be distinguished based on their SFDP minor revision:
> +	 * W25QxxJW:       JESD216B, minor revision == 06h
> +	 * W25Q51/01/02NW: JESD216B, minor revision == 06h
> +	 * W25QxxPW:       JESD216F, minor revision >= 0Ah
> +	 */
> +	return sfdp_h->minor >= SFDP_JESD216F_MINOR;
> +}
> +
> +static bool winbond_pw_match(const struct spi_nor *nor)
> +{
> +	return nor->sfdp && is_w25qxxpw(nor);
> +}
> +
> +static bool winbond_pw_with_ecc_match(const struct spi_nor *nor)
> +{
> +	const struct spi_nor_id *id = nor->info->id;
> +
> +	if (!winbond_pw_match(nor))
> +		return false;
> +
> +	/* W25Q33PW chips (id[2] == 0x16) do not have built-in ECC support */

The commit message mentions ECC is available for >= 32Mbit. Doesn't
align with this comment. Also about the 8MBit and 16Mbit ones?

"W25QxxPW chips smaller than 32MBit doesn't provide built-in ECC support"?

If you like I could rewrite it as I'll apply it. The rest of this
series looks good.

> +	return id->len == 3 && id->bytes[2] >= 0x17;

Then this makes more sense.

-michael

> +}
> +
>  static int
>  w25q128_post_bfpt_fixups(struct spi_nor *nor,
>  			 const struct sfdp_parameter_header *bfpt_header,
> @@ -201,6 +236,22 @@ static const struct spi_nor_fixups winbond_nor_partname_fixups = {
>  	.post_sfdp = winbond_nor_partname_post_sfdp_fixups,
>  };
>  
> +static int winbond_nor_ecc_configuration_post_sfdp_fixups(struct spi_nor *nor)
> +{
> +	/*
> +	 * PW chips feature automatic error correction. Non 16-byte aligned
> +	 * writes work, but disable error correction on the region until next erase.
> +	 */
> +	nor->params->writesize = 16;
> +	nor->params->flags |= SNOR_F_ECC;
> +
> +	return 0;
> +}
> +
> +static const struct spi_nor_fixups winbond_nor_ecc_configuration_fixups = {
> +	.post_sfdp = winbond_nor_ecc_configuration_post_sfdp_fixups,
> +};
> +
>  static const struct flash_info winbond_nor_parts[] = {
>  	{
>  		.id = SNOR_ID(0xef, 0x30, 0x10),
> @@ -622,6 +673,10 @@ static const struct spi_nor_fixup winbond_fixups[] = {
>  	  .fixups = &winbond_nor_multi_die_fixups },
>  	{ .id = SNOR_ID(0xef, 0x40, 0x22), .match = winbond_jv_match,
>  	  .fixups = &winbond_nor_multi_die_fixups },
> +	{ .id = SNOR_ID(0xef, 0x60), .match = winbond_pw_with_ecc_match,
> +	  .fixups = &winbond_nor_ecc_configuration_fixups },
> +	{ .id = SNOR_ID(0xef, 0x60), .match = winbond_pw_match,
> +	  .fixups = &winbond_nor_partname_fixups },
>  	{ .id = SNOR_ID(0xef, 0x70), .match = winbond_rv_match,
>  	  .fixups = &winbond_nor_partname_fixups },
>  	{ .id = SNOR_ID(0xef, 0x70, 0x18), .fixups = &w25q128_fixups },

Attachment: signature.asc
Description: PGP signature