Skip to contentSkip to conversation

mtd: rawnand: Fixes nand infra delay setting and FSMC nand controller

PATCH Applied Herve Codina 2 participants 9 messages Nov 19, 2021

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

v3 cover mtd: rawnand: Fixes nand infra delay setting and FSMC nand controller

Hi,

This patch series is mainly related to FSMC nand controller fixes except for the two first patches which fix some issues in nand base.

Details are given in each patches.

Related to the v1 series:
- the patch related to 8bit accesses has been removed as it was not needed (I misunderstood 8bit accesses meaning).
- A new patch is introduced related to nand_choose_best_timings() issue I discovered during the v2.

Best regards,
Herve Codina

Herve Codina (4):
mtd: rawnand: Fix nand_erase_op delay
mtd: rawnand: Fix nand_choose_best_timings() on unsupported interface
mtd: rawnand: fsmc: Take instruction delay into account
mtd: rawnand: fsmc: Fix timing computation

 drivers/mtd/nand/raw/fsmc_nand.c | 36 +++++++++++++++++++++++++-------
 drivers/mtd/nand/raw/nand_base.c |  6 +++---
 2 files changed, 31 insertions(+), 11 deletions(-)
Signature · 1 line
2.31.1
#1 lore raw

PATCH v3 1/4 mtd: rawnand: Fix nand_erase_op delay

NAND_OP_CMD() expects a delay parameter in nanoseconds. The delay value is wrongly given in milliseconds.

Fix the conversion macro used in order to set this delay in nanoseconds.

Fixes d7a773e8812b ("mtd: rawnand: Access SDR and NV-DDR timings through a common macro")
Fixes 8878b126df76 ("mtd: nand: add ->exec_op() implementation")

---
Changes v1 -> v2:
- Commit log reword

Changes v2 -> v3:
- Fix commit log typo
- Added 'Fixes' references

 drivers/mtd/nand/raw/nand_base.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/mtd/nand/raw/nand_base.c +1 -1
index 3d6c6e880520..5c6b065837ef 100644
@@ -1837,7 +1837,7 @@ int nand_erase_op(struct nand_chip *chip, unsigned int eraseblock)
 			NAND_OP_CMD(NAND_CMD_ERASE1, 0),
 			NAND_OP_ADDR(2, addrs, 0),
 			NAND_OP_CMD(NAND_CMD_ERASE2,
-				    NAND_COMMON_TIMING_MS(conf, tWB_max)),
+				    NAND_COMMON_TIMING_NS(conf, tWB_max)),
 			NAND_OP_WAIT_RDY(NAND_COMMON_TIMING_MS(conf, tBERS_max),
 					 0),
 		};
Signature · 1 line
2.31.1
maintainer Applied
#2 lore raw

Re: [PATCH v3 1/4] mtd: rawnand: Fix nand_erase_op delay

Quoted 9 lines · Herve Codina

NAND_OP_CMD() expects a delay parameter in nanoseconds. The delay value is wrongly given in milliseconds.

Fix the conversion macro used in order to set this delay in nanoseconds.

Fixes d7a773e8812b ("mtd: rawnand: Access SDR and NV-DDR timings through a common macro")
Fixes 8878b126df76 ("mtd: nand: add ->exec_op() implementation")

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git mtd/fixes, thanks.

Miquel

#3 lore raw

PATCH v3 2/4 mtd: rawnand: Fix nand_choose_best_timings() on unsupported interface

In reply to Herve Codina

When the NV-DDR interface is not supported by the NAND chip, the value of onfi->nvddr_timing_modes is 0. In this case, the best_mode variable value in nand_choose_best_nvddr_timings() is -1. The last for-loop is skipped and the function returns an uninitialized value.
If this returned value is 0, the nand_choose_best_sdr_timings() is not executed and no 'best timing' are set. This leads the host controller and the NAND chip working at default mode 0 timing even if a better timing can be used.

Fix this uninitialized returned value.

nand_choose_best_sdr_timings() is pretty similar to nand_choose_best_nvddr_timings(). Even if onfi->sdr_timing_modes should never be seen as 0, nand_choose_best_sdr_timings() returned value is fixed.

Fixes a9ecc8c814e9 ("mtd: rawnand: Choose the best timings, NV-DDR included")

---
Changes v1 to v2:
- New patch in v2 series

Changes v2 -> v3:
- Fix commit log typo

 drivers/mtd/nand/raw/nand_base.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/mtd/nand/raw/nand_base.c +2 -2
