Change in flashrom[master]: flashchips: Add support for Macronix MX25L5121E

[Steve Markgraf (Code Review)]

Steve Markgraf has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/43416 )

Change subject: flashchips: Add support for Macronix MX25L5121E
......................................................................

flashchips: Add support for Macronix MX25L5121E

Change-Id: I881e2cda938083ba271b2ee0c457d2bbd8e1a766
Signed-off-by: Steve Markgraf 
---
M flashchips.c
M flashchips.h
2 files changed, 39 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/16/43416/1

diff --git a/flashchips.c b/flashchips.c
index 48405fe..996865a 100644
--- a/flashchips.c
+++ b/flashchips.c
@@ -8543,6 +8543,44 @@

{
    	.vendor		= "Macronix",
+		.name		= "MX25L5121E",
+		.bustype	= BUS_SPI,
+		.manufacture_id	= MACRONIX_ID,
+		.model_id	= MACRONIX_MX25L5121E,
+		.total_size	= 64,
+		.page_size	= 32,
+		.feature_bits	= FEATURE_WRSR_WREN,
+		.tested		= TEST_OK_PREW,
+		.probe		= probe_spi_rdid,
+		.probe_timing	= TIMING_ZERO,
+		.block_erasers	=
+		{
+			{
+				.eraseblocks = { {4 * 1024, 16} },
+				.block_erase = spi_block_erase_20,
+			}, {
+				.eraseblocks = { {64 * 1024, 1} },
+				.block_erase = spi_block_erase_52,
+			}, {
+				.eraseblocks = { {64 * 1024, 1} },
+				.block_erase = spi_block_erase_d8,
+			}, {
+				.eraseblocks = { {64 * 1024, 1} },
+				.block_erase = spi_block_erase_60,
+			}, {
+				.eraseblocks = { {64 * 1024, 1} },
+				.block_erase = spi_block_erase_c7,
+			},
+		},
+		.printlock	= spi_prettyprint_status_register_bp1_srwd,
+		.unlock		= spi_disable_blockprotect,
+		.write		= spi_chip_write_256,
+		.read		= spi_chip_read, /* Fast read (0x0B) supported, MX25L5121E supports dual I/O */
+		.voltage	= {2700, 3600}, /* 2.35-3.6V for MX25L5121E */
+	},
+
+	{
+		.vendor		= "Macronix",
    	.name		= "MX25L6405",
    	.bustype	= BUS_SPI,
    	.manufacture_id	= MACRONIX_ID,
diff --git a/flashchips.h b/flashchips.h
index c58595d..597f659 100644
--- a/flashchips.h
+++ b/flashchips.h
@@ -482,6 +482,7 @@
  * Generalplus SPI chips seem to be compatible with Macronix
  * and use the same set of IDs. */
 #define MACRONIX_MX25L512	0x2010	/* Same as MX25L512E, MX25V512, MX25V512C */
+#define MACRONIX_MX25L5121E	0x2210
 #define MACRONIX_MX25L1005	0x2011	/* Same as MX25L1005C, MX25L1006E */
 #define MACRONIX_MX25L2005	0x2012	/* Same as MX25L2005C, MX25L2006E */
 #define MACRONIX_MX25L4005	0x2013	/* Same as MX25L4005A, MX25L4005C, MX25L4006E */

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/43416 )

Change subject: flashchips: Add support for Macronix MX25L5121E
......................................................................

Patch Set 1: Code-Review+1

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/43416 )

Change subject: flashchips: Add support for Macronix MX25L5121E
......................................................................

Patch Set 1:

(3 comments)

https://review.coreboot.org/c/flashrom/+/43416/1/flashchips.c 
File flashchips.c:

https://review.coreboot.org/c/flashrom/+/43416/1/flashchips.c@8577 
PS1, Line 8577: spi_chip_write_256
Does this work as intended? I would try to use a layout file to see if partial writes work

https://review.coreboot.org/c/flashrom/+/43416/1/flashchips.c@8578 
PS1, Line 8578: MX25L5121E supports dual I/O
Where do you see this?

https://review.coreboot.org/c/flashrom/+/43416/1/flashchips.c@8579 
PS1, Line 8579: 2.35
I see "2.7 to 3.6 volt for read, erase and program operations"

[Steve Markgraf (Code Review)]

