Change in flashrom[master]: flashchips: Add support for N25Q512..1E

List overview All Threads
Download

newer

older

Change in flashrom[master]:...

Jacob Creedon (Code Review)

19 Jul 2019 19 Jul '19

2 a.m.

Jacob Creedon has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/34413 )

Change subject: flashchips: Add support for N25Q512..1E ......................................................................

flashchips: Add support for N25Q512..1E

This adds support for the 1.8v variant of N25Q512.

Signed-off-by: Jacob Creedon jcreedon@google.com Change-Id: I9e8d2c1fc0c09efea7812fa513b45d1a372d06d7 --- M flashchips.c 1 file changed, 40 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/13/34413/1

diff --git a/flashchips.c b/flashchips.c index 166af6a..cc22414 100644 --- a/flashchips.c +++ b/flashchips.c @@ -10585,6 +10585,46 @@

{ .vendor = "Micron", + .name = "N25Q512..1E/MT25QU512", /* ..1E/U = 1.8V, uniform 64KB/4KB blocks/sectors */ + .bustype = BUS_SPI, + .manufacture_id = ST_ID, + .model_id = ST_N25Q512__1E, + .total_size = 65536, + .page_size = 256, + /* supports SFDP */ + /* OTP: 64B total; read 0x4B, write 0x42 */ + .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_4BA_WREN, + .tested = TEST_OK_PREW, + .probe = probe_spi_rdid, + .probe_timing = TIMING_ZERO, + .block_erasers = + { + { + .eraseblocks = { {4 * 1024, 16384} }, + .block_erase = spi_block_erase_21, + }, { + .eraseblocks = { {4 * 1024, 16384} }, + .block_erase = spi_block_erase_20, + }, { + .eraseblocks = { {64 * 1024, 1024} }, + .block_erase = spi_block_erase_dc, + }, { + .eraseblocks = { {64 * 1024, 1024} }, + .block_erase = spi_block_erase_d8, + }, { + .eraseblocks = { {65536 * 1024, 1} }, + .block_erase = spi_block_erase_c7, + } + }, + .printlock = spi_prettyprint_status_register_n25q, /* TODO: config, lock, flag regs */ + .unlock = spi_disable_blockprotect_n25q, /* TODO: per 64kB sector lock registers */ + .write = spi_chip_write_256, /* Multi I/O supported */ + .read = spi_chip_read, /* Fast read (0x0B) and multi I/O supported */ + .voltage = {2700, 3600}, + }, + + { + .vendor = "Micron", .name = "N25Q512..3E/MT25QL512", /* ..3E/L = 3V, uniform 64KB/4KB blocks/sectors */ .bustype = BUS_SPI, .manufacture_id = ST_ID,

Attachments:

attachment.htm (text/html — 5.5 KB)

Show replies by date

Jacob Creedon (Code Review)

19 Jul 19 Jul

5:02 p.m.

Hello build bot (Jenkins),

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

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

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

Change subject: flashchips: Add support for N25Q512..1E ......................................................................

flashchips: Add support for N25Q512..1E

This adds support for the 1.8v variant of N25Q512.

Signed-off-by: Jacob Creedon jcreedon@google.com Change-Id: I9e8d2c1fc0c09efea7812fa513b45d1a372d06d7 --- M flashchips.c 1 file changed, 40 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/13/34413/2

Jacob Creedon (Code Review)

5:17 p.m.

Jacob Creedon has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/34413 )

Change subject: flashchips: Add support for N25Q512..1E ......................................................................

Patch Set 2:

Hi Nico,

Please review this change for an additional variant of an existing chip. The entire N25Qxxx..1E series should be identical but I only have hardware to test the 512Mb version. Should I also add the other sizes but marked with TEST_UNTESTED?

Thanks!

-- To view, visit https://review.coreboot.org/c/flashrom/+/34413 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9e8d2c1fc0c09efea7812fa513b45d1a372d06d7 Gerrit-Change-Number: 34413 Gerrit-PatchSet: 2 Gerrit-Owner: Jacob Creedon jcreedon@google.com Gerrit-Reviewer: Jacob Creedon jcreedon@google.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Fri, 19 Jul 2019 15:17:23 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

Nico Huber (Code Review)

6:05 p.m.

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/34413 )

Change subject: flashchips: Add support for N25Q512..1E ......................................................................

Patch Set 2: Code-Review+2

