Re: [PATCH v2 22/25] mtd: spi-nor: winbond: Add support for W25Q33PW-M

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] mtd: spi-nor: winbond: W25Q33PW-M inline ECC incorrectly disabled by density check
--

commit a82dcd60aac8f24c79cf246f21c0a7afd4ac0ae0
Author: Miquel Raynal <miquel.raynal@xxxxxxxxxxx>

mtd: spi-nor: winbond: Add support for W25Q33PW-M

There is an ID collision with chips of same density from the JW family
(which it self conflicted with yet another family). 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.

> diff --git a/drivers/mtd/spi-nor/winbond.c b/drivers/mtd/spi-nor/winbond.c
> index c1792b12db630..c56bb01c197b2 100644
> --- a/drivers/mtd/spi-nor/winbond.c
> +++ b/drivers/mtd/spi-nor/winbond.c
> @@ -467,7 +467,7 @@ static const struct flash_info winbond_nor_parts[] = {
>  		.size = SZ_64M,
>  		.no_sfdp_flags = SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ,
>  	}, {
> -		/* W25Q32JW-M */
> +		/* W25Q32JW-M, W25Q33PW-M */
>  		.id = SNOR_ID(0xef, 0x80, 0x16),
>  		.name = "w25q32jwm",

[Severity: High]
The commit message states that the W25Q33PW-M chip supports inline ECC, but
does this newly added chip actually get its ECC configuration applied?

Looking at winbond_pw_with_ecc_match() in drivers/mtd/spi-nor/winbond.c:

static bool winbond_pw_with_ecc_match(const struct spi_nor *nor)
{
	...
	/* W25QxxPW densities <= 32 Mbit (id[2] <= 0x16) do not have built-in ECC support */
	return id->len == 3 && id->bytes[2] >= 0x17;
}

Since the W25Q33PW-M has an ID of 0xef, 0x80, 0x16, the density byte is 0x16.
This causes winbond_pw_with_ecc_match() to unconditionally evaluate to false,
preventing the winbond_nor_ecc_configuration_post_sfdp_fixups from being
applied.

As a result, SNOR_F_ECC is never set and writesize remains 1 instead of 16.
Will this cause writes to be unaligned to the chip's 16-byte chunks, which
according to the commit message disables ECC locally until the next erase?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260920-winbond-master-spi-nor-jw-cleanup-pw-addition-v2-0-ab9c9688d071@xxxxxxxxxxx?part=22