Re: [PATCH] mtd: spi-nor: support eon en25

Re: [PATCH] mtd: spi-nor: support eon en25

[Leon M. George]

Hi,

The original commit message contained typos and two of the paragraphs have
been reworked.
A 'Tested-By:' tag was been added.

Please consider this updated version of the commit.

kind regards,
Leon George


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH] mtd: spi-nor: support eon en25qh256a variant

[Leon M. George]

This patch allows accessing the upper 8m on the A variant (EN25QH256A) of
the EN25QH256 that shares same JEDEC ID.

Without this patch, addr_with is detected to be '4' but the read_opcode is
a plain READ (supporting only 3 byte addresses).
Setting PARSE_SFDP is enough to detect the read_opcode READ_4B on the A
variant. READ_4B is not available on the no-A variant.

Both variants support 4-byte address mode (spi_nor_set_4byte_addr_mode)
but that is prone to breaking on unexpected reboots if the reset pin isn't
connected (broken-flash-reset).

The no-A variant supports a 'high bank latch mode' that affects read,
program, and erase commands - similar to the extended address register
(EAR).
The HBL bit is manipulated using the ENHBL (0x67) and EXHBL (0x98)
opcodes.

Should it become necessary to distinguish the two variants in the future,
the A variant sets the SNOR_HWCAPS_READ_1_1_4 SFDP param - the no-A
variant doesn't.

Tested with and without fast read on the A variant only.

Signed-off-by: Leon M. George <leon@georgemail.eu>
Tested-by: Leon M. George <leon@georgemail.eu>
---
 drivers/mtd/spi-nor/eon.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/spi-nor/eon.c b/drivers/mtd/spi-nor/eon.c
index 8c1c57530281..50a11053711f 100644
--- a/drivers/mtd/spi-nor/eon.c
+++ b/drivers/mtd/spi-nor/eon.c
@@ -25,7 +25,8 @@ static const struct flash_info eon_nor_parts[] = {
 	{ "en25qh64",   INFO(0x1c7017, 0, 64 * 1024,  128)
 		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ) },
 	{ "en25qh128",  INFO(0x1c7018, 0, 64 * 1024,  256) },
-	{ "en25qh256",  INFO(0x1c7019, 0, 64 * 1024,  512) },
+	{ "en25qh256",  INFO(0x1c7019, 0, 64 * 1024,  512)
+		PARSE_SFDP },
 	{ "en25s64",	INFO(0x1c3817, 0, 64 * 1024,  128)
 		NO_SFDP_FLAGS(SECT_4K) },
 };
-- 
2.35.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] mtd: spi-nor: support eon en25qh256a variant

[Michael Walle]

Hi,

> This patch allows accessing the upper 8m on the A variant (EN25QH256A) of

8m? It's a 32MiB flash right? So it should be the upper 16MiB?

> the EN25QH256 that shares same JEDEC ID.
> 
> Without this patch, addr_with is detected to be '4' but the read_opcode is
> a plain READ (supporting only 3 byte addresses).
> Setting PARSE_SFDP is enough to detect the read_opcode READ_4B on the A
> variant. READ_4B is not available on the no-A variant.
> 
> Both variants support 4-byte address mode (spi_nor_set_4byte_addr_mode)
> but that is prone to breaking on unexpected reboots if the reset pin isn't
> connected (broken-flash-reset).
> 
> The no-A variant supports a 'high bank latch mode' that affects read,
> program, and erase commands - similar to the extended address register
> (EAR).
> The HBL bit is manipulated using the ENHBL (0x67) and EXHBL (0x98)
> opcodes.

So the upper 16MiB never really worked, correct?

> Should it become necessary to distinguish the two variants in the future,
> the A variant sets the SNOR_HWCAPS_READ_1_1_4 SFDP param - the no-A
> variant doesn't.
> 
> Tested with and without fast read on the A variant only.

Do you have the A variant only? Where did you get the SFDP from the
non-A variant? datasheet?

> 
> Signed-off-by: Leon M. George <leon@georgemail.eu>
> Tested-by: Leon M. George <leon@georgemail.eu>

No need for the Tested-by:. That is already implied by the SoB (unless
noted otherwise).

> ---
>  drivers/mtd/spi-nor/eon.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/spi-nor/eon.c b/drivers/mtd/spi-nor/eon.c
> index 8c1c57530281..50a11053711f 100644
> --- a/drivers/mtd/spi-nor/eon.c
> +++ b/drivers/mtd/spi-nor/eon.c
> @@ -25,7 +25,8 @@ static const struct flash_info eon_nor_parts[] = {
>  	{ "en25qh64",   INFO(0x1c7017, 0, 64 * 1024,  128)
>  		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ) },
>  	{ "en25qh128",  INFO(0x1c7018, 0, 64 * 1024,  256) },
> -	{ "en25qh256",  INFO(0x1c7019, 0, 64 * 1024,  512) },
> +	{ "en25qh256",  INFO(0x1c7019, 0, 64 * 1024,  512)
> +		PARSE_SFDP },