(2 comments)

Looks good, but it would be nice to add all erase opcodes.

https://review.coreboot.org/c/flashrom/+/34413/2/flashchips.c File flashchips.c:

https://review.coreboot.org/c/flashrom/+/34413/2/flashchips.c@10608 PS2, Line 10608: }, { Datasheet also mentions 32KiB 0x5c and 0x52.

https://review.coreboot.org/c/flashrom/+/34413/2/flashchips.c@10616 PS2, Line 10616: .block_erase = spi_block_erase_c7, Also 0x60 according to the datasheet. The reason that we usually add all opcodes is that there are programmers that may restrict what we can send to the flash.

-- To view, visit https://review.coreboot.org/c/flashrom/+/34413 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9e8d2c1fc0c09efea7812fa513b45d1a372d06d7 Gerrit-Change-Number: 34413 Gerrit-PatchSet: 2 Gerrit-Owner: Jacob Creedon jcreedon@google.com Gerrit-Reviewer: Jacob Creedon jcreedon@google.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Fri, 19 Jul 2019 16:05:39 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Nico Huber (Code Review)

6:07 p.m.

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/34413 )

Change subject: flashchips: Add support for N25Q512..1E ......................................................................

Patch Set 2:

...

Please review this change for an additional variant of an existing chip. The entire N25Qxxx..1E series should be identical but I only have hardware to test the 512Mb version. Should I also add the other sizes but marked with TEST_UNTESTED?

That would be nice. Usually saves some time to add/review them in one go.

-- To view, visit https://review.coreboot.org/c/flashrom/+/34413 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9e8d2c1fc0c09efea7812fa513b45d1a372d06d7 Gerrit-Change-Number: 34413 Gerrit-PatchSet: 2 Gerrit-Owner: Jacob Creedon jcreedon@google.com Gerrit-Reviewer: Jacob Creedon jcreedon@google.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Fri, 19 Jul 2019 16:07:24 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

Jacob Creedon (Code Review)

7:32 p.m.

New subject: Change in flashrom[master]: flashchips: Fill out support for N25Q/MT25Q chips

Hello build bot (Jenkins), Nico Huber,

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

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

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

Change subject: flashchips: Fill out support for N25Q/MT25Q chips ......................................................................

flashchips: Fill out support for N25Q/MT25Q chips

This adds the names for the MT25Q versions to the 128Mb Micron devices and adds 1.8v variants of the 256Mb and 512Mb Micron devices.

This has been tested in hardware for the MT25QU512 and the MT25QU256 is submitted untested.

Signed-off-by: Jacob Creedon jcreedon@google.com Change-Id: I9e8d2c1fc0c09efea7812fa513b45d1a372d06d7 --- M flashchips.c 1 file changed, 82 insertions(+), 2 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/13/34413/3

-- To view, visit https://review.coreboot.org/c/flashrom/+/34413 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9e8d2c1fc0c09efea7812fa513b45d1a372d06d7 Gerrit-Change-Number: 34413 Gerrit-PatchSet: 3 Gerrit-Owner: Jacob Creedon jcreedon@google.com Gerrit-Reviewer: Jacob Creedon jcreedon@google.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-MessageType: newpatchset

Jacob Creedon (Code Review)

7:32 p.m.

New subject: Change in flashrom[master]: flashchips: Fill out support for N25Q/MT25Q chips

Jacob Creedon has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/34413 )

Change subject: flashchips: Fill out support for N25Q/MT25Q chips ......................................................................

Patch Set 2:

(2 comments)

See my notes about N25Q vs MT25Q compatibility.

https://review.coreboot.org/c/flashrom/+/34413/2/flashchips.c File flashchips.c:

https://review.coreboot.org/c/flashrom/+/34413/2/flashchips.c@10608 PS2, Line 10608: }, {

...

Datasheet also mentions 32KiB 0x5c and 0x52.

My understanding here is that the newer MT25 series supports the 32K subsector erase, but the older N25Q does not, however, both use the same JEDEC ID. :-/ There is a application note, TN-25-01 which highlights the differences between N25Q and MT25Q.

https://review.coreboot.org/c/flashrom/+/34413/2/flashchips.c@10616 PS2, Line 10616: .block_erase = spi_block_erase_c7,

...

Also 0x60 according to the datasheet. The reason that we usually add all […]

