RE: [PATCH] mtd: rawnand: gpmi: Set WAIT_FOR_READY timeout based on program/erase times

[Han Xu <han.xu@nxp.com>]

>-----Original Message-----
>From: Sascha Hauer <s.hauer@pengutronix.de>
>Sent: Friday, July 1, 2022 6:04 AM
>To: linux-mtd@lists.infradead.org
>Cc: Han Xu <han.xu@nxp.com>; Miquel Raynal <miquel.raynal@bootlin.com>;
>kernel@pengutronix.de; Richard Weinberger <richard@nod.at>; Sascha Hauer
><s.hauer@pengutronix.de>; stable@vger.kernel.org
>Subject: [PATCH] mtd: rawnand: gpmi: Set WAIT_FOR_READY timeout based on
>program/erase times
>
>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>

>---
> drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
>b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
>index 889e403299568..93da23682d862 100644
>--- a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
>+++ b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
>@@ -850,9 +850,10 @@ static int 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;
> 	unsigned long clk_rate, min_rate;
>+	u64 busy_timeout_ps;
>
> 	if (sdr->tRC_min >= 30000) {
> 		/* ONFI non-EDO modes [0-3] */
>@@ -885,7 +886,8 @@ static int 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) |
>--
>2.30.2