April 2024 - flashrom-gerrit - mail.coreboot.org

[XS] Change in flashrom[main]: flashrom: Don't throw around "delay 1 second" so lightly
by Peter Marheine (Code Review) April 22, 2024

April 22, 2024

Attention is currently required from: Anastasia Klimchuk, Brian Norris. Peter Marheine has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/80807?usp=email ) Change subject: flashrom: Don't throw around "delay 1 second" so lightly ...................................................................... Patch Set 3: (1 comment) Patchset: PS1: > I'd avoid adding a flag - if there really are chips that need a delay, then there is a hardware or s […] Agree that since a manual workflow can do the same thing, we shouldn't add an option. I think that recommendation should be captured in some kind of changelog so there's "real" documentation of the change, though. -- To view, visit https://review.coreboot.org/c/flashrom/+/80807?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Ie09651fede3f9f03425244c94a2da8bae00315fc Gerrit-Change-Number: 80807 Gerrit-PatchSet: 3 Gerrit-Owner: Brian Norris <briannorris(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-CC: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Attention: Brian Norris <briannorris(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Tue, 23 Apr 2024 03:24:08 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Brian Norris <briannorris(a)chromium.org> Comment-In-Reply-To: Nikolai Artemiev <nartemiev(a)google.com> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Comment-In-Reply-To: Peter Marheine <pmarheine(a)chromium.org> Gerrit-MessageType: comment

1 0

[XS] Change in flashrom[main]: flashrom: Don't throw around "delay 1 second" so lightly
by Nikolai Artemiev (Code Review) April 22, 2024

April 22, 2024

Attention is currently required from: Anastasia Klimchuk, Brian Norris, Peter Marheine. Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/80807?usp=email ) Change subject: flashrom: Don't throw around "delay 1 second" so lightly ...................................................................... Patch Set 3: (1 comment) Patchset: PS1: > Unfortunately, this hack survived so long it migrated from the CLI to the library. […] I'd avoid adding a flag - if there really are chips that need a delay, then there is a hardware or software bug that should be addressed with a chip flag or driver fix instead. Adding a flag makes a temporary fix more permanent. If users need to flash a buggy chip and can't wait for a new flashrom version that fixes it, they can always separate the write & verify operations as Brian suggests. A mailing list post seems like a good option for trying to recover the history around why the delay was added. If we don't get any response there within say 1 week, then I think we should just land this patch. -- To view, visit https://review.coreboot.org/c/flashrom/+/80807?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Ie09651fede3f9f03425244c94a2da8bae00315fc Gerrit-Change-Number: 80807 Gerrit-PatchSet: 3 Gerrit-Owner: Brian Norris <briannorris(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-CC: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Attention: Brian Norris <briannorris(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Tue, 23 Apr 2024 02:56:48 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Brian Norris <briannorris(a)chromium.org> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Comment-In-Reply-To: Nikolai Artemiev <nartemiev(a)google.com> Comment-In-Reply-To: Peter Marheine <pmarheine(a)chromium.org> Gerrit-MessageType: comment

1 0

[L] Change in flashrom[main]: <Flashrom SPI NOR>: adding a few new SPI NOR part numbers
by Nikolai Artemiev (Code Review) April 22, 2024

April 22, 2024

Attention is currently required from: Anastasia Klimchuk, Stefan Reinauer, Victor Lim. Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/81970?usp=email ) Change subject: <Flashrom SPI NOR>: adding a few new SPI NOR part numbers ...................................................................... Patch Set 1: (3 comments) Patchset: PS1: > Victor, this is amazing, thank you so much for adding and updating so many chips! […] +1 Thank you Victor for adding all these chips! File flashchips.c: https://review.coreboot.org/c/flashrom/+/81970/comment/90c2f7a2_428f544c : PS1, Line 6998: Just a small thing but please ensure that the new entries are indented using tabs only. https://review.coreboot.org/c/flashrom/+/81970/comment/516b3ced_d414beb0 : PS1, Line 7278: acts like SEC Should this be `acts like TB`? -- To view, visit https://review.coreboot.org/c/flashrom/+/81970?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I1526eed830845d5391ca14c7fd3ac58a5aee71f2 Gerrit-Change-Number: 81970 Gerrit-PatchSet: 1 Gerrit-Owner: Victor Lim <victorswlim(a)yahoo.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Victor Lim <victorswlim(a)yahoo.com> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Tue, 23 Apr 2024 01:30:45 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

