[PATCH 0/4] mtd: nand: raw: Collected improvements

[PATCH 0/4] 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.
First three patches are minor fixes.  Fourth patch is introducing a new
subcommand for the new atmel nand controller driver, which helped me a
lot when investigating issues.

Series is based on upstream next branch, but should also apply to master
cleanly.

Greets
Alex

Alexander Dahl (4):
  mtd: nand: raw: Use macro nand_to_mtd() where appropriate
  mtd: nand: raw: Port another option flag from Linux
  mtd: nand: raw: Fix (most) Kconfig indentation
  mtd: nand: raw: atmel: Introduce optional debug commands

 drivers/mtd/nand/raw/Kconfig                 | 115 +++++----
 drivers/mtd/nand/raw/atmel/nand-controller.c | 251 ++++++++++++++++++-
 drivers/mtd/nand/raw/nand_base.c             |   6 +-
 include/linux/mtd/rawnand.h                  |   7 +
 4 files changed, 321 insertions(+), 58 deletions(-)


base-commit: 6eb682bc7ea398fad4aadb612c690884e73edc03
-- 
2.39.2

[PATCH 1/4] mtd: nand: raw: Use macro nand_to_mtd() where appropriate

[Alexander Dahl]

In every other place in this file the macro is used, make it consistent.

