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

From: Erez Geva <ErezGeva2@gmail.com>

Add Macronix SPI-NOR mx25l12833f.

Signed-off-by: Erez Geva <ErezGeva2@gmail.com>
---
 Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

On Fri, Jun 28, 2024 at 04:03:27PM +0200, Erez Geva wrote:
> From: Erez Geva <ErezGeva2@gmail.com>
> 
> Add Macronix SPI-NOR mx25l12833f.
> 
> Signed-off-by: Erez Geva <ErezGeva2@gmail.com>

Should the email in here and in the From: field be your nwtime one?
Otherwise
Acked-by: Conor Dooley <conor.dooley@microchip.com>

Hi,

On Fri Jun 28, 2024 at 5:57 PM CEST, Conor Dooley wrote:
> On Fri, Jun 28, 2024 at 04:03:27PM +0200, Erez Geva wrote:
> > From: Erez Geva <ErezGeva2@gmail.com>
> > 
> > Add Macronix SPI-NOR mx25l12833f.
> > 
> > Signed-off-by: Erez Geva <ErezGeva2@gmail.com>
>
> Should the email in here and in the From: field be your nwtime one?
> Otherwise
> Acked-by: Conor Dooley <conor.dooley@microchip.com>

Actually, you're not supposed to add any compatibles to this list.

From the binding:
    description:
      SPI NOR flashes compatible with the JEDEC SFDP standard or which may be
      identified with the READ ID opcode (0x9F) do not deserve a specific
      compatible. They should instead only be matched against the generic
      "jedec,spi-nor" compatible.

I presume the Macronix flash does support the read id opcode.

-michael

On Fri, 28 Jun 2024 at 18:30, Erez <erezgeva2@gmail.com> wrote:
>
>
>
> On Fri, 28 Jun 2024 at 18:04, Michael Walle <mwalle@kernel.org> wrote:
>>
>> Hi,
>>
>> On Fri Jun 28, 2024 at 5:57 PM CEST, Conor Dooley wrote:
>> > On Fri, Jun 28, 2024 at 04:03:27PM +0200, Erez Geva wrote:
>> > > From: Erez Geva <ErezGeva2@gmail.com>
>> > >
>> > > Add Macronix SPI-NOR mx25l12833f.
>> > >
>> > > Signed-off-by: Erez Geva <ErezGeva2@gmail.com>
>> >
>> > Should the email in here and in the From: field be your nwtime one?
>> > Otherwise
>> > Acked-by: Conor Dooley <conor.dooley@microchip.com>
>>
>> Actually, you're not supposed to add any compatibles to this list.
>>
>> From the binding:
>>     description:
>>       SPI NOR flashes compatible with the JEDEC SFDP standard or which may be
>>       identified with the READ ID opcode (0x9F) do not deserve a specific
>>       compatible. They should instead only be matched against the generic
>>       "jedec,spi-nor" compatible.
>>
>> I presume the Macronix flash does support the read id opcode.
>
>
> Yes, they do support.
> The new mx25l12833f also supports SFDP.
>
> I do not know why they decided to use the same JEDEC ID for two chips.
> Your guess is as good as mine.

https://www.macronix.com/en-us/products/NOR-Flash/Serial-NOR-Flash/Pages/spec.aspx?p=MX25L12805D&m=Serial%20NOR%20Flash&n=PM1310

End of Life (EOL)
https://www.macronix.com/Lists/TechDoc/Attachments/9861/PCN31_2009%20PCN_MX25L6405D%20and%20MX25L12805D.pdf
Macronix will continue the support of the existing MX25L6405D and
MX25L12805D as the following schedule:
The Last Time Order Date: 31st Aug., 2010
The Last Time Shipment Date: 30th Nov., 2010

Erez

>
> I know the two chips have the same flash size.
> Though the new mx25l12833f chip has a bigger OTP.
> Secondly, the old mx25l12805d has an asymmetric OTP, the 2 regions are of different size.
>
> As I see it, the first 2 patches are the real contribution.
> As I remember, the kernel maintainers usually like to see something using the code.
> I don't care if it was another Macronix chip. I simply tested it with the mx25l12833f chip.
> "[PATCH 1/4] Add generic functions for accessing the SPI-NOR chip."
> "[PATCH 2/4] Add support for SPI-NOR Macronix OTP."
>
> I did not think it is important to have a kernel configuration for choosing mx25l12833f over mx25l12805d.
>
> Erez
>
>
>
>
>
>>
>>
>> -michael

On Fri Jun 28, 2024 at 6:30 PM CEST, Erez wrote:
> I do not know why they decided to use the same JEDEC ID for two chips.
> Your guess is as good as mine.

