[PATCH v2] mtd: spinand: cache the last read page to avoid redundant SPI operations

[PATCH v2] mtd: spinand: cache the last read page to avoid redundant SPI operations

[fzz]

From: Zongzhen Feng <1768315307@qq.com>

When squashfs reads files through mtdblock, the mtdblock layer
splits I/O into 512-byte sectors. For a 4K-page SPI NAND, this
means reading the same page 8 times (4096 / 512), generating 7x
redundant SPI read-from-cache operations. Each such operation
involves a full SPI bus transaction, significantly slowing down
boot time and file access.

Cache the last successfully read page in spinand_device to avoid
these redundant operations. When the same {target, eraseblock,
page} is requested consecutively, the data is served directly
from the bounce buffer (databuf) via memcpy, skipping the SPI
transaction entirely.

The cache is invalidated on write and erase operations. RAW reads,
OOB reads, and continuous reads are excluded from caching to
maintain correctness.

Signed-off-by: Zongzhen Feng <1768315307@qq.com>
---
v2: fix build error (use req->mode != MTD_OPS_RAW instead of
    req->disable_ecc), fix whitespace noise, use real name
---
 drivers/mtd/nand/spi/core.c | 34 ++++++++++++++++++++++++++++++++++
 include/linux/mtd/spinand.h |  9 +++++++++
 2 files changed, 43 insertions(+)

diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
index 8bf9301f25e7..21bc29ae62b8 100644
--- a/drivers/mtd/nand/spi/core.c
+++ b/drivers/mtd/nand/spi/core.c
@@ -560,6 +560,13 @@ static int spinand_read_from_cache_op(struct spinand_device *spinand,
 			       req->ooblen);
 	}
 
+	if (req->datalen && !req->continuous && req->mode != MTD_OPS_RAW) {
+		spinand->cur_target_cache = req->pos.target;
+		spinand->cur_block_cache = req->pos.eraseblock;
+		spinand->cur_page_cache = req->pos.page;
+		spinand->cache_valid = true;
+	}
+
 	return 0;
 }
 
@@ -833,6 +840,20 @@ static int spinand_mtd_regular_page_read(struct mtd_info *mtd, loff_t from,
 		if (disable_ecc)
 			iter.req.mode = MTD_OPS_RAW;
 
+		if (spinand->cache_valid && !disable_ecc &&
+		    !iter.req.ooblen &&
+		    iter.req.pos.target == spinand->cur_target_cache &&
+		    iter.req.pos.eraseblock == spinand->cur_block_cache &&
+		    iter.req.pos.page == spinand->cur_page_cache) {
+			if (iter.req.datalen)
+				memcpy(iter.req.databuf.in,
+				       spinand->databuf + iter.req.dataoffs,
+				       iter.req.datalen);
+			ops->retlen += iter.req.datalen;
+			ops->oobretlen += iter.req.ooblen;
+			continue;
+		}
+
 		ret = spinand_select_target(spinand, iter.req.pos.target);
 		if (ret)
 			break;
@@ -1064,6 +1085,12 @@ static int spinand_mtd_write(struct mtd_info *mtd, loff_t to,
 		if (ret)
 			break;
 
+		if (spinand->cache_valid &&
+		    iter.req.pos.target == spinand->cur_target_cache &&
+		    iter.req.pos.eraseblock == spinand->cur_block_cache &&
+		    iter.req.pos.page == spinand->cur_page_cache)
+			spinand->cache_valid = false;
+
 		ret = spinand_write_page(spinand, &iter.req);
 		if (ret)
 			break;
@@ -1188,6 +1215,11 @@ static int spinand_erase(struct nand_device *nand, const struct nand_pos *pos)
 	if (!ret && (status & STATUS_ERASE_FAILED))
 		ret = -EIO;
 
+	if (!ret && spinand->cache_valid &&
+	    spinand->cur_target_cache == pos->target &&
+	    spinand->cur_block_cache == pos->eraseblock)
+		spinand->cache_valid = false;
+
 	return ret;
 }
 
@@ -2025,6 +2057,7 @@ static void spinand_cleanup(struct spinand_device *spinand)
 	nanddev_ecc_engine_cleanup(nand);
 	nanddev_cleanup(nand);
 	spinand_manufacturer_cleanup(spinand);
+	spinand->cache_valid = false;
 	kfree(spinand->databuf);
 	kfree(spinand->scratchbuf);
 }
