diff mbox series

[v5,1/5] mtd: spi-nor: core: add manufacturer flags

Message ID 20240920181231.20542-2-erezgeva@nwtime.org
State Superseded
Delegated to: Ambarus Tudor
Headers show
Series Add support for SPI-NOR Macronix OTP | expand

Commit Message

Erez Geva Sept. 20, 2024, 6:12 p.m. UTC
From: Erez Geva <ErezGeva2@gmail.com>

Add flag for always trying reading SFDP:
Some vendors reuse all JEDEC IDs on manufacture table
 with new chips that support SFDP.

Add flag for reading OTP parameters from device tree.
Some vendors reuse JEDEC IDs
 with several chips with different OTP parameters.
Alternatively we read parameters from SFDP.
But the OTP parameters are absent from the SFDP.
So there is not other way but to add the OTP parameters in the device tree.

In this patch series we use the new flags with Macronix.

Signed-off-by: Erez Geva <ErezGeva2@gmail.com>
---
 drivers/mtd/spi-nor/core.c    | 36 ++++++++++++++++++++++++++++++-----
 drivers/mtd/spi-nor/core.h    |  7 ++++++-
 drivers/mtd/spi-nor/otp.c     |  6 +++---
 drivers/mtd/spi-nor/winbond.c |  2 +-
 4 files changed, 41 insertions(+), 10 deletions(-)

Comments

Tudor Ambarus Sept. 23, 2024, 6:04 a.m. UTC | #1
Hi,

On 9/20/24 7:12 PM, Erez Geva wrote:
> From: Erez Geva <ErezGeva2@gmail.com>
> 
> Add flag for always trying reading SFDP:
> Some vendors reuse all JEDEC IDs on manufacture table
>  with new chips that support SFDP.
> 
> Add flag for reading OTP parameters from device tree.
> Some vendors reuse JEDEC IDs
>  with several chips with different OTP parameters.
> Alternatively we read parameters from SFDP.
> But the OTP parameters are absent from the SFDP.

Do you have some specific flashes that you try to identify? Why can't
they be differentiated at runtime?

> So there is not other way but to add the OTP parameters in the device tree.
> 

If there isn't any way to distinguish the flashes at runtime (which I
doubt/challenge btw), then as a last resort we introduce a dedicated
compatible for the flash in cause and specify all needed parameters in a
dedicated flash entry. This shall be more generic as further flash
parameters can be statically specified in the dedicated flash entry,
less invasive for dt, and less confusing for people when they decide
whether to use OTP or not. OTP params in device tree is a no-go.

But again, you have to prove why you can't distinguish the flash at
runtime before introducing a new flash compatible. So don't go this path
before sharing with us what you're trying to achieve.

Cheers,
ta
Erez Sept. 23, 2024, 10:31 a.m. UTC | #2
On Mon, 23 Sept 2024 at 08:04, Tudor Ambarus <tudor.ambarus@linaro.org> wrote:
>
> Hi,
>
> On 9/20/24 7:12 PM, Erez Geva wrote:
> > From: Erez Geva <ErezGeva2@gmail.com>
> >
> > Add flag for always trying reading SFDP:
> > Some vendors reuse all JEDEC IDs on manufacture table
> >  with new chips that support SFDP.
> >
> > Add flag for reading OTP parameters from device tree.
> > Some vendors reuse JEDEC IDs
> >  with several chips with different OTP parameters.
> > Alternatively we read parameters from SFDP.
> > But the OTP parameters are absent from the SFDP.
>
> Do you have some specific flashes that you try to identify? Why can't
> they be differentiated at runtime?

You can not figure OTP parameters based on  JEDEC ID and SFDP existence.
I did send a few examples.

One of them:
"How?

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?
I would gladly remove the obsolete mx25l12805d.
And skp compatibles all together."


>
> > So there is not other way but to add the OTP parameters in the device tree.
> >
>
> If there isn't any way to distinguish the flashes at runtime (which I
> doubt/challenge btw), then as a last resort we introduce a dedicated
> compatible for the flash in cause and specify all needed parameters in a
> dedicated flash entry. This shall be more generic as further flash
> parameters can be statically specified in the dedicated flash entry,
> less invasive for dt, and less confusing for people when they decide
> whether to use OTP or not. OTP params in device tree is a no-go.
>
> But again, you have to prove why you can't distinguish the flash at
> runtime before introducing a new flash compatible. So don't go this path
> before sharing with us what you're trying to achieve.

You keep sending me contradictory messages.

I told you we can not "guess" OTP settings based on JEDEC ID and SFDP existence.
It may be partial and Macronix may add new chips in the future.
They reuse JEDEC ID only retaining flash size and blocks.
This is why compatibilities work with new Macronix chips . Although by
reading the SFDP, we can use higher speeds.
We can use SFDP parameters to read  flash size, blocks and speed.
But it does not contain any OTP parameters.
I found only one Macronix chip with an enterprise SFDP table with a
boolean flag for OTP, this does not help us much.
Macronix technical support was explicit on OTP settings. You can not
deduce them. You must know what chip you use.
As far as I can see, Macronix does not reuse module names (god thanks for that).

I do not mind using flash compatible.
Just clarify that point.
And I will send the patches accordingly.

Thanks
Erez

>
> Cheers,
> ta
Michael Walle Sept. 23, 2024, 10:46 a.m. UTC | #3
Hi,

> I would gladly remove the obsolete mx25l12805d.

Why? I don't see any need for that.

> > If there isn't any way to distinguish the flashes at runtime (which I
> > doubt/challenge btw), then as a last resort we introduce a dedicated
> > compatible for the flash in cause and specify all needed parameters in a
> > dedicated flash entry. This shall be more generic as further flash
> > parameters can be statically specified in the dedicated flash entry,
> > less invasive for dt, and less confusing for people when they decide
> > whether to use OTP or not. OTP params in device tree is a no-go.
> >
> > But again, you have to prove why you can't distinguish the flash at
> > runtime before introducing a new flash compatible. So don't go this path
> > before sharing with us what you're trying to achieve.
>
> You keep sending me contradictory messages.
>
> I told you we can not "guess" OTP settings based on JEDEC ID and
> SFDP existence.

What are you trying to achieve here? I've told you we are trying
hard to figure out everything out at runtime. I'd suggest you start
with one particular device where you want OTP support for. If the
flash id is already in our database, find a way to distinguish
between the old and the new one; probably by looking at some SFDP
parameters. No need for any new compatible. Don't try to solve the
problem for all the chips out there.

Again, the reason why we are trying hard to determine that at
runtime is that these flashes are usually second source devices and
a manufacturer might just replace it with a (more or less)
compatible one. Therefore, the less information we put into the
devicetree the better. So before you are sending a new version with
the flash compatibles, you actually have to convince us that there
is no other way of knowing what kind of flash there is on your
board except for providing the name by the firmware.

-michael
Tudor Ambarus Sept. 23, 2024, 12:07 p.m. UTC | #4
On 9/23/24 11:31 AM, Erez wrote:
> On Mon, 23 Sept 2024 at 08:04, Tudor Ambarus <tudor.ambarus@linaro.org> wrote:
>>
>> Hi,
>>

Hi!

>> On 9/20/24 7:12 PM, Erez Geva wrote:
>>> From: Erez Geva <ErezGeva2@gmail.com>
>>>
>>> Add flag for always trying reading SFDP:
>>> Some vendors reuse all JEDEC IDs on manufacture table
>>>  with new chips that support SFDP.
>>>
>>> Add flag for reading OTP parameters from device tree.
>>> Some vendors reuse JEDEC IDs
>>>  with several chips with different OTP parameters.
>>> Alternatively we read parameters from SFDP.
>>> But the OTP parameters are absent from the SFDP.
>>
>> Do you have some specific flashes that you try to identify? Why can't
>> they be differentiated at runtime?
> 
> You can not figure OTP parameters based on  JEDEC ID and SFDP existence.
> I did send a few examples.
> 
> One of them:
> "How?
> 
> 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.

Ok, so you want to add support for mx25l12833f which shares the same ID
as mx25l12805d and has different OTP settings. Is that correct?

Which flash do you have at hand, both, none, just one of them?
> 
> How do we handle it?
> I would gladly remove the obsolete mx25l12805d.
> And skp compatibles all together."

I need to understand first what you're trying to do. Don't assume that I
remember what we discussed one month ago. Describe the why in the commit
message.
> 
> 
>>
>>> So there is not other way but to add the OTP parameters in the device tree.
>>>
>>
>> If there isn't any way to distinguish the flashes at runtime (which I
>> doubt/challenge btw), then as a last resort we introduce a dedicated
>> compatible for the flash in cause and specify all needed parameters in a
>> dedicated flash entry. This shall be more generic as further flash
>> parameters can be statically specified in the dedicated flash entry,
>> less invasive for dt, and less confusing for people when they decide
>> whether to use OTP or not. OTP params in device tree is a no-go.
>>
>> But again, you have to prove why you can't distinguish the flash at
>> runtime before introducing a new flash compatible. So don't go this path
>> before sharing with us what you're trying to achieve.
> 
> You keep sending me contradictory messages.

when? Please accept my apologies if that's the case, it's not in my
intention. Provide better commit message, help me help you.

> 
> I told you we can not "guess" OTP settings based on JEDEC ID and SFDP existence.

When? And more importantly, why?

> It may be partial and Macronix may add new chips in the future.

I don't understand what you mean by partial, please elaborate.

And we don't add support for what we assume new chips will look like.

> They reuse JEDEC ID only retaining flash size and blocks.

Yes, I know macronix shares flash IDs among flavors of flashes or new
chips altogether.

> This is why compatibilities work with new Macronix chips . Although by

In the last 7 years we haven't add any new compatible for SPI NOR, I
don't understand what are you referring to.

