Change in flashrom[master]: flashchips: Add Macronix MX25R3235F

[Samir Ibradžić (Code Review)]

Samir Ibradžić has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/39532 )

Change subject: flashchips: Add Macronix MX25R3235F
......................................................................

flashchips: Add Macronix MX25R3235F

32Mbit (4MB) [x1/x2/x4] Wide Voltage Range (VCC 1.65V-3.6V), data-sheet:
https://www.macronix.com/Lists/Datasheet/Attachments/7534/MX25R3235F,%20Wide...

Similar to already supported MX25R6435F, just half the size.

Tested on ch341a, 4232H & PicoTAP (2232H) programmers, confirmed working
probe, read, erase & write.

Change-Id: I6e79115adba17d13d24bc85d78707d53fd4a0be5
Signed-off-by: Samir Ibradzic 
---
M flashchips.c
M flashchips.h
2 files changed, 40 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/32/39532/1

diff --git a/flashchips.c b/flashchips.c
index 58dd4f3..69ea270 100644
--- a/flashchips.c
+++ b/flashchips.c
@@ -8735,6 +8735,45 @@

{
    	.vendor		= "Macronix",
+		.name		= "MX25R3235F",
+		.bustype	= BUS_SPI,
+		.manufacture_id	= MACRONIX_ID,
+		.model_id	= MACRONIX_MX25R3235F,
+		.total_size	= 4096,
+		.page_size	= 256,
+		/* OTP: 1024B total; enter 0xB1, exit 0xC1 */
+		.feature_bits	= FEATURE_WRSR_WREN | FEATURE_OTP,
+		.tested		= TEST_OK_PREW,
+		.probe		= probe_spi_rdid,
+		.probe_timing	= TIMING_ZERO,
+		.block_erasers	=
+		{
+			{
+				.eraseblocks = { {4 * 1024, 1024} },
+				.block_erase = spi_block_erase_20,
+			}, {
+				.eraseblocks = { {64 * 1024, 64} },
+				.block_erase = spi_block_erase_d8,
+			}, {
+				.eraseblocks = { {32 * 1024, 128} },
+				.block_erase = spi_block_erase_52,
+			}, {
+				.eraseblocks = { {4 * 1024 * 1024, 1} },
+				.block_erase = spi_block_erase_60,
+			}, {
+				.eraseblocks = { {4 * 1024 * 1024, 1} },
+				.block_erase = spi_block_erase_c7,
+			}
+		},
+		.printlock	= spi_prettyprint_status_register_bp3_srwd, /* bit6 is quad enable */
+		.unlock		= spi_disable_blockprotect_bp3_srwd,
+		.write		= spi_chip_write_256,
+		.read		= spi_chip_read, /* Fast read (0x0B) and multi I/O supported */
+		.voltage	= {1650, 3600},
+	},
+
+	{
+		.vendor		= "Macronix",
    	.name		= "MX25R6435F",
    	.bustype	= BUS_SPI,
    	.manufacture_id	= MACRONIX_ID,
diff --git a/flashchips.h b/flashchips.h
index 14ab6de..36990e2 100644
--- a/flashchips.h
+++ b/flashchips.h
@@ -503,6 +503,7 @@
 #define MACRONIX_MX25L3235D	0x5E16	/* MX25L3225D/MX25L3235D/MX25L3237D */
 #define MACRONIX_MX25L6495F	0x9517

+#define MACRONIX_MX25R3235F	0x2816
 #define MACRONIX_MX25R6435F	0x2817

#define MACRONIX_MX29F001B	0x19

[Samir Ibradžić (Code Review)]

Samir Ibradžić has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/39532 )

Change subject: flashchips: Add Macronix MX25R3235F
......................................................................

Patch Set 1:

Also fixes https://github.com/flashrom/flashrom/issues/43

[Paul Menzel (Code Review)]

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/39532 )

Change subject: flashchips: Add Macronix MX25R3235F
......................................................................

Patch Set 1: Code-Review+1
> Patch Set 1:
>
> Also fixes https://github.com/flashrom/flashrom/issues/43

Please add
> Fixes: https://github.com/flashrom/flashrom/issues/43

to the commit messages, so GitHub will make the connection, and close the issue (if configured that way).

[Samir Ibradžić (Code Review)]

Samir Ibradžić has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/39532 )

Change subject: flashchips: Add Macronix MX25R3235F
......................................................................

Patch Set 1:
> Patch Set 1: Code-Review+1
> > Patch Set 1:
> >
> > Also fixes https://github.com/flashrom/flashrom/issues/43
>
> Please add
> > Fixes: https://github.com/flashrom/flashrom/issues/43
>
> to the commit messages, so GitHub will make the connection, and close the issue (if configured that way).

