[PATCH v3 0/3] mtd: nand: raw: Collected improvements

[PATCH v3 0/3] mtd: nand: raw: Collected improvements

[Alexander Dahl]

Hello everyone,

while working on NAND flash support for a custom board based on the at91
SAM9X60 SoC I stumbled over some issues in the raw nand subsystem.

Some trivial patches of previous iterations of this series were already
applied.

Patch 1 introduces a new subcommand for the new atmel nand controller
driver.  Patch 2 introduces a new subcommand for the nand command to
override ONFI timing mode.  Both are are for debugging purposes only and
thus optional, and need to be enabled through menu.  Both helped me a
lot when investigating issues.  Patch 3 is a fix carried over from
at91bootstrap for faster at91 SoCs with certain raw NAND chips.

Series is based on post v2024.04 master now.

Greets
Alex

v3:

- 4 patches removed, applied to master
- other 2 patches from v2 unchanged, still under test
- added new third patch with a fix for atmel nand timings (forgot to
  send that with v2)

v2:

Link: https://lore.kernel.org/u-boot/20240320090214.40465-1-ada@thorsis.com/

- rebased on recent next
- collected tags
- improved patch 4 after feedback from Mihai
- added new patch 5 with another help text fix
- added new patch 6 with a new debug command
- reworded cover letter

v1:

Link: https://lore.kernel.org/u-boot/20240307091014.39796-1-ada@thorsis.com/

See per patch changes in patches for more detailed changes.

Alexander Dahl (3):
  mtd: nand: raw: atmel: Introduce optional debug commands
  cmd: nand: Add new optional sub-command 'onfi'
  mtd: nand: raw: atmel: Fix pulse read timing for certain NAND flashes

 cmd/Kconfig                                  |  10 +
 cmd/nand.c                                   |  61 ++++
 drivers/mtd/nand/raw/Kconfig                 |   9 +
 drivers/mtd/nand/raw/atmel/nand-controller.c | 308 ++++++++++++++++++-
 drivers/mtd/nand/raw/nand_base.c             |   2 +-
 include/linux/mtd/rawnand.h                  |   1 +
 6 files changed, 386 insertions(+), 5 deletions(-)


base-commit: b03b49046af5dfca599d2ce8f0aafed89b97aa91
-- 
2.39.2

[PATCH v3 1/3] mtd: nand: raw: atmel: Introduce optional debug commands

[Alexander Dahl]

For now adds one new command 'hsmc' with a single subcommand 'decode' to
read and display the content of the registers of the Static Memory
Controllers (SMC/HSMC) found in different at91 SoCs.  Needed to get a
better picture on what raw nand core and atmel nand controller driver
try to set as timings based on ONFI parameters of the connected NAND
chip.

Tested on SAMA5D2 and SAM9X60 based boards.  Example output:

    U-Boot> hsmc decode

    MCK rate: 200 MHz

    SMC_SETUP3:     0x00000002
    SMC_PULSE3:     0x06030703
    SMC_CYCLE3:     0x00060007
    SMC_MODE3:      0x001f0003
    NCS_RD: setup: 0 (0 ns), pulse: 6 (30 ns), hold: 0 (0 ns), cycle: 6 (30 ns)
       NRD: setup: 0 (0 ns), pulse: 3 (15 ns), hold: 3 (15 ns), cycle: 6 (30 ns)
    NCS_WR: setup: 0 (0 ns), pulse: 7 (35 ns), hold: 0 (0 ns), cycle: 7 (35 ns)
       NWE: setup: 2 (10 ns), pulse: 3 (15 ns), hold: 2 (10 ns), cycle: 7 (35 ns)
    Standard read applied
    TDF optimization enabled
    TDF cycles: 15 (75 ns)
    Data Bus Width: 8-bit bus
    NWAIT Mode: 0
    Write operation controlled by NWE signal
    Read operation controlled by NRD signal

Signed-off-by: Alexander Dahl <ada@thorsis.com>
Tested-by: Mihai Sain <mihai.sain@microchip.com>
---

Notes:
    v3:
    - no changes to this patch
    
    v2:
    - Use SMC register definitions
    - Implement atmel_hsmc_print_timings()
    - Improve hsmc command output
    - Collected tags
    
    v1:
    - initial patch version

 drivers/mtd/nand/raw/Kconfig                 |   9 +
 drivers/mtd/nand/raw/atmel/nand-controller.c | 295 +++++++++++++++++++
 2 files changed, 304 insertions(+)

diff --git a/drivers/mtd/nand/raw/Kconfig b/drivers/mtd/nand/raw/Kconfig
index 9f3f1267cbd..1c0084fd521 100644
--- a/drivers/mtd/nand/raw/Kconfig
+++ b/drivers/mtd/nand/raw/Kconfig
@@ -50,12 +50,21 @@ config SYS_NAND_NO_SUBPAGE_WRITE
 
 config DM_NAND_ATMEL
 	bool "Support Atmel NAND controller with DM support"
+	select MFD_ATMEL_SMC
 	select SYS_NAND_SELF_INIT
 	imply SYS_NAND_USE_FLASH_BBT
 	help
 	  Enable this driver for NAND flash platforms using an Atmel NAND
 	  controller.
 
+config CMD_NAND_ATMEL_DEBUG
+	bool "Optional debug commands for Atmel NAND controller"
+	depends on DM_NAND_ATMEL
+	help
+	  Add commands for debugging internals of the Atmel NAND flash
+	  controller, for example:
+	  - Decode Static Memory Controller (SMC) registers
+
 config NAND_ATMEL
 	bool "Support Atmel NAND controller"
 	select SYS_NAND_SELF_INIT
diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c b/drivers/mtd/nand/raw/atmel/nand-controller.c
index ee4ec6da587..bbafc88e44c 100644
--- a/drivers/mtd/nand/raw/atmel/nand-controller.c
+++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
@@ -51,11 +51,13 @@
 
 #include <asm-generic/gpio.h>
 #include <clk.h>
+#include <command.h>
 #include <dm/device_compat.h>
 #include <dm/devres.h>
 #include <dm/of_addr.h>
 #include <dm/of_access.h>
 #include <dm/uclass.h>
+#include <linux/bitops.h>
 #include <linux/completion.h>
 #include <linux/io.h>
 #include <linux/iopoll.h>
@@ -69,6 +71,7 @@
 #include <nand.h>
 #include <regmap.h>
 #include <syscon.h>
+#include <vsprintf.h>
 
 #include "pmecc.h"
 
@@ -216,6 +219,7 @@ struct atmel_nand_controller_ops {
 	int (*ecc_init)(struct nand_chip *chip);
 	int (*setup_data_interface)(struct atmel_nand *nand, int csline,
 				    const struct nand_data_interface *conf);
+	void (*print_info)(struct atmel_nand *nand, int csline);
 };
 
 struct atmel_nand_controller_caps {
@@ -2041,12 +2045,260 @@ err:
 	return ret;
 }
 