@@ -2044,6 +2077,7 @@ static int spinand_probe(struct spi_mem *mem)
 	spi_mem_set_drvdata(mem, spinand);
 	spinand_set_of_node(spinand, mem->spi->dev.of_node);
 	mutex_init(&spinand->lock);
+	spinand->cache_valid = false;
 	mtd = spinand_to_mtd(spinand);
 	mtd->dev.parent = &mem->spi->dev;
 
diff --git a/include/linux/mtd/spinand.h b/include/linux/mtd/spinand.h
index 5f4c00ae72a7..887ef6d34a41 100644
--- a/include/linux/mtd/spinand.h
+++ b/include/linux/mtd/spinand.h
@@ -757,6 +757,10 @@ struct spinand_mem_ops {
  *		   a command addressing a page or an eraseblock embedded in
  *		   this die. Only required if your chip exposes several dies
  * @cur_target: currently selected target/die
+ * @cur_target_cache: target of the cached page
+ * @cur_block_cache: eraseblock of the cached page
+ * @cur_page_cache: page number of the cached page
+ * @cache_valid: whether the cached page is valid
  * @eccinfo: on-die ECC information
  * @cfg_cache: config register cache. One entry per die
  * @databuf: bounce buffer for data
@@ -798,6 +802,11 @@ struct spinand_device {
 			     unsigned int target);
 	unsigned int cur_target;
 
+	unsigned int cur_target_cache;
+	unsigned int cur_block_cache;
+	unsigned int cur_page_cache;
+	bool cache_valid;
+
 	struct spinand_ecc_info eccinfo;
 
 	u8 *cfg_cache;
-- 
2.25.1

Re: [PATCH v2] mtd: spinand: cache the last read page to avoid redundant SPI operations

[Miquel Raynal]

Hello Zongzhen,

On 08/09/2026 at 11:25:30 +08, fzz <1768315307@qq.com> wrote:

> From: Zongzhen Feng <1768315307@qq.com>
>
> When squashfs reads files through mtdblock, the mtdblock layer
> splits I/O into 512-byte sectors. For a 4K-page SPI NAND, this
> means reading the same page 8 times (4096 / 512), generating 7x
> redundant SPI read-from-cache operations. Each such operation
> involves a full SPI bus transaction, significantly slowing down
> boot time and file access.
>
> Cache the last successfully read page in spinand_device to avoid
> these redundant operations. When the same {target, eraseblock,
> page} is requested consecutively, the data is served directly
> from the bounce buffer (databuf) via memcpy, skipping the SPI
> transaction entirely.
>
> The cache is invalidated on write and erase operations. RAW reads,
> OOB reads, and continuous reads are excluded from caching to
> maintain correctness.
>
> Signed-off-by: Zongzhen Feng <1768315307@qq.com>

Sashiko reports interesting warnings, can you please address them?

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] The page caching mechanism uses the shared bounce buffer
`spinand->databuf` to store cached data, but fails to unconditionally
invalidate the cache when this buffer is overwritten by other
operations, leading to severe cross-page data corruption.
- [Critical] Pages with uncorrectable ECC errors are prematurely cached
as valid, causing subsequent reads to bypass ECC checks and silently
return corrupted data as successful.

Thanks,
Miquèl

[PATCH v3] mtd: spinand: cache the last read page to avoid redundant SPI operations

[Zongzhen Feng]