[XS] Change in flashrom[main]: flashrom: Don't throw around "delay 1 second" so lightly
by Brian Norris (Code Review) April 22, 2024

April 22, 2024

Attention is currently required from: Anastasia Klimchuk, Nikolai Artemiev, Peter Marheine. Brian Norris has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/80807?usp=email ) Change subject: flashrom: Don't throw around "delay 1 second" so lightly ...................................................................... Patch Set 3: (1 comment) Patchset: PS1: > > If nobody knows why this is necessary or how to reproduce, I think it's okay to remove. […] Unfortunately, this hack survived so long it migrated from the CLI to the library. So a CLI flag won't really provide an escape hatch for all users -- unless you also want to also add a libflashrom flag for it?? I'm still somewhat of a lazy opinion here: 1. I doubt that anything will come of either this CL, nor of a CLI flag that provides a bug-report-escape-hatch for it and 2. the whole `--verify` sequence is trivial enough to emulate on one's own [1]. That gets slightly more complex to implement if people are using environments without basic stuff like a decent shell or tools like `cmp`, or if they're using surgical flags (like `--include`). But it's not that hard. And the good news is we don't have to try #2 if I'm right about #1 😊 But if you still think we need a flag, I can give it a shot. And either way, do you want me to start such a mailing list thread, or were you suggesting I do this? [1] e.g., `flashrom --read /tmp/foo.bin`; `flashrom --write /my/new/image.bin --noverify`; `sleep 1`; `flashrom --read /tmp/bar.bin`; `cmp /tmp/foo.bin /tmp/bar.bin` -- To view, visit https://review.coreboot.org/c/flashrom/+/80807?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Ie09651fede3f9f03425244c94a2da8bae00315fc Gerrit-Change-Number: 80807 Gerrit-PatchSet: 3 Gerrit-Owner: Brian Norris <briannorris(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-CC: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Tue, 23 Apr 2024 00:23:30 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Brian Norris <briannorris(a)chromium.org> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Comment-In-Reply-To: Nikolai Artemiev <nartemiev(a)google.com> Comment-In-Reply-To: Peter Marheine <pmarheine(a)chromium.org> Gerrit-MessageType: comment

1 0

[XS] Change in flashrom[main]: flashrom: Don't throw around "delay 1 second" so lightly
by Anastasia Klimchuk (Code Review) April 22, 2024

April 22, 2024

Attention is currently required from: Brian Norris, Nikolai Artemiev, Peter Marheine. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/80807?usp=email ) Change subject: flashrom: Don't throw around "delay 1 second" so lightly ...................................................................... Patch Set 3: (1 comment) Patchset: PS1: > If nobody knows why this is necessary or how to reproduce, I think it's okay to remove. Yes, true. I was trying to find out which exactly chips needed that extra delay, but no success so far. If we knew which chips we could add a feature flag as Nikolai suggested, and mark them in flashchips. But it seems, we don't know. So your plan looks very reasonable: an option will cover for the edge cases (if they exist). I was thinking to start from a post on a mailing list, very specifically about removing 1s delay (there has been none dedicated thread), so that if someone suddenly recalls information about those odd chips, they can share. Even if no one responds, it stays in archives as a record of decision being made with explanation why. And then update this patch with an option, post link to patch to the thread. That should be enough for the "paperwork" :) -- To view, visit https://review.coreboot.org/c/flashrom/+/80807?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Ie09651fede3f9f03425244c94a2da8bae00315fc Gerrit-Change-Number: 80807 Gerrit-PatchSet: 3 Gerrit-Owner: Brian Norris <briannorris(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-CC: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Attention: Brian Norris <briannorris(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Mon, 22 Apr 2024 11:03:24 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Brian Norris <briannorris(a)chromium.org> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Comment-In-Reply-To: Nikolai Artemiev <nartemiev(a)google.com> Comment-In-Reply-To: Peter Marheine <pmarheine(a)chromium.org> Gerrit-MessageType: comment

1 0

[L] Change in flashrom[main]: <Flashrom SPI NOR>: adding a few new SPI NOR part numbers
by Anastasia Klimchuk (Code Review) April 22, 2024