Steve Markgraf has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/43416 )

Change subject: flashchips: Add support for Macronix MX25L5121E
......................................................................

Patch Set 1:

(3 comments)

https://review.coreboot.org/c/flashrom/+/43416/1/flashchips.c 
File flashchips.c:

https://review.coreboot.org/c/flashrom/+/43416/1/flashchips.c@8577 
PS1, Line 8577: spi_chip_write_256
> Does this work as intended? I would try to use a layout file to see if partial writes work

Yes, I just did some testing with a layout file. spi_write_chunked() does check for page_size and split to 32 byte writes.

https://review.coreboot.org/c/flashrom/+/43416/1/flashchips.c@8578 
PS1, Line 8578: MX25L5121E supports dual I/O
> Where do you see this?

Oops, I thought I removed that.

https://review.coreboot.org/c/flashrom/+/43416/1/flashchips.c@8579 
PS1, Line 8579: 2.35
> I see "2.7 to 3. […]

Same here, 2.7V-3.6V is correct.

[Steve Markgraf (Code Review)]

Hello build bot (Jenkins), Angel Pons,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/flashrom/+/43416

to look at the new patch set (#2).

Change subject: flashchips: Add support for Macronix MX25L5121E
......................................................................

flashchips: Add support for Macronix MX25L5121E

Change-Id: I881e2cda938083ba271b2ee0c457d2bbd8e1a766
Signed-off-by: Steve Markgraf 
---
M flashchips.c
M flashchips.h
2 files changed, 39 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/16/43416/2

[Steve Markgraf (Code Review)]

Steve Markgraf has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/43416 )

Change subject: flashchips: Add support for Macronix MX25L5121E
......................................................................

Patch Set 2:

(3 comments)

https://review.coreboot.org/c/flashrom/+/43416/1/flashchips.c 
File flashchips.c:

https://review.coreboot.org/c/flashrom/+/43416/1/flashchips.c@8577 
PS1, Line 8577: spi_chip_write_256
> Yes, I just did some testing with a layout file. […]

Done

https://review.coreboot.org/c/flashrom/+/43416/1/flashchips.c@8578 
PS1, Line 8578: MX25L5121E supports dual I/O
> Oops, I thought I removed that.

Done

https://review.coreboot.org/c/flashrom/+/43416/1/flashchips.c@8579 
PS1, Line 8579: 2.35
> Same here, 2.7V-3.6V is correct.

Done

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/43416 )

Change subject: flashchips: Add support for Macronix MX25L5121E
......................................................................

Patch Set 2: Code-Review+1

(2 comments)

https://review.coreboot.org/c/flashrom/+/43416/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/flashrom/+/43416/1//COMMIT_MSG@8 
PS1, Line 8: 
With which programmer did you test this?

https://review.coreboot.org/c/flashrom/+/43416/1/flashchips.c 
File flashchips.c:

https://review.coreboot.org/c/flashrom/+/43416/1/flashchips.c@8577 
PS1, Line 8577: spi_chip_write_256
> Done

Hrm, I'm not sure but I can see FIXMEs.

[Steve Markgraf (Code Review)]

Steve Markgraf has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/43416 )

Change subject: flashchips: Add support for Macronix MX25L5121E
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/flashrom/+/43416/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/flashrom/+/43416/1//COMMIT_MSG@8 
PS1, Line 8:
> With which programmer did you test this?

I tested with ch341a_spi.

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/43416 )

Change subject: flashchips: Add support for Macronix MX25L5121E
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/flashrom/+/43416/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/flashrom/+/43416/1//COMMIT_MSG@8 
PS1, Line 8:
> I tested with ch341a_spi.

OK, please mention so in the commit message

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/43416 )

Change subject: flashchips: Add support for Macronix MX25L5121E
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/flashrom/+/43416/1/flashchips.c 
File flashchips.c:

https://review.coreboot.org/c/flashrom/+/43416/1/flashchips.c@8577 
PS1, Line 8577: spi_chip_write_256
> Hrm, I'm not sure but I can see FIXMEs.

I checked, should work.

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/43416 )

Change subject: flashchips: Add support for Macronix MX25L5121E
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/flashrom/+/43416/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/flashrom/+/43416/1//COMMIT_MSG@8 
PS1, Line 8:
> OK, please mention so in the commit message

*poke*

