linux-kernel - Re: [PATCH v2 3/4] dt-bindings: mtd: macronix,mx25l12833f: add SPI-NOR chip

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]

Date: Mon, 1 Jul 2024 18:12:42 +0200
From: Erez <erezgeva2@...il.com>
To: Tudor Ambarus <tudor.ambarus@...aro.org>
Cc: Esben Haabendal <esben@...nix.com>, Erez Geva <erezgeva@...ime.org>, linux-mtd@...ts.infradead.org, 
	Pratyush Yadav <pratyush@...nel.org>, Michael Walle <mwalle@...nel.org>, linux-kernel@...r.kernel.org, 
	Miquel Raynal <miquel.raynal@...tlin.com>, Richard Weinberger <richard@....at>, 
	Vignesh Raghavendra <vigneshr@...com>, devicetree@...r.kernel.org, Rob Herring <robh@...nel.org>, 
	Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>
Subject: Re: [PATCH v2 3/4] dt-bindings: mtd: macronix,mx25l12833f: add
 SPI-NOR chip

On Mon, 1 Jul 2024 at 14:53, Tudor Ambarus <tudor.ambarus@...aro.org> wrote:
>
>
>
> On 7/1/24 12:03 PM, Erez wrote:
> > On Mon, 1 Jul 2024 at 12:23, Tudor Ambarus <tudor.ambarus@...aro.org> wrote:
> >>
> >>
> >>
> >> On 7/1/24 11:15 AM, Tudor Ambarus wrote:
> >>>
> >>>
> >>> On 7/1/24 10:46 AM, Erez wrote:
> >>>> When using mx25l12805d, we do not read SFDP.
> >>>> As it uses the no-SFDP flags.
> >>>> When using mx25l12833f hardware with mx25l12805d driver, it did not
> >>>> try to read the SFDP.
> >>>> Yet mx25l12833f does have SFDP, when I remove the no-SFDP flags, the
> >>>> driver fetch the SFDP.
> >>>>
> >>>> Secondly SFDP does not contain OTP information.
> >>>>
> >>>> mx25l12805d has two OTP regions of 128 KiB and 384 KiB (yes asymmetric).
> >>>> While mx25l12833f has two OTP regions of 512 KiB.
> >>>>
> >>>> How do we handle it?
> >>>
> >>> You would first try to parse SFDP and initialize the flash based on
> >>> SFDP. If there's no SFDP then you fallback to the flags declared at
> >>> flash declaration. Esben had a try recently, see [1]. I don't know if
> >>> there's any progress in that direction.
> >>>
> >>
> >> And you can then decide which OTP org to use based on whether SFDP is
> >> present or not.
> >
> > That can work, but sound like a hack.
>
> It's not a hack, we're just doing our best to dynamically identify the
> flash.

Call it whatever you want.

>
> > Is that really that important to hack?
>
> we push really hard against new compatibles. Users shouldn't care about
> what SPI NOR flash is there.

I am in. I do not like compatibility.
I do not like to see them in the device tree.
I did not create the mess.
It seems like Macronix likes to reuse JEDEC IDs.
As most new chips have SPDF, the mess is with OTP mainly.
We can think about a different model.
Perhaps allow the user to define the OTP size and number of regions
using a 'flash_otp_set' tool?
I am open to ideas.

>
> > Just for OTP, that very few use?
> > And if in the future Macronix adds a newer one with the same JEDEC ID,
> > but a different OTP size?
>
> we'll compare SFDP data and choose based on the differences. This is not
> encouraged. Instead ask for unique IDs or choose other flash.

That requires additional callback in the SFDP, not a big challenge.
Yet most SFDP tables are not sufficient to determine OTP.
Found only one table with OTP support flag.
No size, offset  and number of regions to work with.
The only hint is do we have SFDP or not.

>
> > Macronix does not consult with the Linux Kernel on these matters.
> >
>
> I'm complaining about unique flash IDs for years now. Hopefully vendors
> have learnt their lesson. I didn't see new flash designs reusing old IDs.

I can not speak in Macronix's name.
I know they did that, not sure if they stop.

>
> > Anyhow as I do not have the hardware anymore, I can not do more
> > changes and test them.
> >
>
> Be aware that we're not queuing patches without some minimal tests. If
> you don't have the hardware, contact mchp and see if they care,
> otherwise we're wasting our time. Here are the minimum testing requirements:
> https://docs.kernel.org/driver-api/mtd/spi-nor.html#minimum-testing-requirements
> For OTP we'll also need some OTP tests.

OTP as its name can be written once.
So a simple OTP test would be nice.

Anyhow I ordered 3 Macronix's MX25L3233F that have OTP.
The OTP works the same as the other Macronix chips.
The MX25L3233F ID is 0xc22016 same as "mx25l3205d".
Seems Macronix are persistent on reusing IDs.
First revision of MX25L3233F was in 2015,

I will connect to my old BeagleBone-black.
I think I can perform the test with it.

As I say, I am more interested in testing the code.
Less on which Macronix chip we use.

Thanks for your time
 Erez

>
> Cheers,
> ta