April 22, 2024

Attention is currently required from: Nikolai Artemiev, Stefan Reinauer, Victor Lim. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/81970?usp=email ) Change subject: <Flashrom SPI NOR>: adding a few new SPI NOR part numbers ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: Victor, this is amazing, thank you so much for adding and updating so many chips! What you need to do now, is to split this large patch into several smaller ones. I can see some of the models are related and can be grouped together. So for example, GD25LB128E, GD25LQ128E, GD25LR128E can be in one patch. And then GD25LB256F, GD25LQ256H, GD25LR256F can be in the next patch, and so on. When you split your git commit locally, you will have a chain of commits in your local branch. And then when you push the whole chain to Gerrit, you will see all the patches in a chain as well. For commit titles, you can use the pattern: `flashchips: Add (or update) chip models <list models>` Another thing is, you need to provide links to public datasheets for the models added or updated in each patch. Thanks! -- To view, visit https://review.coreboot.org/c/flashrom/+/81970?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I1526eed830845d5391ca14c7fd3ac58a5aee71f2 Gerrit-Change-Number: 81970 Gerrit-PatchSet: 1 Gerrit-Owner: Victor Lim <victorswlim(a)yahoo.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Victor Lim <victorswlim(a)yahoo.com> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Mon, 22 Apr 2024 10:42:58 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in flashrom[main]: flashchips: Split and add write-protect support for MX25L12833F
by Anastasia Klimchuk (Code Review) April 22, 2024