> reading the SFDP, we can use higher speeds.

I don't see what's your point with this sentence.

> We can use SFDP parameters to read  flash size, blocks and speed.
> But it does not contain any OTP parameters.
> I found only one Macronix chip with an enterprise SFDP table with a
> boolean flag for OTP, this does not help us much.

So you say that there's a specific vendor SFDP table that contains a bit
indicating whether OTP is supported or not? Use that then.

> Macronix technical support was explicit on OTP settings. You can not

Provide us the answer for your specific flash. I don't care yet about
all their flashes.

> deduce them. You must know what chip you use.

And I think I already said that you can differentiate between the two
based on SFDP presence. mx25l12833f has SFDP, thus when SFDP present use
the mx25l12833f-OTP configuration. When SFDP is not presence one may add
support for the mx25l12805d-OTP configuration.

Is there any case that I miss?

> As far as I can see, Macronix does not reuse module names (god thanks for that).
> 
> I do not mind using flash compatible.
> Just clarify that point.
> And I will send the patches accordingly.
> 

I'm afraid I haven't understood yet what you're trying to achieve to
provide some guidance.
Erez Sept. 23, 2024, 1:01 p.m. UTC | #5
On Mon, 23 Sept 2024 at 14:07, Tudor Ambarus <tudor.ambarus@linaro.org> wrote:
>
>
>
> On 9/23/24 11:31 AM, Erez wrote:
> > On Mon, 23 Sept 2024 at 08:04, Tudor Ambarus <tudor.ambarus@linaro.org> wrote:
> >>
> >> Hi,
> >>
>
> Hi!
>
> >> On 9/20/24 7:12 PM, Erez Geva wrote:
> >>> From: Erez Geva <ErezGeva2@gmail.com>
> >>>
> >>> Add flag for always trying reading SFDP:
> >>> Some vendors reuse all JEDEC IDs on manufacture table
> >>>  with new chips that support SFDP.
> >>>
> >>> Add flag for reading OTP parameters from device tree.
> >>> Some vendors reuse JEDEC IDs
> >>>  with several chips with different OTP parameters.
> >>> Alternatively we read parameters from SFDP.
> >>> But the OTP parameters are absent from the SFDP.
> >>
> >> Do you have some specific flashes that you try to identify? Why can't
> >> they be differentiated at runtime?
> >
> > You can not figure OTP parameters based on  JEDEC ID and SFDP existence.
> > I did send a few examples.
> >
> > One of them:
> > "How?
> >
> > 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.
>
> Ok, so you want to add support for mx25l12833f which shares the same ID
> as mx25l12805d and has different OTP settings. Is that correct?

My patch purpose was initially adding Mactronix OTP.
After reading a lot of Mactronix datasheets, I suggest also reading
all SFDP to all  Mactronix chips.

I add compatibility as I follow Kernel rule, that new code must be used!

>
> Which flash do you have at hand, both, none, just one of them?

When I started working on the OTP code, I used MX25L12833F.
But later I left the company.
So I use my beaglebone black and connect it to a MX25L3233F.

> >
> > How do we handle it?
> > I would gladly remove the obsolete mx25l12805d.
> > And skp compatibles all together."
>
> I need to understand first what you're trying to do. Don't assume that I
> remember what we discussed one month ago. Describe the why in the commit
> message.

"Add support for SPI-NOR Macronix OTP."
I wrote in the cover letter.
No, I do not expect you to remember everything.
I did write my intention in the cover letter.



> >
> >
> >>
> >>> So there is not other way but to add the OTP parameters in the device tree.
> >>>
> >>
> >> If there isn't any way to distinguish the flashes at runtime (which I
> >> doubt/challenge btw), then as a last resort we introduce a dedicated
> >> compatible for the flash in cause and specify all needed parameters in a
> >> dedicated flash entry. This shall be more generic as further flash
> >> parameters can be statically specified in the dedicated flash entry,
> >> less invasive for dt, and less confusing for people when they decide
> >> whether to use OTP or not. OTP params in device tree is a no-go.
> >>
> >> But again, you have to prove why you can't distinguish the flash at
> >> runtime before introducing a new flash compatible. So don't go this path
> >> before sharing with us what you're trying to achieve.
> >
> > You keep sending me contradictory messages.
>
> when? Please accept my apologies if that's the case, it's not in my
> intention. Provide better commit message, help me help you.

I tried adding a new compatibility.
You say you do not want new compatibility.
You ask if it is possible to deduce it from JEDEC ID and SFDP,
I answer that this is not possible, at least in some cases..
I try to add OTP parameters in DT. You do not like it, fair enough.
What am I to do?
Seems like a dead end.

Erez


>
> >
> > I told you we can not "guess" OTP settings based on JEDEC ID and SFDP existence.
>
> When? And more importantly, why?

I send you example of 3/4 chips that using JEDEC ID and SFDP existence
is not enough.

>
> > It may be partial and Macronix may add new chips in the future.
>
> I don't understand what you mean by partial, please elaborate.

I think Kernel like using bullet proof methods.
methods that will produce a working results.
If I find one example we can not probe the OTP parameters this way, it
means this method is NOT bullet proof.

>
> And we don't add support for what we assume new chips will look like.

This is not what I ask for.
Just trying to guess OTP parameters in current chips will break with new chips.


>
> > They reuse JEDEC ID only retaining flash size and blocks.
>
> Yes, I know macronix shares flash IDs among flavors of flashes or new
> chips altogether.

I am glad you know.

>
> > This is why compatibilities work with new Macronix chips . Although by
>
> In the last 7 years we haven't add any new compatible for SPI NOR, I
> don't understand what are you referring to.

The fact we can not deduce OTP parameters with current methods.
I do not mind in what way we do it.
But it seems there is no way according to your suggestions.

>
> > reading the SFDP, we can use higher speeds.
>
> I don't see what's your point with this sentence.

SFDP specifies different methods of reading and writing. Double, block.
But it does not contain any information on OTP.


>
> > We can use SFDP parameters to read  flash size, blocks and speed.
> > But it does not contain any OTP parameters.
> > I found only one Macronix chip with an enterprise SFDP table with a
> > boolean flag for OTP, this does not help us much.
>
> So you say that there's a specific vendor SFDP table that contains a bit
> indicating whether OTP is supported or not? Use that then.

I found only one chip like that.
All the others do not have it.
Nor any information on OTP size and number of regions.
So, no it does not help much.
I do not hold any information.

>
> > Macronix technical support was explicit on OTP settings. You can not
>
> Provide us the answer for your specific flash. I don't care yet about
> all their flashes.

There are two parts:

SFDP - all Mactronix chips whether they are in the compatibilities or
not, have SFDP.
 There is no logic to skip SFDP just because we have an old chip entry
in the compatibilities table.

OTP - I want to add only one chip with OTP, the one I test.
 But I can not rely on JEDEC ID or SFDP to deduce the OTP parameters.
 So as I understand we have two options:Add compatibility for this
chip, or add dynamic OTP parameters (in DT or sysfs).


>
> > deduce them. You must know what chip you use.
>
> And I think I already said that you can differentiate between the two
> based on SFDP presence. mx25l12833f has SFDP, thus when SFDP present use
> the mx25l12833f-OTP configuration. When SFDP is not presence one may add
> support for the mx25l12805d-OTP configuration.

No, we have 3 chips.
2 are using the same JEDEC ID and both using SFDP, yet they use a different OTP.
So, the problem is here, and probably be bigger in the future, despite
you "do not care".


>
> Is there any case that I miss?

According to your reply, I would say pretty much most.

>
> > As far as I can see, Macronix does not reuse module names (god thanks for that).
> >
> > I do not mind using flash compatible.
> > Just clarify that point.
> > And I will send the patches accordingly.
> >
>
> I'm afraid I haven't understood yet what you're trying to achieve to
> provide some guidance.

I do not understand you either.
But I try to work on my communication skills.

Thank you for your feedback

Erez
Erez Sept. 23, 2024, 1:25 p.m. UTC | #6
On Mon, 23 Sept 2024 at 12:46, Michael Walle <mwalle@kernel.org> wrote:
>
> Hi,
>
> > I would gladly remove the obsolete mx25l12805d.
>
> Why? I don't see any need for that.

Maybe because we do not want compatibility table?

>
> > > If there isn't any way to distinguish the flashes at runtime (which I
> > > doubt/challenge btw), then as a last resort we introduce a dedicated
> > > compatible for the flash in cause and specify all needed parameters in a
> > > dedicated flash entry. This shall be more generic as further flash
> > > parameters can be statically specified in the dedicated flash entry,
> > > less invasive for dt, and less confusing for people when they decide
> > > whether to use OTP or not. OTP params in device tree is a no-go.
> > >
> > > But again, you have to prove why you can't distinguish the flash at
> > > runtime before introducing a new flash compatible. So don't go this path
> > > before sharing with us what you're trying to achieve.
> >
> > You keep sending me contradictory messages.
> >
> > I told you we can not "guess" OTP settings based on JEDEC ID and
> > SFDP existence.
>
> What are you trying to achieve here? I've told you we are trying
> hard to figure out everything out at runtime. I'd suggest you start
> with one particular device where you want OTP support for. If the
> flash id is already in our database, find a way to distinguish
> between the old and the new one; probably by looking at some SFDP
> parameters. No need for any new compatibility. Don't try to solve the
> problem for all the chips out there.

I start with "Add support for SPI-NOR Macronix OTP".
With one flash chip and move to another one chip.
I never suggested adding multiple.

Yet, after some research I find that all Macronix chips in the last 15
years have SFDP.
So I added a second patch to always read the SFDP of Macronix chips.
Perhaps I should send it separately,as it seems to confuse you.

