[PATCH v2] mtd: spinand: Do not re-enable continuous reads vetoed during variant selection

[Frieder Schrempf]

From: Frieder Schrempf <frieder.schrempf@xxxxxxxxxx>

spinand_match_and_init() tries to disable continuous reads by clearing
cont_read_possible when the fastest continuous read variant is slower
than the fastest read from cache variant, or when no SSDR/ODTR continuous
read variant is supported by the controller. However, at that point
cont_read_possible has not been set yet, and spinand_cont_read_init(),
which runs later during spinand_init(), unconditionally sets it to true
for any chip implementing ->set_cont_read().

As a result, continuous reads end up being used while
op_templates->cont_read_cache is NULL. spinand_create_dirmap() then does
not provide a secondary template and spinand_read_from_cache_op() falls
back to the regular read from cache operation while the chip is in
continuous read mode. On Winbond chips, which do not expect the column
address in this mode, this misaligns the data returned.

The SSDR and ODTR variants are evaluated independently and the bus
interface is only selected later on, so the decision cannot be taken in
spinand_match_and_init(). For example, a quad-only W25N01JW attached to
an octal DTR capable controller has no ODTR continuous read variant, but
ends up using SSDR, where continuous reads work fine.

Instead of clearing cont_read_possible, leave the continuous read
template of the affected bus interface empty and remember whether the
chip relies on dedicated continuous read operations. In
spinand_cont_read_init(), which runs once the bus interface is known,
only enable continuous reads for such chips if a continuous read
template is available for the bus interface in use.

Fixes: 6eb7c193e751 ("mtd: spinand: Use secondary ops for continuous reads")
Assisted-by: LLM
Signed-off-by: Frieder Schrempf <frieder.schrempf@xxxxxxxxxx>
---
I was using an LLM to check sync patches for U-Boot and the LLM reported
a bug in the upstream kernel code. To me it looks like this is indeed a
bug and the fix also looks correct to me, but I would like to mention, that
this is purely LLM-generated and theoretical. I didn't verify the bug and
the fix on actual hardware.

@Miquel: Do you have a hardware setup that would be affected by this? Do
you think there is a better fix?
---
Changes in v2:
- Take the decision in spinand_cont_read_init() once the bus interface
  is known, instead of using a single veto flag shared between SSDR and
  ODTR. v1 wrongly disabled continuous reads e.g. for a W25N01JW on an
  octal DTR capable controller (reported by Sashiko).
- Drop the no-op cont_read_possible = false assignments.
- Link to v1: https://patch.msgid.link/20261007-spinand-cont-read-supported-v1-1-411facc04d99@xxxxxxxxxx

To: Miquel Raynal <miquel.raynal@xxxxxxxxxxx>
To: Richard Weinberger <richard@xxxxxx>
To: Vignesh Raghavendra <vigneshr@xxxxxx>
Cc: linux-mtd@xxxxxxxxxxxxxxxxxxx
Cc: linux-kernel@xxxxxxxxxxxxxxx
---
 drivers/mtd/nand/spi/core.c | 23 +++++++++++++----------
 include/linux/mtd/spinand.h |  4 ++++
 2 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
index 95353777d7f7..11278820675c 100644
--- a/drivers/mtd/nand/spi/core.c
+++ b/drivers/mtd/nand/spi/core.c
@@ -993,9 +993,16 @@ static void spinand_cont_read_init(struct spinand_device *spinand)
 		 */
 		spinand_cont_read_enable(spinand, false);
 
+		/*
+		 * Chips needing dedicated continuous read operations can only
+		 * use continuous reads if such an operation was selected for
+		 * the bus interface in use.
+		 */
 		if ((engine_type == NAND_ECC_ENGINE_TYPE_ON_DIE ||
 		     engine_type == NAND_ECC_ENGINE_TYPE_NONE) &&
-		    !spi_mem_controller_is_capable(ctlr, no_cs_assertion))
+		    !spi_mem_controller_is_capable(ctlr, no_cs_assertion) &&
+		    (!spinand->has_cont_read_variants ||
+		     spinand->op_templates->cont_read_cache))
 			spinand->cont_read_possible = true;
 	}
 }
@@ -1653,6 +1660,7 @@ int spinand_match_and_init(struct spinand_device *spinand,
 		spinand->read_retries = table[i].read_retries;
 		spinand->set_read_retry = table[i].set_read_retry;
 		spinand->set_randomizer = table[i].set_randomizer;
+		spinand->has_cont_read_variants = !!info->op_variants.cont_read_cache;
 
 		/* I/O variants selection with single-spi SDR commands */
 
@@ -1700,12 +1708,10 @@ int spinand_match_and_init(struct spinand_device *spinand,
 				    (read_op->addr.dtr && !op->addr.dtr) ||
 				    read_op->data.buswidth > op->data.buswidth ||
 				    (read_op->data.dtr && !op->data.dtr))
-					spinand->cont_read_possible = false;
-				else
-					spinand->ssdr_op_templates.cont_read_cache = op;
-			} else {
-				spinand->cont_read_possible = false;
+					op = NULL;
 			}
+
+			spinand->ssdr_op_templates.cont_read_cache = op;
 		}
 
 		/* I/O variants selection with octo-spi DDR commands (optional) */
@@ -1733,10 +1739,7 @@ int spinand_match_and_init(struct spinand_device *spinand,
 		if (info->op_variants.cont_read_cache) {
 			op = spinand_select_op_variant(spinand, ODTR,
 						       info->op_variants.cont_read_cache);
-			if (op)
-				spinand->odtr_op_templates.cont_read_cache = op;
-			else
-				spinand->cont_read_possible = false;
+			spinand->odtr_op_templates.cont_read_cache = op;
 		}
 
 		return 0;
diff --git a/include/linux/mtd/spinand.h b/include/linux/mtd/spinand.h
index 5f4c00ae72a7..83f1036fe9bc 100644
--- a/include/linux/mtd/spinand.h
+++ b/include/linux/mtd/spinand.h
@@ -772,6 +772,9 @@ struct spinand_mem_ops {
  *		suitable to use or not in general with this chip/configuration.
  *		A per-transfer check must of course be done to ensure it is
  *		actually relevant to enable this feature.
+ * @has_cont_read_variants: The chip needs dedicated continuous read from
+ *			    cache operations, which must then be available for
+ *			    the bus interface in use
  * @set_cont_read: Enable/disable the continuous read feature
  * @priv: manufacturer private data
  * @fact_otp: SPI NAND factory OTP info.
@@ -810,6 +813,7 @@ struct spinand_device {
 	int (*configure_chip)(struct spinand_device *spinand,
 			      enum spinand_bus_interface iface);
 	bool cont_read_possible;
+	bool has_cont_read_variants;
 	int (*set_cont_read)(struct spinand_device *spinand,
 			     bool enable);
 	int (*set_randomizer)(struct spinand_device *spinand,

---
base-commit: a90ee4305c4a5df72c11b31dacfdc76e00fcf78a
change-id: 20261007-spinand-cont-read-supported-cdaf25f0bf45

Best regards,
--  
Frieder Schrempf <frieder.schrempf@xxxxxxxxxx>