LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] mtd: spi-nor: mt25qu: Ignore 6th ID byte
@ 2021-11-19  8:04 Alexander A Sverdlin
  2021-11-19 21:19 ` Michael Walle
  2022-07-18 15:03 ` Tudor.Ambarus
  0 siblings, 2 replies; 14+ messages in thread
From: Alexander A Sverdlin @ 2021-11-19  8:04 UTC (permalink / raw)
  To: linux-mtd
  Cc: Alexander Sverdlin, Tudor Ambarus, Pratyush Yadav, Michael Walle,
	Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
	linux-kernel

From: Alexander Sverdlin <alexander.sverdlin@nokia.com>

Ignore 6th ID byte, secure version of mt25qu256a has 0x73 as 6th byte.

Signed-off-by: Alexander Sverdlin <alexander.sverdlin@nokia.com>
---
 drivers/mtd/spi-nor/micron-st.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/mtd/spi-nor/micron-st.c b/drivers/mtd/spi-nor/micron-st.c
index f3d19b7..509a732 100644
--- a/drivers/mtd/spi-nor/micron-st.c
+++ b/drivers/mtd/spi-nor/micron-st.c
@@ -155,9 +155,9 @@ static const struct flash_info st_parts[] = {
 	{ "n25q256a",    INFO(0x20ba19, 0, 64 * 1024,  512, SECT_4K |
 			      USE_FSR | SPI_NOR_DUAL_READ |
 			      SPI_NOR_QUAD_READ) },
-	{ "mt25qu256a",  INFO6(0x20bb19, 0x104400, 64 * 1024,  512,
-			       SECT_4K | USE_FSR | SPI_NOR_DUAL_READ |
-			       SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) },
+	{ "mt25qu256a",  INFO(0x20bb19, 0x1044, 64 * 1024,  512,
+			      SECT_4K | USE_FSR | SPI_NOR_DUAL_READ |
+			      SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) },
 	{ "n25q256ax1",  INFO(0x20bb19, 0, 64 * 1024,  512,
 			      SECT_4K | USE_FSR | SPI_NOR_QUAD_READ) },
 	{ "mt25ql512a",  INFO6(0x20ba20, 0x104400, 64 * 1024, 1024,
@@ -167,9 +167,9 @@ static const struct flash_info st_parts[] = {
 			      SECT_4K | USE_FSR | SPI_NOR_QUAD_READ |
 			      SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB |
 			      SPI_NOR_4BIT_BP | SPI_NOR_BP3_SR_BIT6) },
-	{ "mt25qu512a",  INFO6(0x20bb20, 0x104400, 64 * 1024, 1024,
-			       SECT_4K | USE_FSR | SPI_NOR_DUAL_READ |
-			       SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) },
+	{ "mt25qu512a",  INFO(0x20bb20, 0x1044, 64 * 1024, 1024,
+			      SECT_4K | USE_FSR | SPI_NOR_DUAL_READ |
+			      SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) },
 	{ "n25q512a",    INFO(0x20bb20, 0, 64 * 1024, 1024,
 			      SECT_4K | USE_FSR | SPI_NOR_QUAD_READ |
 			      SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB |
-- 
2.10.2


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] mtd: spi-nor: mt25qu: Ignore 6th ID byte
  2021-11-19  8:04 [PATCH] mtd: spi-nor: mt25qu: Ignore 6th ID byte Alexander A Sverdlin
@ 2021-11-19 21:19 ` Michael Walle
  2021-11-22  7:06   ` Alexander Sverdlin
  2022-07-18 15:03 ` Tudor.Ambarus
  1 sibling, 1 reply; 14+ messages in thread
From: Michael Walle @ 2021-11-19 21:19 UTC (permalink / raw)
  To: Alexander A Sverdlin
  Cc: linux-mtd, Tudor Ambarus, Pratyush Yadav, Miquel Raynal,
	Richard Weinberger, Vignesh Raghavendra, linux-kernel

Hi,

Am 2021-11-19 09:04, schrieb Alexander A Sverdlin:
> From: Alexander Sverdlin <alexander.sverdlin@nokia.com>
> 
> Ignore 6th ID byte, secure version of mt25qu256a has 0x73 as 6th byte.

What is the secure version? What is the difference? Do you have some
links to datasheets for both?

Also please provide the SFDP data for this flash, see [1].

-michael

[1] 
https://lore.kernel.org/linux-mtd/7038f037de3e224016d269324517400d@walle.cc/

> 
> Signed-off-by: Alexander Sverdlin <alexander.sverdlin@nokia.com>
> ---
>  drivers/mtd/spi-nor/micron-st.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/micron-st.c 
> b/drivers/mtd/spi-nor/micron-st.c
> index f3d19b7..509a732 100644
> --- a/drivers/mtd/spi-nor/micron-st.c
> +++ b/drivers/mtd/spi-nor/micron-st.c
> @@ -155,9 +155,9 @@ static const struct flash_info st_parts[] = {
>  	{ "n25q256a",    INFO(0x20ba19, 0, 64 * 1024,  512, SECT_4K |
>  			      USE_FSR | SPI_NOR_DUAL_READ |
>  			      SPI_NOR_QUAD_READ) },
> -	{ "mt25qu256a",  INFO6(0x20bb19, 0x104400, 64 * 1024,  512,
> -			       SECT_4K | USE_FSR | SPI_NOR_DUAL_READ |
> -			       SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) },
> +	{ "mt25qu256a",  INFO(0x20bb19, 0x1044, 64 * 1024,  512,
> +			      SECT_4K | USE_FSR | SPI_NOR_DUAL_READ |
> +			      SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) },
>  	{ "n25q256ax1",  INFO(0x20bb19, 0, 64 * 1024,  512,
>  			      SECT_4K | USE_FSR | SPI_NOR_QUAD_READ) },
>  	{ "mt25ql512a",  INFO6(0x20ba20, 0x104400, 64 * 1024, 1024,
> @@ -167,9 +167,9 @@ static const struct flash_info st_parts[] = {
>  			      SECT_4K | USE_FSR | SPI_NOR_QUAD_READ |
>  			      SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB |
>  			      SPI_NOR_4BIT_BP | SPI_NOR_BP3_SR_BIT6) },
> -	{ "mt25qu512a",  INFO6(0x20bb20, 0x104400, 64 * 1024, 1024,
> -			       SECT_4K | USE_FSR | SPI_NOR_DUAL_READ |
> -			       SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) },
> +	{ "mt25qu512a",  INFO(0x20bb20, 0x1044, 64 * 1024, 1024,
> +			      SECT_4K | USE_FSR | SPI_NOR_DUAL_READ |
> +			      SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) },
>  	{ "n25q512a",    INFO(0x20bb20, 0, 64 * 1024, 1024,
>  			      SECT_4K | USE_FSR | SPI_NOR_QUAD_READ |
>  			      SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB |

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] mtd: spi-nor: mt25qu: Ignore 6th ID byte
  2021-11-19 21:19 ` Michael Walle
@ 2021-11-22  7:06   ` Alexander Sverdlin
  2021-11-22 15:05     ` Michael Walle
  0 siblings, 1 reply; 14+ messages in thread
From: Alexander Sverdlin @ 2021-11-22  7:06 UTC (permalink / raw)
  To: Michael Walle
  Cc: linux-mtd, Tudor Ambarus, Pratyush Yadav, Miquel Raynal,
	Richard Weinberger, Vignesh Raghavendra, linux-kernel

Hi!

On 19/11/2021 22:19, Michael Walle wrote:
>> Ignore 6th ID byte, secure version of mt25qu256a has 0x73 as 6th byte.
> 
> What is the secure version? What is the difference? Do you have some
> links to datasheets for both?

For instance:
https://www.micron.com/products/nor-flash/serial-nor-flash/part-catalog/mt25qu256aba1ew7-0sit
https://media-www.micron.com/-/media/client/global/documents/products/data-sheet/nor-flash/serial-nor/mt25q/die-rev-a/mt25q_qljs_u_256_aba_0.pdf?rev=594079234c1b496496b062c21ce162d6

https://www.micron.com/products/nor-flash/serial-nor-flash/part-catalog/mt25qu256aba8e12-1sit
https://media-www.micron.com/-/media/client/global/documents/products/data-sheet/nor-flash/serial-nor/mt25q/die-rev-a/mt25q_qljs_u_256_aba_0.pdf?rev=594079234c1b496496b062c21ce162d6

But the differences are in "MT25Q Security Addendum":
"The additional protection features available on the secure MT25Q device include a
lock status register bit, top/bottom block address protection lock, volatile
configuration lock register at power up, protection management register lock,
and a nonvolatile configuration lock register."
This is only available under NDA from Micron.

However as long as one doesn't use these security features, it appears compatible with
non-secure version. That's why just ignoring the non-standard configuration allows
to support it.
 
> Also please provide the SFDP data for this flash, see [1].

sfdp:
53464450060101ff00060110300000ff84000102800000ffffffffffffff
ffffffffffffffffffffffffffffffffffffe520fbffffffff0f29eb276b
273b27bbffffffffffff27bbffff29eb0c2010d80f520000244a99008b8e
03d4ac0127387a757a75fbbdd55c4a0f82ff81bd3d36ffffffffffffffff
ffffffffffffffffffe7ffff21dcffff

md5sum:
5ea738216f68c9f98987bb3725699a32

jedec_id:
20bb191044

partname:
mt25qu256a

manufacturer:
st

(But last 3 do not make sense to me, as they come from the table I modify,
not from the chip itself). Further, I don't have 512Mbit chip to provide
SFDP for it.

> [1] https://lore.kernel.org/linux-mtd/7038f037de3e224016d269324517400d@walle.cc/
> 
>>
>> Signed-off-by: Alexander Sverdlin <alexander.sverdlin@nokia.com>
>> ---
>>  drivers/mtd/spi-nor/micron-st.c | 12 ++++++------
>>  1 file changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/mtd/spi-nor/micron-st.c b/drivers/mtd/spi-nor/micron-st.c
>> index f3d19b7..509a732 100644
>> --- a/drivers/mtd/spi-nor/micron-st.c
>> +++ b/drivers/mtd/spi-nor/micron-st.c
>> @@ -155,9 +155,9 @@ static const struct flash_info st_parts[] = {
>>      { "n25q256a",    INFO(0x20ba19, 0, 64 * 1024,  512, SECT_4K |
>>                    USE_FSR | SPI_NOR_DUAL_READ |
>>                    SPI_NOR_QUAD_READ) },
>> -    { "mt25qu256a",  INFO6(0x20bb19, 0x104400, 64 * 1024,  512,
>> -                   SECT_4K | USE_FSR | SPI_NOR_DUAL_READ |
>> -                   SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) },
>> +    { "mt25qu256a",  INFO(0x20bb19, 0x1044, 64 * 1024,  512,
>> +                  SECT_4K | USE_FSR | SPI_NOR_DUAL_READ |
>> +                  SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) },
>>      { "n25q256ax1",  INFO(0x20bb19, 0, 64 * 1024,  512,
>>                    SECT_4K | USE_FSR | SPI_NOR_QUAD_READ) },
>>      { "mt25ql512a",  INFO6(0x20ba20, 0x104400, 64 * 1024, 1024,
>> @@ -167,9 +167,9 @@ static const struct flash_info st_parts[] = {
>>                    SECT_4K | USE_FSR | SPI_NOR_QUAD_READ |
>>                    SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB |
>>                    SPI_NOR_4BIT_BP | SPI_NOR_BP3_SR_BIT6) },
>> -    { "mt25qu512a",  INFO6(0x20bb20, 0x104400, 64 * 1024, 1024,
>> -                   SECT_4K | USE_FSR | SPI_NOR_DUAL_READ |
>> -                   SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) },
>> +    { "mt25qu512a",  INFO(0x20bb20, 0x1044, 64 * 1024, 1024,
>> +                  SECT_4K | USE_FSR | SPI_NOR_DUAL_READ |
>> +                  SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) },
>>      { "n25q512a",    INFO(0x20bb20, 0, 64 * 1024, 1024,
>>                    SECT_4K | USE_FSR | SPI_NOR_QUAD_READ |
>>                    SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB |

-- 
Best regards,
Alexander Sverdlin.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] mtd: spi-nor: mt25qu: Ignore 6th ID byte
  2021-11-22  7:06   ` Alexander Sverdlin
@ 2021-11-22 15:05     ` Michael Walle
  2021-11-23  7:45       ` Alexander Sverdlin
  2021-11-23 12:13       ` Alexander Sverdlin
  0 siblings, 2 replies; 14+ messages in thread
From: Michael Walle @ 2021-11-22 15:05 UTC (permalink / raw)
  To: Alexander Sverdlin
  Cc: linux-mtd, Tudor Ambarus, Pratyush Yadav, Miquel Raynal,
	Richard Weinberger, Vignesh Raghavendra, linux-kernel

Hi,

Am 2021-11-22 08:06, schrieb Alexander Sverdlin:
> On 19/11/2021 22:19, Michael Walle wrote:
>>> Ignore 6th ID byte, secure version of mt25qu256a has 0x73 as 6th 
>>> byte.
>> 
>> What is the secure version? What is the difference? Do you have some
>> links to datasheets for both?
> 
> For instance:
> https://www.micron.com/products/nor-flash/serial-nor-flash/part-catalog/mt25qu256aba1ew7-0sit
> https://media-www.micron.com/-/media/client/global/documents/products/data-sheet/nor-flash/serial-nor/mt25q/die-rev-a/mt25q_qljs_u_256_aba_0.pdf?rev=594079234c1b496496b062c21ce162d6
> 
> https://www.micron.com/products/nor-flash/serial-nor-flash/part-catalog/mt25qu256aba8e12-1sit
> https://media-www.micron.com/-/media/client/global/documents/products/data-sheet/nor-flash/serial-nor/mt25q/die-rev-a/mt25q_qljs_u_256_aba_0.pdf?rev=594079234c1b496496b062c21ce162d6
> 
> But the differences are in "MT25Q Security Addendum":
> "The additional protection features available on the secure MT25Q
> device include a
> lock status register bit, top/bottom block address protection lock, 
> volatile
> configuration lock register at power up, protection management register 
> lock,
> and a nonvolatile configuration lock register."
> This is only available under NDA from Micron.

So I take a wild guess here. There are lock bits (either in OTP or
maybe until the next power cycle), to lock the corresponding
top/bottom, block protection bits, right?

> However as long as one doesn't use these security features, it appears
> compatible with
> non-secure version. That's why just ignoring the non-standard
> configuration allows
> to support it.

But if we ever support it, then we have to distiguish them. Thus,
I was asking.

>> Also please provide the SFDP data for this flash, see [1].
> 
> sfdp:
> 53464450060101ff00060110300000ff84000102800000ffffffffffffff
> ffffffffffffffffffffffffffffffffffffe520fbffffffff0f29eb276b
> 273b27bbffffffffffff27bbffff29eb0c2010d80f520000244a99008b8e
> 03d4ac0127387a757a75fbbdd55c4a0f82ff81bd3d36ffffffffffffffff
> ffffffffffffffffffe7ffff21dcffff
> 
> md5sum:
> 5ea738216f68c9f98987bb3725699a32
> 
> jedec_id:
> 20bb191044
> 
> partname:
> mt25qu256a
> 
> manufacturer:
> st
> 
> (But last 3 do not make sense to me, as they come from the table I 
> modify,
> not from the chip itself). Further, I don't have 512Mbit chip to 
> provide
> SFDP for it.

Thanks, so that's the SFDP data for the mt25qu256aba8e12-1sit part. and 
the
jedec id is 20bb19104473, correct?

You don't have the non-security part by chance?

Mh, I'm undecided whether we should just duplicate the entry or if we
should ignore the last byte ("Device configuration information", where 
00h
is standard). The commit which introduced the flash was 7f412111e276b.
Vingesh?

Can you elaborate on the 0x73? Is that a bitmask? If it was an 
enumeration,
I'd assumed it would be 01h (or some smaller value).

-michael

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] mtd: spi-nor: mt25qu: Ignore 6th ID byte
  2021-11-22 15:05     ` Michael Walle
@ 2021-11-23  7:45       ` Alexander Sverdlin
  2021-11-23  8:14         ` Michael Walle
  2021-11-23 12:13       ` Alexander Sverdlin
  1 sibling, 1 reply; 14+ messages in thread
From: Alexander Sverdlin @ 2021-11-23  7:45 UTC (permalink / raw)
  To: Michael Walle
  Cc: linux-mtd, Tudor Ambarus, Pratyush Yadav, Miquel Raynal,
	Richard Weinberger, Vignesh Raghavendra, linux-kernel

Hi!

On 22/11/2021 16:05, Michael Walle wrote:
>>>> Ignore 6th ID byte, secure version of mt25qu256a has 0x73 as 6th byte.

...

> Thanks, so that's the SFDP data for the mt25qu256aba8e12-1sit part. and the
> jedec id is 20bb19104473, correct?

Yes!

> You don't have the non-security part by chance?

Unfortunately no. And this is exactly the trigger for this patch:
one can get "secure" parts from Micron even though these "features" are not
required.

> Mh, I'm undecided whether we should just duplicate the entry or if we
> should ignore the last byte ("Device configuration information", where 00h
> is standard). The commit which introduced the flash was 7f412111e276b.
> Vingesh?

Some people ask themselves why this table keeps growing if there is SFDP...
I see the point in fixups, but maybe at some point we will be able to support
some devices just out of the box?

> Can you elaborate on the 0x73? Is that a bitmask? If it was an enumeration,
> I'd assumed it would be 01h (or some smaller value).

This "security addendum" where one need NDA just says "73h = Secure".
There is no explanation for it and no other variants.

I'd really suggest to try to autodetect whatever features are going to be
supported from this chip and only duplicate the entry if this auto-detection
fails.

-- 
Best regards,
Alexander Sverdlin.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] mtd: spi-nor: mt25qu: Ignore 6th ID byte
  2021-11-23  7:45       ` Alexander Sverdlin
@ 2021-11-23  8:14         ` Michael Walle
  2021-11-23 12:40           ` Alexander Sverdlin
  0 siblings, 1 reply; 14+ messages in thread
From: Michael Walle @ 2021-11-23  8:14 UTC (permalink / raw)
  To: Alexander Sverdlin
  Cc: linux-mtd, Tudor Ambarus, Pratyush Yadav, Miquel Raynal,
	Richard Weinberger, Vignesh Raghavendra, linux-kernel

Hi,

Am 2021-11-23 08:45, schrieb Alexander Sverdlin:
> On 22/11/2021 16:05, Michael Walle wrote:
>>>>> Ignore 6th ID byte, secure version of mt25qu256a has 0x73 as 6th 
>>>>> byte.

>> You don't have the non-security part by chance?
> 
> Unfortunately no. And this is exactly the trigger for this patch:
> one can get "secure" parts from Micron even though these "features" are 
> not
> required.
> 
>> Mh, I'm undecided whether we should just duplicate the entry or if we
>> should ignore the last byte ("Device configuration information", where 
>> 00h
>> is standard). The commit which introduced the flash was 7f412111e276b.
>> Vingesh?
> 
> Some people ask themselves why this table keeps growing if there is 
> SFDP...
> I see the point in fixups, but maybe at some point we will be able to 
> support
> some devices just out of the box?

Are these features detectable by SFDP? Without knowing anything, as you 
ignored
my former question, I'd say no. So there will be two flashes, one with 
these
features and one without, both presumably have the same SFDP. Thus we'd 
need
these two entries anyway if we ever support these features. I get that 
this
might be under NDA, but then talk to Micron; I for myself can't get a 
complete
picture here.

>> Can you elaborate on the 0x73? Is that a bitmask? If it was an 
>> enumeration,
>> I'd assumed it would be 01h (or some smaller value).
> 
> This "security addendum" where one need NDA just says "73h = Secure".
> There is no explanation for it and no other variants.

Ok.

> I'd really suggest to try to autodetect whatever features are going to 
> be
> supported from this chip and only duplicate the entry if this 
> auto-detection
> fails.

There is a bigger patch series [1] from Tudor which you can try. You'd 
need to
respin your patch against that anyway.

-michael

[1] 
https://lore.kernel.org/linux-mtd/20211122095020.393346-1-tudor.ambarus@microchip.com/

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] mtd: spi-nor: mt25qu: Ignore 6th ID byte
  2021-11-22 15:05     ` Michael Walle
  2021-11-23  7:45       ` Alexander Sverdlin
@ 2021-11-23 12:13       ` Alexander Sverdlin
  2021-11-23 17:42         ` Pratyush Yadav
  1 sibling, 1 reply; 14+ messages in thread
From: Alexander Sverdlin @ 2021-11-23 12:13 UTC (permalink / raw)
  To: Michael Walle
  Cc: linux-mtd, Tudor Ambarus, Pratyush Yadav, Miquel Raynal,
	Richard Weinberger, Vignesh Raghavendra, linux-kernel

Dear maintainers!

On 22/11/2021 16:05, Michael Walle wrote:
> Thanks, so that's the SFDP data for the mt25qu256aba8e12-1sit part. and the
> jedec id is 20bb19104473, correct?

While we are at this part, I've encountered another issue:

The chip supports 1-1-1, 1-1-4 and 1-4-4 write OPs in extended SPI mode,
while only 1-1-0 erase. (as well as 4-4-4/4-4-0, but that's not the issue here,
I think).

Now the erase code (chip/sector) uses spi_nor_spimem_setup_op(nor, &op, nor->write_proto)
in both functions.

In my opinion, as I look into Micron or Macronix datasheets, write_proto has little to
do with erase_proto. (there is currently no separate erase_proto)

Before I come up with a totally wrong patch, wanted to ask your opinion, how should
it be solved, what do you think?

I do not see any erase-related tables for this in JESD216C.
I also cannot come up with an example of a chip with erase != 1-1-0.

Shall I hardcode 1-1-0 for erase?
Shall I introduce erase_proto? What would be the logic for its setting/discovery?

-- 
Best regards,
Alexander Sverdlin.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] mtd: spi-nor: mt25qu: Ignore 6th ID byte
  2021-11-23  8:14         ` Michael Walle
@ 2021-11-23 12:40           ` Alexander Sverdlin
  2021-11-23 14:01             ` Michael Walle
  0 siblings, 1 reply; 14+ messages in thread
From: Alexander Sverdlin @ 2021-11-23 12:40 UTC (permalink / raw)
  To: Michael Walle
  Cc: linux-mtd, Tudor Ambarus, Pratyush Yadav, Miquel Raynal,
	Richard Weinberger, Vignesh Raghavendra, linux-kernel

Hi!

On 23/11/2021 09:14, Michael Walle wrote:
>>
>> Some people ask themselves why this table keeps growing if there is SFDP...
>> I see the point in fixups, but maybe at some point we will be able to support
>> some devices just out of the box?
> 
> Are these features detectable by SFDP? Without knowing anything, as you ignored
> my former question, I'd say no.

Well, I just had nothing to say on your question.
It wasn't my intention to study security features of a chip, which I don't use.
There is a background which I didn't tell you, which might even emphasize the
importance of security version support even without security features:
Micron might want to "streamline" the product palette and as security version
is backwards compatible (except 6th byte, which is not an ID, actually), a
buyer can get security version instead of normal one.

And the reason for the above is, mainly, because there are real problems with
this chip, even without new features, sector erase doesn't work properly
(refer to my other email in this thread).

-- 
Best regards,
Alexander Sverdlin.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] mtd: spi-nor: mt25qu: Ignore 6th ID byte
  2021-11-23 12:40           ` Alexander Sverdlin
@ 2021-11-23 14:01             ` Michael Walle
  2021-11-23 16:14               ` Tudor.Ambarus
  0 siblings, 1 reply; 14+ messages in thread
From: Michael Walle @ 2021-11-23 14:01 UTC (permalink / raw)
  To: Alexander Sverdlin
  Cc: linux-mtd, Tudor Ambarus, Pratyush Yadav, Miquel Raynal,
	Richard Weinberger, Vignesh Raghavendra, linux-kernel

Hi,

Am 2021-11-23 13:40, schrieb Alexander Sverdlin:
> On 23/11/2021 09:14, Michael Walle wrote:
>>> 
>>> Some people ask themselves why this table keeps growing if there is 
>>> SFDP...
>>> I see the point in fixups, but maybe at some point we will be able to 
>>> support
>>> some devices just out of the box?
>> 
>> Are these features detectable by SFDP? Without knowing anything, as 
>> you ignored
>> my former question, I'd say no.
> 
> Well, I just had nothing to say on your question.
> It wasn't my intention to study security features of a chip, which I 
> don't use.

Like I said, without that information its hard for me do decide if we 
can just
ignore that last byte. (And yes I've already tried to get that NDA PDF 
via $WORK,
but I don't have high hopes with that).

-michael

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] mtd: spi-nor: mt25qu: Ignore 6th ID byte
  2021-11-23 14:01             ` Michael Walle
@ 2021-11-23 16:14               ` Tudor.Ambarus
  0 siblings, 0 replies; 14+ messages in thread
From: Tudor.Ambarus @ 2021-11-23 16:14 UTC (permalink / raw)
  To: michael, alexander.sverdlin
  Cc: linux-mtd, p.yadav, miquel.raynal, richard, vigneshr, linux-kernel

On 11/23/21 4:01 PM, Michael Walle wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Hi,
> 
> Am 2021-11-23 13:40, schrieb Alexander Sverdlin:
>> On 23/11/2021 09:14, Michael Walle wrote:
>>>>
>>>> Some people ask themselves why this table keeps growing if there is
>>>> SFDP...
>>>> I see the point in fixups, but maybe at some point we will be able to
>>>> support
>>>> some devices just out of the box?
>>>
>>> Are these features detectable by SFDP? Without knowing anything, as
>>> you ignored
>>> my former question, I'd say no.
>>
>> Well, I just had nothing to say on your question.
>> It wasn't my intention to study security features of a chip, which I
>> don't use.
> 
> Like I said, without that information its hard for me do decide if we
> can just
> ignore that last byte. (And yes I've already tried to get that NDA PDF
> via $WORK,
> but I don't have high hopes with that).
> 

It would be nice to understand what is the difference between the two,
otherwise keeping a single flash entry will become a maintenance burden.
For example, what is the difference between the "top/bottom block address
protection lock" and the BIT(5) (Top/Bottom) of the Status Register?

Is the newer flash completely backward compatible with the 0x00 variant?
I'm not talking about the flash_info flags that are currently specified
in the flash_info entry, but about all the features of the 0x00 version
(also the ones that are not implemented in linux SPI NOR).

If the newer flash is fully backward compatible with the older one,
it should be fine to strip the 6th byte of ID. As Michael has suggested,
if the security features are not SFDP discoverable (either via a generic
table or a vendor specific table) and one wants to use/implement the
security features, one will have to revert the patch that drops the 6th
byte of ID, and to introduce a new flash_info entry anyway.

Cheers,
ta

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] mtd: spi-nor: mt25qu: Ignore 6th ID byte
  2021-11-23 12:13       ` Alexander Sverdlin
@ 2021-11-23 17:42         ` Pratyush Yadav
  2021-11-25  7:26           ` Alexander Sverdlin
  0 siblings, 1 reply; 14+ messages in thread
From: Pratyush Yadav @ 2021-11-23 17:42 UTC (permalink / raw)
  To: Alexander Sverdlin
  Cc: Michael Walle, linux-mtd, Tudor Ambarus, Miquel Raynal,
	Richard Weinberger, Vignesh Raghavendra, linux-kernel

On 23/11/21 01:13PM, Alexander Sverdlin wrote:
> Dear maintainers!
> 
> On 22/11/2021 16:05, Michael Walle wrote:
> > Thanks, so that's the SFDP data for the mt25qu256aba8e12-1sit part. and the
> > jedec id is 20bb19104473, correct?
> 
> While we are at this part, I've encountered another issue:
> 
> The chip supports 1-1-1, 1-1-4 and 1-4-4 write OPs in extended SPI mode,
> while only 1-1-0 erase. (as well as 4-4-4/4-4-0, but that's not the issue here,
> I think).
> 
> Now the erase code (chip/sector) uses spi_nor_spimem_setup_op(nor, &op, nor->write_proto)
> in both functions.
> 
> In my opinion, as I look into Micron or Macronix datasheets, write_proto has little to
> do with erase_proto. (there is currently no separate erase_proto)

I think this just worked for most flashes since both writes and erases 
generally use 1-bit mode. 4 or 8 bit modes are generally used for reads 
only.

> 
> Before I come up with a totally wrong patch, wanted to ask your opinion, how should
> it be solved, what do you think?
> 
> I do not see any erase-related tables for this in JESD216C.
> I also cannot come up with an example of a chip with erase != 1-1-0.

See Micron MT35XU512ABA or Cypress S28HS512T (in spansion.c). Both have 
erase in 8D-8D-8D mode.

> 
> Shall I hardcode 1-1-0 for erase?
> Shall I introduce erase_proto? What would be the logic for its setting/discovery?

I think introducing erase_proto would be the sensible thing. You would 
have to see if we can discover erase protocol from SFDP. But my question 
is: is that really worth it? Do you really need that little bit speed 
boost you'd get by transmitting write data in 4 bit mode, since the 
large portion of the time would be spent in the chip actually flashing 
the data.

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] mtd: spi-nor: mt25qu: Ignore 6th ID byte
  2021-11-23 17:42         ` Pratyush Yadav
@ 2021-11-25  7:26           ` Alexander Sverdlin
  2021-11-30  9:49             ` Pratyush Yadav
  0 siblings, 1 reply; 14+ messages in thread
From: Alexander Sverdlin @ 2021-11-25  7:26 UTC (permalink / raw)
  To: Pratyush Yadav
  Cc: Michael Walle, linux-mtd, Tudor Ambarus, Miquel Raynal,
	Richard Weinberger, Vignesh Raghavendra, linux-kernel

Hi Pratyush,

thanks for the quick reply!

On 23/11/2021 18:42, Pratyush Yadav wrote:
>> In my opinion, as I look into Micron or Macronix datasheets, write_proto has little to
>> do with erase_proto. (there is currently no separate erase_proto)
> I think this just worked for most flashes since both writes and erases 
> generally use 1-bit mode. 4 or 8 bit modes are generally used for reads 
> only.
> 
>> Before I come up with a totally wrong patch, wanted to ask your opinion, how should
>> it be solved, what do you think?
>>
>> I do not see any erase-related tables for this in JESD216C.
>> I also cannot come up with an example of a chip with erase != 1-1-0.
> See Micron MT35XU512ABA or Cypress S28HS512T (in spansion.c). Both have 
> erase in 8D-8D-8D mode.
> 
>> Shall I hardcode 1-1-0 for erase?
>> Shall I introduce erase_proto? What would be the logic for its setting/discovery?
> I think introducing erase_proto would be the sensible thing. You would 
> have to see if we can discover erase protocol from SFDP. But my question 
> is: is that really worth it? Do you really need that little bit speed 
> boost you'd get by transmitting write data in 4 bit mode, since the 
> large portion of the time would be spent in the chip actually flashing 
> the data.

The problem I have is not speed, but totally not working erase. And I don't want
to downgrade write functionality for other chips.

-- 
Best regards,
Alexander Sverdlin.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] mtd: spi-nor: mt25qu: Ignore 6th ID byte
  2021-11-25  7:26           ` Alexander Sverdlin
@ 2021-11-30  9:49             ` Pratyush Yadav
  0 siblings, 0 replies; 14+ messages in thread
From: Pratyush Yadav @ 2021-11-30  9:49 UTC (permalink / raw)
  To: Alexander Sverdlin
  Cc: Michael Walle, linux-mtd, Tudor Ambarus, Miquel Raynal,
	Richard Weinberger, Vignesh Raghavendra, linux-kernel

On 25/11/21 08:26AM, Alexander Sverdlin wrote:
> Hi Pratyush,
> 
> thanks for the quick reply!
> 
> On 23/11/2021 18:42, Pratyush Yadav wrote:
> >> In my opinion, as I look into Micron or Macronix datasheets, write_proto has little to
> >> do with erase_proto. (there is currently no separate erase_proto)
> > I think this just worked for most flashes since both writes and erases 
> > generally use 1-bit mode. 4 or 8 bit modes are generally used for reads 
> > only.
> > 
> >> Before I come up with a totally wrong patch, wanted to ask your opinion, how should
> >> it be solved, what do you think?
> >>
> >> I do not see any erase-related tables for this in JESD216C.
> >> I also cannot come up with an example of a chip with erase != 1-1-0.
> > See Micron MT35XU512ABA or Cypress S28HS512T (in spansion.c). Both have 
> > erase in 8D-8D-8D mode.
> > 
> >> Shall I hardcode 1-1-0 for erase?
> >> Shall I introduce erase_proto? What would be the logic for its setting/discovery?
> > I think introducing erase_proto would be the sensible thing. You would 
> > have to see if we can discover erase protocol from SFDP. But my question 
> > is: is that really worth it? Do you really need that little bit speed 
> > boost you'd get by transmitting write data in 4 bit mode, since the 
> > large portion of the time would be spent in the chip actually flashing 
> > the data.
> 
> The problem I have is not speed, but totally not working erase. And I don't want
> to downgrade write functionality for other chips.

Then you need to introduce erase_proto.

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] mtd: spi-nor: mt25qu: Ignore 6th ID byte
  2021-11-19  8:04 [PATCH] mtd: spi-nor: mt25qu: Ignore 6th ID byte Alexander A Sverdlin
  2021-11-19 21:19 ` Michael Walle
@ 2022-07-18 15:03 ` Tudor.Ambarus
  1 sibling, 0 replies; 14+ messages in thread
From: Tudor.Ambarus @ 2022-07-18 15:03 UTC (permalink / raw)
  To: alexander.sverdlin, linux-mtd
  Cc: p.yadav, michael, miquel.raynal, richard, vigneshr, linux-kernel

On 11/19/21 10:04, Alexander A Sverdlin wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> From: Alexander Sverdlin <alexander.sverdlin@nokia.com>
> 
> Ignore 6th ID byte, secure version of mt25qu256a has 0x73 as 6th byte.
>

By reading the thread I understand that we don't have a complete view
about the differences between the two types of flashes, thus I'll
mark the patch with "Changes requested". Feel free to revive the thread
if you think we can progress on this topic.

Thanks,
ta

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2022-07-18 15:03 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-19  8:04 [PATCH] mtd: spi-nor: mt25qu: Ignore 6th ID byte Alexander A Sverdlin
2021-11-19 21:19 ` Michael Walle
2021-11-22  7:06   ` Alexander Sverdlin
2021-11-22 15:05     ` Michael Walle
2021-11-23  7:45       ` Alexander Sverdlin
2021-11-23  8:14         ` Michael Walle
2021-11-23 12:40           ` Alexander Sverdlin
2021-11-23 14:01             ` Michael Walle
2021-11-23 16:14               ` Tudor.Ambarus
2021-11-23 12:13       ` Alexander Sverdlin
2021-11-23 17:42         ` Pratyush Yadav
2021-11-25  7:26           ` Alexander Sverdlin
2021-11-30  9:49             ` Pratyush Yadav
2022-07-18 15:03 ` Tudor.Ambarus

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).