[PATCH] mtd: spi-nor: spansion: Add SMPT fixup for S25FS512S

[Tudor Ambarus]

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