That's a common pattern and we try hard to figure that out during
probe time instead of hardcoding it. E.g. by looking at the SFDP
data. Have a look at various fixups in drivers/mtd/spi-nor/.

compatibles are really the last resort to distinguish flash devices.

Next time, please mention such information in the commit message,
please.

Also please have a look at
https://docs.kernel.org/driver-api/mtd/spi-nor.html

-michael

On Fri, 28 Jun 2024 at 18:51, Michael Walle <mwalle@kernel.org> wrote:
>
> On Fri Jun 28, 2024 at 6:30 PM CEST, Erez wrote:
> > I do not know why they decided to use the same JEDEC ID for two chips.
> > Your guess is as good as mine.
>
> That's a common pattern and we try hard to figure that out during
> probe time instead of hardcoding it. E.g. by looking at the SFDP
> data. Have a look at various fixups in drivers/mtd/spi-nor/.

That's a good approach.
The obsolete mx25l12805d does not support the SFDP table.
The new mx25l12833f does.

What is the kernel policy regarding obsolete flash chips?
Macronix annonce on end of life of mx25l12805d in 2010.
Perhaps we should remove mx25l12805d,
 and leave the mx25l12833f configuration in mtd/spi-nor/macronix.c?

>
> compatibles are really the last resort to distinguish flash devices.

I totally agree. Use JEDEC ID a lot better.
In this case Macronix decided to reuse an obsolete chip ID.

>
> Next time, please mention such information in the commit message,
> please.

Thanks for the tip.
I did write it in the fourth patch, but I can add it in part 3 as well.

> Also please have a look at
> https://docs.kernel.org/driver-api/mtd/spi-nor.html

The new mx25l12833f supports SFDP, the obsolete mx25l12805d does not.
I did manage to read the SFDP, though I do not have a copy of it (I do
not have the hardware any more).
To my best knowledge SFDP table do not contain information on OTP,

Thanks
  Erez Geva

>
> -michael

> What is the kernel policy regarding obsolete flash chips?
> Macronix annonce on end of life of mx25l12805d in 2010.

Which doesn't mean there are no boards using it. But EOL in 2010
might be a strong argument to remove it. But I don't see the need
for it, yet.

> > Also please have a look at
> > https://docs.kernel.org/driver-api/mtd/spi-nor.html
>
> The new mx25l12833f supports SFDP, the obsolete mx25l12805d does not.
> I did manage to read the SFDP, though I do not have a copy of it (I do
> not have the hardware any more).

So how would you test newer versions of this series?

> To my best knowledge SFDP table do not contain information on OTP,

That's true, but we need it to detect flashes during runtime in the
future. Imagine sometime later, there's a third flash with this
exact ID, then we need to have the SFDP to figure out if there are
any differences between yours and the new one.

-michael

On Fri, 28 Jun 2024 at 19:45, Michael Walle <mwalle@kernel.org> wrote:
>
> > What is the kernel policy regarding obsolete flash chips?
> > Macronix annonce on end of life of mx25l12805d in 2010.
>
> Which doesn't mean there are no boards using it. But EOL in 2010
> might be a strong argument to remove it. But I don't see the need
> for it, yet.

Fair enough. That means I was in the right direction.
Just need a better description for the patch :-)
I'll update for version 2.

>
> > > Also please have a look at
> > > https://docs.kernel.org/driver-api/mtd/spi-nor.html
> >
> > The new mx25l12833f supports SFDP, the obsolete mx25l12805d does not.
> > I did manage to read the SFDP, though I do not have a copy of it (I do
> > not have the hardware any more).
>
> So how would you test newer versions of this series?

I develop the OTP callbacks with a mx25l12805d for a company using it.
I am no longer in contact with them.

The testing can be done with any Macronix SPI-NOR with OTP.
I did not check, but I guess most of their chips have an OTP using the
same opcodes/methods.

At least mx25l12805d and mx25l12833f use the same.
I did not add mx25l12833f as I did not use it and as it uses
asymmetric OTP (the MTD supports symmetric OTP).

>
> > To my best knowledge SFDP table do not contain information on OTP,
>
> That's true, but we need it to detect flashes during runtime in the
> future. Imagine sometime later, there's a third flash with this
> exact ID, then we need to have the SFDP to figure out if there are
> any differences between yours and the new one.

Sure, as I said the mx25l12833f does provide SFDP.
In patch 4, I removed the size property to force the driver to read SFDP.
I was not sure about the flags though.
I did not see any difference compared to using the setting of
mx25l12805d without SFDP.
Yet my focus of work was the OTP.

Erez

>
> -michael