index 5c6b065837ef..a130320de412 100644
@@ -926,7 +926,7 @@ int nand_choose_best_sdr_timings(struct nand_chip *chip,
 				 struct nand_sdr_timings *spec_timings)
 {
 	const struct nand_controller_ops *ops = chip->controller->ops;
-	int best_mode = 0, mode, ret;
+	int best_mode = 0, mode, ret = -EOPNOTSUPP;
 
 	iface->type = NAND_SDR_IFACE;
 
@@ -977,7 +977,7 @@ int nand_choose_best_nvddr_timings(struct nand_chip *chip,
 				   struct nand_nvddr_timings *spec_timings)
 {
 	const struct nand_controller_ops *ops = chip->controller->ops;
-	int best_mode = 0, mode, ret;
+	int best_mode = 0, mode, ret = -EOPNOTSUPP;
 
 	iface->type = NAND_NVDDR_IFACE;
 
Signature · 1 line
2.31.1
maintainer Applied
#4 lore raw

Re: [PATCH v3 2/4] mtd: rawnand: Fix nand_choose_best_timings() on unsupported interface

Quoted 12 lines · Herve Codina

When the NV-DDR interface is not supported by the NAND chip, the value of onfi->nvddr_timing_modes is 0. In this case, the best_mode variable value in nand_choose_best_nvddr_timings() is -1. The last for-loop is skipped and the function returns an uninitialized value.
If this returned value is 0, the nand_choose_best_sdr_timings() is not executed and no 'best timing' are set. This leads the host controller and the NAND chip working at default mode 0 timing even if a better timing can be used.

Fix this uninitialized returned value.

Quoted 7 lines

nand_choose_best_sdr_timings() is pretty similar to nand_choose_best_nvddr_timings(). Even if onfi->sdr_timing_modes should never be seen as 0, nand_choose_best_sdr_timings() returned value is fixed.

Fixes a9ecc8c814e9 ("mtd: rawnand: Choose the best timings, NV-DDR included")

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git mtd/fixes, thanks.

Miquel

#5 lore raw

PATCH v3 3/4 mtd: rawnand: fsmc: Take instruction delay into account

In reply to Herve Codina

The FSMC NAND controller should apply a delay after the instruction has been issued on the bus.
The FSMC NAND controller driver did not handle this delay.

Add this waiting delay in the FSMC NAND controller driver.

Fixes 4da712e70294 ("mtd: nand: fsmc: use ->exec_op()")

---
Changes v1 -> v2:
- Commit log reword

Changes v2 -> v3:
- Added 'Fixes' reference

 drivers/mtd/nand/raw/fsmc_nand.c | 4 ++++
 1 file changed, 4 insertions(+)
drivers/mtd/nand/raw/fsmc_nand.c +4 -0
index 658f0cbe7ce8..0a6c9ef0ea8b 100644
@@ -15,6 +15,7 @@
 
 #include <linux/clk.h>
 #include <linux/completion.h>
+#include <linux/delay.h>
 #include <linux/dmaengine.h>
 #include <linux/dma-direction.h>
 #include <linux/dma-mapping.h>
@@ -664,6 +665,9 @@ static int fsmc_exec_op(struct nand_chip *chip, const struct nand_operation *op,
 						instr->ctx.waitrdy.timeout_ms);
 			break;
 		}
+
+		if (instr->delay_ns)
+			ndelay(instr->delay_ns);
 	}
 
 	return ret;
Signature · 1 line
2.31.1
maintainer Applied
#6 lore raw

Re: [PATCH v3 3/4] mtd: rawnand: fsmc: Take instruction delay into account

Quoted 8 lines · Herve Codina

The FSMC NAND controller should apply a delay after the instruction has been issued on the bus.
The FSMC NAND controller driver did not handle this delay.

Add this waiting delay in the FSMC NAND controller driver.

Fixes 4da712e70294 ("mtd: nand: fsmc: use ->exec_op()")

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git mtd/fixes, thanks.

Miquel

#7 lore raw

PATCH v3 4/4 mtd: rawnand: fsmc: Fix timing computation

In reply to Herve Codina

Under certain circumstances, the timing settings calculated by the FSMC NAND controller driver were inaccurate. These settings led to incorrect data reads or fallback to timing mode 0 depending on the NAND chip used.

The timing computation did not take into account the following constraint given in SPEAr3xx reference manual:
twait >= tCEA - (tset * TCLK) + TOUTDEL + TINDEL

Enhance the timings calculation by taking into account this additional constraint.

This change has no impact on slow timing modes such as mode 0. Indeed, on mode 0, computed values are the same with and without the patch.

NANDs which previously stayed in mode 0 because of fallback to mode 0 can now work at higher speeds and NANDs which were not working at all because of the corrupted data work at high speeds without troubles.

Overall improvement on a Micron/MT29F1G08 (flash_speed tool):

                        mode0       mode3
eraseblock write speed  3220 KiB/s  4511 KiB/s
eraseblock read speed   4491 KiB/s  7529 KiB/s
Fixes d9fb079571833 ("mtd: nand: fsmc: add support for SDR timings")