Please also submit a SFDP dump, see [1].

-michael

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

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] mtd: spi-nor: support eon en25qh256a variant

[Leon M. George]

Hello Michael and thanks for your reply :-)

I hope I'm doing this right (replying with send-email).

On Mon,  2 May 2022 13:52:27 +0200
Michael Walle <michael@walle.cc> wrote:

> 8m? It's a 32MiB flash right? So it should be the upper 16MiB?

You're absolutely right. The datasheet calls it the 'higher 128M bank' and
I mixed up the divisors 8 and 16 in my head - glorious!

> So the upper 16MiB never really worked, correct?

Correct. At least for the A variant.
I'm unsure about the no-A variant but I don't have one of those to test.
The information about accessing the higher bank is included in the commit
message just in case anyone wants to work on that.

I could imagine doing an attempt getting the EAR to work but I presume it
makes more sense if somebody with access to the hardware does it properly.

> Do you have the A variant only? Where did you get the SFDP from the
> non-A variant? datasheet?

I only have the A variant - yes - and the part about the datasheet is also
correct.
Sadly, the layout of the SFDP tables is different between the datasheets
but I went through them manually and most of the fields have identical
values.

These are the differences I can spot with my eyes:

        Value          |  address  |   A   |  no-A
-----------------------+-----------+-------+-------
 SFDP and table minor  |    0x04   | 0x06  | 0x00
   revision number     |  / 0x09   |       |
-----------------------+-----------+-------+-------
  param table length   |   0x0b    | 0x10  | 0x09
-----------------------+-----------+-------+-------
 supports 1-1-4 opcode |  0x32@7   |   1   |   0
-----------------------+-----------+-------+-------
  1-1-4 dummy clocks   |  0x3a@0:5 |  0x4  | 0x0
-----------------------+-----------+-------+-------
   1-1-4 read opcode   |   0x3b    | 0x6b  | 0xFF
-----------------------+-----------+-------+-------
  sector type 2 size   |   0x4e    | 0x0f  | 0x00
-----------------------+-----------+-------+-------
 sector type 2 opcode  |   0x4f    | 0x52  | 0xff
-----------------------+-----------+-------+-------


The no-A variant datasheet doesn't list the following address ranges:

  0x10 - 0x1f
  0x54 - 0x63

I guess they'll read as 0xff.


> No need for the Tested-by:. That is already implied by the SoB (unless
> noted otherwise).

I will remove it then.

> Please also submit a SFDP dump, see [1].

Again, I hope I'm doing this right..

Thank you for your valued feedback :-)


kind regards,
Leon

---

$ xxd -p sfdp
53464450060102ff00060110300000ff1c000104100100ff84000102c000
00ffffffffffffffffffffffffffffffffffe520f3ffffffff0f44eb086b
083b04bbfeffffffffff00ffffff44eb0c200f5210d800ff353aa50082ea
14d34c6f76b3fffffffff7bdd55c299689ffe850c081ffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffff0ef0ff215cdcffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffff003600279f49fffffcebffffffffffff
$ md5sum sfdp
e4078f17a495a38c657f4f39f8851080  sfdp
$ cat jedec_id
1c7019
$ cat partname
en25qh256
$ cat manufacturer
eon


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH] mtd: spi-nor: support eon en25qh256a variant

[Leon M. George]

This patch allows accessing the upper 16m on the A variant (EN25QH256A) of
the EN25QH256 that shares same JEDEC ID.

Without this patch, addr_with is detected to be '4' but the read_opcode is
a plain READ (supporting only 3 byte addresses).
Setting PARSE_SFDP is enough to detect the read_opcode READ_4B on the A
variant. READ_4B is not available on the no-A variant.

Both variants support 4-byte address mode (spi_nor_set_4byte_addr_mode)
but that is prone to breaking on unexpected reboots if the reset pin isn't
connected (broken-flash-reset).

The no-A variant supports a 'high bank latch mode' that affects read,
program, and erase commands - similar to the extended address register
(EAR).
The HBL bit is manipulated using the ENHBL (0x67) and EXHBL (0x98)
opcodes.

Should it become necessary to distinguish the two variants in the future,
the A variant sets the SNOR_HWCAPS_READ_1_1_4 SFDP param - the no-A
variant doesn't.

Tested with and without fast read on the A variant only.

Signed-off-by: Leon M. George <leon@georgemail.eu>
---
 drivers/mtd/spi-nor/eon.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/spi-nor/eon.c b/drivers/mtd/spi-nor/eon.c
index 8c1c57530281..50a11053711f 100644
--- a/drivers/mtd/spi-nor/eon.c
+++ b/drivers/mtd/spi-nor/eon.c
@@ -25,7 +25,8 @@ static const struct flash_info eon_nor_parts[] = {
 	{ "en25qh64",   INFO(0x1c7017, 0, 64 * 1024,  128)
 		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ) },
 	{ "en25qh128",  INFO(0x1c7018, 0, 64 * 1024,  256) },