+#ifdef CONFIG_CMD_NAND_ATMEL_DEBUG
+u32 atmel_smc_decode_ncycles(u32 reg, u32 shift, u32 msbpos, u32 msbwidth, u32 msbfactor)
+{
+	/*
+	 *	Examples:
+	 *
+	 *	NRD setup length = (128 * NRD_SETUP[5] + NRD_SETUP[4:0]) clock cycles.
+	 *	NRD pulse length = (256 * NRD_PULSE[6] + NRD_PULSE[5:0]) clock cycles.
+	 *	Read cycle length = (NRD_CYCLE[8:7] * 256) + NRD_CYCLE[6:0] clock cycles.
+	 */
+
+	reg >>= shift;
+
+	u32 lsbmask = GENMASK(msbpos - 1, 0);
+	u32 msbmask = GENMASK(msbwidth - 1, 0) << msbpos;
+	u32 msb = (reg & msbmask) >> msbpos;
+	u32 lsb = (reg & lsbmask);
+
+	return msb * msbfactor + lsb;
+}
+
+static void atmel_smc_cs_conf_print_raw(struct atmel_smc_cs_conf *conf, int cs)
+{
+	printf("SMC_SETUP%d:     0x%08x\n", cs, conf->setup);
+	printf("SMC_PULSE%d:     0x%08x\n", cs, conf->pulse);
+	printf("SMC_CYCLE%d:     0x%08x\n", cs, conf->cycle);
+	printf("SMC_MODE%d:      0x%08x\n", cs, conf->mode);
+}
+
+static void atmel_hsmc_cs_conf_print_raw(struct atmel_smc_cs_conf *conf, int cs)
+{
+	printf("HSMC_SETUP%d:    0x%08x\n", cs, conf->setup);
+	printf("HSMC_PULSE%d:    0x%08x\n", cs, conf->pulse);
+	printf("HSMC_CYCLE%d:    0x%08x\n", cs, conf->cycle);
+	printf("HSMC_TIMINGS%d:  0x%08x\n", cs, conf->timings);
+	printf("HSMC_MODE%d:     0x%08x\n", cs, conf->mode);
+}
+
+static void atmel_smc_print_reg(const char *name, u32 setup, u32 pulse,
+				u32 cycle, u32 clk_period_ns)
+{
+	u32 hold = cycle - pulse - setup;
+
+	printf("%6s: setup: %u (%u ns), pulse: %u (%u ns), hold: %u (%u ns), cycle: %u (%u ns)\n",
+	       name, setup, setup * clk_period_ns, pulse, pulse * clk_period_ns,
+	       hold, hold * clk_period_ns, cycle, cycle * clk_period_ns);
+}
+
+static void atmel_smc_print_ncs_rd(struct atmel_smc_cs_conf *conf, u32 clk_period_ns)
+{
+	u32 ncs_rd_setup = atmel_smc_decode_ncycles(conf->setup,
+						    ATMEL_SMC_NCS_RD_SHIFT,
+						    5, 1, 128);
+	u32 ncs_rd_pulse = atmel_smc_decode_ncycles(conf->pulse,
+						    ATMEL_SMC_NCS_RD_SHIFT,
+						    6, 1, 256);
+	u32 nrd_cycle = atmel_smc_decode_ncycles(conf->cycle, 16, 7, 2, 256);
+
+	atmel_smc_print_reg("NCS_RD", ncs_rd_setup, ncs_rd_pulse,
+			    nrd_cycle, clk_period_ns);
+}
+
+static void atmel_smc_print_nrd(struct atmel_smc_cs_conf *conf, u32 clk_period_ns)
+{
+	u32 nrd_setup = atmel_smc_decode_ncycles(conf->setup,
+						 ATMEL_SMC_NRD_SHIFT,
+						 5, 1, 128);
+	u32 nrd_pulse = atmel_smc_decode_ncycles(conf->pulse,
+						 ATMEL_SMC_NRD_SHIFT,
+						 6, 1, 256);
+	u32 nrd_cycle = atmel_smc_decode_ncycles(conf->cycle, 16, 7, 2, 256);
+
+	atmel_smc_print_reg("NRD", nrd_setup, nrd_pulse, nrd_cycle, clk_period_ns);
+}
+
+static void atmel_smc_print_ncs_wr(struct atmel_smc_cs_conf *conf, u32 clk_period_ns)
+{
+	u32 ncs_wr_setup = atmel_smc_decode_ncycles(conf->setup,
+						    ATMEL_SMC_NCS_WR_SHIFT,
+						    5, 1, 128);
+	u32 ncs_wr_pulse = atmel_smc_decode_ncycles(conf->pulse,
+						    ATMEL_SMC_NCS_WR_SHIFT,
+						    6, 1, 256);
+	u32 nwe_cycle = atmel_smc_decode_ncycles(conf->cycle, 0, 7, 2, 256);
+
+	atmel_smc_print_reg("NCS_WR", ncs_wr_setup, ncs_wr_pulse,
+			    nwe_cycle, clk_period_ns);
+}
+
+static void atmel_smc_print_nwe(struct atmel_smc_cs_conf *conf, u32 clk_period_ns)
+{
+	u32 nwe_setup = atmel_smc_decode_ncycles(conf->setup,
+						 ATMEL_SMC_NWE_SHIFT,
+						 5, 1, 128);
+	u32 nwe_pulse = atmel_smc_decode_ncycles(conf->pulse,
+						 ATMEL_SMC_NWE_SHIFT,
+						 6, 1, 256);
+	u32 nwe_cycle = atmel_smc_decode_ncycles(conf->cycle, 0, 7, 2, 256);
+
+	atmel_smc_print_reg("NWE", nwe_setup, nwe_pulse, nwe_cycle, clk_period_ns);
+}
+
+static void atmel_smc_print_mode(struct atmel_smc_cs_conf *conf, u32 clk_period_ns)
+{
+	u32 tdf;
+	u8 dbw;
+
+	if (conf->mode & BIT(24)) {
+		printf("Asynchronous burst read in Page mode applied on the corresponding chip select\n");
+		printf("Page Size: %u-byte page\n",
+		       4 << ((conf->mode & GENMASK(29, 28)) >> 28));
+	} else {
+		printf("Standard read applied\n");
+	}
+
+	tdf = (conf->mode & GENMASK(19, 16)) >> 16;
+	printf("TDF optimization %s\n",
+	       (conf->mode & BIT(20)) ? "enabled" : "disabled");
+	printf("TDF cycles: %u (%u ns)\n", tdf, tdf * clk_period_ns);
+
+	dbw = 8 << ((conf->mode & GENMASK(13, 12)) >> 12);
+	printf("Data Bus Width: %u-bit bus\n", dbw);
+	if (dbw > 8)
+		printf("Byte %s access type\n",
+		       (conf->mode & BIT(8)) ? "write" : "select");
+
+	printf("NWAIT Mode: %lu\n", (conf->mode & GENMASK(5, 4)) >> 4);
+	printf("Write operation controlled by %s signal\n",
+	       (conf->mode & BIT(1)) ? "NWE" : "NCS");
+	printf("Read operation controlled by %s signal\n",
+	       (conf->mode & BIT(0)) ? "NRD" : "NCS");
+}
+
+static void atmel_hsmc_print_mode(struct atmel_smc_cs_conf *conf, u32 clk_period_ns)
+{
+	u32 tdf;
+	u8 dbw;
+
+	tdf = (conf->mode & GENMASK(19, 16)) >> 16;
+	printf("TDF optimization %s\n",
+	       (conf->mode & BIT(20)) ? "enabled" : "disabled");
+	printf("TDF cycles: %u (%u ns)\n", tdf, tdf * clk_period_ns);
+
+	dbw = 8 << ((conf->mode & BIT(12)) >> 12);
+	printf("Data Bus Width: %u-bit bus\n", dbw);
+	if (dbw > 8)
+		printf("Byte %s access type\n",
+		       (conf->mode & BIT(8)) ? "write" : "select");
+
+	printf("NWAIT Mode: %lu\n", (conf->mode & GENMASK(5, 4)) >> 4);
+	printf("Write operation controlled by %s signal\n",
+	       (conf->mode & BIT(1)) ? "NWE" : "NCS");
+	printf("Read operation controlled by %s signal\n",
+	       (conf->mode & BIT(0)) ? "NRD" : "NCS");
+}
+
+static void atmel_hsmc_print_timings(struct atmel_smc_cs_conf *conf, u32 clk_period_ns)
+{
+	u32 twb = atmel_smc_decode_ncycles(conf->timings,
+					   ATMEL_HSMC_TIMINGS_TWB_SHIFT,
+					   3, 1, 64);
+	u32 trr = atmel_smc_decode_ncycles(conf->timings,
+					   ATMEL_HSMC_TIMINGS_TRR_SHIFT,
+					   3, 1, 64);
+	u32 tar = atmel_smc_decode_ncycles(conf->timings,
+					   ATMEL_HSMC_TIMINGS_TAR_SHIFT,
+					   3, 1, 64);
+	u32 tadl = atmel_smc_decode_ncycles(conf->timings,
+					    ATMEL_HSMC_TIMINGS_TADL_SHIFT,
+					    3, 1, 64);
+	u32 tclr = atmel_smc_decode_ncycles(conf->timings,
+					    ATMEL_HSMC_TIMINGS_TCLR_SHIFT,
+					    3, 1, 64);
+
+	printf("NFSEL (NAND Flash Selection) is %s\n",
+	       conf->timings & ATMEL_HSMC_TIMINGS_NFSEL ? "set" : "cleared");
+	printf("OCMS (Off Chip Memory Scrambling) is %s\n",
+	       conf->timings & ATMEL_HSMC_TIMINGS_OCMS ? "enabled" : "disabled");
+
+	printf("TWB (WEN High to REN to Busy): %u (%u ns)\n",
+	       twb, twb * clk_period_ns);
+	printf("TRR (Ready to REN Low Delay):  %u (%u ns)\n",
+	       trr, trr * clk_period_ns);
+	printf("TAR (ALE to REN Low Delay):    %u (%u ns)\n",
+	       tar, tar * clk_period_ns);
+	printf("TADL (ALE to Data Start):      %u (%u ns)\n",
+	       tadl, tadl * clk_period_ns);
+	printf("TCLR (CLE to REN Low Delay):   %u (%u ns)\n",
+	       tclr, tclr * clk_period_ns);
+}
+
+static void atmel_smc_print_info(struct atmel_nand *nand, int csline)
+{
+	struct atmel_nand_controller *nc;
+	struct atmel_smc_cs_conf smcconf;
+	struct atmel_nand_cs *cs;
+	u32 mck_period_ns;
+
+	nc = to_nand_controller(nand->controller);
+	cs = &nand->cs[csline];
+
+	atmel_smc_cs_conf_init(&smcconf);
+	atmel_smc_cs_conf_get(nc->smc, cs->id, &smcconf);
+
+	atmel_smc_cs_conf_print_raw(&smcconf, cs->id);
+
+	mck_period_ns = NSEC_PER_SEC / clk_get_rate(nc->mck);
+
+	atmel_smc_print_ncs_rd(&smcconf, mck_period_ns);
+	atmel_smc_print_nrd(&smcconf, mck_period_ns);
+	atmel_smc_print_ncs_wr(&smcconf, mck_period_ns);
+	atmel_smc_print_nwe(&smcconf, mck_period_ns);
+
+	atmel_smc_print_mode(&smcconf, mck_period_ns);
+}
+
+static void atmel_hsmc_print_info(struct atmel_nand *nand, int csline)
+{
+	struct atmel_hsmc_nand_controller *hsmc_nc;
+	struct atmel_nand_controller *nc;
+	struct atmel_smc_cs_conf smcconf;
+	struct atmel_nand_cs *cs;
+	u32 mck_period_ns;
+
+	nc = to_nand_controller(nand->controller);
+	hsmc_nc = to_hsmc_nand_controller(nand->controller);
+	cs = &nand->cs[csline];
+
+	atmel_smc_cs_conf_init(&smcconf);
+	atmel_hsmc_cs_conf_get(nc->smc, hsmc_nc->hsmc_layout, cs->id, &smcconf);
+
+	atmel_hsmc_cs_conf_print_raw(&smcconf, cs->id);
+
+	mck_period_ns = NSEC_PER_SEC / clk_get_rate(nc->mck);
+
+	atmel_smc_print_ncs_rd(&smcconf, mck_period_ns);
+	atmel_smc_print_nrd(&smcconf, mck_period_ns);
+	atmel_smc_print_ncs_wr(&smcconf, mck_period_ns);
+	atmel_smc_print_nwe(&smcconf, mck_period_ns);
+
+	atmel_hsmc_print_mode(&smcconf, mck_period_ns);
+	atmel_hsmc_print_timings(&smcconf, mck_period_ns);
+}
+#endif
+
 static const struct atmel_nand_controller_ops atmel_hsmc_nc_ops = {
 	.probe = atmel_hsmc_nand_controller_probe,
 	.remove = atmel_hsmc_nand_controller_remove,
 	.ecc_init = atmel_hsmc_nand_ecc_init,
 	.nand_init = atmel_hsmc_nand_init,
 	.setup_data_interface = atmel_hsmc_nand_setup_data_interface,
+#ifdef CONFIG_CMD_NAND_ATMEL_DEBUG
+	.print_info = atmel_hsmc_print_info,
+#endif
 };
 
 static const struct atmel_nand_controller_caps atmel_sama5_nc_caps = {
@@ -2117,6 +2369,9 @@ static const struct atmel_nand_controller_ops atmel_smc_nc_ops = {
 	.ecc_init = atmel_nand_ecc_init,
 	.nand_init = atmel_smc_nand_init,
 	.setup_data_interface = atmel_smc_nand_setup_data_interface,
+#ifdef CONFIG_CMD_NAND_ATMEL_DEBUG
+	.print_info = atmel_smc_print_info,
+#endif
 };
 
 static const struct atmel_nand_controller_caps atmel_sam9260_nc_caps = {
@@ -2247,3 +2502,43 @@ void board_nand_init(void)
 		printf("Failed to initialize NAND controller. (error %d)\n",
 		       ret);
 }
+
+#ifdef CONFIG_CMD_NAND_ATMEL_DEBUG
+static int do_hsmc_decode(struct cmd_tbl *cmdtp, int flag,
+			  int argc, char * const argv[])
+{
+	struct atmel_nand_controller *nc;
+	struct atmel_nand *nand;
+	struct nand_chip *chip;
+	struct mtd_info *mtd;
+	int i, j;
+
+	for (i = 0; i < CONFIG_SYS_MAX_NAND_DEVICE; i++) {
+		putc('\n');
+		mtd = get_nand_dev_by_index(i);
+		if (!mtd)
+			continue;
+
+		chip = mtd_to_nand(mtd);
+		nand = to_atmel_nand(chip);
+		nc = to_nand_controller(nand->controller);
+		printf("MCK rate: %lu MHz\n", clk_get_rate(nc->mck) / 1000000);
+		if (!nc->caps->ops->print_info)
+			continue;
+
+		for (j = 0; j < nand->numcs; j++) {
+			putc('\n');
+			nc->caps->ops->print_info(nand, j);
+		}
+	}
+
+	return CMD_RET_SUCCESS;
+}
+
+static char hsmc_help_text[] =
+	"decode - Decode SMC registers\n"
+	;
+
+U_BOOT_CMD_WITH_SUBCMDS(hsmc, "Atmel Static Memory Controller (SMC) debugging", hsmc_help_text,
+			U_BOOT_SUBCMD_MKENT(decode, 1, 1, do_hsmc_decode));
+#endif
-- 
2.39.2

[PATCH v3 2/3] cmd: nand: Add new optional sub-command 'onfi'

[Alexander Dahl]

Override the ONFI timing mode at runtime.

Signed-off-by: Alexander Dahl <ada@thorsis.com>
---

Notes:
    v3:
    - no changes to this patch
    
    v2:
    - initial patch version (not present in v1)

 cmd/Kconfig                      | 10 ++++++
 cmd/nand.c                       | 61 ++++++++++++++++++++++++++++++++
 drivers/mtd/nand/raw/nand_base.c |  2 +-
 include/linux/mtd/rawnand.h      |  1 +
 4 files changed, 73 insertions(+), 1 deletion(-)

diff --git a/cmd/Kconfig b/cmd/Kconfig
index 8eeb99eea5e..5dc47bd3f51 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -1382,6 +1382,7 @@ config CMD_NAND
 	  NAND support.
 
 if CMD_NAND
+
 config CMD_NAND_TRIMFFS
 	bool "nand write.trimffs"
 	default y if ARCH_SUNXI
@@ -1398,6 +1399,15 @@ config CMD_NAND_TORTURE
 	help
 	  NAND torture support.
 
+config CMD_NAND_ONFI
+	bool "nand onfi"
+	help
+	  Set ONFI timing modes explicitly.
+	  This is a debugging command to switch to slower ONFI timing
+	  modes for testing.
+	  In normal operation determining the timing mode automatically
+	  should work fine, and you don't need this.
+
 endif # CMD_NAND
 
 config CMD_NVME
diff --git a/cmd/nand.c b/cmd/nand.c
index fe834c4ac5c..2b83a5ad1b8 100644
--- a/cmd/nand.c
+++ b/cmd/nand.c
@@ -494,6 +494,48 @@ static void adjust_size_for_badblocks(loff_t *size, loff_t offset, int dev)
 	}
 }
 