When squashfs reads files through mtdblock, the mtdblock layer
splits I/O into 512-byte sectors. For a 4K-page SPI NAND, this
means reading the same page 8 times (4096 / 512), generating 7x
redundant SPI read-from-cache operations. Each such operation
involves a full SPI bus transaction, significantly slowing down
boot time and file access.

Cache the last successfully read page in spinand_device to avoid
these redundant operations. When the same {target, eraseblock,
page} is requested consecutively, the data is served directly
from the bounce buffer (databuf) via memcpy, skipping the SPI
transaction entirely.

The cache is invalidated on write, erase, ECC errors, and when
the bounce buffer is overwritten by non-cacheable reads (RAW,
continuous, or OOB-only).

Signed-off-by: Zongzhen Feng <1768315307@qq.com>
---
v3:
- Invalidate cache when databuf is overwritten by a non-cacheable
  read (RAW, continuous, OOB-only) to prevent stale cache hits.
- Invalidate cache on uncorrectable ECC errors to prevent
  subsequent reads from bypassing ECC checks.

v2:
- Fix compilation error: req->disable_ecc -> req->mode != MTD_OPS_RAW
- Remove spurious indentation change
- Use real name instead of pseudonym
---
 drivers/mtd/nand/spi/core.c | 39 +++++++++++++++++++++++++++++++++++++
 include/linux/mtd/spinand.h |  9 +++++++++
 2 files changed, 48 insertions(+)

diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
index 8bf9301f25e7..f545dd948604 100644
--- a/drivers/mtd/nand/spi/core.c
+++ b/drivers/mtd/nand/spi/core.c
@@ -560,6 +560,15 @@ static int spinand_read_from_cache_op(struct spinand_device *spinand,
 			       req->ooblen);
 	}
 
+	if (req->datalen && !req->continuous && req->mode != MTD_OPS_RAW) {
+		spinand->cur_target_cache = req->pos.target;
+		spinand->cur_block_cache = req->pos.eraseblock;
+		spinand->cur_page_cache = req->pos.page;
+		spinand->cache_valid = true;
+	} else if (req->datalen) {
+		spinand->cache_valid = false;
+	}
+
 	return 0;
 }
 
