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] [thread-next>] [day] [month] [year] [list]

Message-ID: <be8e5758-8f85-4476-b6c0-4400c8151cbe@linaro.org>
Date: Wed, 26 Mar 2025 14:44:52 +0000
From: Tudor Ambarus <tudor.ambarus@...aro.org>
To: Michael Walle <mwalle@...nel.org>, Rob Herring <robh@...nel.org>
Cc: Takahiro Kuwano <tkuw584924@...il.com>,
 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

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.

> 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