Re: [PATCH 13/13] mtd: spinand: winbond: Add continuous read support for W25N04LW - sashiko-bot

From: sashiko-bot@kernel.org
To: "Miquel Raynal" <miquel.raynal@bootlin.com>,
	"Richard Weinberger" <richard@nod.at>,
	"Vignesh Raghavendra" <vigneshr@ti.com>,
	"Daisuke Mizobuchi" <mizo@atmark-techno.com>,
	"Dominique Martinet" <dominique.martinet@atmark-techno.com>
Cc: "Thomas Petazzoni" <thomas.petazzoni@bootlin.com>,
	"Steam Lin" <stlin2@winbond.com>,
	linux-mtd@lists.infradead.org
Subject: Re: [PATCH 13/13] mtd: spinand: winbond: Add continuous read support for W25N04LW
Date: Fri, 30 Oct 2026 17:57:29 +0000	[thread overview]
Message-ID: <sashiko-outbox-191379@kernel.org> (raw)
In-Reply-To: <20261030-winbond-master-w25nxxlvw-v1-13-91f67407dc8b@bootlin.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] mtd: spinand: winbond: W25N04LW uses wrong OOB layout w25nxxkv_kw_ooblayout
--

commit 806af285b955963cbeff1fd619e9d9a72b5987e3
Author: Miquel Raynal <miquel.raynal@bootlin.com>

mtd: spinand: winbond: Add continuous read support for W25N04LW

This commit enables continuous read support for the Winbond W25N04LW SPI-NAND
flash chip by configuring cont_read_cache_variants and the w25n_set_cont_read
helper in winbond_spinand_table.

> diff --git a/drivers/mtd/nand/spi/winbond.c b/drivers/mtd/nand/spi/winbond.c
> index 17147444f8cbc..c897d1686d259 100644
> --- a/drivers/mtd/nand/spi/winbond.c
> +++ b/drivers/mtd/nand/spi/winbond.c
> @@ -895,11 +895,13 @@ static const struct spinand_info winbond_spinand_table[] = {
>  		     SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0xb2, 0x23),
>  		     NAND_MEMORG(1, 4096, 256, 64, 2048, 40, 1, 1, 1),
>  		     NAND_ECCREQ(8, 512),
> -		     SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
> -					      &write_cache_variants,
> -					      &update_cache_variants),
> +		     SPINAND_INFO_OP_VARIANTS_WITH_CONT(&read_cache_variants,
> +							&write_cache_variants,
> +							&update_cache_variants,
> +							&cont_read_cache_variants),
>  		     0,
> -		     SPINAND_ECCINFO(&w25nxxkv_kw_ooblayout, w25nxxkv_kw_lv_lw_ecc_get_status)),
> +		     SPINAND_ECCINFO(&w25nxxkv_kw_ooblayout, w25nxxkv_kw_lv_lw_ecc_get_status),
> +		     SPINAND_CONT_READ(w25n_set_cont_read)),

[Severity: Medium]
Should the W25N04LW entry in winbond_spinand_table use &w25nxxlv_lw_ooblayout
instead of &w25nxxkv_kw_ooblayout here?

Earlier in this series, commit 4c37f4cb3704 ("mtd: spinand: winbond: Fix an
OOB layout inconsistency with W25NxxLW chips") changed W25N04LW from
&w25n02kv_ooblayout to &w25n08lw_ooblayout so that all W25NxxLV and W25NxxLW
chips share the same OOB layout.

However, commit c7bc36662c75 ("mtd: spinand: winbond: Generalize the KV OOB
layout") appears to have accidentally overwritten &w25n08lw_ooblayout on
W25N04LW with &w25nxxkv_kw_ooblayout, which then caused commit 64760906da7f
("mtd: spinand: winbond: Generalize the LW OOB layout") to miss W25N04LW when
renaming w25n08lw_ooblayout to w25nxxlv_lw_ooblayout.

With &w25nxxkv_kw_ooblayout registered for W25N04LW during spinand_init(),
w25nxxkv_kw_ooblayout_free() exposes 14 free bytes per 16-byte OOB section
starting at offset 2 (including the 2 User Data II bytes not protected by
on-die ECC) instead of only the 12 ECC-protected User Data I bytes at offset 4
returned by w25nxxlv_lw_ooblayout_free(), and w25nxxkv_kw_ooblayout_ecc()
reports ECC parity regions in the upper half of OOB rather than returning
-ERANGE as w25nxxlv_lw_ooblayout_ecc() does for W25N02LV, W25N02LW, W25N04LV,
and W25N08LW.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261030-winbond-master-w25nxxlvw-v1-0-91f67407dc8b@bootlin.com?part=13

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