And no, I do not try to support all chips, just to remove the
restriction that if
Macronix reuses an old chip JEDEC ID, we skip the SFDP of the new
chip, because we have the old chip in the compatibility table,
although there are two distinct chips. They use different model names.
There is no reason to differentiate chips in that way, at least not
with Macronix chips.

>
> Again, the reason why we are trying hard to determine that at
> runtime is that these flashes are usually second source devices and
> a manufacturer might just replace it with a (more or less)
> compatible one. Therefore, the less information we put into the
> devicetree the better. So before you are sending a new version with
> the flash compatibles, you actually have to convince us that there
> is no other way of knowing what kind of flash there is on your
> board except for providing the name by the firmware.

No, reading the SFDP is great.
Except for OTP parameters/configuration.
As there is not way to find OTP parameters using JEDEC ID/SFDP
So as I understand there are only 2 ways to set the OTP parameters:
* Use a compatibility.
* Use dynamic OTP configuration, through DT, sysfs,

Erez


>
> -michael
Tudor Ambarus Sept. 23, 2024, 2:18 p.m. UTC | #7
On 9/23/24 2:01 PM, Erez wrote:

cut

>>>
>>> mx25l12805d has two OTP regions of 128 KiB and 384 KiB (yes asymmetric).
>>> While mx25l12833f has two OTP regions of 512 KiB.
>>
>> Ok, so you want to add support for mx25l12833f which shares the same ID
>> as mx25l12805d and has different OTP settings. Is that correct?
> 
> My patch purpose was initially adding Mactronix OTP.

support needs to be added per flash, not per manufacturer.

> After reading a lot of Mactronix datasheets, I suggest also reading
> all SFDP to all  Mactronix chips.

Why? It seems too invasive. Generally it is not recommended to issue
unsupported commands to flashes. Change just the flash that you use and
can test.

cut

>> Which flash do you have at hand, both, none, just one of them?
> 
> When I started working on the OTP code, I used MX25L12833F.
> But later I left the company.
> So I use my beaglebone black and connect it to a MX25L3233F.

I understand mx25l12805d and mx25l12833f share the same ID. How is
mx25l3233f related, does it share the same ID as the previous two?

> 
>>>
>>> How do we handle it?
>>> I would gladly remove the obsolete mx25l12805d.
>>> And skp compatibles all together."
>>
>> I need to understand first what you're trying to do. Don't assume that I
>> remember what we discussed one month ago. Describe the why in the commit
>> message.
> 
> "Add support for SPI-NOR Macronix OTP."
> I wrote in the cover letter.
> No, I do not expect you to remember everything.
> I did write my intention in the cover letter.
> 

I already read your cover letter and it didn't explain the challenges
that you're facing.

cut

>>>
>>> You keep sending me contradictory messages.
>>
>> when? Please accept my apologies if that's the case, it's not in my
>> intention. Provide better commit message, help me help you.
> 
> I tried adding a new compatibility.
> You say you do not want new compatibility.

I said new compatibility will be introduced as a last resort only if we
can't differentiate the flashes at run-time. You haven't proved me yet
that this is the case.

> You ask if it is possible to deduce it from JEDEC ID and SFDP,
> I answer that this is not possible, at least in some cases..

I'm interested just about your case, not all the possible cases.

> I try to add OTP parameters in DT. You do not like it, fair enough.
> What am I to do?
> Seems like a dead end.
> 

You can better explain what are the challenges you have and answer the
questions that we're asking ...
> Erez
> 
> 
>>
>>>
>>> I told you we can not "guess" OTP settings based on JEDEC ID and SFDP existence.
>>
>> When? And more importantly, why?
> 
> I send you example of 3/4 chips that using JEDEC ID and SFDP existence
> is not enough.

Why? Do they have the exact SFDP tables? Prove me, drop them all.
Any bit that differs in the SFDP tables is enough to differentiate the
flavors of flashes. Vendor tables included ;)
> 
>>
>>> It may be partial and Macronix may add new chips in the future.
>>
>> I don't understand what you mean by partial, please elaborate.
> 
> I think Kernel like using bullet proof methods.
> methods that will produce a working results.
> If I find one example we can not probe the OTP parameters this way, it
> means this method is NOT bullet proof.
> 
>>
>> And we don't add support for what we assume new chips will look like.
> 
> This is not what I ask for.
> Just trying to guess OTP parameters in current chips will break with new chips.
> 

Again, I'm interested just in the flashes that you use and can test. I'm
not interested in addressing theoretical problems.
> 
>>
>>> They reuse JEDEC ID only retaining flash size and blocks.
>>
>> Yes, I know macronix shares flash IDs among flavors of flashes or new
>> chips altogether.
> 
> I am glad you know.

I sense some rage and I find this inappropriate. I'm just trying to help.

cut

>>
>> And I think I already said that you can differentiate between the two
>> based on SFDP presence. mx25l12833f has SFDP, thus when SFDP present use
>> the mx25l12833f-OTP configuration. When SFDP is not presence one may add
>> support for the mx25l12805d-OTP configuration.
> 
> No, we have 3 chips.
> 2 are using the same JEDEC ID and both using SFDP, yet they use a different OTP.

Which ones are these?

I guess mx25l12805d is non-SFDP. Then mx25l12833f and mx25l3233f define
some SFDP tables. Do mx25l12833f and mx25l3233f have the same OTP
organization?

> So, the problem is here, and probably be bigger in the future, despite
> you "do not care".
> 

I hear about your problem just here, after 3 emails exchanged and at
least one hour wasted of my time.

I want to address problems one step at a time. We can address the bigger
theoretical problems in the future, if they'll ever become real.
> 
>>
>> Is there any case that I miss?
> 
> According to your reply, I would say pretty much most.

This is again inappropriate ... I will read your next email as well, but
I'm not going to tolerate such replies anymore.
Erez Sept. 23, 2024, 2:51 p.m. UTC | #8
On Mon, 23 Sept 2024 at 16:18, Tudor Ambarus <tudor.ambarus@linaro.org> wrote:
>
>
>
> On 9/23/24 2:01 PM, Erez wrote:
>
> cut
>
> >>>
> >>> mx25l12805d has two OTP regions of 128 KiB and 384 KiB (yes asymmetric).
> >>> While mx25l12833f has two OTP regions of 512 KiB.
> >>
> >> Ok, so you want to add support for mx25l12833f which shares the same ID
> >> as mx25l12805d and has different OTP settings. Is that correct?
> >
> > My patch purpose was initially adding Mactronix OTP.
>
> support needs to be added per flash, not per manufacturer.

The OTP code is per manufacturer.
Not my inventions, See drivers/mtd/spi-nor/winbond.c OTP callbacks.
My patch adds support after a single chip flash.


>
> > After reading a lot of Mactronix datasheets, I suggest also reading
> > all SFDP to all  Mactronix chips.
>
> Why? It seems too invasive. Generally it is not recommended to issue
> unsupported commands to flashes. Change just the flash that you use and
> can test.

It is not, All Mactronix chips in the last 15 years have SFDP.
The chips in the manufacturer compatibility table are all obsolete.
Mactronix reuse the JEDEC IDs. There is no point pretending these are
the same chips.



>
> cut
>
> >> Which flash do you have at hand, both, none, just one of them?
> >
> > When I started working on the OTP code, I used MX25L12833F.
> > But later I left the company.
> > So I use my beaglebone black and connect it to a MX25L3233F.
>
> I understand mx25l12805d and mx25l12833f share the same ID. How is
> mx25l3233f related, does it share the same ID as the previous two?

They are not. I just replaced the company hardware with a different one.
You ask me to report the hardware I use for testing.

The patch covers the one I use with beaglebone black.
I just mentioned the OTP callbacks are per manufacturer.
But if a new chip in the future would require different callbacks,
then just add them.
My patch is using a single chip, the one I send the testing with.
beaglebone black + MX25L3233F.

>
> >
> >>>
> >>> How do we handle it?
> >>> I would gladly remove the obsolete mx25l12805d.
> >>> And skp compatibles all together."
> >>
> >> I need to understand first what you're trying to do. Don't assume that I
> >> remember what we discussed one month ago. Describe the why in the commit
> >> message.
> >
> > "Add support for SPI-NOR Macronix OTP."
> > I wrote in the cover letter.
> > No, I do not expect you to remember everything.
> > I did write my intention in the cover letter.
> >
>
> I already read your cover letter and it didn't explain the challenges
> that you're facing.

Sorry, I will improve my cover letter.

>
> cut
>
> >>>
> >>> You keep sending me contradictory messages.
> >>
> >> when? Please accept my apologies if that's the case, it's not in my
> >> intention. Provide better commit message, help me help you.
> >
> > I tried adding a new compatibility.
> > You say you do not want new compatibility.
>
> I said new compatibility will be introduced as a last resort only if we
> can't differentiate the flashes at run-time. You haven't proved me yet
> that this is the case.

Then you do not read my explanation.
Do you wish me to send the Macronix datasheet of the 4 chips?

>
> > You ask if it is possible to deduce it from JEDEC ID and SFDP,
> > I answer that this is not possible, at least in some cases..
>
> I'm interested just about your case, not all the possible cases.

Actually it is the MX25L3233F and its previous chips.
Anyway, I will not submit a broken solution.
Whether you like the idea or not.

>
> > I try to add OTP parameters in DT. You do not like it, fair enough.
> > What am I to do?
> > Seems like a dead end.
> >
>
> You can better explain what are the challenges you have and answer the
> questions that we're asking ...

I try.
But seems you are lock in a loop.
You want to eat the cake and leave it as is.
You do not want to add compatibility and reject using dynamic OTP.
You are welcome to find a third way.
And no you can not use JEDEC ID and SFDP for that.



