diff mbox series

[RESEND,stable,5.10] mtd: rawnand: gpmi: Set WAIT_FOR_READY timeout based on program/erase times

Message ID 20221028213315.4171685-1-tharvey@gateworks.com
State Not Applicable
Headers show
Series [RESEND,stable,5.10] mtd: rawnand: gpmi: Set WAIT_FOR_READY timeout based on program/erase times | expand

Commit Message

Tim Harvey Oct. 28, 2022, 9:33 p.m. UTC 
From: Sascha Hauer <s.hauer@pengutronix.de>

06781a5026350 Fixes the calculation of the DEVICE_BUSY_TIMEOUT register
value from busy_timeout_cycles. busy_timeout_cycles is calculated wrong
though: It is calculated based on the maximum page read time, but the
timeout is also used for page write and block erase operations which
require orders of magnitude bigger timeouts.

Fix this by calculating busy_timeout_cycles from the maximum of
tBERS_max and tPROG_max.

This is for now the easiest and most obvious way to fix the driver.
There's room for improvements though: The NAND_OP_WAITRDY_INSTR tells us
the desired timeout for the current operation, so we could program the
timeout dynamically for each operation instead of setting a fixed
timeout. Also we could wire up the interrupt handler to actually detect
and forward timeouts occurred when waiting for the chip being ready.

As a sidenote I verified that the change in 06781a5026350 is really
correct. I wired up the interrupt handler in my tree and measured the
time between starting the operation and the timeout interrupt handler
coming in. The time increases 41us with each step in the timeout
register which corresponds to 4096 clock cycles with the 99MHz clock
that I have.

Fixes: 06781a5026350 ("mtd: rawnand: gpmi: Fix setting busy timeout setting")
Fixes: b1206122069aa ("mtd: rawniand: gpmi: use core timings instead of an empirical derivation")
Cc: stable@vger.kernel.org
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Acked-by: Han Xu <han.xu@nxp.com>
Tested-by: Tomasz Moń <tomasz.mon@camlingroup.com>
Signed-off-by: Richard Weinberger <richard@nod.at>
---
stable-5.10

 drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Greg Kroah-Hartman Oct. 31, 2022, 7:43 a.m. UTC | #1 
On Fri, Oct 28, 2022 at 02:33:15PM -0700, Tim Harvey wrote:
> From: Sascha Hauer <s.hauer@pengutronix.de>
> 
> 06781a5026350 Fixes the calculation of the DEVICE_BUSY_TIMEOUT register
> value from busy_timeout_cycles. busy_timeout_cycles is calculated wrong
> though: It is calculated based on the maximum page read time, but the
> timeout is also used for page write and block erase operations which
> require orders of magnitude bigger timeouts.
> 
> Fix this by calculating busy_timeout_cycles from the maximum of
> tBERS_max and tPROG_max.
> 
> This is for now the easiest and most obvious way to fix the driver.
> There's room for improvements though: The NAND_OP_WAITRDY_INSTR tells us
> the desired timeout for the current operation, so we could program the
> timeout dynamically for each operation instead of setting a fixed
> timeout. Also we could wire up the interrupt handler to actually detect
> and forward timeouts occurred when waiting for the chip being ready.
> 
> As a sidenote I verified that the change in 06781a5026350 is really
> correct. I wired up the interrupt handler in my tree and measured the
> time between starting the operation and the timeout interrupt handler
> coming in. The time increases 41us with each step in the timeout
> register which corresponds to 4096 clock cycles with the 99MHz clock
> that I have.
> 
> Fixes: 06781a5026350 ("mtd: rawnand: gpmi: Fix setting busy timeout setting")
> Fixes: b1206122069aa ("mtd: rawniand: gpmi: use core timings instead of an empirical derivation")
> Cc: stable@vger.kernel.org
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> Acked-by: Han Xu <han.xu@nxp.com>
> Tested-by: Tomasz Moń <tomasz.mon@camlingroup.com>
> Signed-off-by: Richard Weinberger <richard@nod.at>
> ---
> stable-5.10

You did not sign-off on this backport.  Please fix that up and resend
and also give us a hint as to what the upstream commit id is here.

thanks,

greg k-h
diff mbox series

Patch

diff --git a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
index 92e8ca56f566..200d3ab343b0 100644
--- a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
+++ b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
@@ -653,8 +653,9 @@  static void gpmi_nfc_compute_timings(struct gpmi_nand_data *this,
 	unsigned int tRP_ps;
 	bool use_half_period;
 	int sample_delay_ps, sample_delay_factor;
-	u16 busy_timeout_cycles;
+	unsigned int busy_timeout_cycles;
 	u8 wrn_dly_sel;
+	u64 busy_timeout_ps;
 
 	if (sdr->tRC_min >= 30000) {
 		/* ONFI non-EDO modes [0-3] */
@@ -678,7 +679,8 @@  static void gpmi_nfc_compute_timings(struct gpmi_nand_data *this,
 	addr_setup_cycles = TO_CYCLES(sdr->tALS_min, period_ps);
 	data_setup_cycles = TO_CYCLES(sdr->tDS_min, period_ps);
 	data_hold_cycles = TO_CYCLES(sdr->tDH_min, period_ps);
-	busy_timeout_cycles = TO_CYCLES(sdr->tWB_max + sdr->tR_max, period_ps);
+	busy_timeout_ps = max(sdr->tBERS_max, sdr->tPROG_max);
+	busy_timeout_cycles = TO_CYCLES(busy_timeout_ps, period_ps);
 
 	hw->timing0 = BF_GPMI_TIMING0_ADDRESS_SETUP(addr_setup_cycles) |
 		      BF_GPMI_TIMING0_DATA_HOLD(data_hold_cycles) |