Re: [PATCH 2/3] mtd: spi-nor: use rdid-dummy-ncycles DT property

From: Takahiro Kuwano
Date: Thu Mar 27 2025 - 06:44:35 EST


On 3/26/2025 11:44 PM, Tudor Ambarus wrote:
> Hi, Michael,
>
> Sorry, I somehow missed your replies.
>
> On 3/21/25 8:00 AM, Michael Walle wrote:
>
> cut
>
>>>>> The
>>>>> problem that I see with that is that we no longer bind against the
>>>>> generic jedec,spi-nor compatible, so people need to update their DT in
>>>>> case they use/plug-in a different flash on their board.
>>>>
>>>> This chip is clearly *not* compatible with a generic chip.
>>>
>>> I think it is compatible. The chip defines the SFDP (serial flash
>>> discoverable parameters) tables. At probe time we parse those tables and
>>> initialize the flash based on them.
>>
>> I disagree. It's not compatible with "jedec,spi-nor", which is
>> defined as
>>
>
> cut
>
>>
>> See my first reply, on how to possibly fix this mess (new
>> compatible if accepted, just use RDSFDP sequence which is backed by
>> the standard and do some fingerprinting).
>>
>
> this won't work unless there's a unique parameter or ID in the sfdp or
> vendors tables, which I doubt. Takahiro to confirm.
>
No, cyrs17b doesn't have it.

>> FWIW, a new (or rather different) compatible is needed because we
>> cannot distinguish between random data returned during the dummy
>> cycles and a proper manufacturer id. So there is no way we could fix
>> this in the core itself.
>
> Yes, I agree, new compatible it is then.
>
> cut
>
>>> I think the property vs compatible decision resumes at whether we
>>> consider that the dummy cycles requirement for Read ID is/will be
>>> generic or not.
>>
>> It is not generic. Because it will break autodetection. And that is
>> the whole purpose of this. Adding that property means, we can just
>> autodetect flashes within this 'group'. And personally, I think this
>> is a bad precedent.
>>
>
> yes, I agree.
>
>>> I noticed that with higher frequencies or protocol modes (e.g, octal
>>> DTR), flashes tend to require more dummy cycles. I think with time,
>>> we'll have more flashes with such requirement. Takahiro can jump in and
>>> tell if it's already the case with IFX.
>>
>> But hopefully not with RDID. Again this doesn't play nice with other
>> flashes (or all flashes for now). Instead of adding random delay
>> cycles one should rather define a max clock speed for this opcode.
>
> This could work, yes. But not for this flash. Or maybe encourage vendors
> to either contribute and enlarge the SFDP database or define their own
> vendor tables for all the flash properties that are not covered yet.
> It's strange how Block Protection is not yet covered by SFDP after all
> these years.
>
> Thanks,
> ta