> > Erez
> >
> >
> >>
> >>>
> >>> I told you we can not "guess" OTP settings based on JEDEC ID and SFDP existence.
> >>
> >> When? And more importantly, why?
> >
> > I send you example of 3/4 chips that using JEDEC ID and SFDP existence
> > is not enough.
>
> Why? Do they have the exact SFDP tables? Prove me, drop them all.
> Any bit that differs in the SFDP tables is enough to differentiate the
> flavors of flashes. Vendor tables included ;)

Because the SFDP is not related to OTP in any way.
You are planning to decide OTP parameters on non relevant information.
If you wish to implement such a broken feature, you are welcomed.
I'll pass.

> >
> >>
> >>> It may be partial and Macronix may add new chips in the future.
> >>
> >> I don't understand what you mean by partial, please elaborate.
> >
> > I think Kernel like using bullet proof methods.
> > methods that will produce a working results.
> > If I find one example we can not probe the OTP parameters this way, it
> > means this method is NOT bullet proof.
> >
> >>
> >> And we don't add support for what we assume new chips will look like.
> >
> > This is not what I ask for.
> > Just trying to guess OTP parameters in current chips will break with new chips.
> >
>
> Again, I'm interested just in the flashes that you use and can test. I'm
> not interested in addressing theoretical problems.
> >
> >>
> >>> They reuse JEDEC ID only retaining flash size and blocks.
> >>
> >> Yes, I know macronix shares flash IDs among flavors of flashes or new
> >> chips altogether.
> >
> > I am glad you know.
>
> I sense some rage and I find this inappropriate. I'm just trying to help.

Because I repeat myself over and over.
And I find myself in a loop with you.

>
> cut
>
> >>
> >> And I think I already said that you can differentiate between the two
> >> based on SFDP presence. mx25l12833f has SFDP, thus when SFDP present use
> >> the mx25l12833f-OTP configuration. When SFDP is not presence one may add
> >> support for the mx25l12805d-OTP configuration.
> >
> > No, we have 3 chips.
> > 2 are using the same JEDEC ID and both using SFDP, yet they use a different OTP.
>
> Which ones are these?
>
> I guess mx25l12805d is non-SFDP. Then mx25l12833f and mx25l3233f define
> some SFDP tables. Do mx25l12833f and mx25l3233f have the same OTP
> organization?

No, that is the point.


>
> > So, the problem is here, and probably be bigger in the future, despite
> > you "do not care".
> >
>
> I hear about your problem just here, after 3 emails exchanged and at
> least one hour wasted of my time.

Sorry for wasting your time.
I am also wasting mine, I hope you appreciate mine as yourself.

>
> I want to address problems one step at a time. We can address the bigger
> theoretical problems in the future, if they'll ever become real.

As I read more than 15 datasheets of Macronix flash chips, and
specifically ask the Macronix technical staff.
This is not theoretical. But actually.
Having said that, does not mean I plan to support in this patch more
than one chip with OTP.
Only that your suggestion will lead to a broken probing of OTP
parameters, if not in the present, then in the future.
See, until today OTP was not that appealing. I seem to be the first
one who wrote OTP code for Macronix.
So it is not like you have a real experience with it.
I just start the surface of it.





> >
> >>
> >> Is there any case that I miss?
> >
> > According to your reply, I would say pretty much most.
>
> This is again inappropriate ... I will read your next email as well, but
> I'm not going to tolerate such replies anymore.

I agree on this one.
Seems you are looking for something I do not agree on.
This is not because I oppose probing,
 this is because you ask for indirect probing, against Macronix own
recommendation.

Sorry for bothering you

Yours
   Erez
Michael Walle Sept. 23, 2024, 4:19 p.m. UTC | #9
Hi,

> > > I would gladly remove the obsolete mx25l12805d.
> > Why? I don't see any need for that.
> Maybe because we do not want compatibility table?

I don't get this? Anyway, we do not remove support for older
flashes for no reason.

> No, reading the SFDP is great.
> Except for OTP parameters/configuration.
> As there is not way to find OTP parameters using JEDEC ID/SFDP
> So as I understand there are only 2 ways to set the OTP parameters:
> * Use a compatibility.
> * Use dynamic OTP configuration, through DT, sysfs,

* Use the in-kernel database to look up the parameters as it is done
  with any other flash device. If the id is reused, look for
  differences in the SFDP to figure out the correct flash device,
  then add some .fixups to manually add the OTP flags and
  parameters.

-michael
Erez Sept. 23, 2024, 4:31 p.m. UTC | #10
On Mon, 23 Sept 2024 at 18:19, Michael Walle <mwalle@kernel.org> wrote:
>
> Hi,
>
> > > > I would gladly remove the obsolete mx25l12805d.
> > > Why? I don't see any need for that.
> > Maybe because we do not want compatibility table?
>
> I don't get this? Anyway, we do not remove support for older
> flashes for no reason.

I did not insist, you asked.
Macronix stopped selling these chips 15 year ago.
How long do you want to support old chips?

>
> > No, reading the SFDP is great.
> > Except for OTP parameters/configuration.
> > As there is not way to find OTP parameters using JEDEC ID/SFDP
> > So as I understand there are only 2 ways to set the OTP parameters:
> > * Use a compatibility.
> > * Use dynamic OTP configuration, through DT, sysfs,
>
> * Use the in-kernel database to look up the parameters as it is done
>   with any other flash device. If the id is reused, look for

All IDs in table are reused.
The change I suggest is to read SFDP to all Macronix chips.
As today, we skip the RDSFDP. "to avoid no-op".

>   differences in the SFDP to figure out the correct flash device,

You can not do that unless you actually read the SFDP.

>   then add some .fixups to manually add the OTP flags and
>   parameters.

I am in favour.
As we can not use JEDEC ID or SFDP for OTP parameters.
How do you suggest manually OTP parameters?
Tudor Ambarus rejected my proposal.
I am open to implementing your suggestion.

I will split the two patches to avoid confusion.
So I will submit 2 patches:
* Always read Macronix chips SFDP, as Macronix replaced all old chips
in the Manufacture table.
* Support Macronix OTP.

Thanks for the feedback
Erez



>
> -michael
Tudor Ambarus Sept. 23, 2024, 5:52 p.m. UTC | #11
On 9/23/24 3:51 PM, Erez wrote:
> On Mon, 23 Sept 2024 at 16:18, Tudor Ambarus <tudor.ambarus@linaro.org> wrote:

cut

>>> After reading a lot of Mactronix datasheets, I suggest also reading
>>> all SFDP to all  Mactronix chips.
>>
>> Why? It seems too invasive. Generally it is not recommended to issue
>> unsupported commands to flashes. Change just the flash that you use and
>> can test.
> 
> It is not, All Mactronix chips in the last 15 years have SFDP.

This is false. I worked with an octal macronix flash that didn't define
SFDP tables, mx66something.

> The chips in the manufacturer compatibility table are all obsolete.
> Mactronix reuse the JEDEC IDs. There is no point pretending these are
> the same chips.
> 

If macronix doesn't care about backward compatibility, we/I do, and we
can't break it.

>> cut
>>
>>>> Which flash do you have at hand, both, none, just one of them?
>>>
>>> When I started working on the OTP code, I used MX25L12833F.
>>> But later I left the company.
>>> So I use my beaglebone black and connect it to a MX25L3233F.
>>
>> I understand mx25l12805d and mx25l12833f share the same ID. How is
>> mx25l3233f related, does it share the same ID as the previous two?
> 
> They are not. I just replaced the company hardware with a different one.
> You ask me to report the hardware I use for testing.

So MX25L3233F does not share the same ID as MX25L12833F and mx25l12805d?
Then why do we talk about ID reuse?
> 
> The patch covers the one I use with beaglebone black.
> I just mentioned the OTP callbacks are per manufacturer.
> But if a new chip in the future would require different callbacks,
> then just add them.
> My patch is using a single chip, the one I send the testing with.
> beaglebone black + MX25L3233F.

Sounds good.

cut

>> I said new compatibility will be introduced as a last resort only if we
>> can't differentiate the flashes at run-time. You haven't proved me yet
>> that this is the case.
> 
> Then you do not read my explanation.

What explanation? I've read your cover letter, commit message and I
didn't understood what you're trying to achieve.

> Do you wish me to send the Macronix datasheet of the 4 chips?

No, I need just a paragraph where you explain what are the challenges
and how you want to address them.
> 
>>
>>> You ask if it is possible to deduce it from JEDEC ID and SFDP,
>>> I answer that this is not possible, at least in some cases..
>>
>> I'm interested just about your case, not all the possible cases.
> 
> Actually it is the MX25L3233F and its previous chips.

Which previous chips? Do you have any such chip at hand? If not, why are
we talking about collisions?

> Anyway, I will not submit a broken solution.
> Whether you like the idea or not.
> 

Fine by me.

cut

>>>>> I told you we can not "guess" OTP settings based on JEDEC ID and SFDP existence.
>>>>
>>>> When? And more importantly, why?
>>>
>>> I send you example of 3/4 chips that using JEDEC ID and SFDP existence
>>> is not enough.
>>
>> Why? Do they have the exact SFDP tables? Prove me, drop them all.
>> Any bit that differs in the SFDP tables is enough to differentiate the
>> flavors of flashes. Vendor tables included ;)
> 
> Because the SFDP is not related to OTP in any way.
> You are planning to decide OTP parameters on non relevant information.
> If you wish to implement such a broken feature, you are welcomed.
> I'll pass.

Ideally we have a single jedec,spi-nor compatible. If there are flash ID
collisions we try very hard to differentiate the flashes at run-time.
New compatibles are introduced only if we can't differentiate the flavor
at runtime (be it by parsing SFDP or some other way).

cut