---
Changes v1 to v2:
- Commit log reword
- Added performance details in commit log
- Used #define for TOUTDEL and TINDEL and
Fixed coding style
- Used max3()

Changes v2 -> v3:
- Improved commit log
- Added 'Fixes' reference
- Changed comment in source code

 drivers/mtd/nand/raw/fsmc_nand.c | 32 ++++++++++++++++++++++++--------
 1 file changed, 24 insertions(+), 8 deletions(-)
drivers/mtd/nand/raw/fsmc_nand.c +24 -8
index 0a6c9ef0ea8b..6b2bda815b88 100644
@@ -94,6 +94,14 @@
 
 #define FSMC_BUSY_WAIT_TIMEOUT	(1 * HZ)
 
+/*
+ * According to SPEAr300 Reference Manual (RM0082)
+ *  TOUDEL = 7ns (Output delay from the flip-flops to the board)
+ *  TINDEL = 5ns (Input delay from the board to the flipflop)
+ */
+#define TOUTDEL	7000
+#define TINDEL	5000
+
 struct fsmc_nand_timings {
 	u8 tclr;
 	u8 tar;
@@ -278,7 +286,7 @@ static int fsmc_calc_timings(struct fsmc_nand_data *host,
 {
 	unsigned long hclk = clk_get_rate(host->clk);
 	unsigned long hclkn = NSEC_PER_SEC / hclk;
-	u32 thiz, thold, twait, tset;
+	u32 thiz, thold, twait, tset, twait_min;
 
 	if (sdrt->tRC_min < 30000)
 		return -EOPNOTSUPP;
@@ -310,13 +318,6 @@ static int fsmc_calc_timings(struct fsmc_nand_data *host,
 	else if (tims->thold > FSMC_THOLD_MASK)
 		tims->thold = FSMC_THOLD_MASK;
 
-	twait = max(sdrt->tRP_min, sdrt->tWP_min);
-	tims->twait = DIV_ROUND_UP(twait / 1000, hclkn) - 1;
-	if (tims->twait == 0)
-		tims->twait = 1;
-	else if (tims->twait > FSMC_TWAIT_MASK)
-		tims->twait = FSMC_TWAIT_MASK;
-
 	tset = max(sdrt->tCS_min - sdrt->tWP_min,
 		   sdrt->tCEA_max - sdrt->tREA_max);
 	tims->tset = DIV_ROUND_UP(tset / 1000, hclkn) - 1;
@@ -325,6 +326,21 @@ static int fsmc_calc_timings(struct fsmc_nand_data *host,
 	else if (tims->tset > FSMC_TSET_MASK)
 		tims->tset = FSMC_TSET_MASK;
 
+	/*
+	 * According to SPEAr300 Reference Manual (RM0082) which gives more
+	 * information related to FSMSC timings than the SPEAr600 one (RM0305),
+	 *   twait >= tCEA - (tset * TCLK) + TOUTDEL + TINDEL
+	 */
+	twait_min = sdrt->tCEA_max - ((tims->tset + 1) * hclkn * 1000)
+		    + TOUTDEL + TINDEL;
+	twait = max3(sdrt->tRP_min, sdrt->tWP_min, twait_min);
+
+	tims->twait = DIV_ROUND_UP(twait / 1000, hclkn) - 1;
+	if (tims->twait == 0)
+		tims->twait = 1;
+	else if (tims->twait > FSMC_TWAIT_MASK)
+		tims->twait = FSMC_TWAIT_MASK;
+
 	return 0;
 }
 
Signature · 1 line
2.31.1
maintainer Applied
#8 lore raw

Re: [PATCH v3 4/4] mtd: rawnand: fsmc: Fix timing computation

Quoted 21 lines · Herve Codina

Under certain circumstances, the timing settings calculated by the FSMC NAND controller driver were inaccurate. These settings led to incorrect data reads or fallback to timing mode 0 depending on the NAND chip used.

The timing computation did not take into account the following constraint given in SPEAr3xx reference manual:
twait >= tCEA - (tset * TCLK) + TOUTDEL + TINDEL

Enhance the timings calculation by taking into account this additional constraint.

This change has no impact on slow timing modes such as mode 0. Indeed, on mode 0, computed values are the same with and without the patch.

NANDs which previously stayed in mode 0 because of fallback to mode 0 can now work at higher speeds and NANDs which were not working at all because of the corrupted data work at high speeds without troubles.

Quoted 7 lines

Overall improvement on a Micron/MT29F1G08 (flash_speed tool):

                        mode0       mode3
eraseblock write speed  3220 KiB/s  4511 KiB/s
eraseblock read speed   4491 KiB/s  7529 KiB/s
Fixes d9fb079571833 ("mtd: nand: fsmc: add support for SDR timings")

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git mtd/fixes, thanks.

Miquel