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

[Marek Vasut]

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