This is also a N25Q vs MT25Q issue. Also, I discovered a subtle bug here in that the N25Q512 is a stacked device (two 256Mb dies) whereas the MT25Q512 is monolithic. What this means is that 0xC7 (and 0x60) will work for the MT25Q but not for the N25Q. It would instead would require 0xC4, a special "die erase" command that only applies for stacked devices. Once again, they share the same JEDEC ID :-(

I'm new to this codebase so I don't know the best way to address this issue. Probably the safest thing to do short term is remove the bulk erase command altogether for the 512Mb device. In practice, I've found that erasing a sector at a time is only marginally slower than issuing a bulk erase. Thoughts?

(To be clear here I'm actually working on an MT25Q device)

-- To view, visit https://review.coreboot.org/c/flashrom/+/34413 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9e8d2c1fc0c09efea7812fa513b45d1a372d06d7 Gerrit-Change-Number: 34413 Gerrit-PatchSet: 2 Gerrit-Owner: Jacob Creedon jcreedon@google.com Gerrit-Reviewer: Jacob Creedon jcreedon@google.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Fri, 19 Jul 2019 17:32:58 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber nico.h@gmx.de Gerrit-MessageType: comment

Jacob Creedon (Code Review)

7:39 p.m.

New subject: Change in flashrom[master]: flashchips: Fill out support for N25Q/MT25Q chips

Jacob Creedon has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/34413 )

Change subject: flashchips: Fill out support for N25Q/MT25Q chips ......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/flashrom/+/34413/2/flashchips.c File flashchips.c:

https://review.coreboot.org/c/flashrom/+/34413/2/flashchips.c@10616 PS2, Line 10616: .block_erase = spi_block_erase_c7,

...

This is also a N25Q vs MT25Q issue. […]

Also, to be clear, this bug affects the pre-existing N25Q512..3E in addition to the N25Q512..1E that I am adding.

-- To view, visit https://review.coreboot.org/c/flashrom/+/34413 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9e8d2c1fc0c09efea7812fa513b45d1a372d06d7 Gerrit-Change-Number: 34413 Gerrit-PatchSet: 3 Gerrit-Owner: Jacob Creedon jcreedon@google.com Gerrit-Reviewer: Jacob Creedon jcreedon@google.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Fri, 19 Jul 2019 17:39:46 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Jacob Creedon jcreedon@google.com Comment-In-Reply-To: Nico Huber nico.h@gmx.de Gerrit-MessageType: comment

Jacob Creedon (Code Review)

11:24 p.m.

New subject: Change in flashrom[master]: flashchips: Fill out support for N25Q/MT25Q chips

Hello build bot (Jenkins), Nico Huber,

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

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

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

Change subject: flashchips: Fill out support for N25Q/MT25Q chips ......................................................................

flashchips: Fill out support for N25Q/MT25Q chips

This adds the names for the MT25Q versions to the 128Mb Micron devices and adds 1.8v variants of the 256Mb and 512Mb Micron devices.

This has been tested in hardware for the MT25QU512 and the MT25QU256 is submitted untested.

Signed-off-by: Jacob Creedon jcreedon@google.com Change-Id: I9e8d2c1fc0c09efea7812fa513b45d1a372d06d7 --- M flashchips.c 1 file changed, 82 insertions(+), 2 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/13/34413/4

-- To view, visit https://review.coreboot.org/c/flashrom/+/34413 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9e8d2c1fc0c09efea7812fa513b45d1a372d06d7 Gerrit-Change-Number: 34413 Gerrit-PatchSet: 4 Gerrit-Owner: Jacob Creedon jcreedon@google.com Gerrit-Reviewer: Jacob Creedon jcreedon@google.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-MessageType: newpatchset

Nico Huber (Code Review)

20 Jul 20 Jul

12:59 p.m.

New subject: Change in flashrom[master]: flashchips: Fill out support for N25Q/MT25Q chips

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/34413 )

Change subject: flashchips: Fill out support for N25Q/MT25Q chips ......................................................................

Patch Set 4:

(2 comments)

https://review.coreboot.org/c/flashrom/+/34413/2/flashchips.c File flashchips.c:

https://review.coreboot.org/c/flashrom/+/34413/2/flashchips.c@10608 PS2, Line 10608: }, {

...

My understanding here is that the newer MT25 series supports the 32K subsector erase, but the older […]