@@ -833,6 +842,20 @@ static int spinand_mtd_regular_page_read(struct mtd_info *mtd, loff_t from,
 		if (disable_ecc)
 			iter.req.mode = MTD_OPS_RAW;
 
+		if (spinand->cache_valid && !disable_ecc &&
+		    !iter.req.ooblen &&
+		    iter.req.pos.target == spinand->cur_target_cache &&
+		    iter.req.pos.eraseblock == spinand->cur_block_cache &&
+		    iter.req.pos.page == spinand->cur_page_cache) {
+			if (iter.req.datalen)
+				memcpy(iter.req.databuf.in,
+				       spinand->databuf + iter.req.dataoffs,
+				       iter.req.datalen);
+			ops->retlen += iter.req.datalen;
+			ops->oobretlen += iter.req.ooblen;
+			continue;
+		}
+
 		ret = spinand_select_target(spinand, iter.req.pos.target);
 		if (ret)
 			break;
@@ -842,6 +865,9 @@ static int spinand_mtd_regular_page_read(struct mtd_info *mtd, loff_t from,
 		if (ret < 0 && ret != -EBADMSG)
 			break;
 
+		if (ret == -EBADMSG)
+			spinand->cache_valid = false;
+
 		if (ret == -EBADMSG && spinand->set_read_retry) {
 			if (spinand->read_retries && (++retry_mode <= spinand->read_retries)) {
 				ret = spinand->set_read_retry(spinand, retry_mode);
@@ -1064,6 +1090,12 @@ static int spinand_mtd_write(struct mtd_info *mtd, loff_t to,
 		if (ret)
 			break;
 
+		if (spinand->cache_valid &&
+		    iter.req.pos.target == spinand->cur_target_cache &&
+		    iter.req.pos.eraseblock == spinand->cur_block_cache &&
+		    iter.req.pos.page == spinand->cur_page_cache)
+			spinand->cache_valid = false;
+
 		ret = spinand_write_page(spinand, &iter.req);
 		if (ret)
 			break;
@@ -1188,6 +1220,11 @@ static int spinand_erase(struct nand_device *nand, const struct nand_pos *pos)
 	if (!ret && (status & STATUS_ERASE_FAILED))
 		ret = -EIO;
 
+	if (!ret && spinand->cache_valid &&
+	    spinand->cur_target_cache == pos->target &&
+	    spinand->cur_block_cache == pos->eraseblock)
+		spinand->cache_valid = false;
+
 	return ret;
 }
 
@@ -2025,6 +2062,7 @@ static void spinand_cleanup(struct spinand_device *spinand)
 	nanddev_ecc_engine_cleanup(nand);
 	nanddev_cleanup(nand);
 	spinand_manufacturer_cleanup(spinand);
+	spinand->cache_valid = false;
 	kfree(spinand->databuf);
 	kfree(spinand->scratchbuf);
 }
@@ -2044,6 +2082,7 @@ static int spinand_probe(struct spi_mem *mem)
 	spi_mem_set_drvdata(mem, spinand);
 	spinand_set_of_node(spinand, mem->spi->dev.of_node);
 	mutex_init(&spinand->lock);
+	spinand->cache_valid = false;
 	mtd = spinand_to_mtd(spinand);
 	mtd->dev.parent = &mem->spi->dev;
 
diff --git a/include/linux/mtd/spinand.h b/include/linux/mtd/spinand.h
index 5f4c00ae72a7..887ef6d34a41 100644
--- a/include/linux/mtd/spinand.h
+++ b/include/linux/mtd/spinand.h
@@ -757,6 +757,10 @@ struct spinand_mem_ops {
  *		   a command addressing a page or an eraseblock embedded in
  *		   this die. Only required if your chip exposes several dies
  * @cur_target: currently selected target/die
+ * @cur_target_cache: target of the cached page
+ * @cur_block_cache: eraseblock of the cached page
+ * @cur_page_cache: page number of the cached page
+ * @cache_valid: whether the cached page is valid
  * @eccinfo: on-die ECC information
  * @cfg_cache: config register cache. One entry per die
  * @databuf: bounce buffer for data
@@ -798,6 +802,11 @@ struct spinand_device {
 			     unsigned int target);
 	unsigned int cur_target;
 
+	unsigned int cur_target_cache;
+	unsigned int cur_block_cache;
+	unsigned int cur_page_cache;
+	bool cache_valid;
+
 	struct spinand_ecc_info eccinfo;
 
 	u8 *cfg_cache;
-- 
2.25.1

Re: [PATCH v2] mtd: spinand: cache the last read page to avoid redundant SPI operations

[Zongzhen Feng]

Hi Miquèl,

Thanks for the review. v3 has been sent to address the two issues
found by Sashiko:

- Invalidate cache when the bounce buffer is overwritten by
  non-cacheable reads (RAW, continuous, or OOB-only) to prevent
  serving stale data on subsequent cache hits.

- Invalidate cache on uncorrectable ECC errors to prevent
  subsequent reads from bypassing ECC checks and silently
  returning corrupted data.

Regards,
Zongzhen Feng

Re: [PATCH v3] mtd: spinand: cache the last read page to avoid redundant SPI operations

[Miquel Raynal]

Hello Zongzhen,

On 08/09/2026 at 18:19:58 +08, Zongzhen Feng <1768315307@qq.com> wrote:

> When squashfs reads files through mtdblock, the mtdblock layer
> splits I/O into 512-byte sectors. For a 4K-page SPI NAND, this
> means reading the same page 8 times (4096 / 512), generating 7x
> redundant SPI read-from-cache operations. Each such operation
> involves a full SPI bus transaction, significantly slowing down
> boot time and file access.

I haven't made a review yet, but there are still a lot of Sashiko
complaints and they look reasonable. Can you please check them?

Thanks,
Miquèl

Re: [PATCH v3] mtd: spinand: cache the last read page to avoid redundant SPI operations

[Zongzhen Feng]

Hi Miquel,

Thanks for the feedback. I checked the email thread and patchwork, but
couldn't find the Sashiko review comments for v3. The v2 Sashiko
complaints about:

1. Cache not invalidated when databuf is overwritten by non-cacheable
   reads (RAW, continuous, OOB-only)
2. Pages with uncorrectable ECC errors being cached

have already been addressed in v3.

Could you please forward the remaining Sashiko complaints or point me
to where I can find them? I'd like to address them before your human
review.

Thanks,
Zongzhen

[PATCH v4] mtd: spinand: cache the last read page to avoid redundant SPI operations

[Zongzhen Feng]

When squashfs reads files through mtdblock, the mtdblock layer
splits I/O into 512-byte sectors. For a 4K-page SPI NAND, this
means reading the same page 8 times (4096 / 512), generating 7x
redundant SPI read-from-cache operations. Each such operation
involves a full SPI bus transaction, significantly slowing down
boot time and file access.

Cache the last successfully read page in spinand_device to avoid
these redundant operations. When the same {target, eraseblock,
page} is requested consecutively, the data is served directly
from the bounce buffer (databuf) via memcpy, skipping the SPI
transaction entirely.

The cache is invalidated on write, erase, ECC errors, and
non-cacheable reads.

Signed-off-by: Zongzhen Feng <1768315307@qq.com>
---
v4:
- Invalidate cache before writing to databuf in spinand_read_from_cache_op
  to prevent serving stale data on SPI read errors.
- Copy ECC-corrected data back to spinand->databuf in spinand_read_page
  to ensure cache serves corrected data for software ECC engines.
- Invalidate cache on any write, not just writes to the cached page,
  since spinand_write_to_cache_op unconditionally overwrites databuf.
- Invalidate cache on erase even if the operation fails, to prevent
  serving stale data from a potentially corrupted block.

v3:
- Invalidate cache when databuf is overwritten by a non-cacheable
  read (RAW, continuous, OOB-only) to prevent stale cache hits.
- Invalidate cache on uncorrectable ECC errors to prevent
  subsequent reads from bypassing ECC checks.

v2:
- Fix compilation error: req->disable_ecc -> req->mode != MTD_OPS_RAW
- Remove spurious indentation change
- Use real name instead of pseudonym
---
 drivers/mtd/nand/spi/core.c | 41 ++++++++++++++++++++++++++++++++++++-
 include/linux/mtd/spinand.h |  9 ++++++++
 2 files changed, 49 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
index 8bf9301f25e7..7f66de5af171 100644
--- a/drivers/mtd/nand/spi/core.c
+++ b/drivers/mtd/nand/spi/core.c
@@ -485,6 +485,7 @@ static int spinand_read_from_cache_op(struct spinand_device *spinand,
 
 	if (req->datalen) {
 		buf = spinand->databuf;
+		spinand->cache_valid = false;
 		if (!req->continuous)
 			nbytes = nanddev_page_size(nand);
 		else
@@ -560,6 +561,13 @@ static int spinand_read_from_cache_op(struct spinand_device *spinand,
 			       req->ooblen);
 	}
 
+	if (req->datalen && !req->continuous && req->mode != MTD_OPS_RAW) {
+		spinand->cur_target_cache = req->pos.target;
+		spinand->cur_block_cache = req->pos.eraseblock;
+		spinand->cur_page_cache = req->pos.page;
+		spinand->cache_valid = true;
+	}
+
 	return 0;
 }
 
@@ -764,7 +772,12 @@ int spinand_read_page(struct spinand_device *spinand,
 	if (ret)
 		return ret;
 
-	return nand_ecc_finish_io_req(nand, (struct nand_page_io_req *)req);
+	ret = nand_ecc_finish_io_req(nand, (struct nand_page_io_req *)req);
+	if (ret > 0 && req->datalen && !req->continuous && req->mode != MTD_OPS_RAW)
+		memcpy(spinand->databuf + req->dataoffs, req->databuf.in,
+		       req->datalen);
+
+	return ret;
 }
 
 /**
@@ -833,6 +846,20 @@ static int spinand_mtd_regular_page_read(struct mtd_info *mtd, loff_t from,
 		if (disable_ecc)
 			iter.req.mode = MTD_OPS_RAW;
 
+		if (spinand->cache_valid && !disable_ecc &&
+		    !iter.req.ooblen &&
+		    iter.req.pos.target == spinand->cur_target_cache &&
+		    iter.req.pos.eraseblock == spinand->cur_block_cache &&
+		    iter.req.pos.page == spinand->cur_page_cache) {
+			if (iter.req.datalen)
+				memcpy(iter.req.databuf.in,
+				       spinand->databuf + iter.req.dataoffs,
+				       iter.req.datalen);
+			ops->retlen += iter.req.datalen;
+			ops->oobretlen += iter.req.ooblen;
+			continue;
+		}
+
 		ret = spinand_select_target(spinand, iter.req.pos.target);
 		if (ret)
 			break;
@@ -842,6 +869,9 @@ static int spinand_mtd_regular_page_read(struct mtd_info *mtd, loff_t from,
 		if (ret < 0 && ret != -EBADMSG)
 			break;
 
+		if (ret == -EBADMSG)
+			spinand->cache_valid = false;
+
 		if (ret == -EBADMSG && spinand->set_read_retry) {
 			if (spinand->read_retries && (++retry_mode <= spinand->read_retries)) {
 				ret = spinand->set_read_retry(spinand, retry_mode);
@@ -1064,6 +1094,8 @@ static int spinand_mtd_write(struct mtd_info *mtd, loff_t to,
 		if (ret)
 			break;
 
+		spinand->cache_valid = false;
+
 		ret = spinand_write_page(spinand, &iter.req);
 		if (ret)
 			break;
@@ -1188,6 +1220,11 @@ static int spinand_erase(struct nand_device *nand, const struct nand_pos *pos)
 	if (!ret && (status & STATUS_ERASE_FAILED))
 		ret = -EIO;
 
+	if (spinand->cache_valid &&
+	    spinand->cur_target_cache == pos->target &&
+	    spinand->cur_block_cache == pos->eraseblock)
+		spinand->cache_valid = false;
+
 	return ret;
 }
 
@@ -2025,6 +2062,7 @@ static void spinand_cleanup(struct spinand_device *spinand)
 	nanddev_ecc_engine_cleanup(nand);
 	nanddev_cleanup(nand);
 	spinand_manufacturer_cleanup(spinand);
+	spinand->cache_valid = false;
 	kfree(spinand->databuf);
 	kfree(spinand->scratchbuf);
 }
@@ -2044,6 +2082,7 @@ static int spinand_probe(struct spi_mem *mem)
 	spi_mem_set_drvdata(mem, spinand);
 	spinand_set_of_node(spinand, mem->spi->dev.of_node);
 	mutex_init(&spinand->lock);
+	spinand->cache_valid = false;
 	mtd = spinand_to_mtd(spinand);
 	mtd->dev.parent = &mem->spi->dev;
 
diff --git a/include/linux/mtd/spinand.h b/include/linux/mtd/spinand.h
index 5f4c00ae72a7..887ef6d34a41 100644
--- a/include/linux/mtd/spinand.h
+++ b/include/linux/mtd/spinand.h
@@ -757,6 +757,10 @@ struct spinand_mem_ops {
  *		   a command addressing a page or an eraseblock embedded in
  *		   this die. Only required if your chip exposes several dies
  * @cur_target: currently selected target/die
+ * @cur_target_cache: target of the cached page
+ * @cur_block_cache: eraseblock of the cached page
+ * @cur_page_cache: page number of the cached page
+ * @cache_valid: whether the cached page is valid
  * @eccinfo: on-die ECC information
  * @cfg_cache: config register cache. One entry per die
  * @databuf: bounce buffer for data
@@ -798,6 +802,11 @@ struct spinand_device {
 			     unsigned int target);
 	unsigned int cur_target;
 
+	unsigned int cur_target_cache;
+	unsigned int cur_block_cache;
+	unsigned int cur_page_cache;
+	bool cache_valid;
+
 	struct spinand_ecc_info eccinfo;
 
 	u8 *cfg_cache;
-- 
2.25.1

Re: [PATCH v4] mtd: spinand: cache the last read page to avoid redundant SPI operations

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Critical] Partial page reads with software/host ECC cause silent data corruption on subsequent cache hits due to incomplete cache updates.
- [Critical] Cache hit logic fails to check the Logical Unit Number (LUN), leading to cross-LUN cache aliasing.
- [High] Missing cache invalidation on OOB/OTP writes causes cached reads to return clobbered data.
- [High] Error handling leaves cache valid with uncorrected data on non-EBADMSG errors.
--

commit 5e46633e4cdf72f3fb112f28479efe0c2e5ed5d8
Author: Zongzhen Feng <1768315307@qq.com>

mtd: spinand: cache the last read page to avoid redundant SPI operations

This patch caches the last successfully read page in the SPI NAND driver to
avoid redundant SPI read operations when accessing smaller sectors (like 512
bytes) within a larger page. It serves repeated reads directly from a bounce
buffer and attempts to invalidate the cache during writes, erases, and
errors.

> diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
> index 43bbe2629b85b..a742a1cb572a4 100644
> --- a/drivers/mtd/nand/spi/core.c
> +++ b/drivers/mtd/nand/spi/core.c
[ ... ]
> @@ -577,6 +578,13 @@ static int spinand_read_from_cache_op(struct spinand_device *spinand,
>  			       req->ooblen);
>  	}
>  
> +	if (req->datalen && !req->continuous && req->mode != MTD_OPS_RAW) {
> +		spinand->cur_target_cache = req->pos.target;
> +		spinand->cur_block_cache = req->pos.eraseblock;
> +		spinand->cur_page_cache = req->pos.page;
> +		spinand->cache_valid = true;
> +	}
> +

[Severity: Critical]
Is it necessary to include the Logical Unit Number (LUN) in this cache logic?

The SPI NAND subsystem supports devices with multiple LUNs per target. The
nand_pos structure includes a lun field to address these, but this mechanism
tracks identity using only the target, eraseblock, and page.

Could this cause a read from one LUN to falsely hit the cache and serve data
from a different LUN when accessing the exact same eraseblock and page index?

>  	return 0;
>  }
>  
> @@ -780,7 +788,12 @@ int spinand_read_page(struct spinand_device *spinand,
>  	if (ret)
>  		return ret;
>  
> -	return nand_ecc_finish_io_req(nand, (struct nand_page_io_req *)req);
> +	ret = nand_ecc_finish_io_req(nand, (struct nand_page_io_req *)req);
> +	if (ret > 0 && req->datalen && !req->continuous && req->mode != MTD_OPS_RAW)
> +		memcpy(spinand->databuf + req->dataoffs, req->databuf.in,
> +		       req->datalen);
> +
> +	return ret;
>  }

[Severity: Critical]
Are partial page reads handled correctly here?

When a partial page read occurs, the full page is loaded into the cache and
marked valid, but only the requested req->datalen bytes are corrected by the
ECC engine and explicitly copied back to spinand->databuf. The remainder of
the cached page remains raw and uncorrected.

Will subsequent reads requesting a different chunk of the same page hit the
cache, bypass the SPI read and ECC correction, and return uncorrected data?

[ ... ]
> @@ -849,6 +862,20 @@ static int spinand_mtd_regular_page_read(struct mtd_info *mtd, loff_t from,
>  		if (disable_ecc)
>  			iter.req.mode = MTD_OPS_RAW;
>  
> +		if (spinand->cache_valid && !disable_ecc &&
> +		    !iter.req.ooblen &&
> +		    iter.req.pos.target == spinand->cur_target_cache &&
> +		    iter.req.pos.eraseblock == spinand->cur_block_cache &&
> +		    iter.req.pos.page == spinand->cur_page_cache) {
> +			if (iter.req.datalen)
> +				memcpy(iter.req.databuf.in,
> +				       spinand->databuf + iter.req.dataoffs,
> +				       iter.req.datalen);
> +			ops->retlen += iter.req.datalen;
> +			ops->oobretlen += iter.req.ooblen;
> +			continue;
> +		}
> +
>  		ret = spinand_select_target(spinand, iter.req.pos.target);
>  		if (ret)
>  			break;
> @@ -858,6 +885,9 @@ static int spinand_mtd_regular_page_read(struct mtd_info *mtd, loff_t from,
>  		if (ret < 0 && ret != -EBADMSG)
>  			break;
>  
> +		if (ret == -EBADMSG)
> +			spinand->cache_valid = false;
> +

[Severity: High]
Could the cache be left valid with uncorrected data if a non-EBADMSG error
occurs?

In spinand_read_from_cache_op(), raw data is loaded into the cache buffer and
spinand->cache_valid is prematurely set to true. If the subsequent
nand_ecc_finish_io_req() fails with a non-EBADMSG error (such as -ERANGE from
software BCH ECC layout parsing, or -ETIMEDOUT from a hardware engine),
spinand_read_page() propagates this error.

When the loop breaks here on ret < 0 && ret != -EBADMSG, the cache is not
invalidated.

Will a subsequent access to the same page incorrectly hit the cache and return
the uncorrected data as a success?

[ ... ]
> @@ -1080,6 +1110,8 @@ static int spinand_mtd_write(struct mtd_info *mtd, loff_t to,
>  		if (ret)
>  			break;
>  
> +		spinand->cache_valid = false;
> +
>  		ret = spinand_write_page(spinand, &iter.req);
>  		if (ret)
>  			break;

[Severity: High]
Are other write paths missing cache invalidation?

While this patch correctly invalidates the cache in spinand_mtd_write() and
spinand_erase(), other write paths like spinand_markbad() and
spinand_otp_rw() also invoke spinand_write_page().

Because spinand_write_to_cache_op() unconditionally overwrites the shared
data buffer (spinand->databuf) with 0xFFs during any write operation, this
can cause the cache buffer to be clobbered while spinand->cache_valid
remains true.

Will subsequent reads to the cached coordinates falsely hit the cache and
return clobbered data instead of the actual page contents?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/tencent_522792EE6F63A5581C238FF17E9862F10A08@qq.com?part=1

Re: [PATCH v4] mtd: spinand: cache the last read page to avoid redundant SPI operations

[Miquel Raynal]

Hi Zongzhen,

On 10/09/2026 at 04:26:18 GMT, sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
> - [Critical] Partial page reads with software/host ECC cause silent data corruption on subsequent cache hits due to incomplete cache updates.
> - [Critical] Cache hit logic fails to check the Logical Unit Number (LUN), leading to cross-LUN cache aliasing.
> - [High] Missing cache invalidation on OOB/OTP writes causes cached reads to return clobbered data.
> - [High] Error handling leaves cache valid with uncorrected data on
> non-EBADMSG errors.

These still seem valid :-)

Miquèl