drivers/mtd/spi-nor/micron-st.c | 2 ++ 1 file changed, 2 insertions(+)
The datasheet for n25q00a shows that the status register has the same
layout as for n25q00, so use the same flags to enable locking support.
These flags should have been added back in commit 150ccc181588 ("mtd:
spi-nor: Enable locking for n25q128a11"), but they were removed by the
maintainer...
Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
---
Tested with a mt25qu01gbbb, which shares the same flash ID.
drivers/mtd/spi-nor/micron-st.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/mtd/spi-nor/micron-st.c b/drivers/mtd/spi-nor/micron-st.c
index 187239ccd549..17c7d6322508 100644
--- a/drivers/mtd/spi-nor/micron-st.c
+++ b/drivers/mtd/spi-nor/micron-st.c
@@ -486,6 +486,8 @@ static const struct flash_info st_nor_parts[] = {
.id = SNOR_ID(0x20, 0xbb, 0x21),
.name = "n25q00a",
.size = SZ_128M,
+ .flags = SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB | SPI_NOR_4BIT_BP |
+ SPI_NOR_BP3_SR_BIT6,
.no_sfdp_flags = SECT_4K | SPI_NOR_QUAD_READ,
.mfr_flags = USE_FSR,
.fixups = &n25q00_fixups,
--
2.35.1.1320.gc452695387.dirtyOn Mon, Oct 06 2025, Sean Anderson wrote:
> The datasheet for n25q00a shows that the status register has the same
> layout as for n25q00, so use the same flags to enable locking support.
> These flags should have been added back in commit 150ccc181588 ("mtd:
> spi-nor: Enable locking for n25q128a11"), but they were removed by the
> maintainer...
This makes it sound like the maintainer did something wrong, which is
not true. Tudor had a good reason for removing them. Jungseung did not
have the flash at hand and Tudor didn't want to apply patches that
weren't tested. Both were in agreement for removing the n25q00a changes.
If you are going to mention that commit, then mention the full context,
and then also mention what has changed since that makes it possible to
add those changes back in. Having tested them on the real hardware for
example.
>
> Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
> ---
> Tested with a mt25qu01gbbb, which shares the same flash ID.
Ughh, is this another case of flash ID reuse? Do mt25qu and n25q00a
flashes behave exactly the same and only have two names? If not, then
how do you know if n25q00a will also work with these changes?
>
> drivers/mtd/spi-nor/micron-st.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/mtd/spi-nor/micron-st.c b/drivers/mtd/spi-nor/micron-st.c
> index 187239ccd549..17c7d6322508 100644
> --- a/drivers/mtd/spi-nor/micron-st.c
> +++ b/drivers/mtd/spi-nor/micron-st.c
> @@ -486,6 +486,8 @@ static const struct flash_info st_nor_parts[] = {
> .id = SNOR_ID(0x20, 0xbb, 0x21),
> .name = "n25q00a",
> .size = SZ_128M,
> + .flags = SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB | SPI_NOR_4BIT_BP |
> + SPI_NOR_BP3_SR_BIT6,
> .no_sfdp_flags = SECT_4K | SPI_NOR_QUAD_READ,
> .mfr_flags = USE_FSR,
> .fixups = &n25q00_fixups,
--
Regards,
Pratyush Yadav
On 10/7/25 09:15, Pratyush Yadav wrote:
> On Mon, Oct 06 2025, Sean Anderson wrote:
>
>> The datasheet for n25q00a shows that the status register has the same
>> layout as for n25q00, so use the same flags to enable locking support.
>> These flags should have been added back in commit 150ccc181588 ("mtd:
>> spi-nor: Enable locking for n25q128a11"), but they were removed by the
>> maintainer...
>
> This makes it sound like the maintainer did something wrong, which is
> not true. Tudor had a good reason for removing them.
I disagree. The maintainer used his position of authority to make the
submitter second-guess their correct patch.
These flashes have capacity of greater than the 8 MiB that can be
protected using 3 BP bits. Micron (and ST before them?) addressed this
by adding a fourth BP bit. This is consistent across every flash in this
series, and is clearly documented in every datasheet. Defaulting to 3
bits is buggy behavior: we should assume flashes behave per their
datasheets until proven otherwise, especially for less-popular features
that the original submitter may not have tested.
The original patch was entirely correct, and the maintainer's
justification for removing the second hunk is flawed.
> Jungseung did not
> have the flash at hand and Tudor didn't want to apply patches that
> weren't tested. Both were in agreement for removing the n25q00a changes.
>
> If you are going to mention that commit, then mention the full context,
> and then also mention what has changed since that makes it possible to
> add those changes back in. Having tested them on the real hardware for
> example.
>
>>
>> Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
>> ---
>> Tested with a mt25qu01gbbb, which shares the same flash ID.
>
> Ughh, is this another case of flash ID reuse? Do mt25qu and n25q00a
> flashes behave exactly the same and only have two names? If not, then
> how do you know if n25q00a will also work with these changes?
I examined the datasheet for the n25q00a and determined that it has the
same status register layout.
In fact, every n25q and mt25q flash has the same status register layout,
which (as noted above) is necessary to support capacities greater than 8
MiB (and all flashes in this series have such capacity).
--Sea
>>
>> drivers/mtd/spi-nor/micron-st.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/mtd/spi-nor/micron-st.c b/drivers/mtd/spi-nor/micron-st.c
>> index 187239ccd549..17c7d6322508 100644
>> --- a/drivers/mtd/spi-nor/micron-st.c
>> +++ b/drivers/mtd/spi-nor/micron-st.c
>> @@ -486,6 +486,8 @@ static const struct flash_info st_nor_parts[] = {
>> .id = SNOR_ID(0x20, 0xbb, 0x21),
>> .name = "n25q00a",
>> .size = SZ_128M,
>> + .flags = SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB | SPI_NOR_4BIT_BP |
>> + SPI_NOR_BP3_SR_BIT6,
>> .no_sfdp_flags = SECT_4K | SPI_NOR_QUAD_READ,
>> .mfr_flags = USE_FSR,
>> .fixups = &n25q00_fixups,
>
On Tue, Oct 07 2025, Sean Anderson wrote:
> On 10/7/25 09:15, Pratyush Yadav wrote:
>> On Mon, Oct 06 2025, Sean Anderson wrote:
>>
>>> The datasheet for n25q00a shows that the status register has the same
>>> layout as for n25q00, so use the same flags to enable locking support.
>>> These flags should have been added back in commit 150ccc181588 ("mtd:
>>> spi-nor: Enable locking for n25q128a11"), but they were removed by the
>>> maintainer...
>>
>> This makes it sound like the maintainer did something wrong, which is
>> not true. Tudor had a good reason for removing them.
>
> I disagree. The maintainer used his position of authority to make the
> submitter second-guess their correct patch.
Sean, you are being very combative over such a small issue. You must
test your changes. This is one of the most basic principles in software
engineering. It was perfectly reasonable from Tudor to push back on
untested changes.
There is no abuse of "position of authority" here. When things break, we
get to do the work of putting the pieces back together. So of course, we
are reluctant to take things that increase this burden for us. Having
contributors test their changes is the simplest of things we ask for to
keep the quality bar.
Beyond that, I'd say that a little politeness goes a long way in life.
Especially towards the people maintaining the software for free that you
(or your employer) use. We are both wasting our energy on this debate.
Please stop. Take a step back and think from the other side's
perspective. And try to work _with_ people, not against them.
>
> These flashes have capacity of greater than the 8 MiB that can be
> protected using 3 BP bits. Micron (and ST before them?) addressed this
> by adding a fourth BP bit. This is consistent across every flash in this
> series, and is clearly documented in every datasheet. Defaulting to 3
> bits is buggy behavior: we should assume flashes behave per their
> datasheets until proven otherwise, especially for less-popular features
If I had a euro every time I found a bug in a datasheet, well, I would
have enough money to at least buy a nice dinner. My point is, datasheets
are not perfect. Only running on real hardware gets you the true
picture.
> that the original submitter may not have tested.
>
> The original patch was entirely correct, and the maintainer's
> justification for removing the second hunk is flawed.
>
>> Jungseung did not
>> have the flash at hand and Tudor didn't want to apply patches that
>> weren't tested. Both were in agreement for removing the n25q00a changes.
>>
>> If you are going to mention that commit, then mention the full context,
>> and then also mention what has changed since that makes it possible to
>> add those changes back in. Having tested them on the real hardware for
>> example.
>>
>>>
>>> Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
>>> ---
>>> Tested with a mt25qu01gbbb, which shares the same flash ID.
>>
>> Ughh, is this another case of flash ID reuse? Do mt25qu and n25q00a
>> flashes behave exactly the same and only have two names? If not, then
>> how do you know if n25q00a will also work with these changes?
>
> I examined the datasheet for the n25q00a and determined that it has the
> same status register layout.
Can you share the links to the datasheets?
Also, test logs would be nice to have.
>
> In fact, every n25q and mt25q flash has the same status register layout,
> which (as noted above) is necessary to support capacities greater than 8
> MiB (and all flashes in this series have such capacity).
Do they behave the same? If not, do you know how they differ? If they
behave differently, we might need to have some code that detects which
one is running. Not necessarily as part of this patch though.
>
> --Sea
>
>>>
>>> drivers/mtd/spi-nor/micron-st.c | 2 ++
>>> 1 file changed, 2 insertions(+)
>>>
>>> diff --git a/drivers/mtd/spi-nor/micron-st.c b/drivers/mtd/spi-nor/micron-st.c
>>> index 187239ccd549..17c7d6322508 100644
>>> --- a/drivers/mtd/spi-nor/micron-st.c
>>> +++ b/drivers/mtd/spi-nor/micron-st.c
>>> @@ -486,6 +486,8 @@ static const struct flash_info st_nor_parts[] = {
>>> .id = SNOR_ID(0x20, 0xbb, 0x21),
>>> .name = "n25q00a",
>>> .size = SZ_128M,
>>> + .flags = SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB | SPI_NOR_4BIT_BP |
>>> + SPI_NOR_BP3_SR_BIT6,
>>> .no_sfdp_flags = SECT_4K | SPI_NOR_QUAD_READ,
>>> .mfr_flags = USE_FSR,
>>> .fixups = &n25q00_fixups,
>>
--
Regards,
Pratyush Yadav
On 10/8/25 08:30, Pratyush Yadav wrote:
> On Tue, Oct 07 2025, Sean Anderson wrote:
>
>> On 10/7/25 09:15, Pratyush Yadav wrote:
>>> On Mon, Oct 06 2025, Sean Anderson wrote:
>>>
>>>> The datasheet for n25q00a shows that the status register has the same
>>>> layout as for n25q00, so use the same flags to enable locking support.
>>>> These flags should have been added back in commit 150ccc181588 ("mtd:
>>>> spi-nor: Enable locking for n25q128a11"), but they were removed by the
>>>> maintainer...
>>>
>>> This makes it sound like the maintainer did something wrong, which is
>>> not true. Tudor had a good reason for removing them.
>>
>> I disagree. The maintainer used his position of authority to make the
>> submitter second-guess their correct patch.
>
> Sean, you are being very combative over such a small issue. You must
> test your changes. This is one of the most basic principles in software
> engineering. It was perfectly reasonable from Tudor to push back on
> untested changes.
>
> There is no abuse of "position of authority" here. When things break, we
> get to do the work of putting the pieces back together. So of course, we
> are reluctant to take things that increase this burden for us. Having
> contributors test their changes is the simplest of things we ask for to
> keep the quality bar.
>
> Beyond that, I'd say that a little politeness goes a long way in life.
> Especially towards the people maintaining the software for free that you
> (or your employer) use. We are both wasting our energy on this debate.
> Please stop. Take a step back and think from the other side's
> perspective. And try to work _with_ people, not against them.
>
>>
>> These flashes have capacity of greater than the 8 MiB that can be
>> protected using 3 BP bits. Micron (and ST before them?) addressed this
>> by adding a fourth BP bit. This is consistent across every flash in this
>> series, and is clearly documented in every datasheet. Defaulting to 3
>> bits is buggy behavior: we should assume flashes behave per their
>> datasheets until proven otherwise, especially for less-popular features
>
> If I had a euro every time I found a bug in a datasheet, well, I would
> have enough money to at least buy a nice dinner. My point is, datasheets
> are not perfect. Only running on real hardware gets you the true
> picture.
Well, it's even *more* buggy to pretend that the datasheet doesn't exist
and just do whatever you please. Might as well reverse-engineer every
chip that comes across your desk from first principles with that
attitude.
The locking doesn't work on any of these flashes without these flags. If
you don't believe me you can try it yourself. The people who submitted
the original patches certainly didn't test it.
--Sean
On Thu, Oct 09 2025, Sean Anderson wrote:
> On 10/8/25 08:30, Pratyush Yadav wrote:
>> On Tue, Oct 07 2025, Sean Anderson wrote:
>>
>>> On 10/7/25 09:15, Pratyush Yadav wrote:
>>>> On Mon, Oct 06 2025, Sean Anderson wrote:
>>>>
>>>>> The datasheet for n25q00a shows that the status register has the same
>>>>> layout as for n25q00, so use the same flags to enable locking support.
>>>>> These flags should have been added back in commit 150ccc181588 ("mtd:
>>>>> spi-nor: Enable locking for n25q128a11"), but they were removed by the
>>>>> maintainer...
>>>>
>>>> This makes it sound like the maintainer did something wrong, which is
>>>> not true. Tudor had a good reason for removing them.
>>>
>>> I disagree. The maintainer used his position of authority to make the
>>> submitter second-guess their correct patch.
>>
>> Sean, you are being very combative over such a small issue. You must
>> test your changes. This is one of the most basic principles in software
>> engineering. It was perfectly reasonable from Tudor to push back on
>> untested changes.
>>
>> There is no abuse of "position of authority" here. When things break, we
>> get to do the work of putting the pieces back together. So of course, we
>> are reluctant to take things that increase this burden for us. Having
>> contributors test their changes is the simplest of things we ask for to
>> keep the quality bar.
>>
>> Beyond that, I'd say that a little politeness goes a long way in life.
>> Especially towards the people maintaining the software for free that you
>> (or your employer) use. We are both wasting our energy on this debate.
>> Please stop. Take a step back and think from the other side's
>> perspective. And try to work _with_ people, not against them.
>>
>>>
>>> These flashes have capacity of greater than the 8 MiB that can be
>>> protected using 3 BP bits. Micron (and ST before them?) addressed this
>>> by adding a fourth BP bit. This is consistent across every flash in this
>>> series, and is clearly documented in every datasheet. Defaulting to 3
>>> bits is buggy behavior: we should assume flashes behave per their
>>> datasheets until proven otherwise, especially for less-popular features
>>
>> If I had a euro every time I found a bug in a datasheet, well, I would
>> have enough money to at least buy a nice dinner. My point is, datasheets
>> are not perfect. Only running on real hardware gets you the true
>> picture.
>
> Well, it's even *more* buggy to pretend that the datasheet doesn't exist
> and just do whatever you please. Might as well reverse-engineer every
> chip that comes across your desk from first principles with that
> attitude.
... or, you know, read the data sheet, write the driver, and _test_ if
it actually works?
>
> The locking doesn't work on any of these flashes without these flags. If
> you don't believe me you can try it yourself. The people who submitted
> the original patches certainly didn't test it.
Right. So can you send the patches you _did_ test on the hardware you do
have? So this time we are sure we got it right. And reply to the other
review comments? Without that, I don't think this patch can make
progress.
--
Regards,
Pratyush Yadav
On 10/9/25 19:07, Pratyush Yadav wrote:
> On Thu, Oct 09 2025, Sean Anderson wrote:
>
>> On 10/8/25 08:30, Pratyush Yadav wrote:
>>> On Tue, Oct 07 2025, Sean Anderson wrote:
>>>
>>>> On 10/7/25 09:15, Pratyush Yadav wrote:
>>>>> On Mon, Oct 06 2025, Sean Anderson wrote:
>>>>>
>>>>>> The datasheet for n25q00a shows that the status register has the same
>>>>>> layout as for n25q00, so use the same flags to enable locking support.
>>>>>> These flags should have been added back in commit 150ccc181588 ("mtd:
>>>>>> spi-nor: Enable locking for n25q128a11"), but they were removed by the
>>>>>> maintainer...
>>>>>
>>>>> This makes it sound like the maintainer did something wrong, which is
>>>>> not true. Tudor had a good reason for removing them.
>>>>
>>>> I disagree. The maintainer used his position of authority to make the
>>>> submitter second-guess their correct patch.
>>>
>>> Sean, you are being very combative over such a small issue. You must
>>> test your changes. This is one of the most basic principles in software
>>> engineering. It was perfectly reasonable from Tudor to push back on
>>> untested changes.
>>>
>>> There is no abuse of "position of authority" here. When things break, we
>>> get to do the work of putting the pieces back together. So of course, we
>>> are reluctant to take things that increase this burden for us. Having
>>> contributors test their changes is the simplest of things we ask for to
>>> keep the quality bar.
>>>
>>> Beyond that, I'd say that a little politeness goes a long way in life.
>>> Especially towards the people maintaining the software for free that you
>>> (or your employer) use. We are both wasting our energy on this debate.
>>> Please stop. Take a step back and think from the other side's
>>> perspective. And try to work _with_ people, not against them.
>>>
>>>>
>>>> These flashes have capacity of greater than the 8 MiB that can be
>>>> protected using 3 BP bits. Micron (and ST before them?) addressed this
>>>> by adding a fourth BP bit. This is consistent across every flash in this
>>>> series, and is clearly documented in every datasheet. Defaulting to 3
>>>> bits is buggy behavior: we should assume flashes behave per their
>>>> datasheets until proven otherwise, especially for less-popular features
>>>
>>> If I had a euro every time I found a bug in a datasheet, well, I would
>>> have enough money to at least buy a nice dinner. My point is, datasheets
>>> are not perfect. Only running on real hardware gets you the true
>>> picture.
>>
>> Well, it's even *more* buggy to pretend that the datasheet doesn't exist
>> and just do whatever you please. Might as well reverse-engineer every
>> chip that comes across your desk from first principles with that
>> attitude.
>
> ... or, you know, read the data sheet, write the driver, and _test_ if
> it actually works?
Which I did.
But apparently it's not enough to confirm that the datasheet does
describe the hardware.
Which is frankly absurd because the first generation chips have been
obsolete for a decade and there are literally dozens of variants of
these chips and none of them have any documented difference in the
status register.
The datasheet is innocent until proven guilty. While it's important to
verify its claims, it's up to *you* to prove it's wrong. You have given
me no evidence to believe it is incorrect.
And you would prefer to believe that the existing (broken!) behavior is
better? The one that clearly no one tested locking with because:
- Locking literally can't work with the "default" BP semantics. BP holds
the log2 size of the protected area in eraseblocks plus one. The maximum number
of eraseblocks that can be protected with 3 bits is 1 << (7 - 1) = 64. So
if you have more than 64 eraseblocks (the smallest flash in this
series has 128) you have to come up with different semantics:
- Enlarge the protection granularity (e.g. to some multiple of the
eraseblock size or to
- Add another bit to BP (what Micron did)
Both of these are incompatible with existing behavior and need
feature flags since they can't be discovered via SFDP. If you pretend
that BP only has 3 bits, you will either not lock the whole flash (if
you wanted to lock the whole thing) or you will lock too much of the
flash (if you only wanted to lock part of it).
- Locking is broken based on the documentation. Every datasheet for
flashes in this series clearly shows the 4th BP bit in the status
register:
https://media.digikey.com/pdf/Data%20Sheets/Micron%20Technology%20Inc%20PDFs/N25Q032A_RevJ.pdf
This is the only exception because it has 64 eraseblocks and doesn't
need an extra BP bit.
https://media.digikey.com/pdf/Data%20Sheets/Micron%20Technology%20Inc%20PDFs/N25Q064Ax3.pdf
https://media.digikey.com/pdf/Data%20Sheets/Micron%20Technology%20Inc%20PDFs/N25Q128A.pdf
https://mm.digikey.com/Volume0/opasdata/d220001/medias/docus/6657/MT25QL256ABA8ESF-0SIT.pdf
https://mm.digikey.com/Volume0/opasdata/d220001/medias/docus/789/N25Q256A.pdf
https://mm.digikey.com/Volume0/opasdata/d220001/medias/docus/7156/mt25q-qlkt-l-512-abb-0.pdf
https://www.digikey.com/htmldatasheets/production/1952775/0/0/1/N25Q512Ax3.pdf
https://www.alldatasheet.com/datasheet-pdf/download/585916/MICRON/N25Q00AA.html
https://mm.digikey.com/Volume0/opasdata/d220001/medias/docus/6242/MT25QL02GCBB.pdf
https://www.digikey.com/htmldatasheets/production/2058522/0/0/1/n25q016a.pdf
etc.
- Locking is broken based on my testing.
- Locking is broken whenever anyone else tests it on one of these
flashes. And they all fix it in exactly the same way:
f3f2b7eb2f1c ("mtd: spi-nor: Enable locking for n25q512ax3/n25q512a")
f80ff13135cb ("mtd: spi-nor: micron-st: Enable locking for n25q00")
bcc0c61e6134 ("mtd: spi-nor: micron-st: Enable locking for mt25qu256a")
a2a3e5430e7b ("mtd: spi-nor: micron-st: enable lock/unlock for mt25qu512a")
- Locking is not in your suggested test procedure for new flashes
(although it probably should be if you're so gung ho about mistrusting
datasheets).
- None of the commits adding support for these chips report testing
locking:
c692ba6de1c5 ("mtd: spi-nor: micron-st: Add support for mt25qu01g")
7f412111e276 ("mtd: spi-nor: Add entries for mt25q variants")
bd8a6e31b87b ("mtd: spi-nor: Split mt25qu512a (n25q512a) entry into two")
9607af6f857f ("mtd: spi-nor: Rename "n25q512a" to "mt25qu512a (n25q512a)"")
21ed90acd178 ("mtd: spi-nor: Add Micron MT25QL02 support")
a98086e00420 ("mtd: spi-nor: add entry for mt35xu512aba flash")
56c6855c81c8 ("mtd: spi-nor: Add Micron MT25QU02 support")
835ed7bf1260 ("mtd: spi-nor: Add support for N25Q256A11")
61e4611864b3 ("mtd: spi-nor: Add support for N25Q016A")
cebc1fd06907 ("mtd: spi-nor: Added support for n25q00a.")
f9bcb6dc8013 ("mtd: spi-nor: Add support for Micron n25q032a")
2a06c7b1fd23 ("mtd: spi-nor: Add support for Micron n25q064a serial flash")
c14deddec1fb ("mtd: spi-nor: add support for flag status register on Micron chips")
867f770de812 ("mtd: m25p80: Add support for Micron N25Q512A memory")
98a9e2450667 ("mtd: m25p80: modify info for Micron N25Q128")
3105875f6b89 ("mtd: m25p80: add support for Micron N25Q128")
8da28681eb14 ("mtd: m25p80: add support for Micron N25Q256A")
There is no evidence that the status register has three bits (except
when the flash has 64 eraseblocks or fewer), and there overwhelming
evidence to the contrary.
--Sean
>>
>> The locking doesn't work on any of these flashes without these flags. If
>> you don't believe me you can try it yourself. The people who submitted
>> the original patches certainly didn't test it.
>
> Right. So can you send the patches you _did_ test on the hardware you do
> have? So this time we are sure we got it right. And reply to the other
> review comments? Without that, I don't think this patch can make
> progress.
>
Sean, On 10/10/25 4:45 PM, Sean Anderson wrote: >> ... or, you know, read the data sheet, write the driver, and _test_ if >> it actually works? > > Which I did. cut > - Locking is not in your suggested test procedure for new flashes > (although it probably should be if you're so gung ho about mistrusting > datasheets). cut > There is no evidence that the status register has three bits (except > when the flash has 64 eraseblocks or fewer), and there overwhelming > evidence to the contrary. Nobody challenged that your flash has 4 BP bits. I/we just want the proof that you did the tests, i.e. show the output of the mtd-utils locking tests. I also need you to dump the sysfs/debugs entries. It's true that the locking tests are not yet described in https://docs.kernel.org/driver-api/mtd/spi-nor.html, that's why I encouraged you to show us how you did the testing and maybe to contribute and extend the documentation. Cheers, ta
On Wed, Oct 08 2025, Pratyush Yadav wrote: > On Tue, Oct 07 2025, Sean Anderson wrote: > [...] >>>> Tested with a mt25qu01gbbb, which shares the same flash ID. >>> >>> Ughh, is this another case of flash ID reuse? Do mt25qu and n25q00a >>> flashes behave exactly the same and only have two names? If not, then >>> how do you know if n25q00a will also work with these changes? >> >> I examined the datasheet for the n25q00a and determined that it has the >> same status register layout. > > Can you share the links to the datasheets? > > Also, test logs would be nice to have. > >> >> In fact, every n25q and mt25q flash has the same status register layout, >> which (as noted above) is necessary to support capacities greater than 8 >> MiB (and all flashes in this series have such capacity). > > Do they behave the same? If not, do you know how they differ? If they To clarify, I mean behave the same in things other than the status register. > behave differently, we might need to have some code that detects which > one is running. Not necessarily as part of this patch though. [...] -- Regards, Pratyush Yadav
On 10/6/25 18:34, Sean Anderson wrote:
> The datasheet for n25q00a shows that the status register has the same
> layout as for n25q00, so use the same flags to enable locking support.
> These flags should have been added back in commit 150ccc181588 ("mtd:
> spi-nor: Enable locking for n25q128a11"), but they were removed by the
Sorry, this should be commit f80ff13135cb ("mtd: spi-nor: micron-st: Enable locking for n25q00")
https://lore.kernel.org/all/20200421063313.32655-1-js07.lee@samsung.com/
> maintainer...
>
> Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
> ---
> Tested with a mt25qu01gbbb, which shares the same flash ID.
>
> drivers/mtd/spi-nor/micron-st.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/mtd/spi-nor/micron-st.c b/drivers/mtd/spi-nor/micron-st.c
> index 187239ccd549..17c7d6322508 100644
> --- a/drivers/mtd/spi-nor/micron-st.c
> +++ b/drivers/mtd/spi-nor/micron-st.c
> @@ -486,6 +486,8 @@ static const struct flash_info st_nor_parts[] = {
> .id = SNOR_ID(0x20, 0xbb, 0x21),
> .name = "n25q00a",
> .size = SZ_128M,
> + .flags = SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB | SPI_NOR_4BIT_BP |
> + SPI_NOR_BP3_SR_BIT6,
> .no_sfdp_flags = SECT_4K | SPI_NOR_QUAD_READ,
> .mfr_flags = USE_FSR,
> .fixups = &n25q00_fixups,
Hi, Sean,
On 10/6/25 11:38 PM, Sean Anderson wrote:
> On 10/6/25 18:34, Sean Anderson wrote:
>> The datasheet for n25q00a shows that the status register has the same
>> layout as for n25q00, so use the same flags to enable locking support.
>> These flags should have been added back in commit 150ccc181588 ("mtd:
Were the flags removed upstream and then not added back?
>> spi-nor: Enable locking for n25q128a11"), but they were removed by the
>
> Sorry, this should be commit f80ff13135cb ("mtd: spi-nor: micron-st: Enable locking for n25q00")
>
> https://lore.kernel.org/all/20200421063313.32655-1-js07.lee@samsung.com/
The rule is still true today: I don't queue patches that are not
functionally tested, even if they are based on datasheet info.
>
>> maintainer...
Don't point fingers please. If you feel the context is worth
mentioning, specify it in an impersonal way and add a link to the
discussion in the commit message.
>>
>> Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
>> ---
>> Tested with a mt25qu01gbbb, which shares the same flash ID.
Would you please let us know how you tested the support?
If you feel generous and want to give back to the community, you can also
describe your testing steps in the documentation from:
https://docs.kernel.org/driver-api/mtd/spi-nor.html
Also, if there's going to be a v2, please dump the SPI NOR sysfs and
debugfs data, see how in the link from above. We're keeping a database
and it will help us differentiate flashes that have the same flash ID
but different functionalities.
Cheers,
ta
>>
>> drivers/mtd/spi-nor/micron-st.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/mtd/spi-nor/micron-st.c b/drivers/mtd/spi-nor/micron-st.c
>> index 187239ccd549..17c7d6322508 100644
>> --- a/drivers/mtd/spi-nor/micron-st.c
>> +++ b/drivers/mtd/spi-nor/micron-st.c
>> @@ -486,6 +486,8 @@ static const struct flash_info st_nor_parts[] = {
>> .id = SNOR_ID(0x20, 0xbb, 0x21),
>> .name = "n25q00a",
>> .size = SZ_128M,
>> + .flags = SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB | SPI_NOR_4BIT_BP |
>> + SPI_NOR_BP3_SR_BIT6,
>> .no_sfdp_flags = SECT_4K | SPI_NOR_QUAD_READ,
>> .mfr_flags = USE_FSR,
>> .fixups = &n25q00_fixups,
>
On Wed, Oct 08 2025, Tudor Ambarus wrote:
> Hi, Sean,
>
> On 10/6/25 11:38 PM, Sean Anderson wrote:
>> On 10/6/25 18:34, Sean Anderson wrote:
>>> The datasheet for n25q00a shows that the status register has the same
>>> layout as for n25q00, so use the same flags to enable locking support.
>>> These flags should have been added back in commit 150ccc181588 ("mtd:
>
> Were the flags removed upstream and then not added back?
>
>>> spi-nor: Enable locking for n25q128a11"), but they were removed by the
>>
>> Sorry, this should be commit f80ff13135cb ("mtd: spi-nor: micron-st: Enable locking for n25q00")
>>
>> https://lore.kernel.org/all/20200421063313.32655-1-js07.lee@samsung.com/
>
> The rule is still true today: I don't queue patches that are not
> functionally tested, even if they are based on datasheet info.
>
>>
>>> maintainer...
>
> Don't point fingers please. If you feel the context is worth
> mentioning, specify it in an impersonal way and add a link to the
> discussion in the commit message.
+1.
>
>>>
>>> Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
>>> ---
>>> Tested with a mt25qu01gbbb, which shares the same flash ID.
>
> Would you please let us know how you tested the support?
>
> If you feel generous and want to give back to the community, you can also
> describe your testing steps in the documentation from:
> https://docs.kernel.org/driver-api/mtd/spi-nor.html
>
> Also, if there's going to be a v2, please dump the SPI NOR sysfs and
> debugfs data, see how in the link from above. We're keeping a database
> and it will help us differentiate flashes that have the same flash ID
> but different functionalities.
There will need to be a v2. I'm not applying the commit message in its
current form.
Would be nice to have a sysfs and debugfs dump too. Sean, the data we
need can be found in
https://docs.kernel.org/driver-api/mtd/spi-nor.html.
[...]
--
Regards,
Pratyush Yadav
© 2016 - 2025 Red Hat, Inc.