Yeah, I know, somehow I forgot the thing this time.

[Samir Ibradžić (Code Review)]

Samir Ibradžić has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/39532 )

Change subject: flashchips: Add Macronix MX25R3235F
......................................................................

Patch Set 1:

Hi there,

Someone review please...

R,
S

[Samir Ibradžić (Code Review)]

Samir Ibradžić has removed Paul Menzel from this change.  ( https://review.coreboot.org/c/flashrom/+/39532 )

Change subject: flashchips: Add Macronix MX25R3235F
......................................................................

Removed reviewer Paul Menzel with the following votes:

* Code-Review+1 by Paul Menzel

[Samir Ibradžić (Code Review)]

Samir Ibradžić has removed build bot (Jenkins) from this change.  ( https://review.coreboot.org/c/flashrom/+/39532 )

Change subject: flashchips: Add Macronix MX25R3235F
......................................................................

Removed reviewer build bot (Jenkins) with the following votes:

* Verified+1 by build bot (Jenkins)

[Angel Pons (Code Review)]

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

Change subject: flashchips: Add Macronix MX25R3235F
......................................................................

Patch Set 1: Code-Review+1

(4 comments)

Just a few comments on the commit message. I'm sorry that it took me so long to review.

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

https://review.coreboot.org/c/flashrom/+/39532/1//COMMIT_MSG@9 
PS1, Line 9: 4MB
That would be 4 MiB, actually.

Another option would be to omit the paragraph instead. The URL is rather long, could change over time, and the flash chip information is in the code anyway

https://review.coreboot.org/c/flashrom/+/39532/1//COMMIT_MSG@12 
PS1, Line 12: Similar to already supported MX25R6435F, just half the size.
Make this a sentence, for example:

It is similar to the already-supported MX25R6435F, but the total size is halved.

https://review.coreboot.org/c/flashrom/+/39532/1//COMMIT_MSG@14 
PS1, Line 14: 4232H
Did you mean "FT4232H" and "FT2232H" ?

https://review.coreboot.org/c/flashrom/+/39532/1//COMMIT_MSG@15 
PS1, Line 15: &
just "and"

[Samir Ibradžić (Code Review)]

Hello build bot (Jenkins), Paul Menzel, David Hendricks, Stefan Reinauer, Edward O'Callaghan, Angel Pons,

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

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

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

Change subject: flashchips: Add Macronix MX25R3235F
......................................................................

flashchips: Add Macronix MX25R3235F

32Mbit (4MiB) [x1/x2/x4] Wide Voltage Range (VCC 1.65V-3.6V). It is
similar to the already-supported MX25R6435F, but the total size is halved.

Tested on ch341a, FT4232H and FT2232H (PicoTAP) programmers, confirmed working
probe, read, erase and write.

Fixes: https://github.com/flashrom/flashrom/issues/43

Change-Id: I6e79115adba17d13d24bc85d78707d53fd4a0be5
Signed-off-by: Samir Ibradzic 
---
M flashchips.c
M flashchips.h
2 files changed, 40 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/32/39532/2

[Samir Ibradžić (Code Review)]

Samir Ibradžić has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/39532 )

Change subject: flashchips: Add Macronix MX25R3235F
......................................................................

Patch Set 2:

(4 comments)
> Patch Set 1: Code-Review+1
>
> (4 comments)
>
> Just a few comments on the commit message. I'm sorry that it took me so long to review.

Thnx!

All sorted out & "Fixed:" reference to GH issue added as suggested by Paul.
Can we have it merged please?

Cheers

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

https://review.coreboot.org/c/flashrom/+/39532/1//COMMIT_MSG@9 
PS1, Line 9: 4MB
> That would be 4 MiB, actually. […]

Done

https://review.coreboot.org/c/flashrom/+/39532/1//COMMIT_MSG@12 
PS1, Line 12: Similar to already supported MX25R6435F, just half the size.
> Make this a sentence, for example: […]

Done

https://review.coreboot.org/c/flashrom/+/39532/1//COMMIT_MSG@14 
PS1, Line 14: 4232H
> Did you mean "FT4232H" and "FT2232H" ?

Done

https://review.coreboot.org/c/flashrom/+/39532/1//COMMIT_MSG@15 
PS1, Line 15: &
> just "and"

Done

[Angel Pons (Code Review)]

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

Change subject: flashchips: Add Macronix MX25R3235F
......................................................................

Patch Set 2: Code-Review+2

(2 comments)

One last thing!

https://review.coreboot.org/c/flashrom/+/39532/2//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/flashrom/+/39532/2//COMMIT_MSG@10 
PS2, Line 10:  halved.
This needs to go on the next line. The commit message width is at most 72 characters

https://review.coreboot.org/c/flashrom/+/39532/2//COMMIT_MSG@12 
PS2, Line 12:  working
Same here

[Samir Ibradžić (Code Review)]

Hello build bot (Jenkins), Paul Menzel, David Hendricks, Stefan Reinauer, Edward O'Callaghan, Angel Pons,

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

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

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

Change subject: flashchips: Add Macronix MX25R3235F
......................................................................

flashchips: Add Macronix MX25R3235F

32Mbit (4MiB) [x1/x2/x4] Wide Voltage Range (VCC 1.65V-3.6V). It is
similar to the already-supported MX25R6435F, but the total size is
halved.

Tested on ch341a, FT4232H and FT2232H (PicoTAP) programmers, confirmed
working probe, read, erase and write.

Fixes: https://github.com/flashrom/flashrom/issues/43

Change-Id: I6e79115adba17d13d24bc85d78707d53fd4a0be5
Signed-off-by: Samir Ibradzic 
---
M flashchips.c
M flashchips.h
2 files changed, 40 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/32/39532/3

[Samir Ibradžić (Code Review)]

Samir Ibradžić has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/39532 )

