[PATCH] mtd: spi-nor: spansion: Add SMPT fixup for S25FS512S
Tudor Ambarus
tudor.ambarus at linaro.org
Thu Oct 16 21:39:07 PDT 2025
On 10/17/25 4:44 AM, Takahiro Kuwano wrote:
> Hi Tudor,
Hi!
>
> On 10/14/2025 4:22 PM, Takahiro Kuwano wrote:
>> Hi Tudor,
>>
>> On 10/10/2025 5:34 PM, Tudor Ambarus wrote:
>>> Hi, Takahiro,
>>>
>>>>>>>>> On 9/14/24 11:08 PM, Marek Vasut wrote:
>>>>>>>>>> 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
>>>>>>>>>> and 256kiB uniform sectors 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. Since the device does not
>>>>>>>>>> support 64kiB uniform sectors in any configuration, parsing SMPT table
>>>>>>>>>> cannot find a valid sector map entry and fails.
>>>>>>>>>>
>>>>>>>>>> Fix this up by setting SMPT configuration index bit 0, which is populated
>>>>>>>>>> exactly by the CR3NV bit 1 being 1. This is implemented via new .post_smpt
>>>>>>>>>> fixup hook and a wrapper function. The only implementation of the hook is
>>>>>>>>>> currently specific to S25FS512S.
>>>>>>>>>>
>>>>>>>>>> This fixes the following failure on S25FS512S:
>>>>>>>>>> spi-nor spi0.0: Failed to parse optional parameter table: ff81 (err=-22)
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Marek Vasut <marek.vasut+renesas at mailbox.org>
>>>>>>>>>> ---
>>>>>>>>>> Cc: Geert Uytterhoeven <geert+renesas at glider.be>
>>>>>>>>>> Cc: Michael Walle <mwalle at kernel.org>
>>>>>>>>>> Cc: Miquel Raynal <miquel.raynal at bootlin.com>
>>>>>>>>>> Cc: Pratyush Yadav <pratyush at kernel.org>
>>>>>>>>>> Cc: Richard Weinberger <richard at nod.at>
>>>>>>>>>> Cc: Tudor Ambarus <tudor.ambarus at linaro.org>
>>>>>>>>>> Cc: Vignesh Raghavendra <vigneshr at ti.com>
>>>>>>>>>> Cc: linux-mtd at lists.infradead.org
>>>>>>>>>> Cc: linux-renesas-soc at vger.kernel.org
>>>>>>>>>> ---
>>>>>>>>>> drivers/mtd/spi-nor/core.c | 17 +++++++++++++++++
>>>>>>>>>> drivers/mtd/spi-nor/core.h | 5 +++++
>>>>>>>>>> drivers/mtd/spi-nor/sfdp.c | 4 ++++
>>>>>>>>>> drivers/mtd/spi-nor/spansion.c | 27 ++++++++++++++++++++++++++-
>>>>>>>>>> 4 files changed, 52 insertions(+), 1 deletion(-)
>>>>>>>>>>
>>>>>>>>>> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
>>>>>>>>>> index 9d6e85bf227b..ca65f36e5638 100644
>>>>>>>>>> --- a/drivers/mtd/spi-nor/core.c
>>>>>>>>>> +++ b/drivers/mtd/spi-nor/core.c
>>>>>>>>>> @@ -2405,6 +2405,23 @@ int spi_nor_post_bfpt_fixups(struct spi_nor *nor,
>>>>>>>>>> return 0;
>>>>>>>>>> }
>>>>>>>>>> +int spi_nor_post_smpt_fixups(struct spi_nor *nor, u8 *smpt)
>>>>>>>>>> +{
>>>>>>>>>> + int ret;
>>>>>>>>>> +
>>>>>>>>>> + if (nor->manufacturer && nor->manufacturer->fixups &&
>>>>>>>>>> + nor->manufacturer->fixups->post_smpt) {
>>>>>>>>>> + ret = nor->manufacturer->fixups->post_smpt(nor, smpt);
>>>>>>>>>> + if (ret)
>>>>>>>>>> + return ret;
>>>>>>>>>> + }
>>>>>>>>>> +
>>>>>>>>>> + if (nor->info->fixups && nor->info->fixups->post_smpt)
>>>>>>>>>> + return nor->info->fixups->post_smpt(nor, smpt);
>>>>>>>>>> +
>>>>>>>>>> + return 0;
>>>>>>>>>> +}
>>>>>>>>>> +
>>>>>>>>>> static int spi_nor_select_read(struct spi_nor *nor,
>>>>>>>>>> u32 shared_hwcaps)
>>>>>>>>>> {
>>>>>>>>>> diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
>>>>>>>>>> index 1516b6d0dc37..d5294424ab9d 100644
>>>>>>>>>> --- a/drivers/mtd/spi-nor/core.h
>>>>>>>>>> +++ b/drivers/mtd/spi-nor/core.h
>>>>>>>>>> @@ -413,6 +413,8 @@ struct spi_nor_flash_parameter {
>>>>>>>>>> * parameters that could not be extracted by other means (i.e.
>>>>>>>>>> * when information provided by the SFDP/flash_info tables are
>>>>>>>>>> * incomplete or wrong).
>>>>>>>>>> + * @post_smpt: update sector map configuration ID selector according to
>>>>>>>>>> + * chip-specific quirks.
>>>>>>>>>> * @late_init: used to initialize flash parameters that are not declared in the
>>>>>>>>>> * JESD216 SFDP standard, or where SFDP tables not defined at all.
>>>>>>>>>> * Will replace the default_init() hook.
>>>>>>>>>> @@ -426,6 +428,7 @@ struct spi_nor_fixups {
>>>>>>>>>> const struct sfdp_parameter_header *bfpt_header,
>>>>>>>>>> const struct sfdp_bfpt *bfpt);
>>>>>>>>>> int (*post_sfdp)(struct spi_nor *nor);
>>>>>>>>>> + int (*post_smpt)(struct spi_nor *nor, u8 *smpt);
>>>>>>>>>> int (*late_init)(struct spi_nor *nor);
>>>>>>>>>> };
>>>>>>>>>> @@ -660,6 +663,8 @@ int spi_nor_post_bfpt_fixups(struct spi_nor *nor,
>>>>>>>>>> const struct sfdp_parameter_header *bfpt_header,
>>>>>>>>>> const struct sfdp_bfpt *bfpt);
>>>>>>>>>> +int spi_nor_post_smpt_fixups(struct spi_nor *nor, u8 *stmp);
>>>>>>>>>> +
>>>>>>>>>> void spi_nor_init_default_locking_ops(struct spi_nor *nor);
>>>>>>>>>> void spi_nor_try_unlock_all(struct spi_nor *nor);
>>>>>>>>>> void spi_nor_set_mtd_locking_ops(struct spi_nor *nor);
>>>>>>>>>> diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
>>>>>>>>>> index 5b1117265bd2..542c775918ad 100644
>>>>>>>>>> --- a/drivers/mtd/spi-nor/sfdp.c
>>>>>>>>>> +++ b/drivers/mtd/spi-nor/sfdp.c
>>>>>>>>>> @@ -765,6 +765,10 @@ static const u32 *spi_nor_get_map_in_use(struct spi_nor *nor, const u32 *smpt,
>>>>>>>>>> map_id = map_id << 1 | !!(*buf & read_data_mask);
>>>>>>>>>> }
>>>>>>>>>> + err = spi_nor_post_smpt_fixups(nor, &map_id);
>>>>>>>>>> + if (err)
>>>>>>>>>> + return ERR_PTR(err);
>>>>>>>>>> +
>>>>>>>>>> /*
>>>>>>>>>> * If command descriptors are provided, they always precede map
>>>>>>>>>> * descriptors in the table. There is no need to start the iteration
>>>>>>>>>> diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c
>>>>>>>>>> index d6c92595f6bc..d446d12371e1 100644
>>>>>>>>>> --- a/drivers/mtd/spi-nor/spansion.c
>>>>>>>>>> +++ b/drivers/mtd/spi-nor/spansion.c
>>>>>>>>>> @@ -757,6 +757,31 @@ static const struct spi_nor_fixups s25fs_s_nor_fixups = {
>>>>>>>>>> .post_bfpt = s25fs_s_nor_post_bfpt_fixups,
>>>>>>>>>> };
>>>>>>>>>> +static int s25fs512s_nor_post_smpt_fixups(struct spi_nor *nor, u8 *smpt)
>>>>>>>>>> +{
>>>>>>>>>> + /*
>>>>>>>>>> + * 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
>>>>>>>>>> + * 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.
>>>>>>>>>> + * Since the device does not support 64kiB uniform sectors in
>>>>>>>>>> + * any configuration, parsing SMPT table cannot find a valid
>>>>>>>>>> + * sector map entry and fails. Fix this up by setting SMPT
>>>>>>>>>> + * configuration index bit 0, which is populated exactly by
>>>>>>>>>> + * the CR3NV bit 1 being 1.
>>>>>>>>>> + */
>>>>>>>>>> + *smpt |= BIT(0);
>>>>>>>>>
>>>>>>>>> Please help me understand this. Maybe a link to your revision of
>>>>>>>>> datasheet would help me.
>>>>>>>>
>>>>>>>> https://www.infineon.com/assets/row/public/documents/10/49/infineon-s25fs512s-512-mb-1-datasheet-en.pdf
>>>>>>>>
>>>>>>>> SMPT values:
>>>>>>>>
>>>>>>>> i=0 smpt[i]=08ff65fc
>>>>>>>> i=1 smpt[i]=00000004
>>>>>>>> i=2 smpt[i]=04ff65fc
>>>>>>>> i=3 smpt[i]=00000002
>>>>>>>> i=4 smpt[i]=02ff65fd
>>>>>>>> i=5 smpt[i]=00000004
>>>>>>>> i=6 smpt[i]=ff0201fe
>>>>>>>> i=7 smpt[i]=00007ff1
>>>>>>>> i=8 smpt[i]=00037ff4
>>>>>>>> i=9 smpt[i]=03fbfff4
>>>>>>>> i=10 smpt[i]=ff0203fe
>>>>>>>> i=11 smpt[i]=03fbfff4
>>>>>>>> i=12 smpt[i]=00037ff4
>>>>>>>> i=13 smpt[i]=00007ff1
>>>>>>>> i=14 smpt[i]=ff0005ff
>>>>>>>> i=15 smpt[i]=03fffff4
>>>>>>>>
>>>>>>>
>>>>>>> thanks!
>>>>>>>
>>>>>>>>> In the flash datasheets that I see, there shall
>>>>>>>>> be a "Sector Map Parameter Table Notes" where a "Sector Map Parameter"
>>>>>>>>> table is described showing an Index Value constructed by the CRxNV[y]
>>>>>>>>> return values. That index value is the map_id in the code.
>>>>>>>>>
>>>>>>>>> By reading your description I understand CR3NV[1] has value zero as it
>>>>>>>>> is marked as RFU, but at the same time that bit is expected in SMPT to
>>>>>>>>> always have value 1. That's why datasheets like this one [1] in their
>>>>>>>>> "Table 70. Sector Map Parameter" do not describe CR3NV[1] at all, and
>>>>>>>>> define the index value as CR3NV[3] << 2 | CR1NV[2] << 1 | 1.
>>>>>>>>
>>>>>>>> Where does this last part "define the index value as CR3NV[3] << 2 | CR1NV[2] << 1 | 1" come from ?
>>>>>>>
>>>>>>> In your datasheet flavor, from "Table 70 Sector map parameter", page 152,
>>>>>>> "Index Value" column. I deduced the formula by using the CR3NV[3] and
>>>>>>> CR1NV[2] values. The datasheet says:
>>>>>>>
>>>>>>> "When more than one configuration bit must be read, all the bits are
>>>>>>> concatenated into an index value that is used to select the current address map."
>>>>>>>
>>>>>>> and that the "the following MSb to LSb order to form the configuration map
>>>>>>> index value:
>>>>>>> • CR3NV[3] — 0 = Hybrid Architecture, 1 = Uniform Architecture
>>>>>>> • CR1NV[2] — 0 = 4 KB parameter sectors at bottom, 1 = 4 KB sectors at top
>>>>>>> "
>>>>>>>
>>>>>> In old datasheet, Table 70 had CR3NV[1] column and the description about index
>>>>>> value had another bullet:
>>>>>> • CR3NV[1] — 1 = 256 kB uniform sector size
>>>>>> These were removed when the default CRNV3[1] value was changed from 1 to 0.
>>>>>> As Marek pointed out, SMPT still involves CR3NV[1] expecting the value is
>>>>>> always 1.
>>>
>>> If CR3NV[1] is always 1 even in the old datasheets, then replacing
>>> CR3NV[3] << 2 | CR1NV[2] << 1 | CR3NV[1]
>>> with
>>> CR3NV[3] << 2 | CR1NV[2] << 1 | 1 (CR3NV[1] was always 1 anyway)
>>> for the index value/map_id is backward compatible and okay, even for the
>>> older flashes.
>>>
>>> Then Marek's idea of updating the map_id via a hook is sound. If we're going
>>> to update just the map id, I'd rename spi_nor_post_smpt_fixups() to
>>> spi_nor_smpt_map_id_fixup().
>>>
>>>>>>
>>>>>>> The "Index Value" shall be the map_id that you passed in the code:
>>>>>>> spi_nor_post_smpt_fixups(nor, &map_id);
>>>>>>>
>>>>>>> Can you please print the map_id value that you obtain without updating it?
>>>>>>> Let's also print the values of CR3NV and CR1NV.
>>>>>>>
>>>>>> I did this in my environment and found another, fundamental issue in SMPT
>>>>>> contents and the way to parse it...
>>>>>>
>>>>>> The SMPT describes Configuration detection command latency as '1111b: variable
>>>>>> latency' and in spi_nor_smpt_read_dummy(),
>>>>>>
>>>>>> if (read_dummy == SMPT_CMD_READ_DUMMY_IS_VARIABLE)
>>>>>> return nor->read_dummy;
>>>>>>
>>>>>> This results in 0 latency.
>>>>>> However, in S25FS-S, register read latency is 8 cycles by default. That means
>> Sorry, this was not accurate. In S25FS-S, register read latency of RDAR(65h) is
>> 8 cycle by default. RDSR(05h)/RDCR(35h) has no latency.
>>
>>>
>>> Is there any other reg interaction in the probe path? What number of dummy cycles
>>> is it using?
>>>
>> Only SMPT parse uses RDAR(65h) that requires dummy cycles.
>>
> How about introducing new fixup like get_smpt_read_dummy() and call it when
> read dummy is variable?
>
> 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)
> + if (read_dummy == SMPT_CMD_READ_DUMMY_IS_VARIABLE) {
> + if (nor->info->fixups && nor->info->fixups->get_smpt_read_dummy)
> + return nor->info->fixups->get_smpt_read_dummy(nor);
> +
> return nor->read_dummy;
> + }
> +
> return read_dummy;
> }
That's okay. You'll also need a fixup for the CR3NV[1] bit.
Cheers,
ta
More information about the linux-mtd
mailing list