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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <e1baa67a-d62a-477f-9ccd-83cc2dae501a@gmail.com>
Date: Thu, 27 Mar 2025 19:43:57 +0900
From: Takahiro Kuwano <tkuw584924@...il.com>
To: Tudor Ambarus <tudor.ambarus@...aro.org>,
 Michael Walle <mwalle@...nel.org>, Rob Herring <robh@...nel.org>
Cc: Pratyush Yadav <pratyush@...nel.org>,
 Miquel Raynal <miquel.raynal@...tlin.com>,
 Richard Weinberger <richard@....at>, Vignesh Raghavendra <vigneshr@...com>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
 <conor+dt@...nel.org>, linux-mtd@...ts.infradead.org,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
 Bacem Daassi <Bacem.Daassi@...ineon.com>,
 Takahiro Kuwano <Takahiro.Kuwano@...ineon.com>,
 Mark Brown <broonie@...nel.org>
Subject: Re: [PATCH 2/3] mtd: spi-nor: use rdid-dummy-ncycles DT property

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