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

[Erez]

On Thu, 11 Jul 2024 at 21:57, Michael Walle <mwalle@xxxxxxxxxx> wrote:
>
> Hi Erez,
>
> No top posting please, see also
> https://subspace.kernel.org/etiquette.html

It was a single question. Which I think can be answered in one reply.
In cases where there are different parts in the mail, it makes sense
to avoid top posting.
I do not believe we need to be pedantic.
The guidance is not holy, it is aimed to make communication more comprehensive.

>
> On Thu Jul 11, 2024 at 8:57 PM CEST, Erez wrote:
> > Yes, I think we should.
> >
> > Reading the specification provided publicly by Macronix.
> > For all the JEDEC IDs with the no SFDP flag in drivers/mtd/spi-nor/macronix.c
> > All of them have a new version or a new chip with the same JEDEC ID
> > that supports SFDP.
> > There are 2 chips that Macronix does not provide spec. in public.
> > I can ask Macronix technical support on these 2 chips.
>
> We don't add flashes we cannot test.

I did not suggest adding anything new.
I refer to the list of chips we already have in drivers/mtd/spi-nor/macronix.c
I presume someone tested them before adding them to the list in the past.
And probably the old chip did not have the SFDP table back then.

What I checked with the chip specifications is that all Macronix chips
since 2010 have SFDP.

The situation today is that all Macronix chips that are NOT in the
Macronix table work based on the SFDP table.
But new chips that use a JEDEC found in the Macronix table, skip the
SFDP table and
 use the setting of the old chip.

So I suggest we read the SFDP table for all Macronix chips.
Old Macronix chips that do not have SFDP will use the setting from the
Macronix table. i.e backward compatible.
While new chips which do have an SFDP table will work with the new
setting we find in the table.

Of course, we might have issues in parsing the SFDP table itself.
So we fix them as developers report and send chip ID and part number
with the SFDP table content.
I do not see the point of "hiding" with the old setting.
Anyhow, as we do not like the IDs table and keep it for backward-compatible,
  so it only makes sense we should use the SFDP table as much as possible.

My check was to ensure Tudor that all Macronix chips have SFDP whether
they are in the IDs table or not
 and we are not wasting a no-op on a chip which can not have an SFDP table.

All I suggest is we add the new 'SPI_NOR_TRY_SFDP' flag, to all Macronix chips..
Which will try to read the SFDP to any Macronix chip.

Erez

>
> -michael