April 22, 2024

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/81792?usp=email ) Change subject: flashchips: Split and add write-protect support for MX25L12833F ...................................................................... flashchips: Split and add write-protect support for MX25L12833F MX25L12833F datasheet: https://www.macronix.com/Lists/Datasheet/Attachments/8934/MX25L12833F,%203V… Status register: page 30 table 7 (BP0~BP3, SRWD) Configuration register: page 31 table 8 (TB) Security register: page 57 table 12 (WPSEL) MX25L12835F datasheet: https://www.macronix.com/Lists/Datasheet/Attachments/8653/MX25L12835F,%203V… Status register: page 31(BP0~BP3, SRWD) Configuration register: page 32 table 7 (TB) Security register: page 61 table 9 (WPSEL) MX25L12845E datasheet: (no CONFIG) https://www.mxic.com.tw/Lists/Datasheet/Attachments/8693/MX25L12845E,%203V,… Status register: page 17 (BP0~BP3, SRWD) Security register: page 29 (WPSEL) MX25L12865E datasheet: (no CONFIG) https://media.digikey.com/pdf/Data%20Sheets/Macronix/MX25L6465E,_MX25L12865… Status register: page 19 (BP0~BP3, SRWD) Security register: page 31 (WPSEL) MX25L12873F datasheet: (no hardware WP) https://www.mxic.com.tw/Lists/Datasheet/Attachments/8652/MX25L12873F,%203V,… Status register: page 31(BP0~BP3, SRWD) Configuration register: page 32 table 7 (TB) Security register: page 60 table 9 (WPSEL) Splits the MX25L12833F/MX25L12835F/MX25L12845E/MX25L12865E/MX25L12873F group into three subgroups: * MX25L12833F: This chip have the configuration register and WP tested * MX25L12835F/MX25L12873F: These chips have the configuration register. * MX25L12845E/MX25L12865E: These chips don't have the configuration register. Tests the write protect functionality on the MX25L12833F chip only. BUG=b:332486637 TEST=Test flashrom --wp-disable with MX25L12833FZNI-10 on ChromeOS Change-Id: I379c833eea3ed3487504126f45c6df672a772ddc Signed-off-by: Hsuan Ting Chen <roccochen(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/81792 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M flashchips.c 1 file changed, 103 insertions(+), 3 deletions(-) Approvals: build bot (Jenkins): Verified Anastasia Klimchuk: Looks good to me, approved diff --git a/flashchips.c b/flashchips.c index 022b18a..373f28c 100644 --- a/flashchips.c +++ b/flashchips.c @@ -9005,13 +9005,60 @@ { .vendor = "Macronix", - .name = "MX25L12833F/MX25L12835F/MX25L12845E/MX25L12865E/MX25L12873F", + .name = "MX25L12833F", .bustype = BUS_SPI, .manufacture_id = MACRONIX_ID, .model_id = MACRONIX_MX25L12805D, .total_size = 16384, .page_size = 256, - /* OTP: MX25L12833F has 1KB total, others have 512B total; enter 0xB1, exit 0xC1 */ + /* OTP: 1KB total; enter 0xB1, exit 0xC1 */ + .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP, + .tested = TEST_OK_PREWB, + .probe = PROBE_SPI_RDID, + .probe_timing = TIMING_ZERO, + .block_erasers = + { + { + .eraseblocks = { {4 * 1024, 4096} }, + .block_erase = SPI_BLOCK_ERASE_20, + }, { + .eraseblocks = { {32 * 1024, 512} }, + .block_erase = SPI_BLOCK_ERASE_52, + }, { + .eraseblocks = { {64 * 1024, 256} }, + .block_erase = SPI_BLOCK_ERASE_D8, + }, { + .eraseblocks = { {16 * 1024 * 1024, 1} }, + .block_erase = SPI_BLOCK_ERASE_60, + }, { + .eraseblocks = { {16 * 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_WRITE256, + .read = SPI_CHIP_READ, /* Fast read (0x0B) supported */ + .voltage = {2700, 3600}, + .reg_bits = + { + .srp = {STATUS1, 7, RW}, + .bp = {{STATUS1, 2, RW}, {STATUS1, 3, RW}, {STATUS1, 4, RW}, {STATUS1, 5, RW}}, + .tb = {CONFIG, 3, OTP}, + .wps = {SECURITY, 7, OTP}, /* This bit is set by WPSEL command */ + }, + .decode_range = DECODE_RANGE_SPI25, + }, + + { + .vendor = "Macronix", + .name = "MX25L12835F/MX25L12873F", + .bustype = BUS_SPI, + .manufacture_id = MACRONIX_ID, + .model_id = MACRONIX_MX25L12805D, + .total_size = 16384, + .page_size = 256, + /* OTP: 512B total; enter 0xB1, exit 0xC1 */ .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP, .tested = TEST_OK_PREW, .probe = PROBE_SPI_RDID, @@ -9035,12 +9082,65 @@ .block_erase = SPI_BLOCK_ERASE_C7, } }, - /* TODO: security register and SBLK/SBULK; MX25L12835F: configuration register */ .printlock = SPI_PRETTYPRINT_STATUS_REGISTER_BP3_SRWD, /* bit6 is quad enable */ .unlock = SPI_DISABLE_BLOCKPROTECT_BP3_SRWD, .write = SPI_CHIP_WRITE256, .read = SPI_CHIP_READ, /* Fast read (0x0B) supported */ .voltage = {2700, 3600}, + .reg_bits = + { + .srp = {STATUS1, 7, RW}, + .bp = {{STATUS1, 2, RW}, {STATUS1, 3, RW}, {STATUS1, 4, RW}, {STATUS1, 5, RW}}, + .tb = {CONFIG, 3, OTP}, + .wps = {SECURITY, 7, OTP}, /* This bit is set by WPSEL command */ + }, + .decode_range = DECODE_RANGE_SPI25, + }, + + { + .vendor = "Macronix", + .name = "MX25L12845E/MX25L12865E", + .bustype = BUS_SPI, + .manufacture_id = MACRONIX_ID, + .model_id = MACRONIX_MX25L12805D, + .total_size = 16384, + .page_size = 256, + /* OTP: 512B 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, 4096} }, + .block_erase = SPI_BLOCK_ERASE_20, + }, { + .eraseblocks = { {32 * 1024, 512} }, + .block_erase = SPI_BLOCK_ERASE_52, + }, { + .eraseblocks = { {64 * 1024, 256} }, + .block_erase = SPI_BLOCK_ERASE_D8, + }, { + .eraseblocks = { {16 * 1024 * 1024, 1} }, + .block_erase = SPI_BLOCK_ERASE_60, + }, { + .eraseblocks = { {16 * 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_WRITE256, + .read = SPI_CHIP_READ, /* Fast read (0x0B) supported */ + .voltage = {2700, 3600}, + .reg_bits = + { + .srp = {STATUS1, 7, RW}, + .bp = {{STATUS1, 2, RW}, {STATUS1, 3, RW}, {STATUS1, 4, RW}, {STATUS1, 5, RW}}, + .wps = {SECURITY, 7, OTP}, /* This bit is set by WPSEL command */ + }, + .decode_range = DECODE_RANGE_SPI25, }, { -- To view, visit https://review.coreboot.org/c/flashrom/+/81792?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I379c833eea3ed3487504126f45c6df672a772ddc Gerrit-Change-Number: 81792 Gerrit-PatchSet: 7 Gerrit-Owner: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: DZ <danielzhang(a)mxic.com.cn> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Hsuan Ting Chen <roccochen(a)chromium.org> Gerrit-MessageType: merged

