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/Application... Cc: stable@vger.kernel.org Fixes: f9ce2eddf176 ("mtd: nand: atmel: Add ->setup_data_interface() hooks") Signed-off-by: Alexander Sverdlin alexander.sverdlin@siemens.com --- 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);
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/Application... 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)-- 2.50.1
Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/
Hi Alexander,
On Mon, 2025-08-25 at 13:02 +0200, Alexander Dahl wrote:
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/Application... 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.
thank you for your feedback!
Do you see an opportunity to drop the downstream timing quirks with my patch? I actually have another patch related to timings, but it's based on code-review only and the theoretical issue never manifested itself in practice on our side... (it's about missing ndelay at the end of atmel_smc_nand_exec_instr())
Regards,
Hello Alexander,
Am Mon, Aug 25, 2025 at 12:21:24PM +0000 schrieb Sverdlin, Alexander:
Hi Alexander,
On Mon, 2025-08-25 at 13:02 +0200, Alexander Dahl wrote:
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/Application... 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.
thank you for your feedback!
Do you see an opportunity to drop the downstream timing quirks with my patch?
Which downstream do you refer to?
I actually have another patch related to timings, but it's based on code-review only and the theoretical issue never manifested itself in practice on our side...
I also have another one, for which fixes got merged in at91bootstrap and U-Boot already, but I never found the time to upstream it on Linux. It's related to pulse timing.
(it's about missing ndelay at the end of atmel_smc_nand_exec_instr())
That does not ring a bell here, sorry.
Greets Alex
Hi Alexander,
On Mon, 2025-08-25 at 15:22 +0200, Alexander Dahl wrote:
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.
thank you for your feedback!
Do you see an opportunity to drop the downstream timing quirks with my patch?
Which downstream do you refer to?
That's how I understood the phrase above, that some adjustments are still required on your side additionally to standard timings. Sorry for the confusion, if it turns out to be a misunderstanding on my side!
Hello Alexander,
Am Mon, Aug 25, 2025 at 01:41:05PM +0000 schrieb Sverdlin, Alexander:
Hi Alexander,
On Mon, 2025-08-25 at 15:22 +0200, Alexander Dahl wrote:
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.
thank you for your feedback!
Do you see an opportunity to drop the downstream timing quirks with my patch?
Which downstream do you refer to?
That's how I understood the phrase above, that some adjustments are still required on your side additionally to standard timings. Sorry for the confusion, if it turns out to be a misunderstanding on my side!
I don't think your patch is sufficient to solve our problems with sam9x60 and S34ML02G1. Different parts of the timings are affected. You changed NRD_SETUP while I changed NRD_PULSE. I just wanted to make sure both patches do not interfere.
For reference, these are the changes in at91bootstrap and u-boot I referred to:
https://github.com/linux4sam/at91bootstrap/issues/174 https://github.com/linux4sam/at91bootstrap/commit/e2dfd8141d00613a37acee66ef...
https://source.denx.de/u-boot/u-boot/-/commit/344e2f2cd4a407f847b301804f37d0...
My problem might be fixed with commit f552a7c7e0a1 ("mtd: rawnand: atmel: set pmecc data setup time"), but I did not have time to test that assumption.
Sorry for the confusion.
Greets Alex
On Thu, 21 Aug 2025 14:00:57 +0200, A. Sverdlin wrote:
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.
[...]
Applied to mtd/fixes, thanks!
[1/1] mtd: nand: raw: atmel: Respect tAR, tCLR in read setup timing commit: fd779eac2d659668be4d3dbdac0710afd5d6db12
Patche(s) should be available on mtd/linux.git and will be part of the next PR (provided that no robot complains by then).
Kind regards, Miquèl
linux-stable-mirror@lists.linaro.org
-
A. Sverdlin -
Alexander Dahl -
Miquel Raynal -
Sverdlin, Alexander