Re: [PATCH v2] mtd: nand: raw: atmel: Respect tAR, tCLR in read setup timing

[Alexander Dahl <ada@thorsis.com>]

Hei hei,

Am Thu, Aug 21, 2025 at 02:00:57PM +0200 schrieb A. Sverdlin:
> From: Alexander Sverdlin <alexander.sverdlin@siemens.com>
> 
> Having setup time 0 violates tAR, tCLR of some chips, for instance
> TOSHIBA TC58NVG2S3ETAI0 cannot be detected successfully (first ID byte
> being read duplicated, i.e. 98 98 dc 90 15 76 14 03 instead of
> 98 dc 90 15 76 ...).
> 
> Atmel Application Notes postulated 1 cycle NRD_SETUP without explanation
> [1], but it looks more appropriate to just calculate setup time properly.
> 
> [1] Link: https://ww1.microchip.com/downloads/aemDocuments/documents/MPU32/ApplicationNotes/ApplicationNotes/doc6255.pdf
> Cc: stable@vger.kernel.org
> Fixes: f9ce2eddf176 ("mtd: nand: atmel: Add ->setup_data_interface() hooks")
> Signed-off-by: Alexander Sverdlin <alexander.sverdlin@siemens.com>

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

Threw this on top of 6.12.39-rt11 and tested on two custom platforms
both with a Spansion S34ML02G1 SLC 2GBit flash chip, but with
different SoCs (sama5d2, sam9x60).  We had difficulties with the
timing of those NAND flash chips in the past and I wanted to make sure
this patch does not break our setup.  Seems fine in a quick test,
reading and writing and reading back is successful.

Kernel log output is unobtrusive, here for the first device:

  [  +0.001100] AT91: Detected SoC family: sam9x60
  [  +0.000024] AT91: Detected SoC: sam9x60 64MiB DDR2 SiP, revision 2
  …
  [  +0.011069] atmel-nand-controller 10000000.ebi:nand-controller: using dma0chan5 for DMA transfers
  [  +0.000836] nand: device found, Manufacturer ID: 0x01, Chip ID: 0xda
  [  +0.000031] nand: AMD/Spansion S34ML02G1
  [  +0.000011] nand: 256 MiB, SLC, erase size: 128 KiB, page size: 2048, OOB size: 64
  [  +0.051629] Bad block table found at page 131008, version 0xFF
  [  +0.005350] Bad block table found at page 130944, version 0xFF
  [  +0.000479] 5 fixed-partitions partitions found on MTD device atmel_nand
  [  +0.000047] Creating 5 MTD partitions on "atmel_nand":
  [  +0.000025] 0x000000000000-0x000000040000 : "at91bootstrap"
  [  +0.001104] 0x000000040000-0x000000140000 : "u-boot"
  [  +0.001063] 0x000000140000-0x000000180000 : "U-Boot Env"
  [  +0.026887] 0x000000180000-0x0000001c0000 : "U-Boot Env Redundant"
  [  +0.001022] 0x000000200000-0x00000ff00000 : "UBI"
  …
  [  +0.043872] ubi0: attaching mtd4
  [  +0.932080] ubi0: scanning is finished
  [  +0.032686] ubi0: attached mtd4 (name "UBI", size 253 MiB)
  [  +0.000048] ubi0: PEB size: 131072 bytes (128 KiB), LEB size: 126976 bytes
  [  +0.000022] ubi0: min./max. I/O unit sizes: 2048/2048, sub-page size 2048
  [  +0.000018] ubi0: VID header offset: 2048 (aligned 2048), data offset: 4096
  [  +0.000018] ubi0: good PEBs: 2024, bad PEBs: 0, corrupted PEBs: 0
  [  +0.000017] ubi0: user volume: 4, internal volumes: 1, max. volumes count: 128
  [  +0.000017] ubi0: max/mean erase counter: 7/3, WL threshold: 4096, image sequence number: 1905565080
  [  +0.000023] ubi0: available PEBs: 2, total reserved PEBs: 2022, PEBs reserved for bad PEB handling: 40
  [  +0.000043] ubi0: background thread "ubi_bgt0d" started, PID 47