1 0

[S] Change in flashrom[main]: flashchips: Add write protect function support for MX25R1635F
by Anastasia Klimchuk (Code Review) April 21, 2024

April 21, 2024

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/81840?usp=email ) Change subject: flashchips: Add write protect function support for MX25R1635F ...................................................................... flashchips: Add write protect function support for MX25R1635F The MX25R1635F has been tested by ch341a programmer : read, write, erase and wp. We have tested --wp-enable, --wp-disable, --wp-list and --wp-range commands for write-protect feature. MX25R1635F datasheet is available at the following URL: https://www.macronix.com/Lists/Datasheet/Attachments/8702/MX25R1635F,%20Wid… Change-Id: I6e2b417ab177039618069d8e35132ddbfb814f03 Signed-off-by: DanielZhang <danielzhang(a)mxic.com.cn> Reviewed-on: https://review.coreboot.org/c/flashrom/+/81840 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M flashchips.c 1 file changed, 10 insertions(+), 2 deletions(-) Approvals: build bot (Jenkins): Verified Anastasia Klimchuk: Looks good to me, approved diff --git a/flashchips.c b/flashchips.c index e1e774c..022b18a 100644 --- a/flashchips.c +++ b/flashchips.c @@ -10103,8 +10103,8 @@ .total_size = 2048, .page_size = 256, /* OTP: 1024B total; enter 0xB1, exit 0xC1 */ - .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP, - .tested = TEST_OK_PREW, + .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_CFGR, + .tested = TEST_OK_PREWB, .probe = PROBE_SPI_RDID, .probe_timing = TIMING_ZERO, .block_erasers = @@ -10131,6 +10131,14 @@ .write = SPI_CHIP_WRITE256, .read = SPI_CHIP_READ, /* Fast read (0x0B) and multi I/O supported */ .voltage = {1650, 3600}, + .reg_bits = + { + .srp = {STATUS1, 7, RW}, + .bp = {{STATUS1, 2, RW}, {STATUS1, 3, RW}, {STATUS1, 4, RW}, {STATUS1, 5, RW}}, + .tb = {CONFIG, 3, OTP} + }, + + .decode_range = DECODE_RANGE_SPI25, }, { -- To view, visit https://review.coreboot.org/c/flashrom/+/81840?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I6e2b417ab177039618069d8e35132ddbfb814f03 Gerrit-Change-Number: 81840 Gerrit-PatchSet: 2 Gerrit-Owner: DZ <danielzhang(a)mxic.com.cn> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[S] Change in flashrom[main]: flashchips: Add support for MXIC MX25R8035F
by Anastasia Klimchuk (Code Review) April 21, 2024