>>>> And I think I already said that you can differentiate between the two
>>>> based on SFDP presence. mx25l12833f has SFDP, thus when SFDP present use
>>>> the mx25l12833f-OTP configuration. When SFDP is not presence one may add
>>>> support for the mx25l12805d-OTP configuration.
>>>
>>> No, we have 3 chips.
>>> 2 are using the same JEDEC ID and both using SFDP, yet they use a different OTP.
>>
>> Which ones are these?
>>
>> I guess mx25l12805d is non-SFDP. Then mx25l12833f and mx25l3233f define
>> some SFDP tables. Do mx25l12833f and mx25l3233f have the same OTP
>> organization?
> 
> No, that is the point.
> 
? Do you care to explain?

cut

> 
>>>
>>>>
>>>> Is there any case that I miss?
>>>
>>> According to your reply, I would say pretty much most.
>>
>> This is again inappropriate ... I will read your next email as well, but
>> I'm not going to tolerate such replies anymore.
> 
> I agree on this one.
> Seems you are looking for something I do not agree on.

Michael disagrees with OTP being specified in DT too. We both already
suggested how to deal with flash ID collisions but you keep ignoring us ...

> This is not because I oppose probing,
>  this is because you ask for indirect probing, against Macronix own
> recommendation.

What did macronix exactly recommend? Did they say that we shouldn't
interrogate the SFDP data in order to differentiate the flashes at
run-time? If yes, why is that?
Tudor Ambarus Sept. 23, 2024, 6:04 p.m. UTC | #12
On 9/23/24 5:31 PM, Erez wrote:
> On Mon, 23 Sept 2024 at 18:19, Michael Walle <mwalle@kernel.org> wrote:
>>
>> Hi,
>>
>>>>> I would gladly remove the obsolete mx25l12805d.
>>>> Why? I don't see any need for that.
>>> Maybe because we do not want compatibility table?
>>
>> I don't get this? Anyway, we do not remove support for older
>> flashes for no reason.
> 
> I did not insist, you asked.
> Macronix stopped selling these chips 15 year ago.
> How long do you want to support old chips?
> 

as long as there's at least a user in the kernel.
>>
>>> No, reading the SFDP is great.
>>> Except for OTP parameters/configuration.
>>> As there is not way to find OTP parameters using JEDEC ID/SFDP
>>> So as I understand there are only 2 ways to set the OTP parameters:
>>> * Use a compatibility.
>>> * Use dynamic OTP configuration, through DT, sysfs,
>>
>> * Use the in-kernel database to look up the parameters as it is done
>>   with any other flash device. If the id is reused, look for
> 
> All IDs in table are reused.
> The change I suggest is to read SFDP to all Macronix chips.
> As today, we skip the RDSFDP. "to avoid no-op".
> 
>>   differences in the SFDP to figure out the correct flash device,
> 
> You can not do that unless you actually read the SFDP.
> 
>>   then add some .fixups to manually add the OTP flags and
>>   parameters.
> 
> I am in favour.
> As we can not use JEDEC ID or SFDP for OTP parameters.
> How do you suggest manually OTP parameters?

Michael literally just said how to do it. Here it is again:
```
* Use the in-kernel database to look up the parameters as it is done
  with any other flash device. If the id is reused, look for
  differences in the SFDP to figure out the correct flash device,
  then add some .fixups to manually add the OTP flags and
  parameters.
```
> Tudor Ambarus rejected my proposal.
> I am open to implementing your suggestion.
> 
> I will split the two patches to avoid confusion.
> So I will submit 2 patches:
> * Always read Macronix chips SFDP, as Macronix replaced all old chips
> in the Manufacture table.

I'll NACK it unless you prove that the old flavors of flashes are not
used anymore in the kernel.
> * Support Macronix OTP.

Hopefully this time you describe your problem in the commit message and
convince us it is worth fixing and that it makes sense for us to read
past the first paragraph.
Tudor Ambarus Sept. 23, 2024, 6:21 p.m. UTC | #13
On 9/23/24 7:04 PM, Tudor Ambarus wrote:
>> * Always read Macronix chips SFDP, as Macronix replaced all old chips
>> in the Manufacture table.
> I'll NACK it unless you prove that the old flavors of flashes are not
> used anymore in the kernel.

Even if you can prove that the older flashes are not used in the kernel
anymore, we can't just switch to parsing SFDP, because we have seen in
the past flashes with wrong SFDP data that made the flashes misbehave.
The recommended way is to update just the flashes that you can test.
Erez Sept. 23, 2024, 7:30 p.m. UTC | #14
On Mon, 23 Sept 2024 at 19:53, Tudor Ambarus <tudor.ambarus@linaro.org> wrote:
>
>
>
> On 9/23/24 3:51 PM, Erez wrote:
> > On Mon, 23 Sept 2024 at 16:18, Tudor Ambarus <tudor.ambarus@linaro.org> wrote:
>
> cut
>
> >>> After reading a lot of Mactronix datasheets, I suggest also reading
> >>> all SFDP to all  Mactronix chips.
> >>
> >> Why? It seems too invasive. Generally it is not recommended to issue
> >> unsupported commands to flashes. Change just the flash that you use and
> >> can test.
> >
> > It is not, All Mactronix chips in the last 15 years have SFDP.
>
> This is false. I worked with an octal macronix flash that didn't define
> SFDP tables, mx66something.

I did not use the mx66XXX. Is it an SPI-NOR?
I would guess that mx66something you use is already obsolete.
Or marked by Mactronix as 'not recommended for new HW'.
It might be used by a big customer using the HW for a long time.


>
> > The chips in the manufacturer compatibility table are all obsolete.
> > Mactronix reuse the JEDEC IDs. There is no point pretending these are
> > the same chips.
> >
>
> If macronix doesn't care about backward compatibility, we/I do, and we
> can't break it.

As I said, I do not ask you to do that.

I do not represent Macronix, so I can not speak in their name.
My conclusions are based on examining their datasheet.
I did ask their technical representor.
The only straight answer I got from the technical support is
that you can not derive OTP configuration based on JEDEC ID/SFDP,
and you must know what chip you use.

>
> >> cut
> >>
> >>>> Which flash do you have at hand, both, none, just one of them?
> >>>
> >>> When I started working on the OTP code, I used MX25L12833F.
> >>> But later I left the company.
> >>> So I use my beaglebone black and connect it to a MX25L3233F.
> >>
> >> I understand mx25l12805d and mx25l12833f share the same ID. How is
> >> mx25l3233f related, does it share the same ID as the previous two?
> >
> > They are not. I just replaced the company hardware with a different one.
> > You ask me to report the hardware I use for testing.
>
> So MX25L3233F does not share the same ID as MX25L12833F and mx25l12805d?
> Then why do we talk about ID reuse?

I replaced the hardware I use.
Most of the reused IDs are of old chips, usually obsolete, not selled
or not recommended for new HW.
So the chance to use 2 new chips with the same ID is limited.

I respect the fact you want to keep backward compatibility.
I just extend the approach to OTP parameters.
If an old chip with the same JEDEC ID uses different OTP parameters.
We will break backward compatibility with this old chip.

> >
> > The patch covers the one I use with beaglebone black.
> > I just mentioned the OTP callbacks are per manufacturer.
> > But if a new chip in the future would require different callbacks,
> > then just add them.
> > My patch is using a single chip, the one I send the testing with.
> > beaglebone black + MX25L3233F.
>
> Sounds good.

+

>
> cut
>
> >> I said new compatibility will be introduced as a last resort only if we
> >> can't differentiate the flashes at run-time. You haven't proved me yet
> >> that this is the case.
> >
> > Then you do not read my explanation.
>
> What explanation? I've read your cover letter, commit message and I
> didn't understood what you're trying to achieve.

This is on me.
I'll try harder with the cover letter.
I apologize.

>
> > Do you wish me to send the Macronix datasheet of the 4 chips?
>
> No, I need just a paragraph where you explain what are the challenges
> and how you want to address them.

+

I try to explain that we can not based on JEDEC ID + SFDP to figure
out the correct OTP parameters.
This is also the only straight answer from Maxtronix I got.



> >
> >>
> >>> You ask if it is possible to deduce it from JEDEC ID and SFDP,
> >>> I answer that this is not possible, at least in some cases..
> >>
> >> I'm interested just about your case, not all the possible cases.
> >
> > Actually it is the MX25L3233F and its previous chips.
>
> Which previous chips? Do you have any such chip at hand? If not, why are
> we talking about collisions?

JEDEC ID 0xc22016
MX25L3205D - No SFDP, 2 OTP  regions of 128-bit, 384-bit, Status:End of Life,
Recommended Product MX25L3206E
MX25L3206E - support SFDP, 2 OTP  regions of 128-bit, 384-bit, Status:
Not recommend for new design Recommended Product MX25L3233F
MX25L3233F - support SFDP, 1 OTP region of 4096-bit, Status Production

JEDEC ID 0xc22017
MX25L6405D - No SFDP,  2 OTP  regions of 128-bit, 384-bit, Status: End
of Life, recommend Product MX25L6406E.
MX25L6406E - support SFDP, 2 OTP  regions of 128-bit, 384-bit,
Status:Not recommended for new design,
Recommended Product MX25L6433F.
MX25L6433F - support SFDP, 2 OTP regions of 4096-bit, 4096-bit, Status
Production.

JEDEC ID 0xc22015
MX25L1606E - support SFDP, 2 OTP  regions of 128-bit, 384-bit,Status:
Not recommend for new design,
Recommended Product MX25V16066
MX25V16066 - support SFDP, No OTP. Status Production.

The older chips with the same JEDEC IDs are at end of life or not
commended for new design.
But if we talk about backward compatibility, we can have them on old Hardware.

I think that I miss a chip in the list, I remember finding 4 chips
with the same JEDEC ID.

>
> > Anyway, I will not submit a broken solution.
> > Whether you like the idea or not.
> >
>
> Fine by me.

+