+#ifdef CONFIG_CMD_NAND_ONFI
+static int do_nand_onfi(struct mtd_info *mtd, int mode)
+{
+	struct nand_chip *chip;
+	int ret;
+	int i;
+
+	if (mtd->type != MTD_NANDFLASH) {
+		printf("MTD device is no NAND flash!\n");
+		return CMD_RET_FAILURE;
+	}
+
+	chip = mtd_to_nand(mtd);
+
+	if (mode < 0) {
+		printf("Reporting current ONFI settings not yet supported!\n");
+		return CMD_RET_FAILURE;
+	}
+
+	ret = onfi_init_data_interface(chip, chip->data_interface,
+				       NAND_SDR_IFACE, mode);
+	if (ret) {
+		printf("onfi_init_data_interface() for mode %d failed with error %d\n",
+		       mode, ret);
+		return CMD_RET_FAILURE;
+	}
+
+	for (i = 0; i < chip->numchips; i++) {
+		chip->select_chip(mtd, i);
+		ret = nand_setup_data_interface(chip, i);
+		chip->select_chip(mtd, -1);
+		if (ret) {
+			printf("nand_setup_data_interface() for mode %d failed with error %d\n",
+			       mode, ret);
+			return CMD_RET_FAILURE;
+		}
+	}
+
+	return CMD_RET_SUCCESS;
+}
+#endif
+
 static int do_nand(struct cmd_tbl *cmdtp, int flag, int argc,
 		   char *const argv[])
 {
@@ -919,6 +961,21 @@ static int do_nand(struct cmd_tbl *cmdtp, int flag, int argc,
 	}
 #endif
 
+#ifdef CONFIG_CMD_NAND_ONFI
+	/*
+	 * Syntax is:
+	 *   0    1     2
+	 *   nand onfi [mode]
+	 */
+	if (strcmp(cmd, "onfi") == 0) {
+		int mode = -1;
+
+		if (argc > 2)
+			mode = dectoul(argv[2], NULL);
+		return do_nand_onfi(mtd, mode);
+	}
+#endif
+
 usage:
 	return CMD_RET_USAGE;
 }