[Steve Markgraf (Code Review)]

Hello build bot (Jenkins), Angel Pons,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/flashrom/+/43416

to look at the new patch set (#3).

Change subject: flashchips: Add support for Macronix MX25L5121E
......................................................................

flashchips: Add support for Macronix MX25L5121E

Tested with ch341a_spi.

Change-Id: I881e2cda938083ba271b2ee0c457d2bbd8e1a766
Signed-off-by: Steve Markgraf 
---
M flashchips.c
M flashchips.h
2 files changed, 39 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/16/43416/3

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/43416 )

Change subject: flashchips: Add support for Macronix MX25L5121E
......................................................................

Patch Set 4: Code-Review+2

(1 comment)

https://review.coreboot.org/c/flashrom/+/43416/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/flashrom/+/43416/1//COMMIT_MSG@8 
PS1, Line 8:
> *poke*

Done

[Angel Pons (Code Review)]

Angel Pons has submitted this change. ( https://review.coreboot.org/c/flashrom/+/43416 )

Change subject: flashchips: Add support for Macronix MX25L5121E
......................................................................

flashchips: Add support for Macronix MX25L5121E

Tested with ch341a_spi.

Change-Id: I881e2cda938083ba271b2ee0c457d2bbd8e1a766
Signed-off-by: Steve Markgraf 
Reviewed-on: https://review.coreboot.org/c/flashrom/+/43416
Reviewed-by: Angel Pons 
Tested-by: build bot (Jenkins) 
---
M flashchips.c
M flashchips.h
2 files changed, 39 insertions(+), 0 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Angel Pons: Looks good to me, approved

diff --git a/flashchips.c b/flashchips.c
index 3d8042d..09ac9b4 100644
--- a/flashchips.c
+++ b/flashchips.c
@@ -8543,6 +8543,44 @@

{
    	.vendor		= "Macronix",
+		.name		= "MX25L5121E",
+		.bustype	= BUS_SPI,
+		.manufacture_id	= MACRONIX_ID,
+		.model_id	= MACRONIX_MX25L5121E,
+		.total_size	= 64,
+		.page_size	= 32,
+		.feature_bits	= FEATURE_WRSR_WREN,
+		.tested		= TEST_OK_PREW,
+		.probe		= probe_spi_rdid,
+		.probe_timing	= TIMING_ZERO,
+		.block_erasers	=
+		{
+			{
+				.eraseblocks = { {4 * 1024, 16} },
+				.block_erase = spi_block_erase_20,
+			}, {
+				.eraseblocks = { {64 * 1024, 1} },
+				.block_erase = spi_block_erase_52,
+			}, {
+				.eraseblocks = { {64 * 1024, 1} },
+				.block_erase = spi_block_erase_d8,
+			}, {
+				.eraseblocks = { {64 * 1024, 1} },
+				.block_erase = spi_block_erase_60,
+			}, {
+				.eraseblocks = { {64 * 1024, 1} },
+				.block_erase = spi_block_erase_c7,
+			},
+		},
+		.printlock	= spi_prettyprint_status_register_bp1_srwd,
+		.unlock		= spi_disable_blockprotect,
+		.write		= spi_chip_write_256,
+		.read		= spi_chip_read, /* Fast read (0x0B) supported */
+		.voltage	= {2700, 3600},
+	},
+
+	{
+		.vendor		= "Macronix",
    	.name		= "MX25L6405",
    	.bustype	= BUS_SPI,
    	.manufacture_id	= MACRONIX_ID,
diff --git a/flashchips.h b/flashchips.h
index f6e2588..4ae6c07 100644
--- a/flashchips.h
+++ b/flashchips.h
@@ -482,6 +482,7 @@
  * Generalplus SPI chips seem to be compatible with Macronix
  * and use the same set of IDs. */
 #define MACRONIX_MX25L512	0x2010	/* Same as MX25L512E, MX25V512, MX25V512C */
+#define MACRONIX_MX25L5121E	0x2210
 #define MACRONIX_MX25L1005	0x2011	/* Same as MX25L1005C, MX25L1006E */
 #define MACRONIX_MX25L2005	0x2012	/* Same as MX25L2005C, MX25L2006E */
 #define MACRONIX_MX25L4005	0x2013	/* Same as MX25L4005A, MX25L4005C, MX25L4006E */