Re: [PATCH v3] mtd: spinand: winbond: add support for W25N04LW

From: Dominique Martinet <dominique.martinet@atmark-techno.com>
To: Miquel Raynal <miquel.raynal@bootlin.com>,
	Richard Weinberger <richard@nod.at>,
	Md Sadre Alam <md.alam@oss.qualcomm.com>,
	Vignesh Raghavendra <vigneshr@ti.com>
Cc: linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org,
	Daisuke Mizobuchi <mizo@atmark-techno.com>
Subject: Re: [PATCH v3] mtd: spinand: winbond: add support for W25N04LW
Date: Mon, 17 Aug 2026 14:21:04 +0900	[thread overview]
Message-ID: <aoKaQLEY6bAyg5Iv@atmark-techno.com> (raw)
In-Reply-To: <20260814-w25n04lw-v3-1-c6a7423ac7d0@atmark-techno.com>

Dominique Martinet wrote on Fri, Aug 14, 2026 at 08:12:11AM +0000:
> - I still need to check about the high number of ecc errors next week,
> but I don't think it's likely to be related to this patch so sending
> ahead... Feel free to wait a bit (there's no hurry on my end for this to
> get into 7.3; it's fine to skip a cycle as you see fit)

That looks like to have been hardware related, I've tried two other
identical boards and these worked fine (one had zero ECC error, the
other had a handful but nothing like what I was seeing)

That was the last item left on my list of things to verify,
so I thought this patch was now OK, but sashiko's comment is spot on...

> diff --git a/drivers/mtd/nand/spi/winbond.c b/drivers/mtd/nand/spi/winbond.c
> index 9b78c1e6cbc9..171f4c264220 100644
> --- a/drivers/mtd/nand/spi/winbond.c
> +++ b/drivers/mtd/nand/spi/winbond.c
> @@ -326,9 +327,16 @@ static int w25n02kv_ooblayout_ecc(struct mtd_info *mtd, int section,
>  static int w25n02kv_ooblayout_free(struct mtd_info *mtd, int section,
>  				   struct mtd_oob_region *region)
>  {
> -	if (section > 3)
> +	if (section >= mtd->oobsize / 32)
>  		return -ERANGE;

From sashiko:
> Does this change cause a regression for the W25N01KV device?
> The W25N01KV chip uses this shared w25n02kv_ooblayout_free() function
> and has an oobsize of 96 bytes.
> With mtd->oobsize = 96, 96 / 32 evaluates to 3. The new check 
> (section >= mtd->oobsize / 32) will return -ERANGE for section 3, whereas 
> the original code (section > 3) allowed it. This drops the 4th free 
> OOB region for W25N01KV devices.

I could find the W25N01KV datasheet online and it's correctly 96 bytes
with 4x 16 bytes user data and 4x 8 bytes ECC, so this patch is
incorrect (I had only checked w25n02kv_ooblayout was only used with
oobsize = 128, but w25n01kv_ooblayout also uses
w25n02kv_ooblayout_free...)

I'll send a v4 with fixed functions, even if it's less elegant.

-- 
Dominique

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