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

From: Miquel Raynal <miquel.raynal@bootlin.com>
To: "Michael Walle" <mwalle@kernel.org>
Cc: "Pratyush Yadav" <pratyush@kernel.org>,
	 "Takahiro Kuwano" <takahiro.kuwano@infineon.com>,
	 "Richard Weinberger" <richard@nod.at>,
	"Vignesh Raghavendra" <vigneshr@ti.com>,
	 "Thomas Petazzoni" <thomas.petazzoni@bootlin.com>,
	 "Steam Lin" <STLin2@winbond.com>,
	<linux-mtd@lists.infradead.org>,  <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 16/25] mtd: spi-nor: winbond: Prepare introduction of W25QxxPW-Q/N parts
Date: Fri, 18 Sep 2026 12:20:20 +0200	[thread overview]
Message-ID: <8733v6hndn.fsf@bootlin.com> (raw)
In-Reply-To: <DLIBQKG04DIF.1FKAYOEZH6ZU0@kernel.org> (Michael Walle's message of "Fri, 18 Sep 2026 11:11:10 +0200")

>> +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?

You're right, there are 8 and 16Mb chips which I haven't in hands and
therefore forgot about. The return line below is correct.

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

32Mb chips do not have ECC capability either, so maybe:

/* W25QxxPW densities <= 32 Mbit (id[2] <= 0x16) do not have built-in ECC support */

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

Yes, thank you!

>
>> +	return id->len == 3 && id->bytes[2] >= 0x17;
>
> Then this makes more sense.
>
> -michael

Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/