[regression 5.4.y][RFC][PATCH 0/1] mtd: rawnand: gpmi: busy_timeout_cycles

[regression 5.4.y][RFC][PATCH 0/1] mtd: rawnand: gpmi: busy_timeout_cycles

[max.oss.09]

From: Max Krummenacher <max.krummenacher@toradex.com>

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

[regression 5.4.y][RFC][PATCH mtd: rawnand: gpmi: busy_timeout_cycles 1/1] Revert "Revert "mtd: rawnand: gpmi: Fix setting busy timeout setting""

[max.oss.09]

From: Max Krummenacher <max.krummenacher@toradex.com>

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")

Signed-off-by: Max Krummenacher <max.krummenacher@toradex.com>
---
 drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
index b806a762d079f..fdf5cf5565f99 100644
--- a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
+++ b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
@@ -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

Re: [regression 5.4.y][RFC][PATCH 0/1] mtd: rawnand: gpmi: busy_timeout_cycles

[Greg KH]

On Wed, Feb 07, 2024 at 06:49:10PM +0100, max.oss.09@gmail.com wrote:
> From: Max Krummenacher <max.krummenacher@toradex.com>
> 
> 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
> 
> 

Now queeud up, thanks.

greg k-h