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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <87ikkcou0k.fsf@bootlin.com>
Date: Tue, 01 Jul 2025 16:39:07 +0200
From: Miquel Raynal <miquel.raynal@...tlin.com>
To: "A. Sverdlin" <alexander.sverdlin@...mens.com>
Cc: Boris Brezillon <bbrezillon@...nel.org>,  linux-mtd@...ts.infradead.org,
  Richard Weinberger <richard@....at>,  Vignesh Raghavendra
 <vigneshr@...com>,  Nicolas Ferre <nicolas.ferre@...rochip.com>,
  Alexandre Belloni <alexandre.belloni@...tlin.com>,  Claudiu Beznea
 <claudiu.beznea@...on.dev>,  linux-arm-kernel@...ts.infradead.org,
  linux-kernel@...r.kernel.org
Subject: Re: [PATCH] mtd: nand: raw: atmel: Respect tAR, tCLR in read setup
 timing

Hello Alexander,

On 01/07/2025 at 15:33:28 +02, "A. Sverdlin" <alexander.sverdlin@...mens.com> wrote:

> From: Alexander Sverdlin <alexander.sverdlin@...mens.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.
>
> Without the fix we've measured -2ns tAR delay (REn asserted before ALE
> deassert!); with the fix -- 60ns (subject to module clock).
>
> [1] Link: https://ww1.microchip.com/downloads/aemDocuments/documents/MPU32/ApplicationNotes/ApplicationNotes/doc6255.pdf
> Fixes: f9ce2eddf176 ("mtd: nand: atmel: Add ->setup_data_interface() hooks")
> Signed-off-by: Alexander Sverdlin <alexander.sverdlin@...mens.com>

Please add Cc: stable

> ---
>  drivers/mtd/nand/raw/atmel/nand-controller.c | 17 ++++++++++++++---
>  1 file changed, 14 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..844df72f45063 100644
> --- a/drivers/mtd/nand/raw/atmel/nand-controller.c
> +++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
> @@ -1377,14 +1377,25 @@ 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);

Nit as you'll need to resend anyway :), this can be on the previous line.

Otherwise lgtm.

Thanks,
Miquèl