-	{ "en25qh256",  INFO(0x1c7019, 0, 64 * 1024,  512) },
+	{ "en25qh256",  INFO(0x1c7019, 0, 64 * 1024,  512)
+		PARSE_SFDP },
 	{ "en25s64",	INFO(0x1c3817, 0, 64 * 1024,  128)
 		NO_SFDP_FLAGS(SECT_4K) },
 };
-- 
2.35.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] mtd: spi-nor: support eon en25qh256a variant

[Michael Walle]

Hi,

For the next time if you submit a new patch version: the subject should
contain v2, eg. [PATCH v2]. See 
Documentation/process/submitting-patches.rst.
Also a changelog would be good.

Am 2022-05-03 01:33, schrieb Leon M. George:
> This patch allows accessing the upper 16m on the A variant (EN25QH256A) 
> of
> the EN25QH256 that shares same JEDEC ID.
> 
> Without this patch, addr_with is detected to be '4' but the read_opcode 
> is
> a plain READ (supporting only 3 byte addresses).
> Setting PARSE_SFDP is enough to detect the read_opcode READ_4B on the A
> variant. READ_4B is not available on the no-A variant.
> 
> Both variants support 4-byte address mode (spi_nor_set_4byte_addr_mode)
> but that is prone to breaking on unexpected reboots if the reset pin 
> isn't
> connected (broken-flash-reset).
> 
> The no-A variant supports a 'high bank latch mode' that affects read,
> program, and erase commands - similar to the extended address register
> (EAR).
> The HBL bit is manipulated using the ENHBL (0x67) and EXHBL (0x98)
> opcodes.
> 
> Should it become necessary to distinguish the two variants in the 
> future,
> the A variant sets the SNOR_HWCAPS_READ_1_1_4 SFDP param - the no-A
> variant doesn't.
> 
> Tested with and without fast read on the A variant only.
> 
> Signed-off-by: Leon M. George <leon@georgemail.eu>

Reviewed-by: Michael Walle <michael@walle.cc>

-michael

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] mtd: spi-nor: support eon en25qh256a variant

[Pratyush Yadav]

On Tue, 3 May 2022 01:33:10 +0200, Leon M. George wrote:
> This patch allows accessing the upper 16m on the A variant (EN25QH256A) of
> the EN25QH256 that shares same JEDEC ID.
> 
> Without this patch, addr_with is detected to be '4' but the read_opcode is
> a plain READ (supporting only 3 byte addresses).
> Setting PARSE_SFDP is enough to detect the read_opcode READ_4B on the A
> variant. READ_4B is not available on the no-A variant.
> 
> [...]

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git spi-nor/next, thanks!
[1/1] mtd: spi-nor: support eon en25qh256a variant
      https://git.kernel.org/mtd/c/cdbc44dbb2

--
Regards,
Pratyush Yadav
Texas Instruments Inc.


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH] mtd: spi-nor: support eon en25qh256a variant

[Leon M. George]

This patch allows accessing the uppoer 8m on the A variant (EN25QH256A) of
the EN25QH256 that shares same JEDEC ID.

Without this patch, addr_with is detected to be '4' but the read_opcode is
a plain READ (supporting only 3 byte addresses).
Setting PARSE_SFDP is enough to detect the read_opcode to READ_4B.
READ_4B is only supported on the A variant.

Both support 4-byte address mode (spi_nor_set_4byte_addr_mode) but that is
prone to breaking on unexpected reboots if the reset pin isn't connected
(broken-flash-reset).

The no-A variant supports a 'high bank latch mode' that affects read,
program, and erase commands - similar to the extended address register
(EAR).
The HBL bit is manipulated using the ENHBL (0x67) and EXHBL (0x98)
opcodes.

Should it become necessary to distinguish the two variants in the future,
the A variant sets the SNOR_HWCAPS_READ_1_1_4 SFDP param - the no-A
variant doesn't.

Tested with and without fast read on the A variant only.

Signed-off-by: Leon M. George <leon@georgemail.eu>
---
 drivers/mtd/spi-nor/eon.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/spi-nor/eon.c b/drivers/mtd/spi-nor/eon.c
index 8c1c57530281..50a11053711f 100644
--- a/drivers/mtd/spi-nor/eon.c
+++ b/drivers/mtd/spi-nor/eon.c
@@ -25,7 +25,8 @@ static const struct flash_info eon_nor_parts[] = {
 	{ "en25qh64",   INFO(0x1c7017, 0, 64 * 1024,  128)
 		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ) },
 	{ "en25qh128",  INFO(0x1c7018, 0, 64 * 1024,  256) },
-	{ "en25qh256",  INFO(0x1c7019, 0, 64 * 1024,  512) },
+	{ "en25qh256",  INFO(0x1c7019, 0, 64 * 1024,  512)
+		PARSE_SFDP },
 	{ "en25s64",	INFO(0x1c3817, 0, 64 * 1024,  128)
 		NO_SFDP_FLAGS(SECT_4K) },
 };
-- 
2.35.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/