And for the second:

  [  +0.001519] AT91: Detected SoC family: sama5d2
  [  +0.000030] AT91: Detected SoC: sama5d27c 128MiB SiP, revision 2
  …
  [  +0.002585] atmel-nand-controller 10000000.ebi:nand-controller: using dma0chan5 for DMA transfers
  [  +0.036590] nand: device found, Manufacturer ID: 0x01, Chip ID: 0xda
  [  +0.000044] nand: AMD/Spansion S34ML02G1
  [  +0.000017] nand: 256 MiB, SLC, erase size: 128 KiB, page size: 2048, OOB size: 64
  [  +0.001571] Bad block table found at page 131008, version 0xFF
  [  +0.001249] Bad block table found at page 130944, version 0xFF
  [  +0.000572] 6 cmdlinepart partitions found on MTD device atmel_nand
  [  +0.000037] Creating 6 MTD partitions on "atmel_nand":
  [  +0.000026] 0x000000000000-0x000000040000 : "bootstrap"
  [  +0.001496] 0x000000040000-0x000000100000 : "uboot"
  [  +0.031417] 0x000000100000-0x000000140000 : "env1"
  [  +0.013718] 0x000000140000-0x000000180000 : "env2"
  [  +0.001276] 0x000000180000-0x000000200000 : "reserved"
  [  +0.001209] 0x000000200000-0x000010000000 : "UBI"
  …
  [  +0.055885] ubi0: attaching mtd5
  [  +1.468075] ubi0: scanning is finished
  [  +0.047323] ubi0: attached mtd5 (name "UBI", size 254 MiB)
  [  +0.000065] ubi0: PEB size: 131072 bytes (128 KiB), LEB size: 126976 bytes
  [  +0.000022] ubi0: min./max. I/O unit sizes: 2048/2048, sub-page size 2048
  [  +0.000017] ubi0: VID header offset: 2048 (aligned 2048), data offset: 4096
  [  +0.000017] ubi0: good PEBs: 2028, bad PEBs: 4, corrupted PEBs: 0
  [  +0.000016] ubi0: user volume: 4, internal volumes: 1, max. volumes count: 128
  [  +0.000018] ubi0: max/mean erase counter: 6/3, WL threshold: 4096, image sequence number: 1534026892
  [  +0.000022] ubi0: available PEBs: 0, total reserved PEBs: 2028, PEBs reserved for bad PEB handling: 36
  [  +0.000050] ubi0: background thread "ubi_bgt0d" started, PID 49

Thanks and greets
Alex

> ---
> v2:
> - Cc'ed stable
> - reformatted atmel_smc_cs_conf_set_setup() call
> - rebased onto mtd/fixes
> 
>  drivers/mtd/nand/raw/atmel/nand-controller.c | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c b/drivers/mtd/nand/raw/atmel/nand-controller.c
> index dedcca87defc7..ad0eff385e123 100644
> --- a/drivers/mtd/nand/raw/atmel/nand-controller.c
> +++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
> @@ -1377,14 +1377,24 @@ static int atmel_smc_nand_prepare_smcconf(struct atmel_nand *nand,
>  	if (ret)
>  		return ret;
>  
> +	/*
> +	 * Read setup timing depends on the operation done on the NAND:
> +	 *
> +	 * NRD_SETUP = max(tAR, tCLR)
> +	 */
> +	timeps = max(conf->timings.sdr.tAR_min, conf->timings.sdr.tCLR_min);
> +	ncycles = DIV_ROUND_UP(timeps, mckperiodps);
> +	totalcycles += ncycles;
> +	ret = atmel_smc_cs_conf_set_setup(smcconf, ATMEL_SMC_NRD_SHIFT, ncycles);
> +	if (ret)
> +		return ret;
> +
>  	/*
>  	 * The read cycle timing is directly matching tRC, but is also
>  	 * dependent on the setup and hold timings we calculated earlier,
>  	 * which gives:
>  	 *
> -	 * NRD_CYCLE = max(tRC, NRD_PULSE + NRD_HOLD)
> -	 *
> -	 * NRD_SETUP is always 0.
> +	 * NRD_CYCLE = max(tRC, NRD_SETUP + NRD_PULSE + NRD_HOLD)
>  	 */
>  	ncycles = DIV_ROUND_UP(conf->timings.sdr.tRC_min, mckperiodps);
>  	ncycles = max(totalcycles, ncycles);
> -- 
> 2.50.1
> 
> 
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/