>
> cut
>
> >>>>> I told you we can not "guess" OTP settings based on JEDEC ID and SFDP existence.
> >>>>
> >>>> When? And more importantly, why?
> >>>
> >>> I send you example of 3/4 chips that using JEDEC ID and SFDP existence
> >>> is not enough.
> >>
> >> Why? Do they have the exact SFDP tables? Prove me, drop them all.
> >> Any bit that differs in the SFDP tables is enough to differentiate the
> >> flavors of flashes. Vendor tables included ;)
> >
> > Because the SFDP is not related to OTP in any way.
> > You are planning to decide OTP parameters on non relevant information.
> > If you wish to implement such a broken feature, you are welcomed.
> > I'll pass.
>
> Ideally we have a single jedec,spi-nor compatible. If there are flash ID
> collisions we try very hard to differentiate the flashes at run-time.
> New compatibles are introduced only if we can't differentiate the flavor
> at runtime (be it by parsing SFDP or some other way).
>

All I say is that it is a dangerous approach to deduce in this way.
Macronix does not care about breaking, they might introduce new chips
with different SFDP.
They usually do not sell new chips with the same JEDEC IDs. but apart
from that, we can not rely on any assumption.

I can understand if you say that you do not wish to go into this mess.
But indirect probing based on JEDEC ID + SFDP is a risk, I don't think
we should take with Macronix.


> cut
>
> >>>> And I think I already said that you can differentiate between the two
> >>>> based on SFDP presence. mx25l12833f has SFDP, thus when SFDP present use
> >>>> the mx25l12833f-OTP configuration. When SFDP is not presence one may add
> >>>> support for the mx25l12805d-OTP configuration.
> >>>
> >>> No, we have 3 chips.
> >>> 2 are using the same JEDEC ID and both using SFDP, yet they use a different OTP.
> >>
> >> Which ones are these?
> >>
> >> I guess mx25l12805d is non-SFDP. Then mx25l12833f and mx25l3233f define
> >> some SFDP tables. Do mx25l12833f and mx25l3233f have the same OTP
> >> organization?
> >
> > No, that is the point.
> >
> ? Do you care to explain?

The point is that you can not predict OTP based on JEDEC ID + SFDP.
It seems as if Macronix does the OTP in parallel.
See the list above.

>
> cut
>
> >
> >>>
> >>>>
> >>>> Is there any case that I miss?
> >>>
> >>> According to your reply, I would say pretty much most.
> >>
> >> This is again inappropriate ... I will read your next email as well, but
> >> I'm not going to tolerate such replies anymore.
> >
> > I agree on this one.
> > Seems you are looking for something I do not agree on.
>
> Michael disagrees with OTP being specified in DT too. We both already
> suggested how to deal with flash ID collisions but you keep ignoring us ...

I apologize, but I do not insist on DT.
Any solution that configures OTP regardless of JEDEC ID + SFDP is OK by me,
I am open to testing and submit a patch accordingly.


>
> > This is not because I oppose probing,
> >  this is because you ask for indirect probing, against Macronix own
> > recommendation.
>
> What did macronix exactly recommend? Did they say that we shouldn't
> interrogate the SFDP data in order to differentiate the flashes at
> run-time? If yes, why is that?


I forward the reply from Holger Schulze, Field Application Engineer,
Macronix Europe N.V. I received on the 3 Jul.

I ask:
"But the OTP setting is not in the SFDP.
How can we know which OTP size and number of regions to set?"

And the reply:
"You are correct, the OTP setting is not defined in the JEDEC spec for
the SFDP table. The only way is to refer to the data sheet."

Thanks for your feedback
Erez Geva
Erez Sept. 23, 2024, 9:41 p.m. UTC | #15
On Mon, 23 Sept 2024 at 21:30, Erez <erezgeva2@gmail.com> wrote:
>
> On Mon, 23 Sept 2024 at 19:53, Tudor Ambarus <tudor.ambarus@linaro.org> wrote:
> >
> >
> >
> > On 9/23/24 3:51 PM, Erez wrote:
> > > On Mon, 23 Sept 2024 at 16:18, Tudor Ambarus <tudor.ambarus@linaro.org> wrote:
> >
> > cut
> >
> > >>> After reading a lot of Mactronix datasheets, I suggest also reading
> > >>> all SFDP to all  Mactronix chips.
> > >>
> > >> Why? It seems too invasive. Generally it is not recommended to issue
> > >> unsupported commands to flashes. Change just the flash that you use and
> > >> can test.
> > >
> > > It is not, All Mactronix chips in the last 15 years have SFDP.
> >
> > This is false. I worked with an octal macronix flash that didn't define
> > SFDP tables, mx66something.
>
> I did not use the mx66XXX. Is it an SPI-NOR?
> I would guess that mx66something you use is already obsolete.
> Or marked by Mactronix as 'not recommended for new HW'.
> It might be used by a big customer using the HW for a long time.
>
>
> >
> > > The chips in the manufacturer compatibility table are all obsolete.
> > > Mactronix reuse the JEDEC IDs. There is no point pretending these are
> > > the same chips.
> > >
> >
> > If macronix doesn't care about backward compatibility, we/I do, and we
> > can't break it.
>
> As I said, I do not ask you to do that.
>
> I do not represent Macronix, so I can not speak in their name.
> My conclusions are based on examining their datasheet.
> I did ask their technical representor.
> The only straight answer I got from the technical support is
> that you can not derive OTP configuration based on JEDEC ID/SFDP,
> and you must know what chip you use.
>
> >
> > >> cut
> > >>
> > >>>> Which flash do you have at hand, both, none, just one of them?
> > >>>
> > >>> When I started working on the OTP code, I used MX25L12833F.
> > >>> But later I left the company.
> > >>> So I use my beaglebone black and connect it to a MX25L3233F.
> > >>
> > >> I understand mx25l12805d and mx25l12833f share the same ID. How is
> > >> mx25l3233f related, does it share the same ID as the previous two?
> > >
> > > They are not. I just replaced the company hardware with a different one.
> > > You ask me to report the hardware I use for testing.
> >
> > So MX25L3233F does not share the same ID as MX25L12833F and mx25l12805d?
> > Then why do we talk about ID reuse?
>
> I replaced the hardware I use.
> Most of the reused IDs are of old chips, usually obsolete, not selled
> or not recommended for new HW.
> So the chance to use 2 new chips with the same ID is limited.
>
> I respect the fact you want to keep backward compatibility.
> I just extend the approach to OTP parameters.
> If an old chip with the same JEDEC ID uses different OTP parameters.
> We will break backward compatibility with this old chip.
>
> > >
> > > The patch covers the one I use with beaglebone black.
> > > I just mentioned the OTP callbacks are per manufacturer.
> > > But if a new chip in the future would require different callbacks,
> > > then just add them.
> > > My patch is using a single chip, the one I send the testing with.
> > > beaglebone black + MX25L3233F.
> >
> > Sounds good.
>
> +
>
> >
> > cut
> >
> > >> I said new compatibility will be introduced as a last resort only if we
> > >> can't differentiate the flashes at run-time. You haven't proved me yet
> > >> that this is the case.
> > >
> > > Then you do not read my explanation.
> >
> > What explanation? I've read your cover letter, commit message and I
> > didn't understood what you're trying to achieve.
>
> This is on me.
> I'll try harder with the cover letter.
> I apologize.
>
> >
> > > Do you wish me to send the Macronix datasheet of the 4 chips?
> >
> > No, I need just a paragraph where you explain what are the challenges
> > and how you want to address them.
>
> +
>
> I try to explain that we can not based on JEDEC ID + SFDP to figure
> out the correct OTP parameters.
> This is also the only straight answer from Maxtronix I got.
>
>
>
> > >
> > >>
> > >>> You ask if it is possible to deduce it from JEDEC ID and SFDP,
> > >>> I answer that this is not possible, at least in some cases..
> > >>
> > >> I'm interested just about your case, not all the possible cases.
> > >
> > > Actually it is the MX25L3233F and its previous chips.
> >
> > Which previous chips? Do you have any such chip at hand? If not, why are
> > we talking about collisions?
>
> JEDEC ID 0xc22016
> MX25L3205D - No SFDP, 2 OTP  regions of 128-bit, 384-bit, Status:End of Life,
> Recommended Product MX25L3206E
> MX25L3206E - support SFDP, 2 OTP  regions of 128-bit, 384-bit, Status:
> Not recommend for new design Recommended Product MX25L3233F
> MX25L3233F - support SFDP, 1 OTP region of 4096-bit, Status Production
>
> JEDEC ID 0xc22017
> MX25L6405D - No SFDP,  2 OTP  regions of 128-bit, 384-bit, Status: End
> of Life, recommend Product MX25L6406E.
> MX25L6406E - support SFDP, 2 OTP  regions of 128-bit, 384-bit,
> Status:Not recommended for new design,
> Recommended Product MX25L6433F.
> MX25L6433F - support SFDP, 2 OTP regions of 4096-bit, 4096-bit, Status
> Production.
>
> JEDEC ID 0xc22015
> MX25L1606E - support SFDP, 2 OTP  regions of 128-bit, 384-bit,Status:
> Not recommend for new design,
> Recommended Product MX25V16066
> MX25V16066 - support SFDP, No OTP. Status Production.
>
> The older chips with the same JEDEC IDs are at end of life or not
> commended for new design.
> But if we talk about backward compatibility, we can have them on old Hardware.
>
> I think that I miss a chip in the list, I remember finding 4 chips
> with the same JEDEC ID.
>
> >
> > > Anyway, I will not submit a broken solution.
> > > Whether you like the idea or not.
> > >
> >
> > Fine by me.
>
> +
>
> >
> > cut
> >
> > >>>>> I told you we can not "guess" OTP settings based on JEDEC ID and SFDP existence.
> > >>>>
> > >>>> When? And more importantly, why?
> > >>>
> > >>> I send you example of 3/4 chips that using JEDEC ID and SFDP existence
> > >>> is not enough.
> > >>
> > >> Why? Do they have the exact SFDP tables? Prove me, drop them all.
> > >> Any bit that differs in the SFDP tables is enough to differentiate the
> > >> flavors of flashes. Vendor tables included ;)
> > >
> > > Because the SFDP is not related to OTP in any way.
> > > You are planning to decide OTP parameters on non relevant information.
> > > If you wish to implement such a broken feature, you are welcomed.
> > > I'll pass.
> >
> > Ideally we have a single jedec,spi-nor compatible. If there are flash ID
> > collisions we try very hard to differentiate the flashes at run-time.
> > New compatibles are introduced only if we can't differentiate the flavor
> > at runtime (be it by parsing SFDP or some other way).
> >
>
> All I say is that it is a dangerous approach to deduce in this way.
> Macronix does not care about breaking, they might introduce new chips
> with different SFDP.
> They usually do not sell new chips with the same JEDEC IDs. but apart
> from that, we can not rely on any assumption.
>
> I can understand if you say that you do not wish to go into this mess.
> But indirect probing based on JEDEC ID + SFDP is a risk, I don't think
> we should take with Macronix.
>
>
> > cut
> >
> > >>>> And I think I already said that you can differentiate between the two
> > >>>> based on SFDP presence. mx25l12833f has SFDP, thus when SFDP present use
> > >>>> the mx25l12833f-OTP configuration. When SFDP is not presence one may add
> > >>>> support for the mx25l12805d-OTP configuration.
> > >>>
> > >>> No, we have 3 chips.
> > >>> 2 are using the same JEDEC ID and both using SFDP, yet they use a different OTP.
> > >>
> > >> Which ones are these?
> > >>
> > >> I guess mx25l12805d is non-SFDP. Then mx25l12833f and mx25l3233f define
> > >> some SFDP tables. Do mx25l12833f and mx25l3233f have the same OTP
> > >> organization?
> > >
> > > No, that is the point.
> > >
> > ? Do you care to explain?
>
> The point is that you can not predict OTP based on JEDEC ID + SFDP.
> It seems as if Macronix does the OTP in parallel.
> See the list above.
>
> >
> > cut
> >
> > >
> > >>>
> > >>>>
> > >>>> Is there any case that I miss?
> > >>>
> > >>> According to your reply, I would say pretty much most.
> > >>
> > >> This is again inappropriate ... I will read your next email as well, but
> > >> I'm not going to tolerate such replies anymore.
> > >
> > > I agree on this one.
> > > Seems you are looking for something I do not agree on.
> >
> > Michael disagrees with OTP being specified in DT too. We both already
> > suggested how to deal with flash ID collisions but you keep ignoring us ...
>
> I apologize, but I do not insist on DT.
> Any solution that configures OTP regardless of JEDEC ID + SFDP is OK by me,
> I am open to testing and submit a patch accordingly.