We handle such differences with multiple chip definitions that share the ID. Flashrom supports that and would query the user to decide which chip it is. See for instance Macronix definitions (e.g. MX25L6405), they very often recycle IDs.

So you would have to add two entries, one for "N25Q512..1E" and one for "MT25QU512".

https://review.coreboot.org/c/flashrom/+/34413/2/flashchips.c@10616 PS2, Line 10616: .block_erase = spi_block_erase_c7,

...

I'm new to this codebase so I don't know the best way to address this issue. Probably the safest thing to do short term is remove the bulk erase command altogether for the 512Mb device.

I think the chip definitions should be as comprehensive as possible (within flashrom's code limits).

...

In practice, I've found that erasing a sector at a time is only marginally slower than issuing a bulk erase. Thoughts?

Actually, flashrom tries the list of erase functions from top to bottom anyway. It would only choose the bulk erase if the programmer situation doesn't allow any preceding function. The whole logic is not very smart and should change in the future. Hence, the chip definitions should be free from any assumptions.

...

Also, to be clear, this bug affects the pre-existing N25Q512..3E in addition to the N25Q512..1E that I am adding.

That should be addressed in a separate commit. Let me know if you want to do so, otherwise I'll take it.

-- To view, visit https://review.coreboot.org/c/flashrom/+/34413 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9e8d2c1fc0c09efea7812fa513b45d1a372d06d7 Gerrit-Change-Number: 34413 Gerrit-PatchSet: 4 Gerrit-Owner: Jacob Creedon jcreedon@google.com Gerrit-Reviewer: Jacob Creedon jcreedon@google.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Sat, 20 Jul 2019 10:59:04 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Jacob Creedon jcreedon@google.com Comment-In-Reply-To: Nico Huber nico.h@gmx.de Gerrit-MessageType: comment

Jacob Creedon (Code Review)

22 Jul 22 Jul

10:59 p.m.

New subject: Change in flashrom[master]: flashchips: Fill out support for N25Q/MT25Q chips

Jacob Creedon has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/34413 )

Change subject: flashchips: Fill out support for N25Q/MT25Q chips ......................................................................

Patch Set 4:

(2 comments)

I've cleaned up the patch and split them out into more easily reviewed patches. I will abandon this review and open up new ones for the new patch stack. Thanks!

https://review.coreboot.org/c/flashrom/+/34413/2/flashchips.c File flashchips.c:

https://review.coreboot.org/c/flashrom/+/34413/2/flashchips.c@10608 PS2, Line 10608: }, {

...

We handle such differences with multiple chip definitions that share the ID. Flashrom […]

Done

https://review.coreboot.org/c/flashrom/+/34413/2/flashchips.c@10616 PS2, Line 10616: .block_erase = spi_block_erase_c7,

...

...
I'm new to this codebase so I don't know the best way to address this issue. […]

Done

-- To view, visit https://review.coreboot.org/c/flashrom/+/34413 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9e8d2c1fc0c09efea7812fa513b45d1a372d06d7 Gerrit-Change-Number: 34413 Gerrit-PatchSet: 4 Gerrit-Owner: Jacob Creedon jcreedon@google.com Gerrit-Reviewer: Jacob Creedon jcreedon@google.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Mon, 22 Jul 2019 20:59:54 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Jacob Creedon jcreedon@google.com Comment-In-Reply-To: Nico Huber nico.h@gmx.de Gerrit-MessageType: comment

Jacob Creedon (Code Review)

11 p.m.

New subject: Change in flashrom[master]: flashchips: Fill out support for N25Q/MT25Q chips

Jacob Creedon has abandoned this change. ( https://review.coreboot.org/c/flashrom/+/34413 )

Change subject: flashchips: Fill out support for N25Q/MT25Q chips ......................................................................

Abandoned

Will submit new patch stack.

-- To view, visit https://review.coreboot.org/c/flashrom/+/34413 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9e8d2c1fc0c09efea7812fa513b45d1a372d06d7 Gerrit-Change-Number: 34413 Gerrit-PatchSet: 4 Gerrit-Owner: Jacob Creedon jcreedon@google.com Gerrit-Reviewer: Jacob Creedon jcreedon@google.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-MessageType: abandon

1980

days inactive

1983

days old

flashrom-gerrit@flashrom.org

11 comments

2 participants

tags (0)

participants (2)

Jacob Creedon (Code Review)
Nico Huber (Code Review)