[PATCH] mtd: spinand: confirm ready state on SkyHigh S35ML chips

[PATCH] mtd: spinand: confirm ready state on SkyHigh S35ML chips

[Vitaliy Sochnev]

On a Nokia XG-040G-MD with a SkyHigh 256 MiB SPI-NAND, UBI reports
fixable bit-flips during rootfs mount without this workaround:

  ubi0: fixable bit-flip detected at PEB 1979
  ubi0: schedule PEB 1979 for scrubbing
  ubi0: scrubbed PEB 1979 (LEB 5:53), data moved to PEB 645

Other reboots also report fixable bit-flips. A tester reports that after
applying the workaround, sysupgrade, multiple reboots and an active
workload produced no such warnings. Long-term operation is unverified.

spinand_wait() accepts a single non-busy status sample. On these chips,
the ready bit can briefly clear before the device is actually ready,
allowing a premature cache read or a new program/erase command. The
fallback status read for controllers with poll_status offload does not
provide a second ready confirmation in the usual case.

Mark the affected SkyHigh parts with a quirk and require a second ready
status sample after spinand_wait() for those parts. If the chip becomes
busy again, repeat the wait while the outer timeout has not elapsed.
Each inner wait has its own timeout, so the quirk can wait up to roughly
twice as long. Leave other SPI-NAND parts unchanged.

Suggested-by: Mikhail Zhilkin <csharper2005@gmail.com>
Signed-off-by: Vitaliy Sochnev <sochnev.v.74@gmail.com>
---
Notes for reviewers:

The two ready samples are taken in separate SPI transactions, without an
explicit delay. The duration of the premature-ready interval has not been
measured. If both samples fall within that interval, the wait can still
return too early. Would a measured, explicit delay be preferable here?

The on-device result comes from a Nokia XG-040G-MD with a 256 MiB SkyHigh
chip running OpenWrt with this workaround. The linux-next port has only
been compile-tested; the other S35ML entries flagged by this patch have
not been tested on hardware with this workaround.

 drivers/mtd/nand/spi/core.c    | 85 +++++++++++++++++++++++++++-------
 drivers/mtd/nand/spi/skyhigh.c | 12 +++--
 include/linux/mtd/spinand.h    |  1 +
 3 files changed, 76 insertions(+), 22 deletions(-)

diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
index 133776ba53da0..b051ff1d6be5c 100644
--- a/drivers/mtd/nand/spi/core.c
+++ b/drivers/mtd/nand/spi/core.c
@@ -710,6 +710,53 @@ int spinand_wait(struct spinand_device *spinand, unsigned long initial_delay_us,
 	return status & STATUS_BUSY ? -ETIMEDOUT : 0;
 }
 
+/**
+ * spinand_wait_ready_confirmed() - Wait for the device to be ready
+ * @spinand: the spinand device
+ * @initial_delay_us: delay in us before starting to poll
+ * @poll_delay_us: time to sleep between reads in us
+ * @s: the pointer to variable to store the value of REG_STATUS
+ *
+ * Same as spinand_wait(), except for devices flagged with
+ * SPINAND_PREMATURE_READY_QUIRK, whose ready bit can flip back to busy
+ * shortly after first reporting ready. For those, a ready result from
+ * spinand_wait() is only trusted once a follow-up status read still shows
+ * the device ready, and spinand_wait() is called again otherwise. Each
+ * spinand_wait() gets its own SPINAND_WAITRDY_TIMEOUT_MS budget and no
+ * new one is started once that span has elapsed, so such a device can
+ * spend up to roughly twice that time here.
+ *
+ * Return: 0 on success, a negative error code otherwise.
+ */
+static int spinand_wait_ready_confirmed(struct spinand_device *spinand,
+					unsigned long initial_delay_us,
+					unsigned long poll_delay_us, u8 *s)
+{
+	unsigned long timeo;
+	u8 status;
+	int ret;
+
+	if (!(spinand->flags & SPINAND_PREMATURE_READY_QUIRK))
+		return spinand_wait(spinand, initial_delay_us, poll_delay_us, s);
+
+	timeo = jiffies + msecs_to_jiffies(SPINAND_WAITRDY_TIMEOUT_MS);
+	do {
+		ret = spinand_wait(spinand, initial_delay_us, poll_delay_us,
+				   &status);
+		if (ret < 0)
+			return ret;
+
+		ret = spinand_read_status(spinand, &status);
+		if (ret)
+			return ret;
+	} while ((status & STATUS_BUSY) && time_before(jiffies, timeo));
+
+	if (s)
+		*s = status;
+
+	return status & STATUS_BUSY ? -ETIMEDOUT : 0;
+}
+
 static int spinand_read_id_op(struct spinand_device *spinand, u8 naddr,
 			      u8 ndummy, u8 *buf)
 {
@@ -733,10 +780,10 @@ static int spinand_reset_op(struct spinand_device *spinand)
 	if (ret)
 		return ret;
 
-	return spinand_wait(spinand,
-			    SPINAND_RESET_INITIAL_DELAY_US,
-			    SPINAND_RESET_POLL_DELAY_US,
-			    NULL);
+	return spinand_wait_ready_confirmed(spinand,
+					    SPINAND_RESET_INITIAL_DELAY_US,
+					    SPINAND_RESET_POLL_DELAY_US,
+					    NULL);
 }
 
 static int spinand_lock_block(struct spinand_device *spinand, u8 lock)