Just a thought.
What if we put a JEDEC ID + SFDP Macronix OTP probing code under a
kernel configuration with a poorer warning?

Erez

>
>
> >
> > > This is not because I oppose probing,
> > >  this is because you ask for indirect probing, against Macronix own
> > > recommendation.
> >
> > What did macronix exactly recommend? Did they say that we shouldn't
> > interrogate the SFDP data in order to differentiate the flashes at
> > run-time? If yes, why is that?
>
>
> I forward the reply from Holger Schulze, Field Application Engineer,
> Macronix Europe N.V. I received on the 3 Jul.
>
> I ask:
> "But the OTP setting is not in the SFDP.
> How can we know which OTP size and number of regions to set?"
>
> And the reply:
> "You are correct, the OTP setting is not defined in the JEDEC spec for
> the SFDP table. The only way is to refer to the data sheet."
>
> Thanks for your feedback
> Erez Geva
Tudor Ambarus Sept. 24, 2024, 6:01 a.m. UTC | #16
On 9/23/24 8:30 PM, Erez wrote:
> On Mon, 23 Sept 2024 at 19:53, Tudor Ambarus <tudor.ambarus@linaro.org> wrote:

cut

>>>>> You ask if it is possible to deduce it from JEDEC ID and SFDP,
>>>>> I answer that this is not possible, at least in some cases..
>>>>
>>>> I'm interested just about your case, not all the possible cases.
>>>
>>> Actually it is the MX25L3233F and its previous chips.
>>
>> Which previous chips? Do you have any such chip at hand? If not, why are
>> we talking about collisions?
> 
> JEDEC ID 0xc22016
> MX25L3205D - No SFDP, 2 OTP  regions of 128-bit, 384-bit, Status:End of Life,
> Recommended Product MX25L3206E
> MX25L3206E - support SFDP, 2 OTP  regions of 128-bit, 384-bit, Status:
> Not recommend for new design Recommended Product MX25L3233F
> MX25L3233F - support SFDP, 1 OTP region of 4096-bit, Status Production
> 

Okay. So you have just MX25L3233F on your table and you are worried
about possible ID collisions with MX25L3206E and MX25L3205D. But you
don't have access to MX25L3206E and MX25L3205D. Is my understanding correct?

If it's a yes, then don't worry about ID collisions at all, you can't
test the other flashes anyway. OTP is not used by the older flashes as
there's no support right now, so you don't break anything. Just add your
OTP organization to your flash and I (or other SPI NOR maintainer) will
decide on how to handle the ID collisions when/if they appear.

> JEDEC ID 0xc22017
> MX25L6405D - No SFDP,  2 OTP  regions of 128-bit, 384-bit, Status: End
> of Life, recommend Product MX25L6406E.
> MX25L6406E - support SFDP, 2 OTP  regions of 128-bit, 384-bit,
> Status:Not recommended for new design,
> Recommended Product MX25L6433F.
> MX25L6433F - support SFDP, 2 OTP regions of 4096-bit, 4096-bit, Status
> Production.
> 
> JEDEC ID 0xc22015
> MX25L1606E - support SFDP, 2 OTP  regions of 128-bit, 384-bit,Status:
> Not recommend for new design,
> Recommended Product MX25V16066
> MX25V16066 - support SFDP, No OTP. Status Production.

Thanks for the dive. We can ignore these because you can't test any of
them. I'll worry about them when someone adds OTP support for them.

cut

>>
>>> This is not because I oppose probing,
>>>  this is because you ask for indirect probing, against Macronix own
>>> recommendation.
>>
>> What did macronix exactly recommend? Did they say that we shouldn't
>> interrogate the SFDP data in order to differentiate the flashes at
>> run-time? If yes, why is that?
> 
> 
> I forward the reply from Holger Schulze, Field Application Engineer,
> Macronix Europe N.V. I received on the 3 Jul.
> 
> I ask:
> "But the OTP setting is not in the SFDP.
> How can we know which OTP size and number of regions to set?"
> 
> And the reply:
> "You are correct, the OTP setting is not defined in the JEDEC spec for
> the SFDP table. The only way is to refer to the data sheet."

We already know that the OTP is not defined in any SFDP table. This
doesn't mean that we can't compare SFDP tables to determine which flavor
of flash is present. That is the reason why we ask for SFDP dumps when
someone proposes a flash update or addition. So that we can compare the
SFDP dumps and correctly identify the flash or assure backward
compatibility for it.
Tudor Ambarus Sept. 24, 2024, 6:04 a.m. UTC | #17
On 9/23/24 10:41 PM, Erez wrote:
> What if we put a JEDEC ID + SFDP Macronix OTP probing code under a
> kernel configuration with a poorer warning?

No, we don't add kernel configs for such things.
Esben Haabendal Sept. 26, 2024, 7:46 a.m. UTC | #18
Erez <erezgeva2@gmail.com> writes:

> On Mon, 23 Sept 2024 at 18:19, Michael Walle <mwalle@kernel.org> wrote:
>>
>> > > > I would gladly remove the obsolete mx25l12805d.
>> > > Why? I don't see any need for that.
>> > Maybe because we do not want compatibility table?
>>
>> I don't get this? Anyway, we do not remove support for older
>> flashes for no reason.
>
> I did not insist, you asked.
> Macronix stopped selling these chips 15 year ago.
> How long do you want to support old chips?

It is not unusual for embedded products to have a support span of more
than 20 years. And chips such as these flashes might not be entirely new
when the product is introduced. So dropping support for SPI-NOR flashes
that are newer than 25-30 years is definitely a risk. Somebody out there
might not be able to upgrade to latest kernel versions anymore, which is
not a position we should put anyone in. With the increasing pressure to
upgrade product for better security, we definitely should not make it
more difficult to run newer kernel versions than absolutely necessary.

/Esben
Erez Sept. 26, 2024, 11:08 a.m. UTC | #19
On Thu, 26 Sept 2024 at 09:46, Esben Haabendal <esben@geanix.com> wrote:
>
> Erez <erezgeva2@gmail.com> writes:
>
> > On Mon, 23 Sept 2024 at 18:19, Michael Walle <mwalle@kernel.org> wrote:
> >>
> >> > > > I would gladly remove the obsolete mx25l12805d.
> >> > > Why? I don't see any need for that.
> >> > Maybe because we do not want compatibility table?
> >>
> >> I don't get this? Anyway, we do not remove support for older
> >> flashes for no reason.
> >
> > I did not insist, you asked.
> > Macronix stopped selling these chips 15 year ago.
> > How long do you want to support old chips?
>
> It is not unusual for embedded products to have a support span of more
> than 20 years. And chips such as these flashes might not be entirely new
> when the product is introduced. So dropping support for SPI-NOR flashes
> that are newer than 25-30 years is definitely a risk. Somebody out there
> might not be able to upgrade to latest kernel versions anymore, which is
> not a position we should put anyone in. With the increasing pressure to
> upgrade product for better security, we definitely should not make it
> more difficult to run newer kernel versions than absolutely necessary.