Fixes: 9d1806fadc24 ("mtd: nand: Get rid of mtd variable in function calls")
Signed-off-by: Alexander Dahl <ada@thorsis.com>
---
 drivers/mtd/nand/raw/nand_base.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index c40a0f23d7b..688d17ba3c2 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -4118,7 +4118,7 @@ static int nand_get_bits_per_cell(u8 cellinfo)
  */
 void nand_decode_ext_id(struct nand_chip *chip)
 {
-	struct mtd_info *mtd = &chip->mtd;
+	struct mtd_info *mtd = nand_to_mtd(chip);
 	int extid;
 	/* The 3rd id byte holds MLC / multichip data */
 	chip->bits_per_cell = nand_get_bits_per_cell(chip->id.data[2]);
@@ -4185,7 +4185,7 @@ static int nand_manufacturer_init(struct nand_chip *chip)
  */
 static void nand_decode_id(struct nand_chip *chip, struct nand_flash_dev *type)
 {
-	struct mtd_info *mtd = &chip->mtd;
+	struct mtd_info *mtd = nand_to_mtd(chip);
 
 	mtd->erasesize = type->erasesize;
 	mtd->writesize = type->pagesize;
@@ -4265,7 +4265,7 @@ static const struct nand_manufacturer *nand_get_manufacturer_desc(u8 id)
 int nand_detect(struct nand_chip *chip, int *maf_id,
 		int *dev_id, struct nand_flash_dev *type)
 {
-	struct mtd_info *mtd = &chip->mtd;
+	struct mtd_info *mtd = nand_to_mtd(chip);
 	const struct nand_manufacturer *manufacturer_desc;
 	int busw, ret;
 	u8 *id_data = chip->id.data;
-- 
2.39.2

Re: [PATCH 1/4] mtd: nand: raw: Use macro nand_to_mtd() where appropriate

[Michael Nazzareno Trimarchi]

On Thu, Mar 7, 2024 at 10:10 AM Alexander Dahl <ada@thorsis.com> wrote:
>
> In every other place in this file the macro is used, make it consistent.
>
> Fixes: 9d1806fadc24 ("mtd: nand: Get rid of mtd variable in function calls")
> Signed-off-by: Alexander Dahl <ada@thorsis.com>
> ---
>  drivers/mtd/nand/raw/nand_base.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> index c40a0f23d7b..688d17ba3c2 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -4118,7 +4118,7 @@ static int nand_get_bits_per_cell(u8 cellinfo)
>   */
>  void nand_decode_ext_id(struct nand_chip *chip)
>  {
> -       struct mtd_info *mtd = &chip->mtd;
> +       struct mtd_info *mtd = nand_to_mtd(chip);
>         int extid;
>         /* The 3rd id byte holds MLC / multichip data */
>         chip->bits_per_cell = nand_get_bits_per_cell(chip->id.data[2]);
> @@ -4185,7 +4185,7 @@ static int nand_manufacturer_init(struct nand_chip *chip)
>   */
>  static void nand_decode_id(struct nand_chip *chip, struct nand_flash_dev *type)
>  {
> -       struct mtd_info *mtd = &chip->mtd;
> +       struct mtd_info *mtd = nand_to_mtd(chip);
>
>         mtd->erasesize = type->erasesize;
>         mtd->writesize = type->pagesize;
> @@ -4265,7 +4265,7 @@ static const struct nand_manufacturer *nand_get_manufacturer_desc(u8 id)
>  int nand_detect(struct nand_chip *chip, int *maf_id,
>                 int *dev_id, struct nand_flash_dev *type)
>  {
> -       struct mtd_info *mtd = &chip->mtd;
> +       struct mtd_info *mtd = nand_to_mtd(chip);
>         const struct nand_manufacturer *manufacturer_desc;
>         int busw, ret;
>         u8 *id_data = chip->id.data;
> --
> 2.39.2
>

Reviewed-By: Michael Trimarchi <michael@amarulasolutions.com>

-- 
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

[PATCH 2/4] mtd: nand: raw: Port another option flag from Linux

[Alexander Dahl]

Introduced in upstream Linux with commit 7a08dbaedd365 for release v5.0.

When the new atmel nand driver was backported to U-Boot with commit
6a8dfd57220d ("nand: atmel: Add DM based NAND driver") that definition
was added to the driver instead of the header file.  Move it over to the
other definitions with the same help text it has in Linux.

Code actually using this has not been ported over to raw nand base yet.

Signed-off-by: Alexander Dahl <ada@thorsis.com>
---
 drivers/mtd/nand/raw/atmel/nand-controller.c | 2 --
 include/linux/mtd/rawnand.h                  | 7 +++++++
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c b/drivers/mtd/nand/raw/atmel/nand-controller.c
index 0e0441472b8..e06523f3298 100644
--- a/drivers/mtd/nand/raw/atmel/nand-controller.c
+++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
@@ -1429,8 +1429,6 @@ static int atmel_nand_setup_data_interface(struct mtd_info *mtd, int csline,
 	return nc->caps->ops->setup_data_interface(nand, csline, conf);
 }
 
-#define NAND_KEEP_TIMINGS       0x00800000
-
 static void atmel_nand_init(struct atmel_nand_controller *nc,
 			    struct atmel_nand *nand)
 {
diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
index fb002ae6411..4abaf4734cf 100644
--- a/include/linux/mtd/rawnand.h
+++ b/include/linux/mtd/rawnand.h
@@ -249,6 +249,13 @@ enum nand_ecc_algo {
  */
 #define NAND_USE_BOUNCE_BUFFER	0x00100000
 
+/*
+ * Do not try to tweak the timings at runtime. This is needed when the
+ * controller initializes the timings on itself or when it relies on
+ * configuration done by the bootloader.
+ */
+#define NAND_KEEP_TIMINGS	0x00800000
+
 /* Options set by nand scan */
 /* bbt has already been read */
 #define NAND_BBT_SCANNED	0x40000000
-- 
2.39.2

Re: [PATCH 2/4] mtd: nand: raw: Port another option flag from Linux

[Michael Nazzareno Trimarchi]

Hi

On Thu, Mar 7, 2024 at 10:10 AM Alexander Dahl <ada@thorsis.com> wrote:
>
> Introduced in upstream Linux with commit 7a08dbaedd365 for release v5.0.
>
> When the new atmel nand driver was backported to U-Boot with commit
> 6a8dfd57220d ("nand: atmel: Add DM based NAND driver") that definition
> was added to the driver instead of the header file.  Move it over to the
> other definitions with the same help text it has in Linux.
>
> Code actually using this has not been ported over to raw nand base yet.
>
> Signed-off-by: Alexander Dahl <ada@thorsis.com>
> ---
>  drivers/mtd/nand/raw/atmel/nand-controller.c | 2 --
>  include/linux/mtd/rawnand.h                  | 7 +++++++
>  2 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c b/drivers/mtd/nand/raw/atmel/nand-controller.c
> index 0e0441472b8..e06523f3298 100644
> --- a/drivers/mtd/nand/raw/atmel/nand-controller.c
> +++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
> @@ -1429,8 +1429,6 @@ static int atmel_nand_setup_data_interface(struct mtd_info *mtd, int csline,
>         return nc->caps->ops->setup_data_interface(nand, csline, conf);
>  }
>
> -#define NAND_KEEP_TIMINGS       0x00800000
> -
>  static void atmel_nand_init(struct atmel_nand_controller *nc,
>                             struct atmel_nand *nand)
>  {
> diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
> index fb002ae6411..4abaf4734cf 100644
> --- a/include/linux/mtd/rawnand.h
> +++ b/include/linux/mtd/rawnand.h
> @@ -249,6 +249,13 @@ enum nand_ecc_algo {
>   */
>  #define NAND_USE_BOUNCE_BUFFER 0x00100000
>
> +/*
> + * Do not try to tweak the timings at runtime. This is needed when the
> + * controller initializes the timings on itself or when it relies on
> + * configuration done by the bootloader.
> + */
> +#define NAND_KEEP_TIMINGS      0x00800000
> +
>  /* Options set by nand scan */
>  /* bbt has already been read */
>  #define NAND_BBT_SCANNED       0x40000000

Reviewed-by: Michael Trimarchi <michael@amarulasolutions.com>

> --
> 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

[PATCH 3/4] mtd: nand: raw: Fix (most) Kconfig indentation

[Alexander Dahl]

One tab in general.  One tab plus two spaces for help text.

Signed-off-by: Alexander Dahl <ada@thorsis.com>
---
 drivers/mtd/nand/raw/Kconfig | 106 +++++++++++++++++------------------
 1 file changed, 53 insertions(+), 53 deletions(-)

diff --git a/drivers/mtd/nand/raw/Kconfig b/drivers/mtd/nand/raw/Kconfig
index bb9994b8626..f6644899b0a 100644
--- a/drivers/mtd/nand/raw/Kconfig
+++ b/drivers/mtd/nand/raw/Kconfig
@@ -1,6 +1,6 @@
-
 menuconfig MTD_RAW_NAND
 	bool "Raw NAND Device Support"
+
 if MTD_RAW_NAND
 
 config SYS_NAND_SELF_INIT
@@ -49,12 +49,12 @@ config SYS_NAND_NO_SUBPAGE_WRITE
 	depends on NAND_ARASAN || NAND_DAVINCI || NAND_KIRKWOOD
 
 config DM_NAND_ATMEL
-       bool "Support Atmel NAND controller with DM support"
-       select SYS_NAND_SELF_INIT
-       imply SYS_NAND_USE_FLASH_BBT
-       help
-         Enable this driver for NAND flash platforms using an Atmel NAND
-         controller.
+	bool "Support Atmel NAND controller with DM support"
+	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 NAND_ATMEL
 	bool "Support Atmel NAND controller"
@@ -133,35 +133,35 @@ config NAND_BRCMNAND_6753
 	  Enable support for broadcom nand driver on bcm6753.
 
 config NAND_BRCMNAND_68360
-       bool "Support Broadcom NAND controller on bcm68360"
-       depends on NAND_BRCMNAND && BCM6856
-       help
-         Enable support for broadcom nand driver on bcm68360.
+	bool "Support Broadcom NAND controller on bcm68360"
+	depends on NAND_BRCMNAND && BCM6856
+	help
+	  Enable support for broadcom nand driver on bcm68360.
 
 config NAND_BRCMNAND_6838
-       bool "Support Broadcom NAND controller on bcm6838"
-       depends on NAND_BRCMNAND && ARCH_BMIPS && SOC_BMIPS_BCM6838
-       help
-         Enable support for broadcom nand driver on bcm6838.
+	bool "Support Broadcom NAND controller on bcm6838"
+	depends on NAND_BRCMNAND && ARCH_BMIPS && SOC_BMIPS_BCM6838
+	help
+	  Enable support for broadcom nand driver on bcm6838.
 
 config NAND_BRCMNAND_6858
-       bool "Support Broadcom NAND controller on bcm6858"
-       depends on NAND_BRCMNAND && BCM6858
-       help
-         Enable support for broadcom nand driver on bcm6858.
+	bool "Support Broadcom NAND controller on bcm6858"
+	depends on NAND_BRCMNAND && BCM6858
+	help
+	  Enable support for broadcom nand driver on bcm6858.
 
 config NAND_BRCMNAND_63158
-       bool "Support Broadcom NAND controller on bcm63158"
-       depends on NAND_BRCMNAND && BCM63158
-       help
-         Enable support for broadcom nand driver on bcm63158.
+	bool "Support Broadcom NAND controller on bcm63158"
+	depends on NAND_BRCMNAND && BCM63158
+	help
+	  Enable support for broadcom nand driver on bcm63158.
 
 config NAND_BRCMNAND_IPROC
-       bool "Support Broadcom NAND controller on the iproc family"
-       depends on NAND_BRCMNAND
-       help
-         Enable support for broadcom nand driver on the Broadcom
-         iproc family such as Northstar (BCM5301x, BCM4708...)
+	bool "Support Broadcom NAND controller on the iproc family"
+	depends on NAND_BRCMNAND
+	help
+	  Enable support for broadcom nand driver on the Broadcom
+	  iproc family such as Northstar (BCM5301x, BCM4708...)
 
 config NAND_DAVINCI
 	bool "Support TI Davinci NAND controller"
@@ -413,10 +413,10 @@ config NAND_VF610_NFC
 if NAND_VF610_NFC
 
 config NAND_VF610_NFC_DT
-        bool "Support Vybrid's vf610 NAND controller as a DT device"
-        depends on OF_CONTROL && DM_MTD
-        help
-          Enable the driver for Vybrid's vf610 NAND flash on platforms
+	bool "Support Vybrid's vf610 NAND controller as a DT device"
+	depends on OF_CONTROL && DM_MTD
+	help
+	  Enable the driver for Vybrid's vf610 NAND flash on platforms
 	  using device tree.
 
 choice
@@ -472,11 +472,11 @@ config NAND_SUNXI
 	select SPL_NAND_SUPPORT
 	select SPL_SYS_NAND_SELF_INIT
 	imply CMD_NAND
-	---help---
-	Enable support for NAND. This option enables the standard and
-	SPL drivers.
-	The SPL driver only supports reading from the NAND using DMA
-	transfers.
+	help
+	  Enable support for NAND. This option enables the standard and
+	  SPL drivers.
+	  The SPL driver only supports reading from the NAND using DMA
+	  transfers.
 
 if NAND_SUNXI
 
@@ -577,16 +577,16 @@ config NAND_OCTEONTX
 	select SYS_NAND_SELF_INIT
 	imply CMD_NAND
 	help
-	 This enables Nand flash controller hardware found on the OcteonTX
-	 processors.
+	  This enables Nand flash controller hardware found on the OcteonTX
+	  processors.
 
 config NAND_OCTEONTX_HW_ECC
 	bool "Support Hardware ECC for OcteonTX NAND controller"
 	depends on NAND_OCTEONTX
 	default y
 	help
-	 This enables Hardware BCH engine found on the OcteonTX processors to
-	 support ECC for NAND flash controller.
+	  This enables Hardware BCH engine found on the OcteonTX processors to
+	  support ECC for NAND flash controller.
 
 config NAND_STM32_FMC2
 	bool "Support for NAND controller on STM32MP SoCs"
@@ -751,37 +751,37 @@ config SYS_NAND_BAD_BLOCK_POS
 config SYS_NAND_U_BOOT_LOCATIONS
 	bool "Define U-Boot binaries locations in NAND"
 	help
-	Enable CONFIG_SYS_NAND_U_BOOT_OFFS though Kconfig.
-	This option should not be enabled when compiling U-Boot for boards
-	defining CONFIG_SYS_NAND_U_BOOT_OFFS in their include/configs/<board>.h
-	file.
+	  Enable CONFIG_SYS_NAND_U_BOOT_OFFS though Kconfig.
+	  This option should not be enabled when compiling U-Boot for boards
+	  defining CONFIG_SYS_NAND_U_BOOT_OFFS in their include/configs/<board>.h
+	  file.
 
 config SYS_NAND_U_BOOT_OFFS
 	hex "Location in NAND to read U-Boot from"
 	default 0x800000 if NAND_SUNXI
 	depends on SYS_NAND_U_BOOT_LOCATIONS
 	help
-	Set the offset from the start of the nand where u-boot should be
-	loaded from.
+	  Set the offset from the start of the nand where u-boot should be
+	  loaded from.
 
 config SYS_NAND_U_BOOT_OFFS_REDUND
 	hex "Location in NAND to read U-Boot from"
 	default SYS_NAND_U_BOOT_OFFS
 	depends on SYS_NAND_U_BOOT_LOCATIONS
 	help
-	Set the offset from the start of the nand where the redundant u-boot
-	should be loaded from.
+	  Set the offset from the start of the nand where the redundant u-boot
+	  should be loaded from.
 
 config SPL_NAND_AM33XX_BCH
 	bool "Enables SPL-NAND driver which supports ELM based"
 	depends on SPL_NAND_SUPPORT && NAND_OMAP_GPMC && !OMAP34XX
 	default y
-        help
+	help
 	  Hardware ECC correction. This is useful for platforms which have ELM
 	  hardware engine and use NAND boot mode.
 	  Some legacy platforms like OMAP3xx do not have in-built ELM h/w engine,
 	  so those platforms should use CONFIG_SPL_NAND_SIMPLE for enabling
-          SPL-NAND driver with software ECC correction support.
+	  SPL-NAND driver with software ECC correction support.
 
 config SPL_NAND_DENALI
 	bool "Support Denali NAND controller for SPL"
@@ -810,6 +810,6 @@ config SYS_NAND_HW_ECC_OOBFIRST
 	bool "In SPL, read the OOB first and then the data from NAND"
 	depends on SPL_NAND_SIMPLE
 
-endif
+endif	# if SPL
 
-endif   # if NAND
+endif	# if MTD_RAW_NAND
-- 
2.39.2

Re: [PATCH 3/4] mtd: nand: raw: Fix (most) Kconfig indentation

[Michael Nazzareno Trimarchi]

Hi

On Thu, Mar 7, 2024 at 10:10 AM Alexander Dahl <ada@thorsis.com> wrote:
>
> One tab in general.  One tab plus two spaces for help text.
>
> Signed-off-by: Alexander Dahl <ada@thorsis.com>
> ---
>  drivers/mtd/nand/raw/Kconfig | 106 +++++++++++++++++------------------
>  1 file changed, 53 insertions(+), 53 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/Kconfig b/drivers/mtd/nand/raw/Kconfig
> index bb9994b8626..f6644899b0a 100644
> --- a/drivers/mtd/nand/raw/Kconfig
> +++ b/drivers/mtd/nand/raw/Kconfig
> @@ -1,6 +1,6 @@
> -
>  menuconfig MTD_RAW_NAND
>         bool "Raw NAND Device Support"
> +
>  if MTD_RAW_NAND
>
>  config SYS_NAND_SELF_INIT
> @@ -49,12 +49,12 @@ config SYS_NAND_NO_SUBPAGE_WRITE
>         depends on NAND_ARASAN || NAND_DAVINCI || NAND_KIRKWOOD
>
>  config DM_NAND_ATMEL
> -       bool "Support Atmel NAND controller with DM support"
> -       select SYS_NAND_SELF_INIT
> -       imply SYS_NAND_USE_FLASH_BBT
> -       help
> -         Enable this driver for NAND flash platforms using an Atmel NAND
> -         controller.
> +       bool "Support Atmel NAND controller with DM support"
> +       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 NAND_ATMEL
>         bool "Support Atmel NAND controller"
> @@ -133,35 +133,35 @@ config NAND_BRCMNAND_6753
>           Enable support for broadcom nand driver on bcm6753.
>
>  config NAND_BRCMNAND_68360
> -       bool "Support Broadcom NAND controller on bcm68360"
> -       depends on NAND_BRCMNAND && BCM6856
> -       help
> -         Enable support for broadcom nand driver on bcm68360.
> +       bool "Support Broadcom NAND controller on bcm68360"
> +       depends on NAND_BRCMNAND && BCM6856
> +       help
> +         Enable support for broadcom nand driver on bcm68360.
>
>  config NAND_BRCMNAND_6838
> -       bool "Support Broadcom NAND controller on bcm6838"
> -       depends on NAND_BRCMNAND && ARCH_BMIPS && SOC_BMIPS_BCM6838
> -       help
> -         Enable support for broadcom nand driver on bcm6838.
> +       bool "Support Broadcom NAND controller on bcm6838"
> +       depends on NAND_BRCMNAND && ARCH_BMIPS && SOC_BMIPS_BCM6838
> +       help
> +         Enable support for broadcom nand driver on bcm6838.
>
>  config NAND_BRCMNAND_6858
> -       bool "Support Broadcom NAND controller on bcm6858"
> -       depends on NAND_BRCMNAND && BCM6858
> -       help
> -         Enable support for broadcom nand driver on bcm6858.
> +       bool "Support Broadcom NAND controller on bcm6858"
> +       depends on NAND_BRCMNAND && BCM6858
> +       help
> +         Enable support for broadcom nand driver on bcm6858.
>
>  config NAND_BRCMNAND_63158
> -       bool "Support Broadcom NAND controller on bcm63158"
> -       depends on NAND_BRCMNAND && BCM63158
> -       help
> -         Enable support for broadcom nand driver on bcm63158.
> +       bool "Support Broadcom NAND controller on bcm63158"
> +       depends on NAND_BRCMNAND && BCM63158
> +       help
> +         Enable support for broadcom nand driver on bcm63158.
>
>  config NAND_BRCMNAND_IPROC
> -       bool "Support Broadcom NAND controller on the iproc family"
> -       depends on NAND_BRCMNAND
> -       help
> -         Enable support for broadcom nand driver on the Broadcom
> -         iproc family such as Northstar (BCM5301x, BCM4708...)
> +       bool "Support Broadcom NAND controller on the iproc family"
> +       depends on NAND_BRCMNAND
> +       help
> +         Enable support for broadcom nand driver on the Broadcom
> +         iproc family such as Northstar (BCM5301x, BCM4708...)
>
>  config NAND_DAVINCI
>         bool "Support TI Davinci NAND controller"
> @@ -413,10 +413,10 @@ config NAND_VF610_NFC
>  if NAND_VF610_NFC
>
>  config NAND_VF610_NFC_DT
> -        bool "Support Vybrid's vf610 NAND controller as a DT device"
> -        depends on OF_CONTROL && DM_MTD
> -        help
> -          Enable the driver for Vybrid's vf610 NAND flash on platforms
> +       bool "Support Vybrid's vf610 NAND controller as a DT device"
> +       depends on OF_CONTROL && DM_MTD
> +       help
> +         Enable the driver for Vybrid's vf610 NAND flash on platforms
>           using device tree.
>
>  choice
> @@ -472,11 +472,11 @@ config NAND_SUNXI
>         select SPL_NAND_SUPPORT
>         select SPL_SYS_NAND_SELF_INIT
>         imply CMD_NAND
> -       ---help---
> -       Enable support for NAND. This option enables the standard and
> -       SPL drivers.
> -       The SPL driver only supports reading from the NAND using DMA
> -       transfers.
> +       help
> +         Enable support for NAND. This option enables the standard and
> +         SPL drivers.
> +         The SPL driver only supports reading from the NAND using DMA
> +         transfers.
>
>  if NAND_SUNXI
>
> @@ -577,16 +577,16 @@ config NAND_OCTEONTX
>         select SYS_NAND_SELF_INIT
>         imply CMD_NAND
>         help
> -        This enables Nand flash controller hardware found on the OcteonTX
> -        processors.
> +         This enables Nand flash controller hardware found on the OcteonTX
> +         processors.
>
>  config NAND_OCTEONTX_HW_ECC
>         bool "Support Hardware ECC for OcteonTX NAND controller"
>         depends on NAND_OCTEONTX
>         default y
>         help
> -        This enables Hardware BCH engine found on the OcteonTX processors to
> -        support ECC for NAND flash controller.
> +         This enables Hardware BCH engine found on the OcteonTX processors to
> +         support ECC for NAND flash controller.
>
>  config NAND_STM32_FMC2
>         bool "Support for NAND controller on STM32MP SoCs"
> @@ -751,37 +751,37 @@ config SYS_NAND_BAD_BLOCK_POS
>  config SYS_NAND_U_BOOT_LOCATIONS
>         bool "Define U-Boot binaries locations in NAND"
>         help
> -       Enable CONFIG_SYS_NAND_U_BOOT_OFFS though Kconfig.
> -       This option should not be enabled when compiling U-Boot for boards
> -       defining CONFIG_SYS_NAND_U_BOOT_OFFS in their include/configs/<board>.h
> -       file.
> +         Enable CONFIG_SYS_NAND_U_BOOT_OFFS though Kconfig.
> +         This option should not be enabled when compiling U-Boot for boards
> +         defining CONFIG_SYS_NAND_U_BOOT_OFFS in their include/configs/<board>.h
> +         file.
>
>  config SYS_NAND_U_BOOT_OFFS
>         hex "Location in NAND to read U-Boot from"
>         default 0x800000 if NAND_SUNXI
>         depends on SYS_NAND_U_BOOT_LOCATIONS
>         help
> -       Set the offset from the start of the nand where u-boot should be
> -       loaded from.
> +         Set the offset from the start of the nand where u-boot should be
> +         loaded from.
>
>  config SYS_NAND_U_BOOT_OFFS_REDUND
>         hex "Location in NAND to read U-Boot from"
>         default SYS_NAND_U_BOOT_OFFS
>         depends on SYS_NAND_U_BOOT_LOCATIONS
>         help
> -       Set the offset from the start of the nand where the redundant u-boot
> -       should be loaded from.
> +         Set the offset from the start of the nand where the redundant u-boot
> +         should be loaded from.
>
>  config SPL_NAND_AM33XX_BCH
>         bool "Enables SPL-NAND driver which supports ELM based"
>         depends on SPL_NAND_SUPPORT && NAND_OMAP_GPMC && !OMAP34XX
>         default y
> -        help
> +       help
>           Hardware ECC correction. This is useful for platforms which have ELM
>           hardware engine and use NAND boot mode.
>           Some legacy platforms like OMAP3xx do not have in-built ELM h/w engine,
>           so those platforms should use CONFIG_SPL_NAND_SIMPLE for enabling
> -          SPL-NAND driver with software ECC correction support.
> +         SPL-NAND driver with software ECC correction support.
>
>  config SPL_NAND_DENALI
>         bool "Support Denali NAND controller for SPL"
> @@ -810,6 +810,6 @@ config SYS_NAND_HW_ECC_OOBFIRST
>         bool "In SPL, read the OOB first and then the data from NAND"
>         depends on SPL_NAND_SIMPLE
>
> -endif
> +endif  # if SPL
>
> -endif   # if NAND
> +endif  # if MTD_RAW_NAND
> --
> 2.39.2
>
Reviewed-by: Michael Trimarchi <michael@amarulasolutions.com>

-- 
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

[PATCH 4/4] 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 clock rate: 200000000

    SMC_SETUP3:     0x00000002
    SMC_PULSE3:     0x07040703
    SMC_CYCLE3:     0x00070007
    SMC_MODE3:      0x001f0003
    NCS_RD: setup: 0 (0 ns), pulse: 7 (35 ns), hold: 0 (0 ns), cycle: 7 (35 ns)
       NRD: setup: 0 (0 ns), pulse: 4 (20 ns), hold: 3 (15 ns), cycle: 7 (35 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 is 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>
---
 drivers/mtd/nand/raw/Kconfig                 |   9 +
 drivers/mtd/nand/raw/atmel/nand-controller.c | 249 +++++++++++++++++++
 2 files changed, 258 insertions(+)

diff --git a/drivers/mtd/nand/raw/Kconfig b/drivers/mtd/nand/raw/Kconfig
index f6644899b0a..43057aa6c5b 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 e06523f3298..052d9c7b82a 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,214 @@ 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, 24, 5, 1, 128);
+	u32 ncs_rd_pulse = atmel_smc_decode_ncycles(conf->pulse, 24, 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, 16, 5, 1, 128);
+	u32 nrd_pulse = atmel_smc_decode_ncycles(conf->pulse, 16, 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, 8, 5, 1, 128);
+	u32 ncs_wr_pulse = atmel_smc_decode_ncycles(conf->pulse, 8, 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, 0, 5, 1, 128);
+	u32 nwe_pulse = atmel_smc_decode_ncycles(conf->pulse, 0, 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 is 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 is 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)
+{
+	/* tbd */
+}
+
+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);
+}
+#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 +2323,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 +2456,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 clock rate: %lu\n", clk_get_rate(nc->mck));
+		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

Re: [PATCH 4/4] mtd: nand: raw: atmel: Introduce optional debug commands

[Eugen Hristev]

On 3/7/24 11:10, Alexander Dahl wrote:
> 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 clock rate: 200000000
> 
>     SMC_SETUP3:     0x00000002
>     SMC_PULSE3:     0x07040703
>     SMC_CYCLE3:     0x00070007
>     SMC_MODE3:      0x001f0003
>     NCS_RD: setup: 0 (0 ns), pulse: 7 (35 ns), hold: 0 (0 ns), cycle: 7 (35 ns)
>        NRD: setup: 0 (0 ns), pulse: 4 (20 ns), hold: 3 (15 ns), cycle: 7 (35 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 is 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

Adding Mihai as he is usually very interested in such debug information and methods.


> 
> Signed-off-by: Alexander Dahl <ada@thorsis.com>
> ---
>  drivers/mtd/nand/raw/Kconfig                 |   9 +
>  drivers/mtd/nand/raw/atmel/nand-controller.c | 249 +++++++++++++++++++
>  2 files changed, 258 insertions(+)
> 
> diff --git a/drivers/mtd/nand/raw/Kconfig b/drivers/mtd/nand/raw/Kconfig
> index f6644899b0a..43057aa6c5b 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 e06523f3298..052d9c7b82a 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,214 @@ 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, 24, 5, 1, 128);
> +	u32 ncs_rd_pulse = atmel_smc_decode_ncycles(conf->pulse, 24, 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, 16, 5, 1, 128);
> +	u32 nrd_pulse = atmel_smc_decode_ncycles(conf->pulse, 16, 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, 8, 5, 1, 128);
> +	u32 ncs_wr_pulse = atmel_smc_decode_ncycles(conf->pulse, 8, 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, 0, 5, 1, 128);
> +	u32 nwe_pulse = atmel_smc_decode_ncycles(conf->pulse, 0, 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 is 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 is 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)
> +{
> +	/* tbd */
> +}
> +
> +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);
> +}
> +#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 +2323,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 +2456,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 clock rate: %lu\n", clk_get_rate(nc->mck));
> +		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

RE: [PATCH 4/4] mtd: nand: raw: atmel: Introduce optional debug commands

[Mihai.Sain]

On 3/7/24 11:10, Alexander Dahl wrote:
> 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 clock rate: 200000000
>
>     SMC_SETUP3:     0x00000002
>     SMC_PULSE3:     0x07040703
>     SMC_CYCLE3:     0x00070007
>     SMC_MODE3:      0x001f0003
>     NCS_RD: setup: 0 (0 ns), pulse: 7 (35 ns), hold: 0 (0 ns), cycle: 7 (35 ns)
>        NRD: setup: 0 (0 ns), pulse: 4 (20 ns), hold: 3 (15 ns), cycle: 7 (35 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 is 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

Adding Mihai as he is usually very interested in such debug information and methods.

-------------------------------------------------------------------------------------

Hi Alexander,

I tested your work on sama7g54-curiosity board:

U-Boot> nand info

Device 0: nand0, sector size 256 KiB
  Manufacturer  MACRONIX
  Model         MX30LF4G28AD
  Device size        512 MiB
  Page size         4096 b
  OOB size           256 b
  Erase size      262144 b
  ecc strength         8 bits
  ecc step size      512 b
  subpagesize       4096 b
  options       0x00004200
  bbt options   0x00028000

U-Boot> hsmc decode

mck clock rate: 200000000

HSMC_SETUP3:    0x00000001
HSMC_PULSE3:    0x07040804
HSMC_CYCLE3:    0x00070008
HSMC_TIMINGS3:  0x880402f2
HSMC_MODE3:     0x001f0003
NCS_RD: setup: 0 (0 ns), pulse: 7 (35 ns), hold: 0 (0 ns), cycle: 7 (35 ns)
   NRD: setup: 0 (0 ns), pulse: 4 (20 ns), hold: 3 (15 ns), cycle: 7 (35 ns)
NCS_WR: setup: 0 (0 ns), pulse: 8 (40 ns), hold: 0 (0 ns), cycle: 8 (40 ns)
   NWE: setup: 1 (5 ns), pulse: 4 (20 ns), hold: 3 (15 ns), cycle: 8 (40 ns)
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

Best regards,
Mihai Sain

Re: [PATCH 4/4] mtd: nand: raw: atmel: Introduce optional debug commands

[Eugen Hristev]

On 3/18/24 10:09, Mihai.Sain@microchip.com wrote:
> On 3/7/24 11:10, Alexander Dahl wrote:
>> 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 clock rate: 200000000
>>
>>     SMC_SETUP3:     0x00000002
>>     SMC_PULSE3:     0x07040703
>>     SMC_CYCLE3:     0x00070007
>>     SMC_MODE3:      0x001f0003
>>     NCS_RD: setup: 0 (0 ns), pulse: 7 (35 ns), hold: 0 (0 ns), cycle: 7 (35 ns)
>>        NRD: setup: 0 (0 ns), pulse: 4 (20 ns), hold: 3 (15 ns), cycle: 7 (35 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 is 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
> 
> Adding Mihai as he is usually very interested in such debug information and methods.
> 
> -------------------------------------------------------------------------------------
> 
> Hi Alexander,
> 
> I tested your work on sama7g54-curiosity board:
> 
> U-Boot> nand info
> 
> Device 0: nand0, sector size 256 KiB
>   Manufacturer  MACRONIX
>   Model         MX30LF4G28AD
>   Device size        512 MiB
>   Page size         4096 b
>   OOB size           256 b
>   Erase size      262144 b
>   ecc strength         8 bits
>   ecc step size      512 b
>   subpagesize       4096 b
>   options       0x00004200
>   bbt options   0x00028000
> 
> U-Boot> hsmc decode
> 
> mck clock rate: 200000000
> 
> HSMC_SETUP3:    0x00000001
> HSMC_PULSE3:    0x07040804
> HSMC_CYCLE3:    0x00070008
> HSMC_TIMINGS3:  0x880402f2
> HSMC_MODE3:     0x001f0003
> NCS_RD: setup: 0 (0 ns), pulse: 7 (35 ns), hold: 0 (0 ns), cycle: 7 (35 ns)
>    NRD: setup: 0 (0 ns), pulse: 4 (20 ns), hold: 3 (15 ns), cycle: 7 (35 ns)
> NCS_WR: setup: 0 (0 ns), pulse: 8 (40 ns), hold: 0 (0 ns), cycle: 8 (40 ns)
>    NWE: setup: 1 (5 ns), pulse: 4 (20 ns), hold: 3 (15 ns), cycle: 8 (40 ns)
> 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
> 
> Best regards,
> Mihai Sain

Hello Mihai,

If you have any suggestions for improvement, changes, or you are happy with this
command, is it useful ?
You can provide your Tested-by then if you consider this is useful

Eugen

RE: [PATCH 4/4] mtd: nand: raw: atmel: Introduce optional debug commands

[Mihai.Sain]

> Hi Alexander,
>
> I tested your work on sama7g54-curiosity board:
>
> U-Boot> nand info
>
> Device 0: nand0, sector size 256 KiB
>   Manufacturer  MACRONIX
>   Model         MX30LF4G28AD
>   Device size        512 MiB
>   Page size         4096 b
>   OOB size           256 b
>   Erase size      262144 b
>   ecc strength         8 bits
>   ecc step size      512 b
>   subpagesize       4096 b
>   options       0x00004200
>   bbt options   0x00028000
>
> U-Boot> hsmc decode
>
> mck clock rate: 200000000
>
> HSMC_SETUP3:    0x00000001
> HSMC_PULSE3:    0x07040804
> HSMC_CYCLE3:    0x00070008
> HSMC_TIMINGS3:  0x880402f2
> HSMC_MODE3:     0x001f0003
> NCS_RD: setup: 0 (0 ns), pulse: 7 (35 ns), hold: 0 (0 ns), cycle: 7 (35 ns)
>    NRD: setup: 0 (0 ns), pulse: 4 (20 ns), hold: 3 (15 ns), cycle: 7 
> (35 ns)
> NCS_WR: setup: 0 (0 ns), pulse: 8 (40 ns), hold: 0 (0 ns), cycle: 8 (40 ns)
>    NWE: setup: 1 (5 ns), pulse: 4 (20 ns), hold: 3 (15 ns), cycle: 8 
> (40 ns) 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
>
> Best regards,
> Mihai Sain

Hello Mihai,

If you have any suggestions for improvement, changes, or you are happy with this command, is it useful ?
You can provide your Tested-by then if you consider this is useful

Eugen

----------------------------------------------------------------------

Hello Eugen,

Yes.
The command is very useful.
I would like to have also the ONFI timing mode printed for nand-flash 😊
Also I recommend to print the master clock in MHz, and to print the master clock name/label from ccf driver:
https://github.com/u-boot/u-boot/blob/master/drivers/clk/at91/sama7g5.c#L410

Tested-by: Mihai Sain <mihai.sain@microchip.com>

Best regards,
Mihai Sain

Re: [PATCH 4/4] mtd: nand: raw: atmel: Introduce optional debug commands

[Alexander Dahl]

Hello Mihai,

Am Mon, Mar 18, 2024 at 09:15:29AM +0000 schrieb Mihai.Sain@microchip.com:
> > Hi Alexander,
> >
> > I tested your work on sama7g54-curiosity board:
> >
> > U-Boot> nand info
> >
> > Device 0: nand0, sector size 256 KiB
> >   Manufacturer  MACRONIX
> >   Model         MX30LF4G28AD
> >   Device size        512 MiB
> >   Page size         4096 b
> >   OOB size           256 b
> >   Erase size      262144 b
> >   ecc strength         8 bits
> >   ecc step size      512 b
> >   subpagesize       4096 b
> >   options       0x00004200
> >   bbt options   0x00028000
> >
> > U-Boot> hsmc decode
> >
> > mck clock rate: 200000000
> >
> > HSMC_SETUP3:    0x00000001
> > HSMC_PULSE3:    0x07040804
> > HSMC_CYCLE3:    0x00070008
> > HSMC_TIMINGS3:  0x880402f2
> > HSMC_MODE3:     0x001f0003
> > NCS_RD: setup: 0 (0 ns), pulse: 7 (35 ns), hold: 0 (0 ns), cycle: 7 (35 ns)
> >    NRD: setup: 0 (0 ns), pulse: 4 (20 ns), hold: 3 (15 ns), cycle: 7 
> > (35 ns)
> > NCS_WR: setup: 0 (0 ns), pulse: 8 (40 ns), hold: 0 (0 ns), cycle: 8 (40 ns)
> >    NWE: setup: 1 (5 ns), pulse: 4 (20 ns), hold: 3 (15 ns), cycle: 8 
> > (40 ns) 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
> >
> > Best regards,
> > Mihai Sain
> 
> Hello Mihai,
> 
> If you have any suggestions for improvement, changes, or you are happy with this command, is it useful ?
> You can provide your Tested-by then if you consider this is useful
> 
> Eugen
> 
> ----------------------------------------------------------------------
> 
> Hello Eugen,
> 
> Yes.
> The command is very useful.
> I would like to have also the ONFI timing mode printed for nand-flash 😊

As far as I can see the actually set mode is not stored anywhere.  One
could print it after it was successfully set, but that would be in
nand base, not in the atmel driver.

> Also I recommend to print the master clock in MHz, and to print the master clock name/label from ccf driver:
> https://github.com/u-boot/u-boot/blob/master/drivers/clk/at91/sama7g5.c#L410

Should be possible.  I could do this and send a v2?

Greets
Alex

> 
> Tested-by: Mihai Sain <mihai.sain@microchip.com>
> 
> Best regards,
> Mihai Sain

RE: [PATCH 4/4] mtd: nand: raw: atmel: Introduce optional debug commands

[Mihai.Sain]

------------------------------------------------------------
> The command is very useful.
> I would like to have also the ONFI timing mode printed for nand-flash 
> 😊

As far as I can see the actually set mode is not stored anywhere.  One could print it after it was successfully set, but that would be in nand base, not in the atmel driver.

	OK. Understood.
	Thanks.

> Also I recommend to print the master clock in MHz, and to print the master clock name/label from ccf driver:
> https://github.com/u-boot/u-boot/blob/master/drivers/clk/at91/sama7g5.
> c#L410

Should be possible.  I could do this and send a v2?

	Yes Please 😊
	Also please note that older SAM9/SAMA5 series have no ccf drivers ☹
	Only SAM9X6+ and SAMA7 series have ccf 😊
	Thanks.


Greets
Alex

>
> Tested-by: Mihai Sain <mihai.sain@microchip.com>
>
> Best regards,
> Mihai Sain

Re: [PATCH 4/4] mtd: nand: raw: atmel: Introduce optional debug commands

[Alexander Dahl]

Hello everyone,

Am Mon, Mar 18, 2024 at 11:15:18AM +0000 schrieb Mihai.Sain@microchip.com:
> ------------------------------------------------------------
> > The command is very useful.
> > I would like to have also the ONFI timing mode printed for nand-flash 
> > 😊
> 
> As far as I can see the actually set mode is not stored anywhere.  One could print it after it was successfully set, but that would be in nand base, not in the atmel driver.
> 
> 	OK. Understood.
> 	Thanks.
> 
> > Also I recommend to print the master clock in MHz, and to print the master clock name/label from ccf driver:
> > https://github.com/u-boot/u-boot/blob/master/drivers/clk/at91/sama7g5.
> > c#L410
> 
> Should be possible.  I could do this and send a v2?
> 
> 	Yes Please 😊
> 	Also please note that older SAM9/SAMA5 series have no ccf drivers ☹
> 	Only SAM9X6+ and SAMA7 series have ccf 😊

Okay I thought this would be easy, but it seems not.  This is what I
came up with:

-                printf("mck clock rate: %lu\n", clk_get_rate(nc->mck));
+                printf("%s rate: %lu MHz\n",
+                       nc->mck->dev->name ? nc->mck->dev->name : "mck clock",
+                       clk_get_rate(nc->mck) / 1000000);

And this is the output on sam9x60 with CONFIG_CLK_CCF enabled:

    pmc@fffffc00 rate: 200 MHz

The corresponding line from `clk dump` is:

     200000000            5        |       |           |   `-- mck_div

That name, I don't get it where to get that one.

Greets
Alex

> 	Thanks.
> 
> 
> Greets
> Alex
> 
> >
> > Tested-by: Mihai Sain <mihai.sain@microchip.com>
> >
> > Best regards,
> > Mihai Sain

RE: [PATCH 4/4] mtd: nand: raw: atmel: Introduce optional debug commands

[Mihai.Sain]

Hi Alex,

> > Also I recommend to print the master clock in MHz, and to print the master clock name/label from ccf driver:
> > https://github.com/u-boot/u-boot/blob/master/drivers/clk/at91/sama7g5.
> > c#L410
>
> Should be possible.  I could do this and send a v2?
>
>       Yes Please 😊
>       Also please note that older SAM9/SAMA5 series have no ccf drivers ☹
>       Only SAM9X6+ and SAMA7 series have ccf 😊

Okay I thought this would be easy, but it seems not.  This is what I came up with:

-                printf("mck clock rate: %lu\n", clk_get_rate(nc->mck));
+                printf("%s rate: %lu MHz\n",
+                       nc->mck->dev->name ? nc->mck->dev->name : "mck clock",
+                       clk_get_rate(nc->mck) / 1000000);

And this is the output on sam9x60 with CONFIG_CLK_CCF enabled:

    pmc@fffffc00 rate: 200 MHz

The corresponding line from `clk dump` is:

     200000000            5        |       |           |   `-- mck_div

That name, I don't get it where to get that one.

	You can revert to initial print and add MHz 😊
	MCK rate: 200 MHz
	Thanks.

Re: [PATCH 4/4] mtd: nand: raw: atmel: Introduce optional debug commands

[Alexander Dahl]

Hello Mihai,

Am Mon, Mar 18, 2024 at 08:09:00AM +0000 schrieb Mihai.Sain@microchip.com:
> On 3/7/24 11:10, Alexander Dahl wrote:
> > 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 clock rate: 200000000
> >
> >     SMC_SETUP3:     0x00000002
> >     SMC_PULSE3:     0x07040703
> >     SMC_CYCLE3:     0x00070007
> >     SMC_MODE3:      0x001f0003
> >     NCS_RD: setup: 0 (0 ns), pulse: 7 (35 ns), hold: 0 (0 ns), cycle: 7 (35 ns)
> >        NRD: setup: 0 (0 ns), pulse: 4 (20 ns), hold: 3 (15 ns), cycle: 7 (35 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 is 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
> 
> Adding Mihai as he is usually very interested in such debug information and methods.
> 
> -------------------------------------------------------------------------------------
> 
> Hi Alexander,
> 
> I tested your work on sama7g54-curiosity board:

nice, thanks for that.

> U-Boot> nand info
> 
> Device 0: nand0, sector size 256 KiB
>   Manufacturer  MACRONIX
>   Model         MX30LF4G28AD
>   Device size        512 MiB
>   Page size         4096 b
>   OOB size           256 b
>   Erase size      262144 b
>   ecc strength         8 bits
>   ecc step size      512 b
>   subpagesize       4096 b
>   options       0x00004200
>   bbt options   0x00028000

This seems to be the same NAND chip as on the sam9x60 curiosity, but
your output has three additional lines, see mine:

    U-Boot> nand info
    
    Device 0: nand0, sector size 256 KiB
      Page size         4096 b
      OOB size           256 b
      Erase size      262144 b
      ecc strength         8 bits
      ecc step size      512 b
      subpagesize       4096 b
      options       0x40004200
      bbt options   0x00028000

Do you have some additional patches printing manufacturer, model, and
device size?  I can't see those lines printed in
nand_print_and_set_info() here.

> U-Boot> hsmc decode
> 
> mck clock rate: 200000000
> 
> HSMC_SETUP3:    0x00000001
> HSMC_PULSE3:    0x07040804
> HSMC_CYCLE3:    0x00070008
> HSMC_TIMINGS3:  0x880402f2
> HSMC_MODE3:     0x001f0003
> NCS_RD: setup: 0 (0 ns), pulse: 7 (35 ns), hold: 0 (0 ns), cycle: 7 (35 ns)
>    NRD: setup: 0 (0 ns), pulse: 4 (20 ns), hold: 3 (15 ns), cycle: 7 (35 ns)
> NCS_WR: setup: 0 (0 ns), pulse: 8 (40 ns), hold: 0 (0 ns), cycle: 8 (40 ns)
>    NWE: setup: 1 (5 ns), pulse: 4 (20 ns), hold: 3 (15 ns), cycle: 8 (40 ns)
> 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

This is also interesting.  Given the mck clock rate is the same as on
sam9x60, I would have guessed the timings set by
atmel_smc_nand_prepare_smcconf() should give the same results, both
for ONFI timiming mode 3, which is the fastest mode the (H)SMC
supports according to comments in the driver.  This is the output with
the patch in question applied on next for sam9x60:

    U-Boot> hsmc decode 
    
    mck clock rate: 200000000
    
    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 is 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

Notice the pulse times for read are one clock cycle smaller than in
your output, and the timings for write are also different.  Do you
have changes for atmel_smc_nand_prepare_smcconf() applied which are
not upstream yet?  Or is the HSMC on sama7g54 somehow different than
on older SoCs?

Note: I'm currently testing a patch changing the computation of the
read pulse cycles based on a patch for at91bootstrap [1], but that is
not applied here for the output quoted above.

Greets
Alex

[1] https://github.com/linux4sam/at91bootstrap/issues/174#issuecomment-1970698527

> 
> Best regards,
> Mihai Sain

RE: [PATCH 4/4] mtd: nand: raw: atmel: Introduce optional debug commands

[Mihai.Sain]

> U-Boot> nand info
>
> Device 0: nand0, sector size 256 KiB
>   Manufacturer  MACRONIX
>   Model         MX30LF4G28AD
>   Device size        512 MiB
>   Page size         4096 b
>   OOB size           256 b
>   Erase size      262144 b
>   ecc strength         8 bits
>   ecc step size      512 b
>   subpagesize       4096 b
>   options       0x00004200
>   bbt options   0x00028000

This seems to be the same NAND chip as on the sam9x60 curiosity, but your output has three additional lines, see mine:
Do you have some additional patches printing manufacturer, model, and device size?  I can't see those lines printed in
nand_print_and_set_info() here.

Yes. I have 😊
+	printf("  Manufacturer  %s \n", chip->onfi_params.manufacturer);
+	printf("  Model         %s \n", chip->onfi_params.model);
+	printf("  Device size   %8d MiB\n", (int)(chip->chipsize >> 20));

> U-Boot> hsmc decode
>
> mck clock rate: 200000000
>
> HSMC_SETUP3:    0x00000001
> HSMC_PULSE3:    0x07040804
> HSMC_CYCLE3:    0x00070008
> HSMC_TIMINGS3:  0x880402f2
> HSMC_MODE3:     0x001f0003
> NCS_RD: setup: 0 (0 ns), pulse: 7 (35 ns), hold: 0 (0 ns), cycle: 7 (35 ns)
>    NRD: setup: 0 (0 ns), pulse: 4 (20 ns), hold: 3 (15 ns), cycle: 7 
> (35 ns)
> NCS_WR: setup: 0 (0 ns), pulse: 8 (40 ns), hold: 0 (0 ns), cycle: 8 (40 ns)
>    NWE: setup: 1 (5 ns), pulse: 4 (20 ns), hold: 3 (15 ns), cycle: 8 
> (40 ns) 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

This is also interesting.  Given the mck clock rate is the same as on sam9x60, I would have guessed the timings set by
atmel_smc_nand_prepare_smcconf() should give the same results, both for ONFI timiming mode 3, which is the fastest mode the (H)SMC supports according to comments in the driver.  This is the output with the patch in question applied on next for sam9x60:

    U-Boot> hsmc decode

    mck clock rate: 200000000

    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 is 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

Notice the pulse times for read are one clock cycle smaller than in your output, and the timings for write are also different.  Do you have changes for atmel_smc_nand_prepare_smcconf() applied which are not upstream yet?  Or is the HSMC on sama7g54 somehow different than on older SoCs?

Yes. I force timing mode 2 in nand-controller.c:
+	if (conf->timings.sdr.tRC_min < 30001) // force timing mode 2, 35ns for read/write cycle

This will pass the nand torture test 😊

U-Boot> nand torture 0x800000 0x1000000

NAND torture: device 0 offset 0x800000 size 0x1000000 (block size 0x40000)
 Passed: 64, failed: 0

Note: I'm currently testing a patch changing the computation of the read pulse cycles based on a patch for at91bootstrap [1], but that is not applied here for the output quoted above.

Greets
Alex

[1] https://github.com/linux4sam/at91bootstrap/issues/174#issuecomment-1970698527

>
> Best regards,
> Mihai Sain

Re: [PATCH 4/4] mtd: nand: raw: atmel: Introduce optional debug commands

[Alexander Dahl]

Hello Mihai,

Am Mon, Mar 18, 2024 at 09:07:09AM +0000 schrieb Mihai.Sain@microchip.com:
> > U-Boot> nand info
> >
> > Device 0: nand0, sector size 256 KiB
> >   Manufacturer  MACRONIX
> >   Model         MX30LF4G28AD
> >   Device size        512 MiB
> >   Page size         4096 b
> >   OOB size           256 b
> >   Erase size      262144 b
> >   ecc strength         8 bits
> >   ecc step size      512 b
> >   subpagesize       4096 b
> >   options       0x00004200
> >   bbt options   0x00028000
> 
> This seems to be the same NAND chip as on the sam9x60 curiosity, but your output has three additional lines, see mine:
> Do you have some additional patches printing manufacturer, model, and device size?  I can't see those lines printed in
> nand_print_and_set_info() here.
> 
> Yes. I have 😊
> +	printf("  Manufacturer  %s \n", chip->onfi_params.manufacturer);
> +	printf("  Model         %s \n", chip->onfi_params.model);
> +	printf("  Device size   %8d MiB\n", (int)(chip->chipsize >> 20));

This is nice, and I think it would be valuable to have upstream.
Maybe you could send a patch for that?

> > U-Boot> hsmc decode
> >
> > mck clock rate: 200000000
> >
> > HSMC_SETUP3:    0x00000001
> > HSMC_PULSE3:    0x07040804
> > HSMC_CYCLE3:    0x00070008
> > HSMC_TIMINGS3:  0x880402f2
> > HSMC_MODE3:     0x001f0003
> > NCS_RD: setup: 0 (0 ns), pulse: 7 (35 ns), hold: 0 (0 ns), cycle: 7 (35 ns)
> >    NRD: setup: 0 (0 ns), pulse: 4 (20 ns), hold: 3 (15 ns), cycle: 7 
> > (35 ns)
> > NCS_WR: setup: 0 (0 ns), pulse: 8 (40 ns), hold: 0 (0 ns), cycle: 8 (40 ns)
> >    NWE: setup: 1 (5 ns), pulse: 4 (20 ns), hold: 3 (15 ns), cycle: 8 
> > (40 ns) 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
> 
> This is also interesting.  Given the mck clock rate is the same as on sam9x60, I would have guessed the timings set by
> atmel_smc_nand_prepare_smcconf() should give the same results, both for ONFI timiming mode 3, which is the fastest mode the (H)SMC supports according to comments in the driver.  This is the output with the patch in question applied on next for sam9x60:
> 
>     U-Boot> hsmc decode
> 
>     mck clock rate: 200000000
> 
>     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 is 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
> 
> Notice the pulse times for read are one clock cycle smaller than in your output, and the timings for write are also different.  Do you have changes for atmel_smc_nand_prepare_smcconf() applied which are not upstream yet?  Or is the HSMC on sama7g54 somehow different than on older SoCs?
> 
> Yes. I force timing mode 2 in nand-controller.c:
> +	if (conf->timings.sdr.tRC_min < 30001) // force timing mode 2, 35ns for read/write cycle
> 
> This will pass the nand torture test 😊
> 
> U-Boot> nand torture 0x800000 0x1000000
> 
> NAND torture: device 0 offset 0x800000 size 0x1000000 (block size 0x40000)
>  Passed: 64, failed: 0

Ah okay.  I have another patch here for manually setting the ONFI
timing mode from commandline.  This is probably too late for some
scenarios, but it helped me when testing.  If you're interested I
could send it to the public.

Greets
Alex

> 
> Note: I'm currently testing a patch changing the computation of the read pulse cycles based on a patch for at91bootstrap [1], but that is not applied here for the output quoted above.
> 
> Greets
> Alex
> 
> [1] https://github.com/linux4sam/at91bootstrap/issues/174#issuecomment-1970698527
> 
> >
> > Best regards,
> > Mihai Sain

RE: [PATCH 4/4] mtd: nand: raw: atmel: Introduce optional debug commands

[Mihai.Sain]

Hi Alexander,

I'm sorry for quoting because the email is sent from Outlook ☹

> > U-Boot> nand info
> >
> > Device 0: nand0, sector size 256 KiB
> >   Manufacturer  MACRONIX
> >   Model         MX30LF4G28AD
> >   Device size        512 MiB
> >   Page size         4096 b
> >   OOB size           256 b
> >   Erase size      262144 b
> >   ecc strength         8 bits
> >   ecc step size      512 b
> >   subpagesize       4096 b
> >   options       0x00004200
> >   bbt options   0x00028000
>
> This seems to be the same NAND chip as on the sam9x60 curiosity, but your output has three additional lines, see mine:
> Do you have some additional patches printing manufacturer, model, and 
> device size?  I can't see those lines printed in
> nand_print_and_set_info() here.
>
> Yes. I have 😊
> +     printf("  Manufacturer  %s \n", chip->onfi_params.manufacturer);
> +     printf("  Model         %s \n", chip->onfi_params.model);
> +     printf("  Device size   %8d MiB\n", (int)(chip->chipsize >> 20));

This is nice, and I think it would be valuable to have upstream.
Maybe you could send a patch for that?
	Sure. I will send the patch !

> > U-Boot> hsmc decode
> >
> > mck clock rate: 200000000
> >
> > HSMC_SETUP3:    0x00000001
> > HSMC_PULSE3:    0x07040804
> > HSMC_CYCLE3:    0x00070008
> > HSMC_TIMINGS3:  0x880402f2
> > HSMC_MODE3:     0x001f0003
> > NCS_RD: setup: 0 (0 ns), pulse: 7 (35 ns), hold: 0 (0 ns), cycle: 7 (35 ns)
> >    NRD: setup: 0 (0 ns), pulse: 4 (20 ns), hold: 3 (15 ns), cycle: 7
> > (35 ns)
> > NCS_WR: setup: 0 (0 ns), pulse: 8 (40 ns), hold: 0 (0 ns), cycle: 8 (40 ns)
> >    NWE: setup: 1 (5 ns), pulse: 4 (20 ns), hold: 3 (15 ns), cycle: 8
> > (40 ns) 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
>
> This is also interesting.  Given the mck clock rate is the same as on 
> sam9x60, I would have guessed the timings set by
> atmel_smc_nand_prepare_smcconf() should give the same results, both for ONFI timiming mode 3, which is the fastest mode the (H)SMC supports according to comments in the driver.  This is the output with the patch in question applied on next for sam9x60:
>
>     U-Boot> hsmc decode
>
>     mck clock rate: 200000000
>
>     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 is 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
>
> Notice the pulse times for read are one clock cycle smaller than in your output, and the timings for write are also different.  Do you have changes for atmel_smc_nand_prepare_smcconf() applied which are not upstream yet?  Or is the HSMC on sama7g54 somehow different than on older SoCs?
>
> Yes. I force timing mode 2 in nand-controller.c:
> +     if (conf->timings.sdr.tRC_min < 30001) // force timing mode 2, 
> + 35ns for read/write cycle
>
> This will pass the nand torture test 😊
>
> U-Boot> nand torture 0x800000 0x1000000
>
> NAND torture: device 0 offset 0x800000 size 0x1000000 (block size 
> 0x40000)
>  Passed: 64, failed: 0

Ah okay.  I have another patch here for manually setting the ONFI timing mode from commandline.  This is probably too late for some scenarios, but it helped me when testing.  If you're interested I could send it to the public.
	Yes. I'm very interested !
	Maybe we should add an automatic fallback for timing mode in nand-controller.c 😊
	As of now the driver is forcing tRC_min to 30ns (mode 3), which is not reliable for sama7 nfc controller ☹
	https://github.com/u-boot/u-boot/blob/master/drivers/mtd/nand/raw/nand_timings.c#L167
	The nand torture command helped me to manually force tRC_min to 35ns (mode 2).
	Thanks.

Greets
Alex

>
> Note: I'm currently testing a patch changing the computation of the read pulse cycles based on a patch for at91bootstrap [1], but that is not applied here for the output quoted above.
>
> Greets
> Alex
>
> [1] 
> https://github.com/linux4sam/at91bootstrap/issues/174#issuecomment-197
> 0698527
>
> >
> > Best regards,
> > Mihai Sain

Re: [PATCH 4/4] mtd: nand: raw: atmel: Introduce optional debug commands

[Alexander Dahl]

[-- Attachment #1: Type: text/plain, Size: 5683 bytes --]

Hello Mihai,

Am Mon, Mar 18, 2024 at 11:28:39AM +0000 schrieb Mihai.Sain@microchip.com:
> Hi Alexander,
> 
> I'm sorry for quoting because the email is sent from Outlook ☹
> 
> > > U-Boot> nand info
> > >
> > > Device 0: nand0, sector size 256 KiB
> > >   Manufacturer  MACRONIX
> > >   Model         MX30LF4G28AD
> > >   Device size        512 MiB
> > >   Page size         4096 b
> > >   OOB size           256 b
> > >   Erase size      262144 b
> > >   ecc strength         8 bits
> > >   ecc step size      512 b
> > >   subpagesize       4096 b
> > >   options       0x00004200
> > >   bbt options   0x00028000
> >
> > This seems to be the same NAND chip as on the sam9x60 curiosity, but your output has three additional lines, see mine:
> > Do you have some additional patches printing manufacturer, model, and 
> > device size?  I can't see those lines printed in
> > nand_print_and_set_info() here.
> >
> > Yes. I have 😊
> > +     printf("  Manufacturer  %s \n", chip->onfi_params.manufacturer);
> > +     printf("  Model         %s \n", chip->onfi_params.model);
> > +     printf("  Device size   %8d MiB\n", (int)(chip->chipsize >> 20));
> 
> This is nice, and I think it would be valuable to have upstream.
> Maybe you could send a patch for that?
> 	Sure. I will send the patch !
> 
> > > U-Boot> hsmc decode
> > >
> > > mck clock rate: 200000000
> > >
> > > HSMC_SETUP3:    0x00000001
> > > HSMC_PULSE3:    0x07040804
> > > HSMC_CYCLE3:    0x00070008
> > > HSMC_TIMINGS3:  0x880402f2
> > > HSMC_MODE3:     0x001f0003
> > > NCS_RD: setup: 0 (0 ns), pulse: 7 (35 ns), hold: 0 (0 ns), cycle: 7 (35 ns)
> > >    NRD: setup: 0 (0 ns), pulse: 4 (20 ns), hold: 3 (15 ns), cycle: 7
> > > (35 ns)
> > > NCS_WR: setup: 0 (0 ns), pulse: 8 (40 ns), hold: 0 (0 ns), cycle: 8 (40 ns)
> > >    NWE: setup: 1 (5 ns), pulse: 4 (20 ns), hold: 3 (15 ns), cycle: 8
> > > (40 ns) 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
> >
> > This is also interesting.  Given the mck clock rate is the same as on 
> > sam9x60, I would have guessed the timings set by
> > atmel_smc_nand_prepare_smcconf() should give the same results, both for ONFI timiming mode 3, which is the fastest mode the (H)SMC supports according to comments in the driver.  This is the output with the patch in question applied on next for sam9x60:
> >
> >     U-Boot> hsmc decode
> >
> >     mck clock rate: 200000000
> >
> >     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 is 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
> >
> > Notice the pulse times for read are one clock cycle smaller than in your output, and the timings for write are also different.  Do you have changes for atmel_smc_nand_prepare_smcconf() applied which are not upstream yet?  Or is the HSMC on sama7g54 somehow different than on older SoCs?
> >
> > Yes. I force timing mode 2 in nand-controller.c:
> > +     if (conf->timings.sdr.tRC_min < 30001) // force timing mode 2, 
> > + 35ns for read/write cycle
> >
> > This will pass the nand torture test 😊
> >
> > U-Boot> nand torture 0x800000 0x1000000
> >
> > NAND torture: device 0 offset 0x800000 size 0x1000000 (block size 
> > 0x40000)
> >  Passed: 64, failed: 0
> 
> Ah okay.  I have another patch here for manually setting the ONFI timing mode from commandline.  This is probably too late for some scenarios, but it helped me when testing.  If you're interested I could send it to the public.
> 	Yes. I'm very interested !

Okay, I'll add it to v2 of the series then.

> 	Maybe we should add an automatic fallback for timing mode in nand-controller.c 😊
> 	As of now the driver is forcing tRC_min to 30ns (mode 3), which is not reliable for sama7 nfc controller ☹
> 	https://github.com/u-boot/u-boot/blob/master/drivers/mtd/nand/raw/nand_timings.c#L167
> 	The nand torture command helped me to manually force tRC_min to 35ns (mode 2).

This sounds like the same problem encountered in
https://github.com/linux4sam/at91bootstrap/issues/174 and the fix
proposed there works in Linux and U-Boot as well.  I consider the
original commit message of the patch attached to that ticket not easy
to understand however, so I wrote what I think is the problem.  Could
you please test the patch attached to this mail which does the same
thing and should apply to U-Boot cleanly?  I tested that on sam9x60
and sama5, but have no other boards/socs to test with.  If that works
on sama7, I will propose it on U-Boot, too.

(I hope it is okay to attach it as an attachment for now, it's not
ready for submission anyways.)

Greets
Alex

> 	Thanks.
> 
> Greets
> Alex
> 
> >
> > Note: I'm currently testing a patch changing the computation of the read pulse cycles based on a patch for at91bootstrap [1], but that is not applied here for the output quoted above.
> >
> > Greets
> > Alex
> >
> > [1] 
> > https://github.com/linux4sam/at91bootstrap/issues/174#issuecomment-197
> > 0698527
> >
> > >
> > > Best regards,
> > > Mihai Sain

[-- Attachment #2: 0001-mtd-nand-raw-atmel-Port-timing-fix-from-at91bootstra.patch --]
[-- Type: text/x-diff, Size: 2287 bytes --]

From 3ab90ab8a817c79013c6d57d5ac4e52d5242295e Mon Sep 17 00:00:00 2001
From: Alexander Dahl <ada@thorsis.com>
Date: Tue, 12 Mar 2024 13:41:04 +0100
Subject: [PATCH] mtd: nand: raw: atmel: Port timing fix from at91bootstrap

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 must be at least tREA.  The fix works in bootstrap, u-boot,
and Linux, but the explanation might be improved.  It probably worked on
sam9g20 and sama5d2 before, because those have a slower mck clock rate
and thus the resolution of the timings setup is not as tight as with
sam9x60.

Without the fix we got PMECC errors when reading after switching to ONFI
timing mode 3.

Link: https://github.com/linux4sam/at91bootstrap/issues/174
---
 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 4b6c9a79306..405987acd51 100644
--- a/drivers/mtd/nand/raw/atmel/nand-controller.c
+++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
@@ -1168,7 +1168,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;
 
@@ -1294,11 +1294,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 4/4] mtd: nand: raw: atmel: Introduce optional debug commands

[Mihai.Sain]

Hi Alexander,

>       Maybe we should add an automatic fallback for timing mode in nand-controller.c 😊
>       As of now the driver is forcing tRC_min to 30ns (mode 3), which is not reliable for sama7 nfc controller ☹
>       https://github.com/u-boot/u-boot/blob/master/drivers/mtd/nand/raw/nand_timings.c#L167
>       The nand torture command helped me to manually force tRC_min to 35ns (mode 2).

This sounds like the same problem encountered in
https://github.com/linux4sam/at91bootstrap/issues/174 and the fix proposed there works in Linux and U-Boot as well.  I consider the original commit message of the patch attached to that ticket not easy to understand however, so I wrote what I think is the problem.  Could you please test the patch attached to this mail which does the same thing and should apply to U-Boot cleanly?  I tested that on sam9x60 and sama5, but have no other boards/socs to test with.  If that works on sama7, I will propose it on U-Boot, too.

(I hope it is okay to attach it as an attachment for now, it's not ready for submission anyways.)

	I tested your patch on sama7g54-curiosity board.
	I also reverted to (conf->timings.sdr.tRC_min < 30000), to force mode 3 😊
	Indeed the decode command reports tighter timings.
	I tested using nand torture on 16MiB and 32MiB sizes.

U-Boot> nand info

Device 0: nand0, sector size 256 KiB
  Manufacturer  MACRONIX
  Model         MX30LF4G28AD
  Device size        512 MiB
  Page size         4096 b
  OOB size           256 b
  Erase size      262144 b
  ecc strength         8 bits
  ecc step size      512 b
  subpagesize       4096 b
  options       0x40004200
  bbt options   0x00028000

U-Boot> hsmc decode

mck clock rate: 200000000

HSMC_SETUP3:    0x00000002
HSMC_PULSE3:    0x07040703
HSMC_CYCLE3:    0x00070007
HSMC_TIMINGS3:  0x880402f2
HSMC_MODE3:     0x001f0003
NCS_RD: setup: 0 (0 ns), pulse: 7 (35 ns), hold: 0 (0 ns), cycle: 7 (35 ns)
   NRD: setup: 0 (0 ns), pulse: 4 (20 ns), hold: 3 (15 ns), cycle: 7 (35 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)
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

U-Boot> nand torture 0x800000 0x1000000

NAND torture: device 0 offset 0x800000 size 0x1000000 (block size 0x40000)
 Passed: 64, failed: 0

U-Boot> nand torture 0x800000 0x2000000

NAND torture: device 0 offset 0x800000 size 0x2000000 (block size 0x40000)
 Passed: 128, failed: 0

Tested-by: Mihai Sain <mihai.sain@microchip.com>

Best regards,
Mihai Sain