linux-kernel - Re: [PATCH 3/3] mtd: spi-nor: spansion: SMPT fixups for S25FS-S

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 for Android: free password hash cracker in your pocket

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

Message-ID: <b8338932-de87-4328-9d33-d4a8268fb03d@linaro.org>
Date: Thu, 23 Oct 2025 08:43:30 +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 3/3] mtd: spi-nor: spansion: SMPT fixups for S25FS-S



On 10/22/25 11:07 AM, Takahiro Kuwano wrote:
> S25FS-S family supports SMPT that helps to detect sector layout settings
> in configuration registers, but some of parameters in the table are
> wrong or undetermined so the fixups below are required.
> 
> Read Any Register op is used to read configuration registers that
> related to sector map. The op requires 8 cycles latency by default.
> Implement smpt_read_dummy() to set correct dummy cycles.
> 
> Map ID is structured by combination of CR3NV[3], CR1NV[2], and CR3NV[1].
> However, in S25FS512S, CR3NV[1] is RFU and always 0, while map IDs
> defined in the table assume it is always 1. Implement smpt_map_id() to
> fix map ID for S25FS512S. Other densities in S25FS-S family (256Mb and
> 128Mb) don't need this fix as CR3NV[1] in those chips is configurable
> and map IDs are correctly defined in SMPT.
> 
> Co-developed-by: Marek Vasut <marek.vasut+renesas@...lbox.org>
> Signed-off-by: Marek Vasut <marek.vasut+renesas@...lbox.org>
> Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano@...ineon.com>
> ---
>  drivers/mtd/spi-nor/spansion.c | 34 ++++++++++++++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
> 
> diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c
> index a0296c871634678be509cb30d26e18debff3066d..89e85eba2c31a4d74dcbfeffdeff7139b6e0f6b1 100644
> --- a/drivers/mtd/spi-nor/spansion.c
> +++ b/drivers/mtd/spi-nor/spansion.c
> @@ -785,8 +785,42 @@ s25fs_s_nor_post_bfpt_fixups(struct spi_nor *nor,
>  	return 0;
>  }
>  
> +static void s25fs_s_nor_smpt_read_dummy(const struct spi_nor *nor,
> +					u8 *read_dummy)
> +{
> +	/*
> +	 * The configuration detection dwords in S25FS-S SMPT has 65h as
> +	 * command instruction and 'variable' as configuration detection command
> +	 * latency. Set 8 dummy cycles as it is factory default for 65h (read
> +	 * any register) op.
> +	 */
> +	*read_dummy = 8;
> +}
> +
> +static void s25fs_s_nor_smpt_map_id_dummy(const struct spi_nor *nor, u8 *map_id)
> +{
> +	/*
> +	 * The S25FS512S chip datasheet rev.O Table 71 on page 153
> +	 * JEDEC Sector Map Parameter Dword-6 Config. Detect-3 does
> +	 * use CR3NV bit 1 to discern 64kiB/256kiB uniform sectors

you need to update the comment, there's no 64 Kib uniform sectors in
the flash, the comment is misleading. You have:
- Hybrid sector option
• Physical set of eight 4-KB sectors and one 224-KB sector at the top or bottom of address space with all
remaining sectors of 256-KB
- Uniform sector option
• Uniform 256-KB blocks> +	 * device configuration, however according to section 7.5.5.1
> +	 * Configuration Register 3 Non-volatile (CR3NV) page 61, the
> +	 * CR3NV bit 1 is RFU Reserved for Future Use, and is set to
> +	 * 0 on newly manufactured devices, which means 64kiB sectors.
here too> +	 * Since the device does not support 64kiB uniform sectors in
again> +	 * any configuration, parsing SMPT table cannot find a valid
> +	 * sector map entry and fails. Fix this up by setting SMPT

by overwriting the CR3NV[1] value to 1, as the table expects.


The patch looks good, with these minor things addressed. Please fix them
and add
Reviewed-by: Tudor Ambarus <tudor.ambarus@...aro.org>

Thanks for taking care of this, Takahiro!
Cheers,
ta

> +	 * configuration index bit 0, which is populated exactly by
> +	 * the CR3NV bit 1 being 1.
> +	 */
> +	if (nor->params->size == SZ_64M)
> +		*map_id |= BIT(0);
> +}
> +
>  static const struct spi_nor_fixups s25fs_s_nor_fixups = {
>  	.post_bfpt = s25fs_s_nor_post_bfpt_fixups,
> +	.smpt_read_dummy = s25fs_s_nor_smpt_read_dummy,
> +	.smpt_map_id = s25fs_s_nor_smpt_map_id_dummy,
>  };
>  
>  static const struct flash_info spansion_nor_parts[] = {
>