@@ -767,10 +814,10 @@ int spinand_read_page(struct spinand_device *spinand,
 	if (ret)
 		return ret;
 
-	ret = spinand_wait(spinand,
-			   SPINAND_READ_INITIAL_DELAY_US,
-			   SPINAND_READ_POLL_DELAY_US,
-			   &status);
+	ret = spinand_wait_ready_confirmed(spinand,
+					   SPINAND_READ_INITIAL_DELAY_US,
+					   SPINAND_READ_POLL_DELAY_US,
+					   &status);
 	if (ret < 0)
 		return ret;
 
@@ -814,10 +861,10 @@ int spinand_write_page(struct spinand_device *spinand,
 	if (ret)
 		return ret;
 
-	ret = spinand_wait(spinand,
-			   SPINAND_WRITE_INITIAL_DELAY_US,
-			   SPINAND_WRITE_POLL_DELAY_US,
-			   &status);
+	ret = spinand_wait_ready_confirmed(spinand,
+					   SPINAND_WRITE_INITIAL_DELAY_US,
+					   SPINAND_WRITE_POLL_DELAY_US,
+					   &status);
 	if (ret)
 		return ret;
 
@@ -939,8 +986,10 @@ static int spinand_mtd_continuous_page_read(struct mtd_info *mtd, loff_t from,
 		if (ret)
 			goto end_cont_read;
 
-		ret = spinand_wait(spinand, SPINAND_READ_INITIAL_DELAY_US,
-				   SPINAND_READ_POLL_DELAY_US, NULL);
+		ret = spinand_wait_ready_confirmed(spinand,
+						   SPINAND_READ_INITIAL_DELAY_US,
+						   SPINAND_READ_POLL_DELAY_US,
+						   NULL);
 		if (ret < 0)
 			goto end_cont_read;
 
@@ -1196,10 +1245,10 @@ static int spinand_erase(struct nand_device *nand, const struct nand_pos *pos)
 	if (ret)
 		return ret;
 
-	ret = spinand_wait(spinand,
-			   SPINAND_ERASE_INITIAL_DELAY_US,
-			   SPINAND_ERASE_POLL_DELAY_US,
-			   &status);
+	ret = spinand_wait_ready_confirmed(spinand,
+					   SPINAND_ERASE_INITIAL_DELAY_US,
+					   SPINAND_ERASE_POLL_DELAY_US,
+					   &status);
 
 	if (!ret && (status & STATUS_ERASE_FAILED))
 		ret = -EIO;
diff --git a/drivers/mtd/nand/spi/skyhigh.c b/drivers/mtd/nand/spi/skyhigh.c
index bf9ce163e6a7b..19f2380b0427a 100644
--- a/drivers/mtd/nand/spi/skyhigh.c
+++ b/drivers/mtd/nand/spi/skyhigh.c
@@ -88,7 +88,8 @@ static const struct spinand_info skyhigh_spinand_table[] = {
 		     SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
 					      &write_cache_variants,
 					      &update_cache_variants),
-		     SPINAND_NO_RAW_ACCESS,
+		     SPINAND_NO_RAW_ACCESS |
+		     SPINAND_PREMATURE_READY_QUIRK,
 		     SPINAND_ECCINFO(&skyhigh_spinand_ooblayout,
 				     skyhigh_spinand_ecc_get_status)),
 	SPINAND_INFO("S35ML01G300",
@@ -98,7 +99,8 @@ static const struct spinand_info skyhigh_spinand_table[] = {
 		     SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
 					      &write_cache_variants,
 					      &update_cache_variants),
-		     SPINAND_NO_RAW_ACCESS,
+		     SPINAND_NO_RAW_ACCESS |
+		     SPINAND_PREMATURE_READY_QUIRK,
 		     SPINAND_ECCINFO(&skyhigh_spinand_ooblayout,
 				     skyhigh_spinand_ecc_get_status)),
 	SPINAND_INFO("S35ML02G300",
@@ -108,7 +110,8 @@ static const struct spinand_info skyhigh_spinand_table[] = {
 		     SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
 					      &write_cache_variants,
 					      &update_cache_variants),
-		     SPINAND_NO_RAW_ACCESS,
+		     SPINAND_NO_RAW_ACCESS |
+		     SPINAND_PREMATURE_READY_QUIRK,
 		     SPINAND_ECCINFO(&skyhigh_spinand_ooblayout,
 				     skyhigh_spinand_ecc_get_status)),
 	SPINAND_INFO("S35ML04G300",
@@ -118,7 +121,8 @@ static const struct spinand_info skyhigh_spinand_table[] = {
 		     SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
 					      &write_cache_variants,
 					      &update_cache_variants),
-		     SPINAND_NO_RAW_ACCESS,
+		     SPINAND_NO_RAW_ACCESS |
+		     SPINAND_PREMATURE_READY_QUIRK,
 		     SPINAND_ECCINFO(&skyhigh_spinand_ooblayout,
 				     skyhigh_spinand_ecc_get_status)),
 };
diff --git a/include/linux/mtd/spinand.h b/include/linux/mtd/spinand.h
index d7c99fcfb85ef..0767964275cf2 100644
--- a/include/linux/mtd/spinand.h
+++ b/include/linux/mtd/spinand.h
@@ -492,6 +492,7 @@ struct spinand_ecc_info {
 #define SPINAND_HAS_READ_PLANE_SELECT_BIT		BIT(3)
 #define SPINAND_NO_RAW_ACCESS				BIT(4)
 #define SPINAND_ODTR_PACKED_PAGE_READ			BIT(5)
+#define SPINAND_PREMATURE_READY_QUIRK			BIT(6)
 
 /**
  * struct spinand_ondie_ecc_conf - private SPI-NAND on-die ECC engine structure

base-commit: 7649ba73f9e7e1e04d0b46213e39c79ae09dd55c