[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


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[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


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Patch "Revert "Revert "mtd: rawnand: gpmi: Fix setting busy timeout setting""" has been added to the 5.4-stable tree

[gregkh]

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 <stable@vger.kernel.org> know about it.


From max.oss.09@gmail.com  Wed Feb 21 12:00:21 2024
From: max.oss.09@gmail.com
Date: Wed,  7 Feb 2024 18:49:11 +0100
Subject: Revert "Revert "mtd: rawnand: gpmi: Fix setting busy timeout setting""
To: stable@vger.kernel.org
Cc: gregkh@linuxfoundation.org, patches@lists.linux.dev, s.hauer@pengutronix.de, han.xu@nxp.com, tomasz.mon@camlingroup.com, richard@nod.at, tharvey@gateworks.com, linux-mtd@lists.infradead.org, Max Krummenacher <max.krummenacher@toradex.com>
Message-ID: <20240207174911.870822-2-max.oss.09@gmail.com>

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>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- 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(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 max.oss.09@gmail.com are

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

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/