* Re: [PATCH] mtd: spi-nor: spansion: Add SMPT fixup for S25FS512S
2025-10-08 6:20 ` Tudor Ambarus
@ 2025-10-08 6:47 ` Tudor Ambarus
2025-10-10 6:01 ` Takahiro Kuwano
2025-10-12 16:19 ` Marek Vasut
2 siblings, 0 replies; 20+ messages in thread
From: Tudor Ambarus @ 2025-10-08 6:47 UTC (permalink / raw)
To: Marek Vasut, linux-mtd, Takahiro Kuwano
Cc: Geert Uytterhoeven, Michael Walle, Miquel Raynal, Pratyush Yadav,
Richard Weinberger, Vignesh Raghavendra, linux-renesas-soc
+ Takahiro, he works actively with IFX flashes.
Maybe we shall add a per vendor MAINTAINERS line, so that relevant people
are added by scripts/get_maintainer.pl.
No other comment after this line.
On 10/8/25 7:20 AM, Tudor Ambarus wrote:
>
>
> On 9/3/25 2:40 AM, Marek Vasut wrote:
>> On 9/23/24 10:54 AM, Tudor Ambarus wrote:
>>
>> Hello Tudor,
>
> Hi, Marek,
>
>>
>> sorry for the late reply.
>
> That's okay.
>
>>
>>> 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@mailbox.org>
>>>> ---
>>>> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
>>>> Cc: Michael Walle <mwalle@kernel.org>
>>>> Cc: Miquel Raynal <miquel.raynal@bootlin.com>
>>>> Cc: Pratyush Yadav <pratyush@kernel.org>
>>>> Cc: Richard Weinberger <richard@nod.at>
>>>> Cc: Tudor Ambarus <tudor.ambarus@linaro.org>
>>>> Cc: Vignesh Raghavendra <vigneshr@ti.com>
>>>> Cc: linux-mtd@lists.infradead.org
>>>> Cc: linux-renesas-soc@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
> "
>
> 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.
>
> (idea for enthusiasts: we can add a debugfs interface to query registers)
>
>>
>>> I assume what you're doing is fine as it shouldn't break backward
>>> compatibility with other older flashes as their CR3NV[1] has value one
>>> anyway. Correct me if I'm wrong.
>
> Now that I revisited the datasheet, your comment "use CR3NV bit 1 to discern
> 64kiB/256kiB uniform sectors" is misleading because a) I understand that
> CR3NV[1] is not involved in the sector map selection (the "Index Value" is),
> and b) the choice as per table 70 is between uniform 256 KB sectors and
> non-uniform map with 4KB sectors at top or bottom and the remaining 256KB.
>
> Let's get those CR3NV and CR1NV values so we decide how to tackle this.
>
> Cheers,
> ta
>
>>
>> I hope so.
>>
>>> Now looking at the code, what we usually do is to save the flash
>>> parameters described by SFDP in nor->params, then amend those parameters
>>> with fixup hooks if that's needed. Here you modify the map_id and then
>>> let the code use it in order to determine the sector_map. Then that
>>> sector_map (which is SMPT data from the table itself) is used to
>>> initialize erase regions. That sector_map can contain wrong data too.
>>
>> By sector_map, do you refer to the "smpt" array ?
>>
>>> I'd suggest saving a nor->params->sector_map then call a
>>> int spi_nor_post_smpt_fixups(struct spi_nor *nor,
>>> const struct sfdp_parameter_header *smpt_header,
>>> const u32 *smpt)
>>> in case spi_nor_get_map_in_use() fails. This way others can update
>>> sector_map as well if that's ever needed. What do you think?
>>
>> This won't work for me, would it ? In my case, I need to patch content of CR3NV register to assure CR3NV bit 1 is well defined. I don't need to patch the sector_map itself.
>
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH] mtd: spi-nor: spansion: Add SMPT fixup for S25FS512S
2025-10-08 6:20 ` Tudor Ambarus
2025-10-08 6:47 ` Tudor Ambarus
@ 2025-10-10 6:01 ` Takahiro Kuwano
2025-10-10 6:12 ` Tudor Ambarus
2025-10-12 16:19 ` Marek Vasut
2 siblings, 1 reply; 20+ messages in thread
From: Takahiro Kuwano @ 2025-10-10 6:01 UTC (permalink / raw)
To: Tudor Ambarus, Marek Vasut, linux-mtd, Takahiro Kuwano
Cc: Geert Uytterhoeven, Michael Walle, Miquel Raynal, Pratyush Yadav,
Richard Weinberger, Vignesh Raghavendra, linux-renesas-soc
Hi,
On 10/8/2025 3:20 PM, Tudor Ambarus wrote:
>
>
> On 9/3/25 2:40 AM, Marek Vasut wrote:
>> On 9/23/24 10:54 AM, Tudor Ambarus wrote:
>>
>> Hello Tudor,
>
> Hi, Marek,
>
>>
>> sorry for the late reply.
>
> That's okay.
>
>>
>>> 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@mailbox.org>
>>>> ---
>>>> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
>>>> Cc: Michael Walle <mwalle@kernel.org>
>>>> Cc: Miquel Raynal <miquel.raynal@bootlin.com>
>>>> Cc: Pratyush Yadav <pratyush@kernel.org>
>>>> Cc: Richard Weinberger <richard@nod.at>
>>>> Cc: Tudor Ambarus <tudor.ambarus@linaro.org>
>>>> Cc: Vignesh Raghavendra <vigneshr@ti.com>
>>>> Cc: linux-mtd@lists.infradead.org
>>>> Cc: linux-renesas-soc@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.
> 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
the SPI controller reads garbage data as CR3NV and CR1NV value. In my particular
environment they are 0xFF as IO1 is pulled up, so map_id is 111b.
Let me think about how we can solve this.
Thanks,
Takahiro
> (idea for enthusiasts: we can add a debugfs interface to query registers)
>
>>
>>> I assume what you're doing is fine as it shouldn't break backward
>>> compatibility with other older flashes as their CR3NV[1] has value one
>>> anyway. Correct me if I'm wrong.
>
> Now that I revisited the datasheet, your comment "use CR3NV bit 1 to discern
> 64kiB/256kiB uniform sectors" is misleading because a) I understand that
> CR3NV[1] is not involved in the sector map selection (the "Index Value" is),
> and b) the choice as per table 70 is between uniform 256 KB sectors and
> non-uniform map with 4KB sectors at top or bottom and the remaining 256KB.
>
> Let's get those CR3NV and CR1NV values so we decide how to tackle this.
>
> Cheers,
> ta
>
>>
>> I hope so.
>>
>>> Now looking at the code, what we usually do is to save the flash
>>> parameters described by SFDP in nor->params, then amend those parameters
>>> with fixup hooks if that's needed. Here you modify the map_id and then
>>> let the code use it in order to determine the sector_map. Then that
>>> sector_map (which is SMPT data from the table itself) is used to
>>> initialize erase regions. That sector_map can contain wrong data too.
>>
>> By sector_map, do you refer to the "smpt" array ?
>>
>>> I'd suggest saving a nor->params->sector_map then call a
>>> int spi_nor_post_smpt_fixups(struct spi_nor *nor,
>>> const struct sfdp_parameter_header *smpt_header,
>>> const u32 *smpt)
>>> in case spi_nor_get_map_in_use() fails. This way others can update
>>> sector_map as well if that's ever needed. What do you think?
>>
>> This won't work for me, would it ? In my case, I need to patch content of CR3NV register to assure CR3NV bit 1 is well defined. I don't need to patch the sector_map itself.
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH] mtd: spi-nor: spansion: Add SMPT fixup for S25FS512S
2025-10-10 6:01 ` Takahiro Kuwano
@ 2025-10-10 6:12 ` Tudor Ambarus
2025-10-10 6:46 ` Takahiro Kuwano
0 siblings, 1 reply; 20+ messages in thread
From: Tudor Ambarus @ 2025-10-10 6:12 UTC (permalink / raw)
To: Takahiro Kuwano, Marek Vasut, linux-mtd, Takahiro Kuwano
Cc: Geert Uytterhoeven, Michael Walle, Miquel Raynal, Pratyush Yadav,
Richard Weinberger, Vignesh Raghavendra, linux-renesas-soc
On 10/10/25 7:01 AM, Takahiro Kuwano wrote:
> Hi,
Hi!
Thanks for chiming in, Takahiro!
>
> On 10/8/2025 3:20 PM, Tudor Ambarus wrote:
>>
>>
>> On 9/3/25 2:40 AM, Marek Vasut wrote:
>>> On 9/23/24 10:54 AM, Tudor Ambarus wrote:
>>>
>>> Hello Tudor,
>>
>> Hi, Marek,
>>
>>>
>>> sorry for the late reply.
>>
>> That's okay.
>>
>>>
>>>> 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@mailbox.org>
>>>>> ---
>>>>> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
>>>>> Cc: Michael Walle <mwalle@kernel.org>
>>>>> Cc: Miquel Raynal <miquel.raynal@bootlin.com>
>>>>> Cc: Pratyush Yadav <pratyush@kernel.org>
>>>>> Cc: Richard Weinberger <richard@nod.at>
>>>>> Cc: Tudor Ambarus <tudor.ambarus@linaro.org>
>>>>> Cc: Vignesh Raghavendra <vigneshr@ti.com>
>>>>> Cc: linux-mtd@lists.infradead.org
>>>>> Cc: linux-renesas-soc@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.
>
>> 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
If you hack the code and set 8 cycles for the read latency, do you get correct
results?
Can the variable latency be determined by parsing SFDP?
Cheers,
ta
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH] mtd: spi-nor: spansion: Add SMPT fixup for S25FS512S
2025-10-10 6:12 ` Tudor Ambarus
@ 2025-10-10 6:46 ` Takahiro Kuwano
2025-10-10 8:34 ` Tudor Ambarus
0 siblings, 1 reply; 20+ messages in thread
From: Takahiro Kuwano @ 2025-10-10 6:46 UTC (permalink / raw)
To: Tudor Ambarus, Marek Vasut, linux-mtd, Takahiro Kuwano
Cc: Geert Uytterhoeven, Michael Walle, Miquel Raynal, Pratyush Yadav,
Richard Weinberger, Vignesh Raghavendra, linux-renesas-soc
Hi Tudor,
On 10/10/2025 3:12 PM, Tudor Ambarus wrote:
>
>
> On 10/10/25 7:01 AM, Takahiro Kuwano wrote:
>> Hi,
>
> Hi!
>
> Thanks for chiming in, Takahiro!
>
>>
>> On 10/8/2025 3:20 PM, Tudor Ambarus wrote:
>>>
>>>
>>> On 9/3/25 2:40 AM, Marek Vasut wrote:
>>>> On 9/23/24 10:54 AM, Tudor Ambarus wrote:
>>>>
>>>> Hello Tudor,
>>>
>>> Hi, Marek,
>>>
>>>>
>>>> sorry for the late reply.
>>>
>>> That's okay.
>>>
>>>>
>>>>> 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@mailbox.org>
>>>>>> ---
>>>>>> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
>>>>>> Cc: Michael Walle <mwalle@kernel.org>
>>>>>> Cc: Miquel Raynal <miquel.raynal@bootlin.com>
>>>>>> Cc: Pratyush Yadav <pratyush@kernel.org>
>>>>>> Cc: Richard Weinberger <richard@nod.at>
>>>>>> Cc: Tudor Ambarus <tudor.ambarus@linaro.org>
>>>>>> Cc: Vignesh Raghavendra <vigneshr@ti.com>
>>>>>> Cc: linux-mtd@lists.infradead.org
>>>>>> Cc: linux-renesas-soc@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.
>>
>>> 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
>
> If you hack the code and set 8 cycles for the read latency, do you get correct
> results?
>
No, we still need fix like Marek's or something.
> Can the variable latency be determined by parsing SFDP?
>
No, I don't think we can determine by SFDP.
> Cheers,
> ta
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH] mtd: spi-nor: spansion: Add SMPT fixup for S25FS512S
2025-10-10 6:46 ` Takahiro Kuwano
@ 2025-10-10 8:34 ` Tudor Ambarus
2025-10-14 7:22 ` Takahiro Kuwano
0 siblings, 1 reply; 20+ messages in thread
From: Tudor Ambarus @ 2025-10-10 8:34 UTC (permalink / raw)
To: Takahiro Kuwano, Marek Vasut, linux-mtd, Takahiro Kuwano
Cc: Geert Uytterhoeven, Michael Walle, Miquel Raynal, Pratyush Yadav,
Richard Weinberger, Vignesh Raghavendra, linux-renesas-soc
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@mailbox.org>
>>>>>>> ---
>>>>>>> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
>>>>>>> Cc: Michael Walle <mwalle@kernel.org>
>>>>>>> Cc: Miquel Raynal <miquel.raynal@bootlin.com>
>>>>>>> Cc: Pratyush Yadav <pratyush@kernel.org>
>>>>>>> Cc: Richard Weinberger <richard@nod.at>
>>>>>>> Cc: Tudor Ambarus <tudor.ambarus@linaro.org>
>>>>>>> Cc: Vignesh Raghavendra <vigneshr@ti.com>
>>>>>>> Cc: linux-mtd@lists.infradead.org
>>>>>>> Cc: linux-renesas-soc@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
Is there any other reg interaction in the probe path? What number of dummy cycles
is it using?
>>
>> If you hack the code and set 8 cycles for the read latency, do you get correct
>> results?
>>
> No, we still need fix like Marek's or something.
Right
>
>> Can the variable latency be determined by parsing SFDP?
>>
> No, I don't think we can determine by SFDP.
>
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH] mtd: spi-nor: spansion: Add SMPT fixup for S25FS512S
2025-10-10 8:34 ` Tudor Ambarus
@ 2025-10-14 7:22 ` Takahiro Kuwano
2025-10-17 3:44 ` Takahiro Kuwano
0 siblings, 1 reply; 20+ messages in thread
From: Takahiro Kuwano @ 2025-10-14 7:22 UTC (permalink / raw)
To: Tudor Ambarus, Marek Vasut, linux-mtd, Takahiro Kuwano
Cc: Geert Uytterhoeven, Michael Walle, Miquel Raynal, Pratyush Yadav,
Richard Weinberger, Vignesh Raghavendra, linux-renesas-soc
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@mailbox.org>
>>>>>>>> ---
>>>>>>>> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
>>>>>>>> Cc: Michael Walle <mwalle@kernel.org>
>>>>>>>> Cc: Miquel Raynal <miquel.raynal@bootlin.com>
>>>>>>>> Cc: Pratyush Yadav <pratyush@kernel.org>
>>>>>>>> Cc: Richard Weinberger <richard@nod.at>
>>>>>>>> Cc: Tudor Ambarus <tudor.ambarus@linaro.org>
>>>>>>>> Cc: Vignesh Raghavendra <vigneshr@ti.com>
>>>>>>>> Cc: linux-mtd@lists.infradead.org
>>>>>>>> Cc: linux-renesas-soc@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.
>>>
>>> If you hack the code and set 8 cycles for the read latency, do you get correct
>>> results?
>>>
>> No, we still need fix like Marek's or something.
>
> Right
>
>>
>>> Can the variable latency be determined by parsing SFDP?
>>>
>> No, I don't think we can determine by SFDP.
>>
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH] mtd: spi-nor: spansion: Add SMPT fixup for S25FS512S
2025-10-14 7:22 ` Takahiro Kuwano
@ 2025-10-17 3:44 ` Takahiro Kuwano
2025-10-17 4:39 ` Tudor Ambarus
0 siblings, 1 reply; 20+ messages in thread
From: Takahiro Kuwano @ 2025-10-17 3:44 UTC (permalink / raw)
To: Tudor Ambarus, Marek Vasut, linux-mtd, Takahiro Kuwano
Cc: Geert Uytterhoeven, Michael Walle, Miquel Raynal, Pratyush Yadav,
Richard Weinberger, Vignesh Raghavendra, linux-renesas-soc
Hi Tudor,
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@mailbox.org>
>>>>>>>>> ---
>>>>>>>>> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
>>>>>>>>> Cc: Michael Walle <mwalle@kernel.org>
>>>>>>>>> Cc: Miquel Raynal <miquel.raynal@bootlin.com>
>>>>>>>>> Cc: Pratyush Yadav <pratyush@kernel.org>
>>>>>>>>> Cc: Richard Weinberger <richard@nod.at>
>>>>>>>>> Cc: Tudor Ambarus <tudor.ambarus@linaro.org>
>>>>>>>>> Cc: Vignesh Raghavendra <vigneshr@ti.com>
>>>>>>>>> Cc: linux-mtd@lists.infradead.org
>>>>>>>>> Cc: linux-renesas-soc@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;
}
>>>>
>>>> If you hack the code and set 8 cycles for the read latency, do you get correct
>>>> results?
>>>>
>>> No, we still need fix like Marek's or something.
>>
>> Right
>>
>>>
>>>> Can the variable latency be determined by parsing SFDP?
>>>>
>>> No, I don't think we can determine by SFDP.
>>>
>
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH] mtd: spi-nor: spansion: Add SMPT fixup for S25FS512S
2025-10-17 3:44 ` Takahiro Kuwano
@ 2025-10-17 4:39 ` Tudor Ambarus
0 siblings, 0 replies; 20+ messages in thread
From: Tudor Ambarus @ 2025-10-17 4:39 UTC (permalink / raw)
To: Takahiro Kuwano, Marek Vasut, linux-mtd, Takahiro Kuwano
Cc: Geert Uytterhoeven, Michael Walle, Miquel Raynal, Pratyush Yadav,
Richard Weinberger, Vignesh Raghavendra, linux-renesas-soc
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@mailbox.org>
>>>>>>>>>> ---
>>>>>>>>>> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
>>>>>>>>>> Cc: Michael Walle <mwalle@kernel.org>
>>>>>>>>>> Cc: Miquel Raynal <miquel.raynal@bootlin.com>
>>>>>>>>>> Cc: Pratyush Yadav <pratyush@kernel.org>
>>>>>>>>>> Cc: Richard Weinberger <richard@nod.at>
>>>>>>>>>> Cc: Tudor Ambarus <tudor.ambarus@linaro.org>
>>>>>>>>>> Cc: Vignesh Raghavendra <vigneshr@ti.com>
>>>>>>>>>> Cc: linux-mtd@lists.infradead.org
>>>>>>>>>> Cc: linux-renesas-soc@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
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] mtd: spi-nor: spansion: Add SMPT fixup for S25FS512S
2025-10-08 6:20 ` Tudor Ambarus
2025-10-08 6:47 ` Tudor Ambarus
2025-10-10 6:01 ` Takahiro Kuwano
@ 2025-10-12 16:19 ` Marek Vasut
2025-10-13 15:41 ` Tudor Ambarus
2 siblings, 1 reply; 20+ messages in thread
From: Marek Vasut @ 2025-10-12 16:19 UTC (permalink / raw)
To: Tudor Ambarus, linux-mtd
Cc: Geert Uytterhoeven, Michael Walle, Miquel Raynal, Pratyush Yadav,
Richard Weinberger, Vignesh Raghavendra, linux-renesas-soc
On 10/8/25 8:20 AM, Tudor Ambarus wrote:
Hello Tudor,
>> sorry for the late reply.
>
> That's okay.
Thank you
[...]
>>>> 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
> "
>
> 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?
0x4
> Let's also print the values of CR3NV and CR1NV.
Both 0x0 and 0x0 .
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH] mtd: spi-nor: spansion: Add SMPT fixup for S25FS512S
2025-10-12 16:19 ` Marek Vasut
@ 2025-10-13 15:41 ` Tudor Ambarus
2025-10-17 16:04 ` Marek Vasut
0 siblings, 1 reply; 20+ messages in thread
From: Tudor Ambarus @ 2025-10-13 15:41 UTC (permalink / raw)
To: Marek Vasut, linux-mtd, Takahiro Kuwano, Takahiro Kuwano
Cc: Geert Uytterhoeven, Michael Walle, Miquel Raynal, Pratyush Yadav,
Richard Weinberger, Vignesh Raghavendra, linux-renesas-soc
On 10/12/25 5:19 PM, Marek Vasut wrote:
> Hello Tudor,
Hello!
>>>>> 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
>> "
>>
>> 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?
>
> 0x4
This translates to CR3NV[3] = 1, CR1NV[2] = 0, CR3NV[1] = 0.
>
>> Let's also print the values of CR3NV and CR1NV.
>
> Both 0x0 and 0x0 .
But here CR3NV is 0, it contradicts the result from above.
Maybe it's the same problem that Takahiro identified: the flash needs
8 dummy cycles, but the code uses zero dummy cycles, resulting in
reading garbage data, depending on whether your IO lines are pulled up/down
or floating.
Can you redo the test with the following please?
diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
index 21727f9a4ac6..85443c903e59 100644
--- a/drivers/mtd/spi-nor/sfdp.c
+++ b/drivers/mtd/spi-nor/sfdp.c
@@ -752,7 +752,7 @@ static const u32 *spi_nor_get_map_in_use(struct spi_nor *nor, const u32 *smpt,
read_data_mask = SMPT_CMD_READ_DATA(smpt[i]);
nor->addr_nbytes = spi_nor_smpt_addr_nbytes(nor, smpt[i]);
- nor->read_dummy = spi_nor_smpt_read_dummy(nor, smpt[i]);
+ nor->read_dummy = 8;
nor->read_opcode = SMPT_CMD_OPCODE(smpt[i]);
addr = smpt[i + 1];
@@ -767,6 +767,8 @@ static const u32 *spi_nor_get_map_in_use(struct spi_nor *nor, const u32 *smpt,
* Configuration that is currently in use.
*/
map_id = map_id << 1 | !!(*buf & read_data_mask);
+ dev_err(nor->dev, "i = %d, buf = %02x, map_id = %02x\n",
+ i, buf[0], map_id);
}
Cheers,
ta
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH] mtd: spi-nor: spansion: Add SMPT fixup for S25FS512S
2025-10-13 15:41 ` Tudor Ambarus
@ 2025-10-17 16:04 ` Marek Vasut
2025-10-20 11:35 ` Tudor Ambarus
0 siblings, 1 reply; 20+ messages in thread
From: Marek Vasut @ 2025-10-17 16:04 UTC (permalink / raw)
To: Tudor Ambarus, linux-mtd, Takahiro Kuwano, Takahiro Kuwano
Cc: Geert Uytterhoeven, Michael Walle, Miquel Raynal, Pratyush Yadav,
Richard Weinberger, Vignesh Raghavendra, linux-renesas-soc
On 10/13/25 5:41 PM, Tudor Ambarus wrote:
Hello Tudor,
>>> 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?
>>
>> 0x4
>
> This translates to CR3NV[3] = 1, CR1NV[2] = 0, CR3NV[1] = 0.
>>
>>> Let's also print the values of CR3NV and CR1NV.
>>
>> Both 0x0 and 0x0 .
>
> But here CR3NV is 0, it contradicts the result from above.
Maybe I messed up and was reading it the wrong way ?
> Maybe it's the same problem that Takahiro identified: the flash needs
> 8 dummy cycles, but the code uses zero dummy cycles, resulting in
> reading garbage data, depending on whether your IO lines are pulled up/down
> or floating.
>
> Can you redo the test with the following please?
Sure, although I saw some further discussion between you and Kuwano-san
, is this still applicable ?
> diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
> index 21727f9a4ac6..85443c903e59 100644
> --- a/drivers/mtd/spi-nor/sfdp.c
> +++ b/drivers/mtd/spi-nor/sfdp.c
> @@ -752,7 +752,7 @@ static const u32 *spi_nor_get_map_in_use(struct spi_nor *nor, const u32 *smpt,
>
> read_data_mask = SMPT_CMD_READ_DATA(smpt[i]);
> nor->addr_nbytes = spi_nor_smpt_addr_nbytes(nor, smpt[i]);
> - nor->read_dummy = spi_nor_smpt_read_dummy(nor, smpt[i]);
> + nor->read_dummy = 8;
> nor->read_opcode = SMPT_CMD_OPCODE(smpt[i]);
> addr = smpt[i + 1];
>
> @@ -767,6 +767,8 @@ static const u32 *spi_nor_get_map_in_use(struct spi_nor *nor, const u32 *smpt,
> * Configuration that is currently in use.
> */
> map_id = map_id << 1 | !!(*buf & read_data_mask);
> + dev_err(nor->dev, "i = %d, buf = %02x, map_id = %02x\n",
> + i, buf[0], map_id);
> }
Sorry for the late reply.
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] mtd: spi-nor: spansion: Add SMPT fixup for S25FS512S
2025-10-17 16:04 ` Marek Vasut
@ 2025-10-20 11:35 ` Tudor Ambarus
2025-10-22 10:13 ` Takahiro Kuwano
2025-10-27 22:22 ` Marek Vasut
0 siblings, 2 replies; 20+ messages in thread
From: Tudor Ambarus @ 2025-10-20 11:35 UTC (permalink / raw)
To: Marek Vasut, linux-mtd, Takahiro Kuwano, Takahiro Kuwano
Cc: Geert Uytterhoeven, Michael Walle, Miquel Raynal, Pratyush Yadav,
Richard Weinberger, Vignesh Raghavendra, linux-renesas-soc
On 10/17/25 5:04 PM, Marek Vasut wrote:
> On 10/13/25 5:41 PM, Tudor Ambarus wrote:
>
> Hello Tudor,
Hi!
>
>>>> 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?
>>>
>>> 0x4
>>
>> This translates to CR3NV[3] = 1, CR1NV[2] = 0, CR3NV[1] = 0.
>>>
>>>> Let's also print the values of CR3NV and CR1NV.
>>>
>>> Both 0x0 and 0x0 .
>>
>> But here CR3NV is 0, it contradicts the result from above.
>
> Maybe I messed up and was reading it the wrong way ?
>
>> Maybe it's the same problem that Takahiro identified: the flash needs
>> 8 dummy cycles, but the code uses zero dummy cycles, resulting in
>> reading garbage data, depending on whether your IO lines are pulled up/down
>> or floating.
>>
>> Can you redo the test with the following please?
>
> Sure, although I saw some further discussion between you and Kuwano-san , is this still applicable ?
It's still applicable, it will confirm what Kuwano-san discovered:
the flash needs 8 dummy cycles for using that read reg command.
We'll also need a hook to amend the CR3NV[1] value (which is reserved,
zero on some flavors of flash) and replace it with 1, similar to what
you did in this patch.
You can either test or let Kuwano-san come up with a patch addressing
both issues and test his patch afterwards, sync up with him please.
>
>> diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
>> index 21727f9a4ac6..85443c903e59 100644
>> --- a/drivers/mtd/spi-nor/sfdp.c
>> +++ b/drivers/mtd/spi-nor/sfdp.c
>> @@ -752,7 +752,7 @@ static const u32 *spi_nor_get_map_in_use(struct spi_nor *nor, const u32 *smpt,
>> read_data_mask = SMPT_CMD_READ_DATA(smpt[i]);
>> nor->addr_nbytes = spi_nor_smpt_addr_nbytes(nor, smpt[i]);
>> - nor->read_dummy = spi_nor_smpt_read_dummy(nor, smpt[i]);
>> + nor->read_dummy = 8;
>> nor->read_opcode = SMPT_CMD_OPCODE(smpt[i]);
>> addr = smpt[i + 1];
>> @@ -767,6 +767,8 @@ static const u32 *spi_nor_get_map_in_use(struct spi_nor *nor, const u32 *smpt,
>> * Configuration that is currently in use.
>> */
>> map_id = map_id << 1 | !!(*buf & read_data_mask);
>> + dev_err(nor->dev, "i = %d, buf = %02x, map_id = %02x\n",
>> + i, buf[0], map_id);
>> }
> Sorry for the late reply.
yeah, no worries, it's still fresh.
Cheers,
ta
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] mtd: spi-nor: spansion: Add SMPT fixup for S25FS512S
2025-10-20 11:35 ` Tudor Ambarus
@ 2025-10-22 10:13 ` Takahiro Kuwano
2025-10-27 22:35 ` Marek Vasut
2025-10-27 22:22 ` Marek Vasut
1 sibling, 1 reply; 20+ messages in thread
From: Takahiro Kuwano @ 2025-10-22 10:13 UTC (permalink / raw)
To: Tudor Ambarus, Marek Vasut, linux-mtd, Takahiro Kuwano
Cc: Geert Uytterhoeven, Michael Walle, Miquel Raynal, Pratyush Yadav,
Richard Weinberger, Vignesh Raghavendra, linux-renesas-soc
Hi Tudor and Marek,
I have submitted new series.
https://lore.kernel.org/linux-mtd/20251022-s25fs-s-smpt-fixup-v1-0-ce26d4084b2d@infineon.com/T/#m2fa41b89e36d6061df9e979f49c573e46fd0d5c4
Please review and test then give feedback.
Thanks,
Takahiro
On 10/20/2025 8:35 PM, Tudor Ambarus wrote:
>
>
> On 10/17/25 5:04 PM, Marek Vasut wrote:
>> On 10/13/25 5:41 PM, Tudor Ambarus wrote:
>>
>> Hello Tudor,
>
> Hi!
>
>>
>>>>> 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?
>>>>
>>>> 0x4
>>>
>>> This translates to CR3NV[3] = 1, CR1NV[2] = 0, CR3NV[1] = 0.
>>>>
>>>>> Let's also print the values of CR3NV and CR1NV.
>>>>
>>>> Both 0x0 and 0x0 .
>>>
>>> But here CR3NV is 0, it contradicts the result from above.
>>
>> Maybe I messed up and was reading it the wrong way ?
>>
>>> Maybe it's the same problem that Takahiro identified: the flash needs
>>> 8 dummy cycles, but the code uses zero dummy cycles, resulting in
>>> reading garbage data, depending on whether your IO lines are pulled up/down
>>> or floating.
>>>
>>> Can you redo the test with the following please?
>>
>> Sure, although I saw some further discussion between you and Kuwano-san , is this still applicable ?
>
> It's still applicable, it will confirm what Kuwano-san discovered:
> the flash needs 8 dummy cycles for using that read reg command.
>
> We'll also need a hook to amend the CR3NV[1] value (which is reserved,
> zero on some flavors of flash) and replace it with 1, similar to what
> you did in this patch.
>
> You can either test or let Kuwano-san come up with a patch addressing
> both issues and test his patch afterwards, sync up with him please.
>
>>
>>> diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
>>> index 21727f9a4ac6..85443c903e59 100644
>>> --- a/drivers/mtd/spi-nor/sfdp.c
>>> +++ b/drivers/mtd/spi-nor/sfdp.c
>>> @@ -752,7 +752,7 @@ static const u32 *spi_nor_get_map_in_use(struct spi_nor *nor, const u32 *smpt,
>>> read_data_mask = SMPT_CMD_READ_DATA(smpt[i]);
>>> nor->addr_nbytes = spi_nor_smpt_addr_nbytes(nor, smpt[i]);
>>> - nor->read_dummy = spi_nor_smpt_read_dummy(nor, smpt[i]);
>>> + nor->read_dummy = 8;
>>> nor->read_opcode = SMPT_CMD_OPCODE(smpt[i]);
>>> addr = smpt[i + 1];
>>> @@ -767,6 +767,8 @@ static const u32 *spi_nor_get_map_in_use(struct spi_nor *nor, const u32 *smpt,
>>> * Configuration that is currently in use.
>>> */
>>> map_id = map_id << 1 | !!(*buf & read_data_mask);
>>> + dev_err(nor->dev, "i = %d, buf = %02x, map_id = %02x\n",
>>> + i, buf[0], map_id);
>>> }
>> Sorry for the late reply.
> yeah, no worries, it's still fresh.
>
> Cheers,
> ta
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] mtd: spi-nor: spansion: Add SMPT fixup for S25FS512S
2025-10-22 10:13 ` Takahiro Kuwano
@ 2025-10-27 22:35 ` Marek Vasut
0 siblings, 0 replies; 20+ messages in thread
From: Marek Vasut @ 2025-10-27 22:35 UTC (permalink / raw)
To: Takahiro Kuwano, Tudor Ambarus, linux-mtd, Takahiro Kuwano
Cc: Geert Uytterhoeven, Michael Walle, Miquel Raynal, Pratyush Yadav,
Richard Weinberger, Vignesh Raghavendra, linux-renesas-soc
On 10/22/25 12:13 PM, Takahiro Kuwano wrote:
> Hi Tudor and Marek,
Hello Kuwano-san,
I apologize for my delayed reply.
> I have submitted new series.
> https://lore.kernel.org/linux-mtd/20251022-s25fs-s-smpt-fixup-v1-0-ce26d4084b2d@infineon.com/T/#m2fa41b89e36d6061df9e979f49c573e46fd0d5c4
>
> Please review and test then give feedback.
With the patchset applied, the readback looks as follows and the map ID
seems correct now:
spi-nor spi0.0: i = 0, buf = 00, map_id = 00
spi-nor spi0.0: i = 2, buf = 02, map_id = 00
spi-nor spi0.0: i = 4, buf = 00, map_id = 00
spi_nor_get_map_in_use[800] map_id=00000001
Thank you for your help !
--
Best regards,
Marek Vasut
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] mtd: spi-nor: spansion: Add SMPT fixup for S25FS512S
2025-10-20 11:35 ` Tudor Ambarus
2025-10-22 10:13 ` Takahiro Kuwano
@ 2025-10-27 22:22 ` Marek Vasut
1 sibling, 0 replies; 20+ messages in thread
From: Marek Vasut @ 2025-10-27 22:22 UTC (permalink / raw)
To: Tudor Ambarus, linux-mtd, Takahiro Kuwano, Takahiro Kuwano
Cc: Geert Uytterhoeven, Michael Walle, Miquel Raynal, Pratyush Yadav,
Richard Weinberger, Vignesh Raghavendra, linux-renesas-soc
On 10/20/25 1:35 PM, Tudor Ambarus wrote:
Hello Tudor,
I apologize for my slow response.
>>>>> 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?
>>>>
>>>> 0x4
>>>
>>> This translates to CR3NV[3] = 1, CR1NV[2] = 0, CR3NV[1] = 0.
>>>>
>>>>> Let's also print the values of CR3NV and CR1NV.
>>>>
>>>> Both 0x0 and 0x0 .
>>>
>>> But here CR3NV is 0, it contradicts the result from above.
>>
>> Maybe I messed up and was reading it the wrong way ?
I really wonder if I might be reading the CR3NV the wrong way ?
>>> Maybe it's the same problem that Takahiro identified: the flash needs
>>> 8 dummy cycles, but the code uses zero dummy cycles, resulting in
>>> reading garbage data, depending on whether your IO lines are pulled up/down
>>> or floating.
>>>
>>> Can you redo the test with the following please?
>>
>> Sure, although I saw some further discussion between you and Kuwano-san , is this still applicable ?
>
> It's still applicable, it will confirm what Kuwano-san discovered:
> the flash needs 8 dummy cycles for using that read reg command.
>
> We'll also need a hook to amend the CR3NV[1] value (which is reserved,
> zero on some flavors of flash) and replace it with 1, similar to what
> you did in this patch.
>
> You can either test or let Kuwano-san come up with a patch addressing
> both issues and test his patch afterwards, sync up with him please.
>
>>
>>> diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
>>> index 21727f9a4ac6..85443c903e59 100644
>>> --- a/drivers/mtd/spi-nor/sfdp.c
>>> +++ b/drivers/mtd/spi-nor/sfdp.c
>>> @@ -752,7 +752,7 @@ static const u32 *spi_nor_get_map_in_use(struct spi_nor *nor, const u32 *smpt,
>>> read_data_mask = SMPT_CMD_READ_DATA(smpt[i]);
>>> nor->addr_nbytes = spi_nor_smpt_addr_nbytes(nor, smpt[i]);
>>> - nor->read_dummy = spi_nor_smpt_read_dummy(nor, smpt[i]);
>>> + nor->read_dummy = 8;
>>> nor->read_opcode = SMPT_CMD_OPCODE(smpt[i]);
>>> addr = smpt[i + 1];
>>> @@ -767,6 +767,8 @@ static const u32 *spi_nor_get_map_in_use(struct spi_nor *nor, const u32 *smpt,
>>> * Configuration that is currently in use.
>>> */
>>> map_id = map_id << 1 | !!(*buf & read_data_mask);
>>> + dev_err(nor->dev, "i = %d, buf = %02x, map_id = %02x\n",
>>> + i, buf[0], map_id);
>>> }
>> Sorry for the late reply.
> yeah, no worries, it's still fresh.
With current mainline, without the aforementioned patch, with my
CR1NV/CR3NV printing (maybe broken):
CR1NV=00
CR3NV=00
map_id=00000004
With the read_dummy=8 addition above and the dev_err, I get:
spi-nor spi0.0: i = 0, buf = 00, map_id = 00
spi-nor spi0.0: i = 2, buf = 02, map_id = 00
spi-nor spi0.0: i = 4, buf = 00, map_id = 00
map_id=00000000
The reported failure persists:
spi-nor spi0.0: Failed to parse optional parameter table: ff81
Let me test the patchset from Kuwano-san next .
Thank you for your help!
--
Best regards,
Marek Vasut
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 20+ messages in thread