I do not insist. Nor send any patch in this direction.

Each project can define the extent of backward compatibility.
In terms of compilers, linkers and tools, i.e. build environment.
In terms of standards like the C standard we use.
In terms of network protocols.
And also what Hardware do we support.

There is no harm in asking where the boundaries are.
All projects move their boundaries all the time.
The Linux kernel is no exception.

Erez

>
> /Esben
Esben Haabendal Sept. 26, 2024, 11:37 a.m. UTC | #20
Erez <erezgeva2@gmail.com> writes:

> On Thu, 26 Sept 2024 at 09:46, Esben Haabendal <esben@geanix.com> wrote:
>>
>> Erez <erezgeva2@gmail.com> writes:
>>
>> > On Mon, 23 Sept 2024 at 18:19, Michael Walle <mwalle@kernel.org> wrote:
>> >>
>> >> > > > I would gladly remove the obsolete mx25l12805d.
>> >> > > Why? I don't see any need for that.
>> >> > Maybe because we do not want compatibility table?
>> >>
>> >> I don't get this? Anyway, we do not remove support for older
>> >> flashes for no reason.
>> >
>> > I did not insist, you asked.
>> > Macronix stopped selling these chips 15 year ago.
>> > How long do you want to support old chips?
>>
>> It is not unusual for embedded products to have a support span of more
>> than 20 years. And chips such as these flashes might not be entirely new
>> when the product is introduced. So dropping support for SPI-NOR flashes
>> that are newer than 25-30 years is definitely a risk. Somebody out there
>> might not be able to upgrade to latest kernel versions anymore, which is
>> not a position we should put anyone in. With the increasing pressure to
>> upgrade product for better security, we definitely should not make it
>> more difficult to run newer kernel versions than absolutely necessary.
>
> I do not insist. Nor send any patch in this direction.

I did not say or imply that you did any such thing.

You asked an open question, and I gave my response. Nothing more,
nothing less.

> Each project can define the extent of backward compatibility.
> In terms of compilers, linkers and tools, i.e. build environment.
> In terms of standards like the C standard we use.
> In terms of network protocols.
> And also what Hardware do we support.
>
> There is no harm in asking where the boundaries are.
> All projects move their boundaries all the time.
> The Linux kernel is no exception.
Erez Sept. 26, 2024, 1:16 p.m. UTC | #21
On Thu, 26 Sept 2024 at 13:37, Esben Haabendal <esben@geanix.com> wrote:
>
> Erez <erezgeva2@gmail.com> writes:
>
> > On Thu, 26 Sept 2024 at 09:46, Esben Haabendal <esben@geanix.com> wrote:
> >>
> >> Erez <erezgeva2@gmail.com> writes:
> >>
> >> > On Mon, 23 Sept 2024 at 18:19, Michael Walle <mwalle@kernel.org> wrote:
> >> >>
> >> >> > > > I would gladly remove the obsolete mx25l12805d.
> >> >> > > Why? I don't see any need for that.
> >> >> > Maybe because we do not want compatibility table?
> >> >>
> >> >> I don't get this? Anyway, we do not remove support for older
> >> >> flashes for no reason.
> >> >
> >> > I did not insist, you asked.
> >> > Macronix stopped selling these chips 15 year ago.
> >> > How long do you want to support old chips?
> >>
> >> It is not unusual for embedded products to have a support span of more
> >> than 20 years. And chips such as these flashes might not be entirely new
> >> when the product is introduced. So dropping support for SPI-NOR flashes
> >> that are newer than 25-30 years is definitely a risk. Somebody out there
> >> might not be able to upgrade to latest kernel versions anymore, which is
> >> not a position we should put anyone in. With the increasing pressure to
> >> upgrade product for better security, we definitely should not make it
> >> more difficult to run newer kernel versions than absolutely necessary.
> >
> > I do not insist. Nor send any patch in this direction.
>
> I did not say or imply that you did any such thing.
>
> You asked an open question, and I gave my response. Nothing more,
> nothing less.

+

>
> > Each project can define the extent of backward compatibility.
> > In terms of compilers, linkers and tools, i.e. build environment.
> > In terms of standards like the C standard we use.
> > In terms of network protocols.
> > And also what Hardware do we support.
> >
> > There is no harm in asking where the boundaries are.
> > All projects move their boundaries all the time.
> > The Linux kernel is no exception.
diff mbox series

Patch

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 9d6e85bf227b..0238475059f0 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -2891,10 +2891,11 @@  static void spi_nor_init_params_deprecated(struct spi_nor *nor)
 
 	spi_nor_manufacturer_init_params(nor);
 
-	if (nor->info->no_sfdp_flags & (SPI_NOR_DUAL_READ |
-					SPI_NOR_QUAD_READ |
-					SPI_NOR_OCTAL_READ |
-					SPI_NOR_OCTAL_DTR_READ))
+	if ((nor->info->no_sfdp_flags & (SPI_NOR_DUAL_READ |
+					 SPI_NOR_QUAD_READ |
+					 SPI_NOR_OCTAL_READ |
+					 SPI_NOR_OCTAL_DTR_READ)) ||
+	     nor->manufacturer->flags & SPI_NOR_MANUFACT_TRY_SFDP)
 		spi_nor_sfdp_init_params_deprecated(nor);
 }
 
@@ -2911,7 +2912,32 @@  static void spi_nor_init_default_params(struct spi_nor *nor)
 	struct device_node *np = spi_nor_get_flash_node(nor);
 
 	params->quad_enable = spi_nor_sr2_bit1_quad_enable;
-	params->otp.org = info->otp;
+	memset(&params->otp.org, 0, sizeof(struct spi_nor_otp_organization));
+	if (info->otp) {
+		memcpy(&params->otp.org, info->otp, sizeof(struct spi_nor_otp_organization));
+	} else if (nor->manufacturer->flags & SPI_NOR_MANUFACT_DT_OTP) {
+		/* Check for OTP information on device tree */
+		u32 n_regions, len;
+
+		if (!of_property_read_u32(np, "otp-n-regions", &n_regions) &&
+		   n_regions > 0 &&
+		   !of_property_read_u32(np, "otp-len", &len) &&
+		   len > 0) {
+			u32 base, offset = 0;
+
+			if (n_regions > 1) {
+				/* If offset is not defined use length as offset */
+				if (of_property_read_u32(np, "otp-offset", &offset))
+					offset = len;
+			}
+			if (of_property_read_u32(np, "otp-base", &base))
+				base = 0;
+			params->otp.org.n_regions = n_regions;
+			params->otp.org.offset = offset;
+			params->otp.org.base = base;
+			params->otp.org.len = len;
+		}
+	}
 
 	/* Default to 16-bit Write Status (01h) Command */
 	nor->flags |= SNOR_F_HAS_16BIT_SR;
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index 1516b6d0dc37..c862e42c844f 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -326,7 +326,7 @@  struct spi_nor_otp_ops {
  * @ops:	OTP access ops
  */
 struct spi_nor_otp {
-	const struct spi_nor_otp_organization *org;
+	struct spi_nor_otp_organization org;
 	const struct spi_nor_otp_ops *ops;
 };
 
@@ -560,12 +560,17 @@  struct flash_info {
  * @parts: array of parts supported by this manufacturer
  * @nparts: number of entries in the parts array
  * @fixups: hooks called at various points in time during spi_nor_scan()
+ * @flags: manufacturer flags
  */
 struct spi_nor_manufacturer {
 	const char *name;
 	const struct flash_info *parts;
 	unsigned int nparts;
 	const struct spi_nor_fixups *fixups;
+
+	u8 flags;
+#define SPI_NOR_MANUFACT_TRY_SFDP	BIT(0)
+#define SPI_NOR_MANUFACT_DT_OTP		BIT(0)
 };
 
 /**
diff --git a/drivers/mtd/spi-nor/otp.c b/drivers/mtd/spi-nor/otp.c
index 9a729aa3452d..ffb7ffeb9030 100644
--- a/drivers/mtd/spi-nor/otp.c
+++ b/drivers/mtd/spi-nor/otp.c
@@ -11,8 +11,8 @@ 
 
 #include "core.h"
 
-#define spi_nor_otp_region_len(nor) ((nor)->params->otp.org->len)
-#define spi_nor_otp_n_regions(nor) ((nor)->params->otp.org->n_regions)
+#define spi_nor_otp_region_len(nor) ((nor)->params->otp.org.len)
+#define spi_nor_otp_n_regions(nor) ((nor)->params->otp.org.n_regions)
 
 /**
  * spi_nor_otp_read_secr() - read security register
@@ -222,7 +222,7 @@  int spi_nor_otp_is_locked_sr2(struct spi_nor *nor, unsigned int region)
 
 static loff_t spi_nor_otp_region_start(const struct spi_nor *nor, unsigned int region)
 {
-	const struct spi_nor_otp_organization *org = nor->params->otp.org;
+	const struct spi_nor_otp_organization *org = &nor->params->otp.org;
 
 	return org->base + region * org->offset;
 }
diff --git a/drivers/mtd/spi-nor/winbond.c b/drivers/mtd/spi-nor/winbond.c
index 9f7ce5763e71..b60df00e43b9 100644
--- a/drivers/mtd/spi-nor/winbond.c
+++ b/drivers/mtd/spi-nor/winbond.c
@@ -333,7 +333,7 @@  static int winbond_nor_late_init(struct spi_nor *nor)
 {
 	struct spi_nor_flash_parameter *params = nor->params;
 
-	if (params->otp.org)
+	if (params->otp.org.n_regions)
 		params->otp.ops = &winbond_nor_otp_ops;
 
 	/*