linux-kernel - Re: [PATCH 1/3] mtd: spi-nor: sfdp: introduce smpt_read

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <59c3d45b-13ff-4393-a87c-f0504f224acb@linaro.org>
Date: Thu, 23 Oct 2025 08:25:40 +0100
From: Tudor Ambarus <tudor.ambarus@...aro.org>
To: Takahiro Kuwano <tkuw584924@...il.com>,
 Pratyush Yadav <pratyush@...nel.org>, Michael Walle <mwalle@...nel.org>,
 Miquel Raynal <miquel.raynal@...tlin.com>,
 Richard Weinberger <richard@....at>, Vignesh Raghavendra <vigneshr@...com>,
 Marek Vasut <marek.vasut+renesas@...lbox.org>
Cc: linux-mtd@...ts.infradead.org, linux-kernel@...r.kernel.org,
 Takahiro Kuwano <Takahiro.Kuwano@...ineon.com>
Subject: Re: [PATCH 1/3] mtd: spi-nor: sfdp: introduce smpt_read_dummy fixup
 hook

Hi, Takahiro,

On 10/22/25 11:07 AM, Takahiro Kuwano wrote:
> SMPT contains config detection info that descibes opcode, address, and

typo: describes> dummy cycles to read sector map config. The dummy cycles parameter can
> be 'variable' and should be determined in device specific fixup hook.

This doesn't describe why we need the hook. I'd advise to describe what
value has nor->read_dummy before the fixup hook (zero you said), why the
zero value does always fit some flashes like the IFX one, which for that
particular read reg command it always needs 8 dummy cycles. This also
reveals that the SMPT table on that flash instead of defining
SMPT_CMD_READ_DUMMY_IS_VARIABLE is should have actually used the number of
needed dummy cycles: 8.

Please adapt and add something around these lines, otherwise in 5 years
from now we'll forget what was this about.

The patch looks okay to me.
Cheers,
ta

> 
> Inroduce smpt_read_dummy() in struct spi_nor_fixups. It is called when
> the dummy cycle field in SMPT config detection is 'varialble'.
> 
> Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano@...ineon.com>
> ---
>  drivers/mtd/spi-nor/core.h |  3 +++
>  drivers/mtd/spi-nor/sfdp.c | 18 ++++++++++++++++--
>  2 files changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
> index ceff412f7d65ab7b856795ca5c092cddbf598cd6..5ad46d95d09cc9d527f71579a71eed210e726f68 100644
> --- a/drivers/mtd/spi-nor/core.h
> +++ b/drivers/mtd/spi-nor/core.h
> @@ -409,6 +409,8 @@ struct spi_nor_flash_parameter {
>   *                flash parameters when information provided by the flash_info
>   *                table is incomplete or wrong.
>   * @post_bfpt: called after the BFPT table has been parsed
> + * @smpt_read_dummy: called during SMPT table is being parsed. Used to fix the
> + *                   number of dummy cycles in read register ops.
>   * @post_sfdp: called after SFDP has been parsed (is also called for SPI NORs
>   *             that do not support RDSFDP). Typically used to tweak various
>   *             parameters that could not be extracted by other means (i.e.
> @@ -426,6 +428,7 @@ struct spi_nor_fixups {
>  	int (*post_bfpt)(struct spi_nor *nor,
>  			 const struct sfdp_parameter_header *bfpt_header,
>  			 const struct sfdp_bfpt *bfpt);
> +	void (*smpt_read_dummy)(const struct spi_nor *nor, u8 *read_dummy);
>  	int (*post_sfdp)(struct spi_nor *nor);
>  	int (*late_init)(struct spi_nor *nor);
>  };
> diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
> index 21727f9a4ac6926080a116e30830c9533122fdad..9a47dcaca06ae2ad85ac8503658083b1d56d8b96 100644
> --- a/drivers/mtd/spi-nor/sfdp.c
> +++ b/drivers/mtd/spi-nor/sfdp.c
> @@ -699,6 +699,17 @@ static u8 spi_nor_smpt_addr_nbytes(const struct spi_nor *nor, const u32 settings
>  	}
>  }
>  
> +static void spi_nor_smpt_read_dummy_fixups(const struct spi_nor *nor,
> +					   u8 *read_dummy)
> +{
> +	if (nor->manufacturer && nor->manufacturer->fixups &&
> +	    nor->manufacturer->fixups->smpt_read_dummy)
> +		nor->manufacturer->fixups->smpt_read_dummy(nor, read_dummy);
> +
> +	if (nor->info->fixups && nor->info->fixups->smpt_read_dummy)
> +		nor->info->fixups->smpt_read_dummy(nor, read_dummy);
> +}
> +
>  /**
>   * spi_nor_smpt_read_dummy() - return the configuration detection command read
>   *			       latency, in clock cycles.
> @@ -711,8 +722,11 @@ static u8 spi_nor_smpt_read_dummy(const struct spi_nor *nor, const u32 settings)
>  {
>  	u8 read_dummy = SMPT_CMD_READ_DUMMY(settings);
>  
> -	if (read_dummy == SMPT_CMD_READ_DUMMY_IS_VARIABLE)
> -		return nor->read_dummy;
> +	if (read_dummy == SMPT_CMD_READ_DUMMY_IS_VARIABLE) {
> +		read_dummy = nor->read_dummy;
> +		spi_nor_smpt_read_dummy_fixups(nor, &read_dummy);
> +	}
> +
>  	return read_dummy;
>  }
>  
>