@@ -961,6 +1018,10 @@ U_BOOT_LONGHELP(nand,
 	"    bring nand to lock state or display locked pages\n"
 	"nand unlock[.allexcept] [offset] [size] - unlock section"
 #endif
+#ifdef CONFIG_CMD_NAND_ONFI
+	"\n"
+	"nand onfi [mode] - set ONFI mode\n"
+#endif
 #ifdef CONFIG_ENV_OFFSET_OOB
 	"\n"
 	"nand env.oob - environment offset in OOB of block 0 of"
diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index 688d17ba3c2..2384425a746 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -983,7 +983,7 @@ static int nand_onfi_set_timings(struct mtd_info *mtd, struct nand_chip *chip)
  *
  * Returns 0 for success or negative error code otherwise.
  */
-static int nand_setup_data_interface(struct nand_chip *chip, int chipnr)
+int nand_setup_data_interface(struct nand_chip *chip, int chipnr)
 {
 	struct mtd_info *mtd = nand_to_mtd(chip);
 	int ret;
diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
index 4abaf4734cf..07bc4cc9051 100644
--- a/include/linux/mtd/rawnand.h
+++ b/include/linux/mtd/rawnand.h
@@ -1315,6 +1315,7 @@ void nand_write_buf16(struct mtd_info *mtd, const uint8_t *buf, int len);
 void nand_read_buf(struct mtd_info *mtd, uint8_t *buf, int len);
 void nand_read_buf16(struct mtd_info *mtd, uint8_t *buf, int len);
 uint8_t nand_read_byte(struct mtd_info *mtd);
+int nand_setup_data_interface(struct nand_chip *chip, int chipnr);
 
 /* get timing characteristics from ONFI timing mode. */
 const struct nand_sdr_timings *onfi_async_timing_mode_to_sdr_timings(int mode);
-- 
2.39.2

[PATCH v3 3/3] mtd: nand: raw: atmel: Fix pulse read timing for certain NAND flashes

[Alexander Dahl]

From reading the S34ML02G1 and the SAM9X60 datasheets again, it seems
like we have to wait tREA after rising RE# before sampling the data.
Thus pulse time must be at least tREA.

Without this fix we got PMECC errors when reading, after switching to
ONFI timing mode 3 on SAM9X60 SoC with S34ML02G1 raw NAND flash chip.

The approach to set timings used before worked on sam9g20 and sama5d2
with the same flash (S34ML02G1), probably because those have a slower
mck clock rate and thus the resolution of the timings setup is not as
tight as with sam9x60.

The approach to fix the issue was carried over from at91bootstrap, and
has been successfully tested in at91bootstrap, U-Boot and Linux.

Link: https://github.com/linux4sam/at91bootstrap/issues/174
Cc: Li Bin <bin.li@microchip.com>
Signed-off-by: Alexander Dahl <ada@thorsis.com>
---

Notes:
    v3:
    - initial patch version (not present in v1 and v2)

 drivers/mtd/nand/raw/atmel/nand-controller.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c b/drivers/mtd/nand/raw/atmel/nand-controller.c
index bbafc88e44c..00ffeadd113 100644
--- a/drivers/mtd/nand/raw/atmel/nand-controller.c
+++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
@@ -1133,7 +1133,7 @@ static int atmel_smc_nand_prepare_smcconf(struct atmel_nand *nand,
 					  const struct nand_data_interface *conf,
 					  struct atmel_smc_cs_conf *smcconf)
 {
-	u32 ncycles, totalcycles, timeps, mckperiodps;
+	u32 ncycles, totalcycles, timeps, mckperiodps, pulse;
 	struct atmel_nand_controller *nc;
 	int ret;
 
@@ -1259,11 +1259,16 @@ static int atmel_smc_nand_prepare_smcconf(struct atmel_nand *nand,
 			 ATMEL_SMC_MODE_TDFMODE_OPTIMIZED;
 
 	/*
-	 * Read pulse timing directly matches tRP:
+	 * Read pulse timing would directly match tRP,
+	 * but some NAND flash chips (S34ML01G2 and W29N02KVxxAF)
+	 * do not work properly in timing mode 3.
+	 * The workaround is to extend the SMC NRD pulse to meet tREA
+	 * timing.
 	 *
-	 * NRD_PULSE = tRP
+	 * NRD_PULSE = max(tRP, tREA)
 	 */
-	ncycles = DIV_ROUND_UP(conf->timings.sdr.tRP_min, mckperiodps);
+	pulse = max(conf->timings.sdr.tRP_min, conf->timings.sdr.tREA_max);
+	ncycles = DIV_ROUND_UP(pulse, mckperiodps);
 	totalcycles += ncycles;
 	ret = atmel_smc_cs_conf_set_pulse(smcconf, ATMEL_SMC_NRD_SHIFT,
 					  ncycles);
-- 
2.39.2

Re: [PATCH v3 3/3] mtd: nand: raw: atmel: Fix pulse read timing for certain NAND flashes

[Alexander Dahl]

Hei hei,

Am Mon, Apr 15, 2024 at 09:57:55AM +0200 schrieb Alexander Dahl:
> From reading the S34ML02G1 and the SAM9X60 datasheets again, it seems
> like we have to wait tREA after rising RE# before sampling the data.
> Thus pulse time must be at least tREA.
> 
> Without this fix we got PMECC errors when reading, after switching to
> ONFI timing mode 3 on SAM9X60 SoC with S34ML02G1 raw NAND flash chip.
> 
> The approach to set timings used before worked on sam9g20 and sama5d2
> with the same flash (S34ML02G1), probably because those have a slower
> mck clock rate and thus the resolution of the timings setup is not as
> tight as with sam9x60.
> 
> The approach to fix the issue was carried over from at91bootstrap, and
> has been successfully tested in at91bootstrap, U-Boot and Linux.
> 
> Link: https://github.com/linux4sam/at91bootstrap/issues/174
> Cc: Li Bin <bin.li@microchip.com>
> Signed-off-by: Alexander Dahl <ada@thorsis.com>
> ---
> 
> Notes:
>     v3:
>     - initial patch version (not present in v1 and v2)

This patch was send as part of a series, which you wanted to have some
more time to test.  Besides that, has anyone looked into this
particular fix?  Maybe it can be applied separately?

Greets
Alex

> 
>  drivers/mtd/nand/raw/atmel/nand-controller.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c b/drivers/mtd/nand/raw/atmel/nand-controller.c
> index bbafc88e44c..00ffeadd113 100644
> --- a/drivers/mtd/nand/raw/atmel/nand-controller.c
> +++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
> @@ -1133,7 +1133,7 @@ static int atmel_smc_nand_prepare_smcconf(struct atmel_nand *nand,
>  					  const struct nand_data_interface *conf,
>  					  struct atmel_smc_cs_conf *smcconf)
>  {
> -	u32 ncycles, totalcycles, timeps, mckperiodps;
> +	u32 ncycles, totalcycles, timeps, mckperiodps, pulse;
>  	struct atmel_nand_controller *nc;
>  	int ret;
>  
> @@ -1259,11 +1259,16 @@ static int atmel_smc_nand_prepare_smcconf(struct atmel_nand *nand,
>  			 ATMEL_SMC_MODE_TDFMODE_OPTIMIZED;
>  
>  	/*
> -	 * Read pulse timing directly matches tRP:
> +	 * Read pulse timing would directly match tRP,
> +	 * but some NAND flash chips (S34ML01G2 and W29N02KVxxAF)
> +	 * do not work properly in timing mode 3.
> +	 * The workaround is to extend the SMC NRD pulse to meet tREA
> +	 * timing.
>  	 *
> -	 * NRD_PULSE = tRP
> +	 * NRD_PULSE = max(tRP, tREA)
>  	 */
> -	ncycles = DIV_ROUND_UP(conf->timings.sdr.tRP_min, mckperiodps);
> +	pulse = max(conf->timings.sdr.tRP_min, conf->timings.sdr.tREA_max);
> +	ncycles = DIV_ROUND_UP(pulse, mckperiodps);
>  	totalcycles += ncycles;
>  	ret = atmel_smc_cs_conf_set_pulse(smcconf, ATMEL_SMC_NRD_SHIFT,
>  					  ncycles);
> -- 
> 2.39.2
> 

-- 
Alexander Dahl           Thorsis Technologies GmbH   T +49 391 544 563 1000
Industrieautomation      Oststr. 18                  F +49 391 544 563 9099
T +49 391 544 563 3036   39114 Magdeburg             https://www.thorsis.com/

Sitz der Gesellschaft: Magdeburg
Amtsgericht Stendal HRB 30646
Geschäftsführer: Dipl.-Inf. Michael Huschke

Re: [PATCH v3 3/3] mtd: nand: raw: atmel: Fix pulse read timing for certain NAND flashes

[Alexander Dahl]

Hello,

Am Tue, May 28, 2024 at 12:32:44PM +0200 schrieb Alexander Dahl:
> Hei hei,
> 
> Am Mon, Apr 15, 2024 at 09:57:55AM +0200 schrieb Alexander Dahl:
> > From reading the S34ML02G1 and the SAM9X60 datasheets again, it seems
> > like we have to wait tREA after rising RE# before sampling the data.
> > Thus pulse time must be at least tREA.
> > 
> > Without this fix we got PMECC errors when reading, after switching to
> > ONFI timing mode 3 on SAM9X60 SoC with S34ML02G1 raw NAND flash chip.
> > 
> > The approach to set timings used before worked on sam9g20 and sama5d2
> > with the same flash (S34ML02G1), probably because those have a slower
> > mck clock rate and thus the resolution of the timings setup is not as
> > tight as with sam9x60.
> > 
> > The approach to fix the issue was carried over from at91bootstrap, and
> > has been successfully tested in at91bootstrap, U-Boot and Linux.
> > 
> > Link: https://github.com/linux4sam/at91bootstrap/issues/174
> > Cc: Li Bin <bin.li@microchip.com>
> > Signed-off-by: Alexander Dahl <ada@thorsis.com>
> > ---
> > 
> > Notes:
> >     v3:
> >     - initial patch version (not present in v1 and v2)
> 
> This patch was send as part of a series, which you wanted to have some
> more time to test.  Besides that, has anyone looked into this
> particular fix?  Maybe it can be applied separately?

I'd kindly ask what the status of this patch series is from U-Boot
NAND subsystem maintainers POV?  Could you test it?  Should I rebase
and resend?

Two of these three patches are specific to at91 family, what's the
opinion of at91 maintainers on this?

Link to the series discussion for reference:

https://lore.kernel.org/u-boot/20240415074547.779264-1-ada@thorsis.com/T/#u

Greets
Alex

> 
> Greets
> Alex
> 
> > 
> >  drivers/mtd/nand/raw/atmel/nand-controller.c | 13 +++++++++----
> >  1 file changed, 9 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c b/drivers/mtd/nand/raw/atmel/nand-controller.c
> > index bbafc88e44c..00ffeadd113 100644
> > --- a/drivers/mtd/nand/raw/atmel/nand-controller.c
> > +++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
> > @@ -1133,7 +1133,7 @@ static int atmel_smc_nand_prepare_smcconf(struct atmel_nand *nand,
> >  					  const struct nand_data_interface *conf,
> >  					  struct atmel_smc_cs_conf *smcconf)
> >  {
> > -	u32 ncycles, totalcycles, timeps, mckperiodps;
> > +	u32 ncycles, totalcycles, timeps, mckperiodps, pulse;
> >  	struct atmel_nand_controller *nc;
> >  	int ret;
> >  
> > @@ -1259,11 +1259,16 @@ static int atmel_smc_nand_prepare_smcconf(struct atmel_nand *nand,
> >  			 ATMEL_SMC_MODE_TDFMODE_OPTIMIZED;
> >  
> >  	/*
> > -	 * Read pulse timing directly matches tRP:
> > +	 * Read pulse timing would directly match tRP,
> > +	 * but some NAND flash chips (S34ML01G2 and W29N02KVxxAF)
> > +	 * do not work properly in timing mode 3.
> > +	 * The workaround is to extend the SMC NRD pulse to meet tREA
> > +	 * timing.
> >  	 *
> > -	 * NRD_PULSE = tRP
> > +	 * NRD_PULSE = max(tRP, tREA)
> >  	 */
> > -	ncycles = DIV_ROUND_UP(conf->timings.sdr.tRP_min, mckperiodps);
> > +	pulse = max(conf->timings.sdr.tRP_min, conf->timings.sdr.tREA_max);
> > +	ncycles = DIV_ROUND_UP(pulse, mckperiodps);
> >  	totalcycles += ncycles;
> >  	ret = atmel_smc_cs_conf_set_pulse(smcconf, ATMEL_SMC_NRD_SHIFT,
> >  					  ncycles);
> > -- 
> > 2.39.2
> > 
> 

Re: [PATCH v3 3/3] mtd: nand: raw: atmel: Fix pulse read timing for certain NAND flashes

[Eugen Hristev]

On 9/30/24 11:07, Alexander Dahl wrote:
> Hello,
> 
> Am Tue, May 28, 2024 at 12:32:44PM +0200 schrieb Alexander Dahl:
>> Hei hei,
>>
>> Am Mon, Apr 15, 2024 at 09:57:55AM +0200 schrieb Alexander Dahl:
>>> From reading the S34ML02G1 and the SAM9X60 datasheets again, it seems
>>> like we have to wait tREA after rising RE# before sampling the data.
>>> Thus pulse time must be at least tREA.
>>>
>>> Without this fix we got PMECC errors when reading, after switching to
>>> ONFI timing mode 3 on SAM9X60 SoC with S34ML02G1 raw NAND flash chip.
>>>
>>> The approach to set timings used before worked on sam9g20 and sama5d2
>>> with the same flash (S34ML02G1), probably because those have a slower
>>> mck clock rate and thus the resolution of the timings setup is not as
>>> tight as with sam9x60.
>>>
>>> The approach to fix the issue was carried over from at91bootstrap, and
>>> has been successfully tested in at91bootstrap, U-Boot and Linux.
>>>
>>> Link: https://github.com/linux4sam/at91bootstrap/issues/174
>>> Cc: Li Bin <bin.li@microchip.com>
>>> Signed-off-by: Alexander Dahl <ada@thorsis.com>
>>> ---
>>>
>>> Notes:
>>>     v3:
>>>     - initial patch version (not present in v1 and v2)
>>
>> This patch was send as part of a series, which you wanted to have some
>> more time to test.  Besides that, has anyone looked into this
>> particular fix?  Maybe it can be applied separately?
> 
> I'd kindly ask what the status of this patch series is from U-Boot
> NAND subsystem maintainers POV?  Could you test it?  Should I rebase
> and resend?
> 
> Two of these three patches are specific to at91 family, what's the
> opinion of at91 maintainers on this?

Hello Alexander,

From at91 perspective I have no objections.
As these are very NAND specific, I defer the patches to the NAND maintainers.

Eugen

> 
> Link to the series discussion for reference:
> 
> https://lore.kernel.org/u-boot/20240415074547.779264-1-ada@thorsis.com/T/#u
> 
> Greets
> Alex
> 
>>
>> Greets
>> Alex
>>
>>>
>>>  drivers/mtd/nand/raw/atmel/nand-controller.c | 13 +++++++++----
>>>  1 file changed, 9 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c b/drivers/mtd/nand/raw/atmel/nand-controller.c
>>> index bbafc88e44c..00ffeadd113 100644
>>> --- a/drivers/mtd/nand/raw/atmel/nand-controller.c
>>> +++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
>>> @@ -1133,7 +1133,7 @@ static int atmel_smc_nand_prepare_smcconf(struct atmel_nand *nand,
>>>  					  const struct nand_data_interface *conf,
>>>  					  struct atmel_smc_cs_conf *smcconf)
>>>  {
>>> -	u32 ncycles, totalcycles, timeps, mckperiodps;
>>> +	u32 ncycles, totalcycles, timeps, mckperiodps, pulse;
>>>  	struct atmel_nand_controller *nc;
>>>  	int ret;
>>>  
>>> @@ -1259,11 +1259,16 @@ static int atmel_smc_nand_prepare_smcconf(struct atmel_nand *nand,
>>>  			 ATMEL_SMC_MODE_TDFMODE_OPTIMIZED;
>>>  
>>>  	/*
>>> -	 * Read pulse timing directly matches tRP:
>>> +	 * Read pulse timing would directly match tRP,
>>> +	 * but some NAND flash chips (S34ML01G2 and W29N02KVxxAF)
>>> +	 * do not work properly in timing mode 3.
>>> +	 * The workaround is to extend the SMC NRD pulse to meet tREA
>>> +	 * timing.
>>>  	 *
>>> -	 * NRD_PULSE = tRP
>>> +	 * NRD_PULSE = max(tRP, tREA)
>>>  	 */
>>> -	ncycles = DIV_ROUND_UP(conf->timings.sdr.tRP_min, mckperiodps);
>>> +	pulse = max(conf->timings.sdr.tRP_min, conf->timings.sdr.tREA_max);
>>> +	ncycles = DIV_ROUND_UP(pulse, mckperiodps);
>>>  	totalcycles += ncycles;
>>>  	ret = atmel_smc_cs_conf_set_pulse(smcconf, ATMEL_SMC_NRD_SHIFT,
>>>  					  ncycles);
>>> -- 
>>> 2.39.2
>>>
>>

Re: [PATCH v3 3/3] mtd: nand: raw: atmel: Fix pulse read timing for certain NAND flashes

[Michael Nazzareno Trimarchi]

Hi

On Tue, Oct 8, 2024 at 1:41 PM Eugen Hristev
<eugen.hristev@collabora.com> wrote:
>
> On 9/30/24 11:07, Alexander Dahl wrote:
> > Hello,
> >
> > Am Tue, May 28, 2024 at 12:32:44PM +0200 schrieb Alexander Dahl:
> >> Hei hei,
> >>
> >> Am Mon, Apr 15, 2024 at 09:57:55AM +0200 schrieb Alexander Dahl:
> >>> From reading the S34ML02G1 and the SAM9X60 datasheets again, it seems
> >>> like we have to wait tREA after rising RE# before sampling the data.
> >>> Thus pulse time must be at least tREA.
> >>>
> >>> Without this fix we got PMECC errors when reading, after switching to
> >>> ONFI timing mode 3 on SAM9X60 SoC with S34ML02G1 raw NAND flash chip.
> >>>
> >>> The approach to set timings used before worked on sam9g20 and sama5d2
> >>> with the same flash (S34ML02G1), probably because those have a slower
> >>> mck clock rate and thus the resolution of the timings setup is not as
> >>> tight as with sam9x60.
> >>>
> >>> The approach to fix the issue was carried over from at91bootstrap, and
> >>> has been successfully tested in at91bootstrap, U-Boot and Linux.
> >>>
> >>> Link: https://github.com/linux4sam/at91bootstrap/issues/174
> >>> Cc: Li Bin <bin.li@microchip.com>
> >>> Signed-off-by: Alexander Dahl <ada@thorsis.com>
> >>> ---
> >>>
> >>> Notes:
> >>>     v3:
> >>>     - initial patch version (not present in v1 and v2)
> >>
> >> This patch was send as part of a series, which you wanted to have some
> >> more time to test.  Besides that, has anyone looked into this
> >> particular fix?  Maybe it can be applied separately?
> >
> > I'd kindly ask what the status of this patch series is from U-Boot
> > NAND subsystem maintainers POV?  Could you test it?  Should I rebase
> > and resend?
> >
> > Two of these three patches are specific to at91 family, what's the
> > opinion of at91 maintainers on this?
>
> Hello Alexander,
>
> From at91 perspective I have no objections.
> As these are very NAND specific, I defer the patches to the NAND maintainers.
>
> Eugen
>
> >
> > Link to the series discussion for reference:
> >
> > https://lore.kernel.org/u-boot/20240415074547.779264-1-ada@thorsis.com/T/#u
> >
> > Greets
> > Alex
> >

I will review this series

Michael

> >>
> >> Greets
> >> Alex
> >>
> >>>
> >>>  drivers/mtd/nand/raw/atmel/nand-controller.c | 13 +++++++++----
> >>>  1 file changed, 9 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c b/drivers/mtd/nand/raw/atmel/nand-controller.c
> >>> index bbafc88e44c..00ffeadd113 100644
> >>> --- a/drivers/mtd/nand/raw/atmel/nand-controller.c
> >>> +++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
> >>> @@ -1133,7 +1133,7 @@ static int atmel_smc_nand_prepare_smcconf(struct atmel_nand *nand,
> >>>                                       const struct nand_data_interface *conf,
> >>>                                       struct atmel_smc_cs_conf *smcconf)
> >>>  {
> >>> -   u32 ncycles, totalcycles, timeps, mckperiodps;
> >>> +   u32 ncycles, totalcycles, timeps, mckperiodps, pulse;
> >>>     struct atmel_nand_controller *nc;
> >>>     int ret;
> >>>
> >>> @@ -1259,11 +1259,16 @@ static int atmel_smc_nand_prepare_smcconf(struct atmel_nand *nand,
> >>>                      ATMEL_SMC_MODE_TDFMODE_OPTIMIZED;
> >>>
> >>>     /*
> >>> -    * Read pulse timing directly matches tRP:
> >>> +    * Read pulse timing would directly match tRP,
> >>> +    * but some NAND flash chips (S34ML01G2 and W29N02KVxxAF)
> >>> +    * do not work properly in timing mode 3.
> >>> +    * The workaround is to extend the SMC NRD pulse to meet tREA
> >>> +    * timing.
> >>>      *
> >>> -    * NRD_PULSE = tRP
> >>> +    * NRD_PULSE = max(tRP, tREA)
> >>>      */
> >>> -   ncycles = DIV_ROUND_UP(conf->timings.sdr.tRP_min, mckperiodps);
> >>> +   pulse = max(conf->timings.sdr.tRP_min, conf->timings.sdr.tREA_max);
> >>> +   ncycles = DIV_ROUND_UP(pulse, mckperiodps);
> >>>     totalcycles += ncycles;
> >>>     ret = atmel_smc_cs_conf_set_pulse(smcconf, ATMEL_SMC_NRD_SHIFT,
> >>>                                       ncycles);
> >>> --
> >>> 2.39.2
> >>>
> >>
>


-- 
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
michael@amarulasolutions.com
__________________________________

Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
info@amarulasolutions.com
www.amarulasolutions.com

Re: [PATCH v3 3/3] mtd: nand: raw: atmel: Fix pulse read timing for certain NAND flashes

[Balamanikandan.Gunasundar]

On 30/09/24 1:37 pm, Alexander Dahl wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Hello,
> 
> Am Tue, May 28, 2024 at 12:32:44PM +0200 schrieb Alexander Dahl:
>> Hei hei,
>>
>> Am Mon, Apr 15, 2024 at 09:57:55AM +0200 schrieb Alexander Dahl:
>>>  From reading the S34ML02G1 and the SAM9X60 datasheets again, it seems
>>> like we have to wait tREA after rising RE# before sampling the data.
>>> Thus pulse time must be at least tREA.
>>>
>>> Without this fix we got PMECC errors when reading, after switching to
>>> ONFI timing mode 3 on SAM9X60 SoC with S34ML02G1 raw NAND flash chip.
>>>
>>> The approach to set timings used before worked on sam9g20 and sama5d2
>>> with the same flash (S34ML02G1), probably because those have a slower
>>> mck clock rate and thus the resolution of the timings setup is not as
>>> tight as with sam9x60.
>>>
>>> The approach to fix the issue was carried over from at91bootstrap, and
>>> has been successfully tested in at91bootstrap, U-Boot and Linux.
>>>
>>> Link: https://github.com/linux4sam/at91bootstrap/issues/174
>>> Cc: Li Bin <bin.li@microchip.com>
>>> Signed-off-by: Alexander Dahl <ada@thorsis.com>
>>> ---
>>>
>>> Notes:
>>>      v3:
>>>      - initial patch version (not present in v1 and v2)
>>
>> This patch was send as part of a series, which you wanted to have some
>> more time to test.  Besides that, has anyone looked into this
>> particular fix?  Maybe it can be applied separately?
> 
> I'd kindly ask what the status of this patch series is from U-Boot
> NAND subsystem maintainers POV?  Could you test it?  Should I rebase
> and resend?
> 
> Two of these three patches are specific to at91 family, what's the
> opinion of at91 maintainers on this?
> 
> Link to the series discussion for reference:
> 
> https://lore.kernel.org/u-boot/20240415074547.779264-1-ada@thorsis.com/T/#u
> 
> Greets
> Alex
> 

Hi Alex,

Apologies for the delay in response. I also faced the same kind of 
problem while testing our new sam7d65 board with MX30LF4G28AD nand 
flash. I just had a workaround to increase the pulse time by 5 nsecs 
just for testing. The issue has been reported to the validation team and 
an investigation is under progress.

I would request a few more days for this patch alone.

Thanks,
Bala.

>>
>> Greets
>> Alex
>>
>>>
>>>   drivers/mtd/nand/raw/atmel/nand-controller.c | 13 +++++++++----
>>>   1 file changed, 9 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c b/drivers/mtd/nand/raw/atmel/nand-controller.c
>>> index bbafc88e44c..00ffeadd113 100644
>>> --- a/drivers/mtd/nand/raw/atmel/nand-controller.c
>>> +++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
>>> @@ -1133,7 +1133,7 @@ static int atmel_smc_nand_prepare_smcconf(struct atmel_nand *nand,
>>>                                        const struct nand_data_interface *conf,
>>>                                        struct atmel_smc_cs_conf *smcconf)
>>>   {
>>> -   u32 ncycles, totalcycles, timeps, mckperiodps;
>>> +   u32 ncycles, totalcycles, timeps, mckperiodps, pulse;
>>>      struct atmel_nand_controller *nc;
>>>      int ret;
>>>
>>> @@ -1259,11 +1259,16 @@ static int atmel_smc_nand_prepare_smcconf(struct atmel_nand *nand,
>>>                       ATMEL_SMC_MODE_TDFMODE_OPTIMIZED;
>>>
>>>      /*
>>> -    * Read pulse timing directly matches tRP:
>>> +    * Read pulse timing would directly match tRP,
>>> +    * but some NAND flash chips (S34ML01G2 and W29N02KVxxAF)
>>> +    * do not work properly in timing mode 3.
>>> +    * The workaround is to extend the SMC NRD pulse to meet tREA
>>> +    * timing.
>>>       *
>>> -    * NRD_PULSE = tRP
>>> +    * NRD_PULSE = max(tRP, tREA)
>>>       */
>>> -   ncycles = DIV_ROUND_UP(conf->timings.sdr.tRP_min, mckperiodps);
>>> +   pulse = max(conf->timings.sdr.tRP_min, conf->timings.sdr.tREA_max);
>>> +   ncycles = DIV_ROUND_UP(pulse, mckperiodps);
>>>      totalcycles += ncycles;
>>>      ret = atmel_smc_cs_conf_set_pulse(smcconf, ATMEL_SMC_NRD_SHIFT,
>>>                                        ncycles);
>>> --
>>> 2.39.2
>>>
>>

Re: [PATCH v3 3/3] mtd: nand: raw: atmel: Fix pulse read timing for certain NAND flashes

[Alexander Dahl]

Hello Bala,

Am Wed, Oct 09, 2024 at 04:45:02AM +0000 schrieb Balamanikandan.Gunasundar@microchip.com:
> On 30/09/24 1:37 pm, Alexander Dahl wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> > 
> > Hello,
> > 
> > Am Tue, May 28, 2024 at 12:32:44PM +0200 schrieb Alexander Dahl:
> >> Hei hei,
> >>
> >> Am Mon, Apr 15, 2024 at 09:57:55AM +0200 schrieb Alexander Dahl:
> >>>  From reading the S34ML02G1 and the SAM9X60 datasheets again, it seems
> >>> like we have to wait tREA after rising RE# before sampling the data.
> >>> Thus pulse time must be at least tREA.
> >>>
> >>> Without this fix we got PMECC errors when reading, after switching to
> >>> ONFI timing mode 3 on SAM9X60 SoC with S34ML02G1 raw NAND flash chip.
> >>>
> >>> The approach to set timings used before worked on sam9g20 and sama5d2
> >>> with the same flash (S34ML02G1), probably because those have a slower
> >>> mck clock rate and thus the resolution of the timings setup is not as
> >>> tight as with sam9x60.
> >>>
> >>> The approach to fix the issue was carried over from at91bootstrap, and
> >>> has been successfully tested in at91bootstrap, U-Boot and Linux.
> >>>
> >>> Link: https://github.com/linux4sam/at91bootstrap/issues/174
> >>> Cc: Li Bin <bin.li@microchip.com>
> >>> Signed-off-by: Alexander Dahl <ada@thorsis.com>
> >>> ---
> >>>
> >>> Notes:
> >>>      v3:
> >>>      - initial patch version (not present in v1 and v2)
> >>
> >> This patch was send as part of a series, which you wanted to have some
> >> more time to test.  Besides that, has anyone looked into this
> >> particular fix?  Maybe it can be applied separately?
> > 
> > I'd kindly ask what the status of this patch series is from U-Boot
> > NAND subsystem maintainers POV?  Could you test it?  Should I rebase
> > and resend?
> > 
> > Two of these three patches are specific to at91 family, what's the
> > opinion of at91 maintainers on this?
> > 
> > Link to the series discussion for reference:
> > 
> > https://lore.kernel.org/u-boot/20240415074547.779264-1-ada@thorsis.com/T/#u
> > 
> > Greets
> > Alex
> > 
> 
> Hi Alex,
> 
> Apologies for the delay in response. I also faced the same kind of 
> problem while testing our new sam7d65 board with MX30LF4G28AD nand 
> flash. I just had a workaround to increase the pulse time by 5 nsecs 
> just for testing. The issue has been reported to the validation team and 
> an investigation is under progress.
> 
> I would request a few more days for this patch alone.

Did the validation team come to any conclusion on this?

Greets
Alex

> 
> Thanks,
> Bala.
> 
> >>
> >> Greets
> >> Alex
> >>
> >>>
> >>>   drivers/mtd/nand/raw/atmel/nand-controller.c | 13 +++++++++----
> >>>   1 file changed, 9 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c b/drivers/mtd/nand/raw/atmel/nand-controller.c
> >>> index bbafc88e44c..00ffeadd113 100644
> >>> --- a/drivers/mtd/nand/raw/atmel/nand-controller.c
> >>> +++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
> >>> @@ -1133,7 +1133,7 @@ static int atmel_smc_nand_prepare_smcconf(struct atmel_nand *nand,
> >>>                                        const struct nand_data_interface *conf,
> >>>                                        struct atmel_smc_cs_conf *smcconf)
> >>>   {
> >>> -   u32 ncycles, totalcycles, timeps, mckperiodps;
> >>> +   u32 ncycles, totalcycles, timeps, mckperiodps, pulse;
> >>>      struct atmel_nand_controller *nc;
> >>>      int ret;
> >>>
> >>> @@ -1259,11 +1259,16 @@ static int atmel_smc_nand_prepare_smcconf(struct atmel_nand *nand,
> >>>                       ATMEL_SMC_MODE_TDFMODE_OPTIMIZED;
> >>>
> >>>      /*
> >>> -    * Read pulse timing directly matches tRP:
> >>> +    * Read pulse timing would directly match tRP,
> >>> +    * but some NAND flash chips (S34ML01G2 and W29N02KVxxAF)
> >>> +    * do not work properly in timing mode 3.
> >>> +    * The workaround is to extend the SMC NRD pulse to meet tREA
> >>> +    * timing.
> >>>       *
> >>> -    * NRD_PULSE = tRP
> >>> +    * NRD_PULSE = max(tRP, tREA)
> >>>       */
> >>> -   ncycles = DIV_ROUND_UP(conf->timings.sdr.tRP_min, mckperiodps);
> >>> +   pulse = max(conf->timings.sdr.tRP_min, conf->timings.sdr.tREA_max);
> >>> +   ncycles = DIV_ROUND_UP(pulse, mckperiodps);
> >>>      totalcycles += ncycles;
> >>>      ret = atmel_smc_cs_conf_set_pulse(smcconf, ATMEL_SMC_NRD_SHIFT,
> >>>                                        ncycles);
> >>> --
> >>> 2.39.2
> >>>
> >>
> 

Re: [PATCH v3 3/3] mtd: nand: raw: atmel: Fix pulse read timing for certain NAND flashes

[Alexander Dahl]

Hello Michael,

Am Tue, Oct 08, 2024 at 02:12:38PM +0200 schrieb Michael Nazzareno Trimarchi:
> Hi
> 
> On Tue, Oct 8, 2024 at 1:41 PM Eugen Hristev
> <eugen.hristev@collabora.com> wrote:
> >
> > On 9/30/24 11:07, Alexander Dahl wrote:
> > > Hello,
> > >
> > > Am Tue, May 28, 2024 at 12:32:44PM +0200 schrieb Alexander Dahl:
> > >> Hei hei,
> > >>
> > >> Am Mon, Apr 15, 2024 at 09:57:55AM +0200 schrieb Alexander Dahl:
> > >>> From reading the S34ML02G1 and the SAM9X60 datasheets again, it seems
> > >>> like we have to wait tREA after rising RE# before sampling the data.
> > >>> Thus pulse time must be at least tREA.
> > >>>
> > >>> Without this fix we got PMECC errors when reading, after switching to
> > >>> ONFI timing mode 3 on SAM9X60 SoC with S34ML02G1 raw NAND flash chip.
> > >>>
> > >>> The approach to set timings used before worked on sam9g20 and sama5d2
> > >>> with the same flash (S34ML02G1), probably because those have a slower
> > >>> mck clock rate and thus the resolution of the timings setup is not as
> > >>> tight as with sam9x60.
> > >>>
> > >>> The approach to fix the issue was carried over from at91bootstrap, and
> > >>> has been successfully tested in at91bootstrap, U-Boot and Linux.
> > >>>
> > >>> Link: https://github.com/linux4sam/at91bootstrap/issues/174
> > >>> Cc: Li Bin <bin.li@microchip.com>
> > >>> Signed-off-by: Alexander Dahl <ada@thorsis.com>
> > >>> ---
> > >>>
> > >>> Notes:
> > >>>     v3:
> > >>>     - initial patch version (not present in v1 and v2)
> > >>
> > >> This patch was send as part of a series, which you wanted to have some
> > >> more time to test.  Besides that, has anyone looked into this
> > >> particular fix?  Maybe it can be applied separately?
> > >
> > > I'd kindly ask what the status of this patch series is from U-Boot
> > > NAND subsystem maintainers POV?  Could you test it?  Should I rebase
> > > and resend?
> > >
> > > Two of these three patches are specific to at91 family, what's the
> > > opinion of at91 maintainers on this?
> >
> > Hello Alexander,
> >
> > From at91 perspective I have no objections.
> > As these are very NAND specific, I defer the patches to the NAND maintainers.
> >
> > Eugen
> >
> > >
> > > Link to the series discussion for reference:
> > >
> > > https://lore.kernel.org/u-boot/20240415074547.779264-1-ada@thorsis.com/T/#u
> > >
> > > Greets
> > > Alex
> > >
> 
> I will review this series

This might have fallen through the cracks?  The series was initially
based on v2024.04, now we are on v2025.01-rc4 already.  Still applies
cleanly to master, though.  I'm especially interested in the timing
fix of patch 3.  Could you please have a look?

Greets
Alex

> 
> Michael
> 
> > >>
> > >> Greets
> > >> Alex
> > >>
> > >>>
> > >>>  drivers/mtd/nand/raw/atmel/nand-controller.c | 13 +++++++++----
> > >>>  1 file changed, 9 insertions(+), 4 deletions(-)
> > >>>
> > >>> diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c b/drivers/mtd/nand/raw/atmel/nand-controller.c
> > >>> index bbafc88e44c..00ffeadd113 100644
> > >>> --- a/drivers/mtd/nand/raw/atmel/nand-controller.c
> > >>> +++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
> > >>> @@ -1133,7 +1133,7 @@ static int atmel_smc_nand_prepare_smcconf(struct atmel_nand *nand,
> > >>>                                       const struct nand_data_interface *conf,
> > >>>                                       struct atmel_smc_cs_conf *smcconf)
> > >>>  {
> > >>> -   u32 ncycles, totalcycles, timeps, mckperiodps;
> > >>> +   u32 ncycles, totalcycles, timeps, mckperiodps, pulse;
> > >>>     struct atmel_nand_controller *nc;
> > >>>     int ret;
> > >>>
> > >>> @@ -1259,11 +1259,16 @@ static int atmel_smc_nand_prepare_smcconf(struct atmel_nand *nand,
> > >>>                      ATMEL_SMC_MODE_TDFMODE_OPTIMIZED;
> > >>>
> > >>>     /*
> > >>> -    * Read pulse timing directly matches tRP:
> > >>> +    * Read pulse timing would directly match tRP,
> > >>> +    * but some NAND flash chips (S34ML01G2 and W29N02KVxxAF)
> > >>> +    * do not work properly in timing mode 3.
> > >>> +    * The workaround is to extend the SMC NRD pulse to meet tREA
> > >>> +    * timing.
> > >>>      *
> > >>> -    * NRD_PULSE = tRP
> > >>> +    * NRD_PULSE = max(tRP, tREA)
> > >>>      */
> > >>> -   ncycles = DIV_ROUND_UP(conf->timings.sdr.tRP_min, mckperiodps);
> > >>> +   pulse = max(conf->timings.sdr.tRP_min, conf->timings.sdr.tREA_max);
> > >>> +   ncycles = DIV_ROUND_UP(pulse, mckperiodps);
> > >>>     totalcycles += ncycles;
> > >>>     ret = atmel_smc_cs_conf_set_pulse(smcconf, ATMEL_SMC_NRD_SHIFT,
> > >>>                                       ncycles);
> > >>> --
> > >>> 2.39.2
> > >>>
> > >>
> >
> 
> 
> -- 
> Michael Nazzareno Trimarchi
> Co-Founder & Chief Executive Officer
> M. +39 347 913 2170
> michael@amarulasolutions.com
> __________________________________
> 
> Amarula Solutions BV
> Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
> T. +31 (0)85 111 9172
> info@amarulasolutions.com
> www.amarulasolutions.com

Re: [PATCH v3 3/3] mtd: nand: raw: atmel: Fix pulse read timing for certain NAND flashes

[Alexander Dahl]

Hello,

Am Wed, Oct 09, 2024 at 04:45:02AM +0000 schrieb Balamanikandan.Gunasundar@microchip.com:
> On 30/09/24 1:37 pm, Alexander Dahl wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> > 
> > Hello,
> > 
> > Am Tue, May 28, 2024 at 12:32:44PM +0200 schrieb Alexander Dahl:
> >> Hei hei,
> >>
> >> Am Mon, Apr 15, 2024 at 09:57:55AM +0200 schrieb Alexander Dahl:
> >>>  From reading the S34ML02G1 and the SAM9X60 datasheets again, it seems
> >>> like we have to wait tREA after rising RE# before sampling the data.
> >>> Thus pulse time must be at least tREA.
> >>>
> >>> Without this fix we got PMECC errors when reading, after switching to
> >>> ONFI timing mode 3 on SAM9X60 SoC with S34ML02G1 raw NAND flash chip.
> >>>
> >>> The approach to set timings used before worked on sam9g20 and sama5d2
> >>> with the same flash (S34ML02G1), probably because those have a slower
> >>> mck clock rate and thus the resolution of the timings setup is not as
> >>> tight as with sam9x60.
> >>>
> >>> The approach to fix the issue was carried over from at91bootstrap, and
> >>> has been successfully tested in at91bootstrap, U-Boot and Linux.
> >>>
> >>> Link: https://github.com/linux4sam/at91bootstrap/issues/174
> >>> Cc: Li Bin <bin.li@microchip.com>
> >>> Signed-off-by: Alexander Dahl <ada@thorsis.com>
> >>> ---
> >>>
> >>> Notes:
> >>>      v3:
> >>>      - initial patch version (not present in v1 and v2)
> >>
> >> This patch was send as part of a series, which you wanted to have some
> >> more time to test.  Besides that, has anyone looked into this
> >> particular fix?  Maybe it can be applied separately?
> > 
> > I'd kindly ask what the status of this patch series is from U-Boot
> > NAND subsystem maintainers POV?  Could you test it?  Should I rebase
> > and resend?
> > 
> > Two of these three patches are specific to at91 family, what's the
> > opinion of at91 maintainers on this?
> > 
> > Link to the series discussion for reference:
> > 
> > https://lore.kernel.org/u-boot/20240415074547.779264-1-ada@thorsis.com/T/#u
> > 
> > Greets
> > Alex
> > 
> 
> Hi Alex,
> 
> Apologies for the delay in response. I also faced the same kind of 
> problem while testing our new sam7d65 board with MX30LF4G28AD nand 
> flash. I just had a workaround to increase the pulse time by 5 nsecs 
> just for testing. The issue has been reported to the validation team and 
> an investigation is under progress.
> 
> I would request a few more days for this patch alone.

Why did the _same_ change I posted here last year in April appeared
in linux4sam now with different credits? o.O

https://github.com/linux4sam/u-boot-at91/commit/4175c4c8535ea886e2c0fd99e94ead6bc7a4df5f

For reference: the issue was discussed in length here before I made
the patch for U-Boot last year:

https://github.com/linux4sam/at91bootstrap/issues/174

Besides: this patch series was still not reviewed and not merged.
Dear U-Boot NAND subsystem maintainer, what's the status?

Greets
Alex

> 
> Thanks,
> Bala.
> 
> >>
> >> Greets
> >> Alex
> >>
> >>>
> >>>   drivers/mtd/nand/raw/atmel/nand-controller.c | 13 +++++++++----
> >>>   1 file changed, 9 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c b/drivers/mtd/nand/raw/atmel/nand-controller.c
> >>> index bbafc88e44c..00ffeadd113 100644
> >>> --- a/drivers/mtd/nand/raw/atmel/nand-controller.c
> >>> +++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
> >>> @@ -1133,7 +1133,7 @@ static int atmel_smc_nand_prepare_smcconf(struct atmel_nand *nand,
> >>>                                        const struct nand_data_interface *conf,
> >>>                                        struct atmel_smc_cs_conf *smcconf)
> >>>   {
> >>> -   u32 ncycles, totalcycles, timeps, mckperiodps;
> >>> +   u32 ncycles, totalcycles, timeps, mckperiodps, pulse;
> >>>      struct atmel_nand_controller *nc;
> >>>      int ret;
> >>>
> >>> @@ -1259,11 +1259,16 @@ static int atmel_smc_nand_prepare_smcconf(struct atmel_nand *nand,
> >>>                       ATMEL_SMC_MODE_TDFMODE_OPTIMIZED;
> >>>
> >>>      /*
> >>> -    * Read pulse timing directly matches tRP:
> >>> +    * Read pulse timing would directly match tRP,
> >>> +    * but some NAND flash chips (S34ML01G2 and W29N02KVxxAF)
> >>> +    * do not work properly in timing mode 3.
> >>> +    * The workaround is to extend the SMC NRD pulse to meet tREA
> >>> +    * timing.
> >>>       *
> >>> -    * NRD_PULSE = tRP
> >>> +    * NRD_PULSE = max(tRP, tREA)
> >>>       */
> >>> -   ncycles = DIV_ROUND_UP(conf->timings.sdr.tRP_min, mckperiodps);
> >>> +   pulse = max(conf->timings.sdr.tRP_min, conf->timings.sdr.tREA_max);
> >>> +   ncycles = DIV_ROUND_UP(pulse, mckperiodps);
> >>>      totalcycles += ncycles;
> >>>      ret = atmel_smc_cs_conf_set_pulse(smcconf, ATMEL_SMC_NRD_SHIFT,
> >>>                                        ncycles);
> >>> --
> >>> 2.39.2
> >>>
> >>
> 

Re: [PATCH v3 3/3] mtd: nand: raw: atmel: Fix pulse read timing for certain NAND flashes

[Eugen Hristev]

On 2/12/25 09:09, Alexander Dahl wrote:
> Hello,
> 
> Am Wed, Oct 09, 2024 at 04:45:02AM +0000 schrieb Balamanikandan.Gunasundar@microchip.com:
>> On 30/09/24 1:37 pm, Alexander Dahl wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>
>>> Hello,
>>>
>>> Am Tue, May 28, 2024 at 12:32:44PM +0200 schrieb Alexander Dahl:
>>>> Hei hei,
>>>>
>>>> Am Mon, Apr 15, 2024 at 09:57:55AM +0200 schrieb Alexander Dahl:
>>>>>  From reading the S34ML02G1 and the SAM9X60 datasheets again, it seems
>>>>> like we have to wait tREA after rising RE# before sampling the data.
>>>>> Thus pulse time must be at least tREA.
>>>>>
>>>>> Without this fix we got PMECC errors when reading, after switching to
>>>>> ONFI timing mode 3 on SAM9X60 SoC with S34ML02G1 raw NAND flash chip.
>>>>>
>>>>> The approach to set timings used before worked on sam9g20 and sama5d2
>>>>> with the same flash (S34ML02G1), probably because those have a slower
>>>>> mck clock rate and thus the resolution of the timings setup is not as
>>>>> tight as with sam9x60.
>>>>>
>>>>> The approach to fix the issue was carried over from at91bootstrap, and
>>>>> has been successfully tested in at91bootstrap, U-Boot and Linux.
>>>>>
>>>>> Link: https://github.com/linux4sam/at91bootstrap/issues/174
>>>>> Cc: Li Bin <bin.li@microchip.com>
>>>>> Signed-off-by: Alexander Dahl <ada@thorsis.com>
>>>>> ---
>>>>>
>>>>> Notes:
>>>>>      v3:
>>>>>      - initial patch version (not present in v1 and v2)
>>>>
>>>> This patch was send as part of a series, which you wanted to have some
>>>> more time to test.  Besides that, has anyone looked into this
>>>> particular fix?  Maybe it can be applied separately?
>>>
>>> I'd kindly ask what the status of this patch series is from U-Boot
>>> NAND subsystem maintainers POV?  Could you test it?  Should I rebase
>>> and resend?
>>>
>>> Two of these three patches are specific to at91 family, what's the
>>> opinion of at91 maintainers on this?
>>>
>>> Link to the series discussion for reference:
>>>
>>> https://lore.kernel.org/u-boot/20240415074547.779264-1-ada@thorsis.com/T/#u
>>>
>>> Greets
>>> Alex
>>>
>>
>> Hi Alex,
>>
>> Apologies for the delay in response. I also faced the same kind of 
>> problem while testing our new sam7d65 board with MX30LF4G28AD nand 
>> flash. I just had a workaround to increase the pulse time by 5 nsecs 
>> just for testing. The issue has been reported to the validation team and 
>> an investigation is under progress.
>>
>> I would request a few more days for this patch alone.
> 
> Why did the _same_ change I posted here last year in April appeared
> in linux4sam now with different credits? o.O
> 
> https://github.com/linux4sam/u-boot-at91/commit/4175c4c8535ea886e2c0fd99e94ead6bc7a4df5f
> 
> For reference: the issue was discussed in length here before I made
> the patch for U-Boot last year:
> 
> https://github.com/linux4sam/at91bootstrap/issues/174
> 
> Besides: this patch series was still not reviewed and not merged.
> Dear U-Boot NAND subsystem maintainer, what's the status?
> 
> Greets
> Alex

Hello Alex,

As this change appeared downstream, apparently it's validated.

I can apply this as a fix through at91 tree.

Dario do you mind if I do that ? Apparently the patch is now on your list.

Eugen
> 
>>
>> Thanks,
>> Bala.
>>
>>>>
>>>> Greets
>>>> Alex
>>>>
>>>>>
>>>>>   drivers/mtd/nand/raw/atmel/nand-controller.c | 13 +++++++++----
>>>>>   1 file changed, 9 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c b/drivers/mtd/nand/raw/atmel/nand-controller.c
>>>>> index bbafc88e44c..00ffeadd113 100644
>>>>> --- a/drivers/mtd/nand/raw/atmel/nand-controller.c
>>>>> +++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
>>>>> @@ -1133,7 +1133,7 @@ static int atmel_smc_nand_prepare_smcconf(struct atmel_nand *nand,
>>>>>                                        const struct nand_data_interface *conf,
>>>>>                                        struct atmel_smc_cs_conf *smcconf)
>>>>>   {
>>>>> -   u32 ncycles, totalcycles, timeps, mckperiodps;
>>>>> +   u32 ncycles, totalcycles, timeps, mckperiodps, pulse;
>>>>>      struct atmel_nand_controller *nc;
>>>>>      int ret;
>>>>>
>>>>> @@ -1259,11 +1259,16 @@ static int atmel_smc_nand_prepare_smcconf(struct atmel_nand *nand,
>>>>>                       ATMEL_SMC_MODE_TDFMODE_OPTIMIZED;
>>>>>
>>>>>      /*
>>>>> -    * Read pulse timing directly matches tRP:
>>>>> +    * Read pulse timing would directly match tRP,
>>>>> +    * but some NAND flash chips (S34ML01G2 and W29N02KVxxAF)
>>>>> +    * do not work properly in timing mode 3.
>>>>> +    * The workaround is to extend the SMC NRD pulse to meet tREA
>>>>> +    * timing.
>>>>>       *
>>>>> -    * NRD_PULSE = tRP
>>>>> +    * NRD_PULSE = max(tRP, tREA)
>>>>>       */
>>>>> -   ncycles = DIV_ROUND_UP(conf->timings.sdr.tRP_min, mckperiodps);
>>>>> +   pulse = max(conf->timings.sdr.tRP_min, conf->timings.sdr.tREA_max);
>>>>> +   ncycles = DIV_ROUND_UP(pulse, mckperiodps);
>>>>>      totalcycles += ncycles;
>>>>>      ret = atmel_smc_cs_conf_set_pulse(smcconf, ATMEL_SMC_NRD_SHIFT,
>>>>>                                        ncycles);
>>>>> --
>>>>> 2.39.2
>>>>>
>>>>
>>

Re: [PATCH v3 3/3] mtd: nand: raw: atmel: Fix pulse read timing for certain NAND flashes

[Michael Nazzareno Trimarchi]

Hi Eugen

Please apply

Michael

On Wed, Feb 12, 2025 at 9:44 AM Eugen Hristev <eugen.hristev@linaro.org> wrote:
>
>
>
> On 2/12/25 09:09, Alexander Dahl wrote:
> > Hello,
> >
> > Am Wed, Oct 09, 2024 at 04:45:02AM +0000 schrieb Balamanikandan.Gunasundar@microchip.com:
> >> On 30/09/24 1:37 pm, Alexander Dahl wrote:
> >>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> >>>
> >>> Hello,
> >>>
> >>> Am Tue, May 28, 2024 at 12:32:44PM +0200 schrieb Alexander Dahl:
> >>>> Hei hei,
> >>>>
> >>>> Am Mon, Apr 15, 2024 at 09:57:55AM +0200 schrieb Alexander Dahl:
> >>>>>  From reading the S34ML02G1 and the SAM9X60 datasheets again, it seems
> >>>>> like we have to wait tREA after rising RE# before sampling the data.
> >>>>> Thus pulse time must be at least tREA.
> >>>>>
> >>>>> Without this fix we got PMECC errors when reading, after switching to
> >>>>> ONFI timing mode 3 on SAM9X60 SoC with S34ML02G1 raw NAND flash chip.
> >>>>>
> >>>>> The approach to set timings used before worked on sam9g20 and sama5d2
> >>>>> with the same flash (S34ML02G1), probably because those have a slower
> >>>>> mck clock rate and thus the resolution of the timings setup is not as
> >>>>> tight as with sam9x60.
> >>>>>
> >>>>> The approach to fix the issue was carried over from at91bootstrap, and
> >>>>> has been successfully tested in at91bootstrap, U-Boot and Linux.
> >>>>>
> >>>>> Link: https://github.com/linux4sam/at91bootstrap/issues/174
> >>>>> Cc: Li Bin <bin.li@microchip.com>
> >>>>> Signed-off-by: Alexander Dahl <ada@thorsis.com>
> >>>>> ---
> >>>>>
> >>>>> Notes:
> >>>>>      v3:
> >>>>>      - initial patch version (not present in v1 and v2)
> >>>>
> >>>> This patch was send as part of a series, which you wanted to have some
> >>>> more time to test.  Besides that, has anyone looked into this
> >>>> particular fix?  Maybe it can be applied separately?
> >>>
> >>> I'd kindly ask what the status of this patch series is from U-Boot
> >>> NAND subsystem maintainers POV?  Could you test it?  Should I rebase
> >>> and resend?
> >>>
> >>> Two of these three patches are specific to at91 family, what's the
> >>> opinion of at91 maintainers on this?
> >>>
> >>> Link to the series discussion for reference:
> >>>
> >>> https://lore.kernel.org/u-boot/20240415074547.779264-1-ada@thorsis.com/T/#u
> >>>
> >>> Greets
> >>> Alex
> >>>
> >>
> >> Hi Alex,
> >>
> >> Apologies for the delay in response. I also faced the same kind of
> >> problem while testing our new sam7d65 board with MX30LF4G28AD nand
> >> flash. I just had a workaround to increase the pulse time by 5 nsecs
> >> just for testing. The issue has been reported to the validation team and
> >> an investigation is under progress.
> >>
> >> I would request a few more days for this patch alone.
> >
> > Why did the _same_ change I posted here last year in April appeared
> > in linux4sam now with different credits? o.O
> >
> > https://github.com/linux4sam/u-boot-at91/commit/4175c4c8535ea886e2c0fd99e94ead6bc7a4df5f
> >
> > For reference: the issue was discussed in length here before I made
> > the patch for U-Boot last year:
> >
> > https://github.com/linux4sam/at91bootstrap/issues/174
> >
> > Besides: this patch series was still not reviewed and not merged.
> > Dear U-Boot NAND subsystem maintainer, what's the status?
> >
> > Greets
> > Alex
>
> Hello Alex,
>
> As this change appeared downstream, apparently it's validated.
>
> I can apply this as a fix through at91 tree.
>
> Dario do you mind if I do that ? Apparently the patch is now on your list.
>
> Eugen
> >
> >>
> >> Thanks,
> >> Bala.
> >>
> >>>>
> >>>> Greets
> >>>> Alex
> >>>>
> >>>>>
> >>>>>   drivers/mtd/nand/raw/atmel/nand-controller.c | 13 +++++++++----
> >>>>>   1 file changed, 9 insertions(+), 4 deletions(-)
> >>>>>
> >>>>> diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c b/drivers/mtd/nand/raw/atmel/nand-controller.c
> >>>>> index bbafc88e44c..00ffeadd113 100644
> >>>>> --- a/drivers/mtd/nand/raw/atmel/nand-controller.c
> >>>>> +++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
> >>>>> @@ -1133,7 +1133,7 @@ static int atmel_smc_nand_prepare_smcconf(struct atmel_nand *nand,
> >>>>>                                        const struct nand_data_interface *conf,
> >>>>>                                        struct atmel_smc_cs_conf *smcconf)
> >>>>>   {
> >>>>> -   u32 ncycles, totalcycles, timeps, mckperiodps;
> >>>>> +   u32 ncycles, totalcycles, timeps, mckperiodps, pulse;
> >>>>>      struct atmel_nand_controller *nc;
> >>>>>      int ret;
> >>>>>
> >>>>> @@ -1259,11 +1259,16 @@ static int atmel_smc_nand_prepare_smcconf(struct atmel_nand *nand,
> >>>>>                       ATMEL_SMC_MODE_TDFMODE_OPTIMIZED;
> >>>>>
> >>>>>      /*
> >>>>> -    * Read pulse timing directly matches tRP:
> >>>>> +    * Read pulse timing would directly match tRP,
> >>>>> +    * but some NAND flash chips (S34ML01G2 and W29N02KVxxAF)
> >>>>> +    * do not work properly in timing mode 3.
> >>>>> +    * The workaround is to extend the SMC NRD pulse to meet tREA
> >>>>> +    * timing.
> >>>>>       *
> >>>>> -    * NRD_PULSE = tRP
> >>>>> +    * NRD_PULSE = max(tRP, tREA)
> >>>>>       */
> >>>>> -   ncycles = DIV_ROUND_UP(conf->timings.sdr.tRP_min, mckperiodps);
> >>>>> +   pulse = max(conf->timings.sdr.tRP_min, conf->timings.sdr.tREA_max);
> >>>>> +   ncycles = DIV_ROUND_UP(pulse, mckperiodps);
> >>>>>      totalcycles += ncycles;
> >>>>>      ret = atmel_smc_cs_conf_set_pulse(smcconf, ATMEL_SMC_NRD_SHIFT,
> >>>>>                                        ncycles);
> >>>>> --
> >>>>> 2.39.2
> >>>>>
> >>>>
> >>
>


-- 
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
michael@amarulasolutions.com
__________________________________

Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
info@amarulasolutions.com
www.amarulasolutions.com

Re: [PATCH v3 3/3] mtd: nand: raw: atmel: Fix pulse read timing for certain NAND flashes

[Eugen Hristev]

On 2/12/25 10:45, Michael Nazzareno Trimarchi wrote:
> Hi Eugen
> 
> Please apply
> 
> Michael
> 

Ok,

Applied to u-boot-at91/next

Eugen