April 21, 2024

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/81837?usp=email ) Change subject: flashchips: Add support for MXIC MX25R8035F ...................................................................... flashchips: Add support for MXIC MX25R8035F The MX25R8035F has been tested by ch341a programmer : read, write, erase and wp. We have tested --wp-enable, --wp-disable, --wp-list and --wp-range commands for write-protect feature. MX25R8035F datasheet is available at the following URL: https://www.macronix.com/Lists/Datasheet/Attachments/8749/MX25R8035F,%20Wid… Change-Id: Iec244ffc29278c1f8c3ae47d17af2c4fe5fbe498 Signed-off-by: DanielZhang <danielzhang(a)mxic.com.cn> Reviewed-on: https://review.coreboot.org/c/flashrom/+/81837 Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M flashchips.c M include/flashchips.h 2 files changed, 48 insertions(+), 0 deletions(-) Approvals: Anastasia Klimchuk: Looks good to me, approved build bot (Jenkins): Verified diff --git a/flashchips.c b/flashchips.c index 7a40f07..e1e774c 100644 --- a/flashchips.c +++ b/flashchips.c @@ -10213,6 +10213,53 @@ { .vendor = "Macronix", + .name = "MX25R8035F", + .bustype = BUS_SPI, + .manufacture_id = MACRONIX_ID, + .model_id = MACRONIX_MX25R8035F, + .total_size = 1024, + .page_size = 256, + /* OTP: 1024B total; enter 0xB1, exit 0xC1 */ + .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_CFGR, + .tested = TEST_OK_PREWB, + .probe = PROBE_SPI_RDID, + .probe_timing = TIMING_ZERO, + .block_erasers = + { + { + .eraseblocks = { {4 * 1024, 256} }, + .block_erase = SPI_BLOCK_ERASE_20, + }, { + .eraseblocks = { {32 * 1024, 32} }, + .block_erase = SPI_BLOCK_ERASE_52, + }, { + .eraseblocks = { {64 * 1024, 16} }, + .block_erase = SPI_BLOCK_ERASE_D8, + }, { + .eraseblocks = { {1 * 1024 * 1024, 1} }, + .block_erase = SPI_BLOCK_ERASE_60, + }, { + .eraseblocks = { {1 * 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_WRITE256, + .read = SPI_CHIP_READ, /* Fast read (0x0B) and multi I/O supported */ + .voltage = {1650, 3600}, + .reg_bits = + { + .srp = {STATUS1, 7, RW}, + .bp = {{STATUS1, 2, RW}, {STATUS1, 3, RW}, {STATUS1, 4, RW}, {STATUS1, 5, RW}}, + .tb = {CONFIG, 3, OTP} + }, + + .decode_range = DECODE_RANGE_SPI25, + }, + + { + .vendor = "Macronix", .name = "MX25V4035F", .bustype = BUS_SPI, .manufacture_id = MACRONIX_ID, diff --git a/include/flashchips.h b/include/flashchips.h index 103e6ea..c9303d5 100644 --- a/include/flashchips.h +++ b/include/flashchips.h @@ -540,6 +540,7 @@ #define MACRONIX_MX25R1635F 0x2815 #define MACRONIX_MX25R3235F 0x2816 #define MACRONIX_MX25R6435F 0x2817 +#define MACRONIX_MX25R8035F 0x2814 #define MACRONIX_MX29F001B 0x19 #define MACRONIX_MX29F001T 0x18 -- To view, visit https://review.coreboot.org/c/flashrom/+/81837?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Iec244ffc29278c1f8c3ae47d17af2c4fe5fbe498 Gerrit-Change-Number: 81837 Gerrit-PatchSet: 2 Gerrit-Owner: DZ <danielzhang(a)mxic.com.cn> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[XS] Change in flashrom[main]: flashrom: Don't throw around "delay 1 second" so lightly
by Peter Marheine (Code Review) April 21, 2024

April 21, 2024

Attention is currently required from: Anastasia Klimchuk, Brian Norris, Nikolai Artemiev. Peter Marheine has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/80807?usp=email ) Change subject: flashrom: Don't throw around "delay 1 second" so lightly ...................................................................... Patch Set 3: (1 comment) Patchset: PS1: > My main concern is that there may be some chips in the flashchips database which need this delay, bu […] If nobody knows why this is necessary or how to reproduce, I think it's okay to remove. If we're feeling particularly conservative, we could add an option (perhaps `--extra-readback-delay`, but feel free to bikeshed the name) that enables the extra delay so there's an escape hatch available if it turns out somebody has such a chip- then we can suggest using that option as a fallback in `emergency_help_message()` and ask that people report cases where they need to use it (so hopefully over time the escape hatch would become unnecessary). -- To view, visit https://review.coreboot.org/c/flashrom/+/80807?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Ie09651fede3f9f03425244c94a2da8bae00315fc Gerrit-Change-Number: 80807 Gerrit-PatchSet: 3 Gerrit-Owner: Brian Norris <briannorris(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-CC: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Attention: Brian Norris <briannorris(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Mon, 22 Apr 2024 00:50:41 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Brian Norris <briannorris(a)chromium.org> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Comment-In-Reply-To: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-MessageType: comment

1 0