Change subject: flashchips: Add Macronix MX25R3235F
......................................................................

Patch Set 3:

(2 comments)
> Patch Set 2: Code-Review+2
>
> (2 comments)
>
> One last thing!

fixed

https://review.coreboot.org/c/flashrom/+/39532/2//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/flashrom/+/39532/2//COMMIT_MSG@10 
PS2, Line 10:  halved.
> This needs to go on the next line. […]

Done

https://review.coreboot.org/c/flashrom/+/39532/2//COMMIT_MSG@12 
PS2, Line 12:  working
> Same here

Done

[Angel Pons (Code Review)]

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

Change subject: flashchips: Add Macronix MX25R3235F
......................................................................

Patch Set 3:
> Patch Set 2:
>
> (4 comments)
> > Patch Set 1: Code-Review+1
> >
> > (4 comments)
> >
> > Just a few comments on the commit message. I'm sorry that it took me so long to review.
>
> Thnx!
>
> All sorted out & "Fixed:" reference to GH issue added as suggested by Paul.
> Can we have it merged please?
>
> Cheers

Perfect, thank you!

[Angel Pons (Code Review)]

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

Change subject: flashchips: Add Macronix MX25R3235F
......................................................................

Patch Set 3:

Um, could you please manually rebase this change? The build bot does not seem to have picked it up

[Angel Pons (Code Review)]

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

Change subject: flashchips: Add Macronix MX25R3235F
......................................................................

Patch Set 3:

(1 comment)
> Patch Set 3:
>
> Um, could you please manually rebase this change? The build bot does not seem to have picked it up

Maybe change something, for example a comment.

(I know, it's frustrating that such a small change is taking so long to get in, but it will go in as soon as the build bot verifies it)

https://review.coreboot.org/c/flashrom/+/39532/3/flashchips.c 
File flashchips.c:

https://review.coreboot.org/c/flashrom/+/39532/3/flashchips.c@8768 
PS3, Line 8768: bit6
Try to make the build bot re-test this change by changing this to "Bit 6"

[Edward O'Callaghan (Code Review)]

Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/39532 )

Change subject: flashchips: Add Macronix MX25R3235F
......................................................................

Patch Set 3: Code-Review+2

[Samir Ibradžić (Code Review)]

Hello build bot (Jenkins), Paul Menzel, David Hendricks, Stefan Reinauer, Edward O'Callaghan, Angel Pons,

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

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

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

Change subject: flashchips: Add Macronix MX25R3235F
......................................................................

flashchips: Add Macronix MX25R3235F

32Mbit (4MiB) [x1/x2/x4] Wide Voltage Range (VCC 1.65V-3.6V). It is
similar to the already-supported MX25R6435F, but the total size is
halved.

Tested on ch341a, FT4232H and FT2232H (PicoTAP) programmers, confirmed
working probe, read, erase and write.

Fixes: https://github.com/flashrom/flashrom/issues/43

Change-Id: I6e79115adba17d13d24bc85d78707d53fd4a0be5
Signed-off-by: Samir Ibradzic 
---
M flashchips.c
M flashchips.h
2 files changed, 40 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/32/39532/4

[Samir Ibradžić (Code Review)]

Samir Ibradžić has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/39532 )

Change subject: flashchips: Add Macronix MX25R3235F
......................................................................

Patch Set 4:

(1 comment)
> Patch Set 3:
>
> (1 comment)
> > Patch Set 3:
> >
> > Um, could you please manually rebase this change? The build bot does not seem to have picked it up
>
> Maybe change something, for example a comment.
>
> (I know, it's frustrating that such a small change is taking so long to get in, but it will go in as soon as the build bot verifies it)

Done. Looks like gerrit picked it up this time...

https://review.coreboot.org/c/flashrom/+/39532/3/flashchips.c 
File flashchips.c:

https://review.coreboot.org/c/flashrom/+/39532/3/flashchips.c@8768 
PS3, Line 8768: bit6
> Try to make the build bot re-test this change by changing this to "Bit 6"

Done

[Angel Pons (Code Review)]

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

Change subject: flashchips: Add Macronix MX25R3235F
......................................................................

Patch Set 4: Code-Review+2
> Patch Set 4:
>
> (1 comment)
> > Patch Set 3:
> >
> > (1 comment)
> > > Patch Set 3:
> > >
> > > Um, could you please manually rebase this change? The build bot does not seem to have picked it up
> >
> > Maybe change something, for example a comment.
> >
> > (I know, it's frustrating that such a small change is taking so long to get in, but it will go in as soon as the build bot verifies it)
>
> Done. Looks like gerrit picked it up this time...

Yep! Brace for impact...

[Angel Pons (Code Review)]

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

Change subject: flashchips: Add Macronix MX25R3235F
......................................................................

flashchips: Add Macronix MX25R3235F

32Mbit (4MiB) [x1/x2/x4] Wide Voltage Range (VCC 1.65V-3.6V). It is
similar to the already-supported MX25R6435F, but the total size is
halved.

Tested on ch341a, FT4232H and FT2232H (PicoTAP) programmers, confirmed
working probe, read, erase and write.

Fixes: https://github.com/flashrom/flashrom/issues/43

Change-Id: I6e79115adba17d13d24bc85d78707d53fd4a0be5
Signed-off-by: Samir Ibradzic 
Reviewed-on: https://review.coreboot.org/c/flashrom/+/39532
Tested-by: build bot (Jenkins) 
Reviewed-by: Angel Pons 
---
M flashchips.c
M flashchips.h
2 files changed, 40 insertions(+), 0 deletions(-)

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

diff --git a/flashchips.c b/flashchips.c
index 58dd4f3..942cbc2 100644
--- a/flashchips.c
+++ b/flashchips.c
@@ -8735,6 +8735,45 @@

{
    	.vendor		= "Macronix",
+		.name		= "MX25R3235F",
+		.bustype	= BUS_SPI,
+		.manufacture_id	= MACRONIX_ID,
+		.model_id	= MACRONIX_MX25R3235F,
+		.total_size	= 4096,
+		.page_size	= 256,
+		/* OTP: 1024B total; enter 0xB1, exit 0xC1 */
+		.feature_bits	= FEATURE_WRSR_WREN | FEATURE_OTP,
+		.tested		= TEST_OK_PREW,
+		.probe		= probe_spi_rdid,
+		.probe_timing	= TIMING_ZERO,
+		.block_erasers	=
+		{
+			{
+				.eraseblocks = { {4 * 1024, 1024} },
+				.block_erase = spi_block_erase_20,
+			}, {
+				.eraseblocks = { {64 * 1024, 64} },
+				.block_erase = spi_block_erase_d8,
+			}, {
+				.eraseblocks = { {32 * 1024, 128} },
+				.block_erase = spi_block_erase_52,
+			}, {
+				.eraseblocks = { {4 * 1024 * 1024, 1} },
+				.block_erase = spi_block_erase_60,
+			}, {
+				.eraseblocks = { {4 * 1024 * 1024, 1} },
+				.block_erase = spi_block_erase_c7,
+			}
+		},
+		.printlock	= spi_prettyprint_status_register_bp3_srwd, /* bit 6 is quad enable */
+		.unlock		= spi_disable_blockprotect_bp3_srwd,
+		.write		= spi_chip_write_256,
+		.read		= spi_chip_read, /* Fast read (0x0B) and multi I/O supported */
+		.voltage	= {1650, 3600},
+	},
+
+	{
+		.vendor		= "Macronix",
    	.name		= "MX25R6435F",
    	.bustype	= BUS_SPI,
    	.manufacture_id	= MACRONIX_ID,
diff --git a/flashchips.h b/flashchips.h
index 14ab6de..36990e2 100644
--- a/flashchips.h
+++ b/flashchips.h
@@ -503,6 +503,7 @@
 #define MACRONIX_MX25L3235D	0x5E16	/* MX25L3225D/MX25L3235D/MX25L3237D */
 #define MACRONIX_MX25L6495F	0x9517

+#define MACRONIX_MX25R3235F	0x2816
 #define MACRONIX_MX25R6435F	0x2817

#define MACRONIX_MX29F001B	0x19