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

[Takahiro Kuwano]

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