mtd: rawnand: gpmi: busy_timeout_cycles

From: Max Krummenacher <[email protected]>

Hello

With the backported commit e09ff743e30b ("mtd: rawnand: gpmi: Set WAIT_FOR_READY timeout based on program/erase times") in kernel 5.4.y I see corruption of the NAND content during kernel boot. Reverting said commit on top of current 5.4.y fixes the issue.

It seems that the commit relies on commit 71c76f56b97c ("mtd:
rawnand: gpmi: Fix setting busy timeout setting"), but its backport got reverted.
One should either backport both commits or none, having only one results in potential bugs.

I've seen it in 5.4.y, however in 5.10.y and 5.15.y there one of the two backports is also reverted and likely the same regression exists.

Any comments?

Max

Max Krummenacher (1):
Revert "Revert "mtd: rawnand: gpmi: Fix setting busy timeout setting""

 drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

2.42.0

From: Max Krummenacher <[email protected]>

This reverts commit 15a3adfe75937c9e4e0e48f0ed40dd39a0e526e2.

The backport of [1] relies on having [2] also backported. Having only one of the two results in a bogus hw->timing1 setting.

If only [2] is backportet the 16 bit register value likely underflows resulting in a busy_wait_timeout of 0.
Or if only [1] is applied the value likely overflows with chances of having last 16 LSBs all 0 which would then result in a busy_wait_timeout of 0 too.

Both cases may lead to NAND data corruption, e.g. on a Colibri iMX7 setup this has been seen.

[1] commit 0fddf9ad06fd ("mtd: rawnand: gpmi: Set WAIT_FOR_READY timeout based on program/erase times")
[2] commit 06781a5026350 ("mtd: rawnand: gpmi: Fix setting busy timeout setting")

---
 drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index b806a762d079f..fdf5cf5565f99 100644
@@ -684,7 +684,7 @@ static void gpmi_nfc_compute_timings(struct gpmi_nand_data *this,
 	hw->timing0 = BF_GPMI_TIMING0_ADDRESS_SETUP(addr_setup_cycles) |
 		      BF_GPMI_TIMING0_DATA_HOLD(data_hold_cycles) |
 		      BF_GPMI_TIMING0_DATA_SETUP(data_setup_cycles);
-	hw->timing1 = BF_GPMI_TIMING1_BUSY_TIMEOUT(busy_timeout_cycles * 4096);
+	hw->timing1 = BF_GPMI_TIMING1_BUSY_TIMEOUT(DIV_ROUND_UP(busy_timeout_cycles, 4096));
 
 	/*
 	 * Derive NFC ideal delay from {3}:

2.42.0

This is a note to let you know that I've just added the patch titled

Revert "Revert "mtd: rawnand: gpmi: Fix setting busy timeout setting""

to the 5.4-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
revert-revert-mtd-rawnand-gpmi-fix-setting-busy-timeout-setting.patch
and it can be found in the queue-5.4 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree, please let <[email protected]> know about it.

From [email protected] Wed Feb 21 12:00:21 2024
From: [email protected]
Date: Wed, 7 Feb 2024 18:49:11 +0100
Subject: Revert "Revert "mtd: rawnand: gpmi: Fix setting busy timeout setting""
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], Max Krummenacher <[email protected]>
Message-ID: <[email protected]>

From: Max Krummenacher <[email protected]>

This reverts commit 15a3adfe75937c9e4e0e48f0ed40dd39a0e526e2.

The backport of [1] relies on having [2] also backported. Having only one of the two results in a bogus hw->timing1 setting.

If only [2] is backportet the 16 bit register value likely underflows resulting in a busy_wait_timeout of 0.
Or if only [1] is applied the value likely overflows with chances of having last 16 LSBs all 0 which would then result in a busy_wait_timeout of 0 too.

Both cases may lead to NAND data corruption, e.g. on a Colibri iMX7 setup this has been seen.

[1] commit 0fddf9ad06fd ("mtd: rawnand: gpmi: Set WAIT_FOR_READY timeout based on program/erase times")
[2] commit 06781a5026350 ("mtd: rawnand: gpmi: Fix setting busy timeout setting")

---
 drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

@@ -684,7 +684,7 @@ static void gpmi_nfc_compute_timings(str
 	hw->timing0 = BF_GPMI_TIMING0_ADDRESS_SETUP(addr_setup_cycles) |
 		      BF_GPMI_TIMING0_DATA_HOLD(data_hold_cycles) |
 		      BF_GPMI_TIMING0_DATA_SETUP(data_setup_cycles);
-	hw->timing1 = BF_GPMI_TIMING1_BUSY_TIMEOUT(busy_timeout_cycles * 4096);
+	hw->timing1 = BF_GPMI_TIMING1_BUSY_TIMEOUT(DIV_ROUND_UP(busy_timeout_cycles, 4096));
 
 	/*
 	 * Derive NFC ideal delay from {3}:

Patches currently in stable-queue which might be from [email protected] are

queue-5.4/revert-revert-mtd-rawnand-gpmi-fix-setting-busy-timeout-setting.patch

Now queeud up, thanks.

greg k-h