Jacob Creedon has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/34488 )
Change subject: flashchips: Split MT25Q from N25Q ......................................................................
flashchips: Split MT25Q from N25Q
The MT25Q is the successor to the N25Q from Micron/Numonyx/ST. The MT25Q is almost entirely backwards compatible with the N25Q series, however, the MT25Q has additional subsector erase commands available, and there are differences in stacked devices in the higher capacity variants. The N25Q devices are left with "Micron/Numonyx/ST" as the vendor and MT25Q devices are set with "Micron" as the vendor.
Signed-off-by: Jacob Creedon jcreedon@google.com Change-Id: I9d79978544b19cf9acd5f3ea6196cf6f3b3435ef --- M flashchips.c 1 file changed, 82 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/88/34488/1
diff --git a/flashchips.c b/flashchips.c index 166af6a..e6116ae 100644 --- a/flashchips.c +++ b/flashchips.c @@ -10544,8 +10544,88 @@ },
{ + .vendor = "Micron/Numonyx/ST", + .name = "N25Q256..3E", /* ..3E = 3V, uniform 64KB/4KB blocks/sectors */ + .bustype = BUS_SPI, + .manufacture_id = ST_ID, + .model_id = ST_N25Q256__3E, + .total_size = 32768, + .page_size = 256, + /* supports SFDP */ + /* OTP: 64B total; read 0x4B, write 0x42 */ + .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_4BA_WREN, + .tested = TEST_UNTESTED, + .probe = probe_spi_rdid, + .probe_timing = TIMING_ZERO, + .block_erasers = + { + { + .eraseblocks = { {4 * 1024, 8192} }, + .block_erase = spi_block_erase_21, + }, { + .eraseblocks = { {4 * 1024, 8192} }, + .block_erase = spi_block_erase_20, + }, { + .eraseblocks = { {64 * 1024, 512} }, + .block_erase = spi_block_erase_dc, + }, { + .eraseblocks = { {64 * 1024, 512} }, + .block_erase = spi_block_erase_d8, + }, { + .eraseblocks = { {32768 * 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/Numonyx/ST", + .name = "N25Q512..3E", /* ..3E = 3V, uniform 64KB/4KB blocks/sectors */ + .bustype = BUS_SPI, + .manufacture_id = ST_ID, + .model_id = ST_N25Q512__3E, + .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 = "N25Q256..3E/MT25QL256", /* ..3E/L = 3V, uniform 64KB/4KB blocks/sectors */ + .name = "MT25QL256", /* L = 3V, uniform 64KB/4KB blocks/sectors */ .bustype = BUS_SPI, .manufacture_id = ST_ID, .model_id = ST_N25Q256__3E, @@ -10585,7 +10665,7 @@
{ .vendor = "Micron", - .name = "N25Q512..3E/MT25QL512", /* ..3E/L = 3V, uniform 64KB/4KB blocks/sectors */ + .name = "MT25QL512", /* L = 3V, uniform 64KB/4KB blocks/sectors */ .bustype = BUS_SPI, .manufacture_id = ST_ID, .model_id = ST_N25Q512__3E,
Jacob Creedon has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/34488 )
Change subject: flashchips: Split MT25Q from N25Q ......................................................................
Patch Set 1:
This is the beginning of my reworked patch. I am used to using gerrit configured as fast forward and didn't realize this instance was set up for cherry pick. Let me know if you want me to resubmit them all just squashed.
Jacob Creedon has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/34488 )
Change subject: flashchips: Split MT25Q from N25Q ......................................................................
Patch Set 1:
Bumping this review. It would be best to get this through while all my N25Q and MT25Q research is still fresh on my mind.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/34488 )
Change subject: flashchips: Split MT25Q from N25Q ......................................................................
Patch Set 1: Code-Review+2
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/34488 )
Change subject: flashchips: Split MT25Q from N25Q ......................................................................
Patch Set 1: -Code-Review
Sorry, run into the sorting trap. We try to enforce a sort order on .vendor, .name in `flashchips.c`, currently. I sup- pose "Micron" comes before "Micron/Numonyx/ST".
Alan, how about adding a simple check for this to the Make- file, that could be checked by Jenkins.
Alan Green has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/34488 )
Change subject: flashchips: Split MT25Q from N25Q ......................................................................
Patch Set 1:
Patch Set 1: -Code-Review
Sorry, run into the sorting trap. We try to enforce a sort order on .vendor, .name in `flashchips.c`, currently. I sup- pose "Micron" comes before "Micron/Numonyx/ST".
Alan, how about adding a simple check for this to the Make- file, that could be checked by Jenkins.
Ugh, I'm kind of surprised at how hard it is to get ordering "right". I didn't mean to create a burden for other devs.
How about I update my tool to deal with chips in any order and then I can put flashchips.c into the order that we want it to be - which I guess is the order that it was in.
In the meantime, happy for this these chips to go whereever you think makes the most sense.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/34488 )
Change subject: flashchips: Split MT25Q from N25Q ......................................................................
Patch Set 1:
Sorry, run into the sorting trap. We try to enforce a sort order on .vendor, .name in `flashchips.c`, currently. I sup- pose "Micron" comes before "Micron/Numonyx/ST".
Alan, how about adding a simple check for this to the Make- file, that could be checked by Jenkins.
Ugh, I'm kind of surprised at how hard it is to get ordering "right". I didn't mean to create a burden for other devs.
No worries. I actually think that the new ordering is better because it is easier to check automatically. We'd just have to hook that up. Even with the old order, it caused friction. A little less maybe, because we didn't check as much for it ;)
Jacob Creedon has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/34488 )
Change subject: flashchips: Split MT25Q from N25Q ......................................................................
Patch Set 1:
Patch Set 1:
Sorry, run into the sorting trap. We try to enforce a sort order on .vendor, .name in `flashchips.c`, currently. I sup- pose "Micron" comes before "Micron/Numonyx/ST".
Alan, how about adding a simple check for this to the Make- file, that could be checked by Jenkins.
Ugh, I'm kind of surprised at how hard it is to get ordering "right". I didn't mean to create a burden for other devs.
No worries. I actually think that the new ordering is better because it is easier to check automatically. We'd just have to hook that up. Even with the old order, it caused friction. A little less maybe, because we didn't check as much for it ;)
The ordering you see was already present. Can you confirm that this was broken before my patch?
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/34488 )
Change subject: flashchips: Split MT25Q from N25Q ......................................................................
Patch Set 1: Code-Review+2
Sorry, run into the sorting trap. We try to enforce a sort order on .vendor, .name in `flashchips.c`, currently. I sup- pose "Micron" comes before "Micron/Numonyx/ST".
Alan, how about adding a simple check for this to the Make- file, that could be checked by Jenkins.
Ugh, I'm kind of surprised at how hard it is to get ordering "right". I didn't mean to create a burden for other devs.
No worries. I actually think that the new ordering is better because it is easier to check automatically. We'd just have to hook that up. Even with the old order, it caused friction. A little less maybe, because we didn't check as much for it ;)
The ordering you see was already present. Can you confirm that this was broken before my patch?
Um, yap. I don't know any order that would place "Micron/Numonyx/ST" before "Micron", but it seems this is intended?
I guess we have to wait for Alan to publish his scripts before making any more assumptions.
Nico Huber has submitted this change and it was merged. ( https://review.coreboot.org/c/flashrom/+/34488 )
Change subject: flashchips: Split MT25Q from N25Q ......................................................................
flashchips: Split MT25Q from N25Q
The MT25Q is the successor to the N25Q from Micron/Numonyx/ST. The MT25Q is almost entirely backwards compatible with the N25Q series, however, the MT25Q has additional subsector erase commands available, and there are differences in stacked devices in the higher capacity variants. The N25Q devices are left with "Micron/Numonyx/ST" as the vendor and MT25Q devices are set with "Micron" as the vendor.
Signed-off-by: Jacob Creedon jcreedon@google.com Change-Id: I9d79978544b19cf9acd5f3ea6196cf6f3b3435ef Reviewed-on: https://review.coreboot.org/c/flashrom/+/34488 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Nico Huber nico.h@gmx.de --- M flashchips.c 1 file changed, 82 insertions(+), 2 deletions(-)
Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved
diff --git a/flashchips.c b/flashchips.c index 15c33c1..31a922e 100644 --- a/flashchips.c +++ b/flashchips.c @@ -10544,8 +10544,88 @@ },
{ + .vendor = "Micron/Numonyx/ST", + .name = "N25Q256..3E", /* ..3E = 3V, uniform 64KB/4KB blocks/sectors */ + .bustype = BUS_SPI, + .manufacture_id = ST_ID, + .model_id = ST_N25Q256__3E, + .total_size = 32768, + .page_size = 256, + /* supports SFDP */ + /* OTP: 64B total; read 0x4B, write 0x42 */ + .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_4BA_WREN, + .tested = TEST_UNTESTED, + .probe = probe_spi_rdid, + .probe_timing = TIMING_ZERO, + .block_erasers = + { + { + .eraseblocks = { {4 * 1024, 8192} }, + .block_erase = spi_block_erase_21, + }, { + .eraseblocks = { {4 * 1024, 8192} }, + .block_erase = spi_block_erase_20, + }, { + .eraseblocks = { {64 * 1024, 512} }, + .block_erase = spi_block_erase_dc, + }, { + .eraseblocks = { {64 * 1024, 512} }, + .block_erase = spi_block_erase_d8, + }, { + .eraseblocks = { {32768 * 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/Numonyx/ST", + .name = "N25Q512..3E", /* ..3E = 3V, uniform 64KB/4KB blocks/sectors */ + .bustype = BUS_SPI, + .manufacture_id = ST_ID, + .model_id = ST_N25Q512__3E, + .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 = "N25Q256..3E/MT25QL256", /* ..3E/L = 3V, uniform 64KB/4KB blocks/sectors */ + .name = "MT25QL256", /* L = 3V, uniform 64KB/4KB blocks/sectors */ .bustype = BUS_SPI, .manufacture_id = ST_ID, .model_id = ST_N25Q256__3E, @@ -10585,7 +10665,7 @@
{ .vendor = "Micron", - .name = "N25Q512..3E/MT25QL512", /* ..3E/L = 3V, uniform 64KB/4KB blocks/sectors */ + .name = "MT25QL512", /* L = 3V, uniform 64KB/4KB blocks/sectors */ .bustype = BUS_SPI, .manufacture_id = ST_ID, .model_id = ST_N25Q512__3E,
Alan Green has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/34488 )
Change subject: flashchips: Split MT25Q from N25Q ......................................................................
Patch Set 3:
The script that defined the current order is in the downstream repo at https://chromium.googlesource.com/chromiumos/third_party/flashrom/+/b72a47a5...
It's ugly, but only temporary, and will be deleted along with the downstream repo when the merge with upstream is complete.
-
Alan Green (Code Review) -
Jacob Creedon (Code Review)
-
Nico Huber (Code Review)