December 2023 - flashrom-gerrit

[S] Change in flashrom[main]: flashrom_tester: Drop technical debt
by Edward O'Callaghan (Code Review) Dec. 21, 2023

Dec. 21, 2023

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/79303?usp=email ) Change subject: flashrom_tester: Drop technical debt ...................................................................... flashrom_tester: Drop technical debt Drop `-p host` ChromeOS miss-feature and use `-p internal` instead. BUG=b:296978620 BRANCH=none TEST=run flashrom-tester with newer flashrom Change-Id: I1ea174c1760fee3e41a2ef0e9779badf0c51298d Signed-off-by: roccochen(a)chromium.com <roccochen(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/79303 Reviewed-by: Evan Benn <evanbenn(a)gmail.com> Reviewed-by: David Wu <david_wu(a)quanta.corp-partner.google.com> Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> Reviewed-by: Nick Vaccaro <nvaccaro(a)google.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Bob Moragues <moragues(a)google.com> --- M util/flashrom_tester/flashrom/src/lib.rs M util/flashrom_tester/src/main.rs M util/flashrom_tester/src/tests.rs 3 files changed, 7 insertions(+), 7 deletions(-) Approvals: Evan Benn: Looks good to me, but someone else must approve Nick Vaccaro: Looks good to me, but someone else must approve David Wu: Looks good to me, but someone else must approve Edward O'Callaghan: Looks good to me, approved build bot (Jenkins): Verified Bob Moragues: Looks good to me, but someone else must approve diff --git a/util/flashrom_tester/flashrom/src/lib.rs b/util/flashrom_tester/flashrom/src/lib.rs index 41393e8..e2c9149 100644 --- a/util/flashrom_tester/flashrom/src/lib.rs +++ b/util/flashrom_tester/flashrom/src/lib.rs @@ -51,19 +51,19 @@ #[derive(Copy, Clone, PartialEq, Eq, Debug)] pub enum FlashChip { - HOST, + INTERNAL, } impl FlashChip { pub fn from(s: &str) -> Result<FlashChip, &str> { match s { - "host" => Ok(FlashChip::HOST), + "internal" => Ok(FlashChip::INTERNAL), _ => Err("cannot convert str to enum"), } } pub fn to(fc: FlashChip) -> &'static str { match fc { - FlashChip::HOST => "host", + FlashChip::INTERNAL => "internal", } } @@ -80,7 +80,7 @@ /// disabled. pub fn can_control_hw_wp(&self) -> bool { match self { - FlashChip::HOST => true, + FlashChip::INTERNAL => true, } } } diff --git a/util/flashrom_tester/src/main.rs b/util/flashrom_tester/src/main.rs index b8a2581..3035ae5 100644 --- a/util/flashrom_tester/src/main.rs +++ b/util/flashrom_tester/src/main.rs @@ -82,7 +82,7 @@ .arg( Arg::with_name("ccd_target_type") .required(true) - .possible_values(&["host"]), + .possible_values(&["internal"]), ) .arg( Arg::with_name("print-layout") diff --git a/util/flashrom_tester/src/tests.rs b/util/flashrom_tester/src/tests.rs index 721a789..d1e9d61 100644 --- a/util/flashrom_tester/src/tests.rs +++ b/util/flashrom_tester/src/tests.rs @@ -219,8 +219,8 @@ /// is actually able to write to the Flash. This only makes sense for chips /// running Coreboot, which we assume is just host. fn elog_sanity_test(env: &mut TestEnv) -> TestResult { - if env.chip_type() != FlashChip::HOST { - info!("Skipping ELOG sanity check for non-host chip"); + if env.chip_type() != FlashChip::INTERNAL { + info!("Skipping ELOG sanity check for non-internal chip"); return Ok(()); } // flash should be back in the golden state -- To view, visit https://review.coreboot.org/c/flashrom/+/79303?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: I1ea174c1760fee3e41a2ef0e9779badf0c51298d Gerrit-Change-Number: 79303 Gerrit-PatchSet: 2 Gerrit-Owner: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Reviewer: Bob Moragues <moragues(a)google.com> Gerrit-Reviewer: David Wu <david_wu(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Evan Benn <evanbenn(a)gmail.com> Gerrit-Reviewer: Hsuan Ting Chen <roccochen(a)chromium.org> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Tyler Wang <tyler.wang(a)quanta.corp-partner.google.com> Gerrit-MessageType: merged

1 0

[M] Change in flashrom[main]: Add Idaville platform into chipset enable list and add update IRC reg...
by Kevin Yu (Code Review) Dec. 21, 2023

Dec. 21, 2023

Attention is currently required from: Anastasia Klimchuk. Kevin Yu has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/79340?usp=email ) Change subject: Add Idaville platform into chipset enable list and add update IRC region support ...................................................................... Patch Set 6: (1 comment) Patchset: PS6: Hi Anastasia Klimchuk Any concern with you? Thanks -- To view, visit https://review.coreboot.org/c/flashrom/+/79340?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: Icc526b22a9efc8071e972bb1d8cfc51d6a83651b Gerrit-Change-Number: 79340 Gerrit-PatchSet: 6 Gerrit-Owner: Kevin Yu <ykevin(a)celestica.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Thu, 21 Dec 2023 00:54:27 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in flashrom[main]: flashchips: Remove Macronix MX25U25635F from chip list
by DZ (Code Review) Dec. 20, 2023

Dec. 20, 2023

DZ has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/79633?usp=email ) Change subject: flashchips: Remove Macronix MX25U25635F from chip list ...................................................................... flashchips: Remove Macronix MX25U25635F from chip list The MX25U25643G has same ID as MX25U25635F, and the former one is the next generation of the latter. Therefore, we replace it with MX25U25643G and backward compatible. We have tested read/write/erase and write protection function for MX25U25643G. MX25U25643G datasheet is available at the following URL: https://www.mxic.com.tw/en-us/products/NOR-Flash/Serial-NOR-Flash/Pages/spe… MX25U25635F datasheet is available at the following URL: https://www.macronix.com/en-us/products/NOR-Flash/Serial-NOR-Flash/Pages/sp… Change-Id: Ief3fd7641bed817066692c4abffff6d3b0df16b9 Signed-off-by: danielzhang <danielzhang(a)mxic.com.cn> --- M flashchips.c M include/flashchips.h 2 files changed, 2 insertions(+), 51 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/33/79633/1 diff --git a/flashchips.c b/flashchips.c index a4605bd..8ae2680 100644 --- a/flashchips.c +++ b/flashchips.c @@ -10257,59 +10257,10 @@ { .vendor = "Macronix", - .name = "MX25U25635F", - .bustype = BUS_SPI, - .manufacture_id = MACRONIX_ID, - .model_id = MACRONIX_MX25U25635F, - .total_size = 32768, - .page_size = 256, - /* OTP: 512B total; enter 0xB1, exit 0xC1 */ - .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_QPI | FEATURE_4BA, - .tested = TEST_OK_PREW, - .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 = { {32 * 1024, 1024} }, - .block_erase = SPI_BLOCK_ERASE_5C, - }, { - .eraseblocks = { {32 * 1024, 1024} }, - .block_erase = SPI_BLOCK_ERASE_52, - }, { - .eraseblocks = { {64 * 1024, 512} }, - .block_erase = SPI_BLOCK_ERASE_DC, - }, { - .eraseblocks = { {64 * 1024, 512} }, - .block_erase = SPI_BLOCK_ERASE_D8, - }, { - .eraseblocks = { {32 * 1024 * 1024, 1} }, - .block_erase = SPI_BLOCK_ERASE_60, - }, { - .eraseblocks = { {32 * 1024 * 1024, 1} }, - .block_erase = SPI_BLOCK_ERASE_C7, - } - }, - /* TODO: security register */ - .printlock = SPI_PRETTYPRINT_STATUS_REGISTER_BP3_SRWD, /* bit6 is quad enable */ - .unlock = SPI_DISABLE_BLOCKPROTECT_BP3_SRWD, - .write = SPI_CHIP_WRITE256, /* Multi I/O supported */ - .read = SPI_CHIP_READ, /* Fast read (0x0B) and multi I/O supported */ - .voltage = {1650, 2000}, - }, - - { - .vendor = "Macronix", .name = "MX25U25643G", .bustype = BUS_SPI, .manufacture_id = MACRONIX_ID, - .model_id = MACRONIX_MX25U25635F, + .model_id = MACRONIX_MX25U25643G, .total_size = 32768, .page_size = 256, /* OTP: 512B total; enter 0xB1, exit 0xC1 */ diff --git a/include/flashchips.h b/include/flashchips.h index 17a6fc4..654196f 100644 --- a/include/flashchips.h +++ b/include/flashchips.h @@ -529,7 +529,7 @@ #define MACRONIX_MX25U3235E 0x2536 /* Same as MX25U6435F */ #define MACRONIX_MX25U6435E 0x2537 /* Same as MX25U6435F */ #define MACRONIX_MX25U12835E 0x2538 /* Same as MX25U12835F */ -#define MACRONIX_MX25U25635F 0x2539 /* Same as MX25U25643G */ +#define MACRONIX_MX25U25643G 0x2539 #define MACRONIX_MX25U51245G 0x253a #define MACRONIX_MX25L3235D 0x5E16 /* MX25L3225D/MX25L3235D/MX25L3237D */ #define MACRONIX_MX25L6495F 0x9517 -- To view, visit https://review.coreboot.org/c/flashrom/+/79633?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: Ief3fd7641bed817066692c4abffff6d3b0df16b9 Gerrit-Change-Number: 79633 Gerrit-PatchSet: 1 Gerrit-Owner: DZ Gerrit-MessageType: newchange

1 0

[S] Change in flashrom[main]: flashchips: Add GD25LQ255E
by Tyler Wang (Code Review) Dec. 19, 2023

Dec. 19, 2023

Attention is currently required from: Anastasia Klimchuk, David Wu, Kapil Porwal, Maximilian Brune, Nikolai Artemiev, Subrata Banik. Tyler Wang has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/79088?usp=email ) Change subject: flashchips: Add GD25LQ255E ...................................................................... Patch Set 9: (1 comment) File flashchips.c: https://review.coreboot.org/c/flashrom/+/79088/comment/b3fed484_bbc55af6 : PS9, Line 6559: FEATURE_WRSR_EXT2 > Also for my education: how do I find out from datasheet that FEATURE_WRSR_EXT2 is supported? Thank y […] Umm.. I was refer to funciton spi_prepare_wrsr_ext(): ``` /* * Writing SR2 or higher with an extended WRSR command requires * writing all lower SRx along with it, so just read the lower * SRx and write them back. */ ``` If it has SR2(or higher), FEATURE_WRSR_EXT2 is needed. But after check the code again, I think my opinion is incorrect. Do you know how to check FEATURE_WRSR_EXT2 is supported? -- To view, visit https://review.coreboot.org/c/flashrom/+/79088?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: I0d780255ed6772f4aa406584acf071a7ddd6da47 Gerrit-Change-Number: 79088 Gerrit-PatchSet: 9 Gerrit-Owner: Tyler Wang <tyler.wang(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: David Wu <david_wu(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com> 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: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: David Wu <david_wu(a)quanta.corp-partner.google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Tue, 19 Dec 2023 05:14:45 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[main]: flashchips: Add GD25LQ255E
by Tyler Wang (Code Review) Dec. 19, 2023

Dec. 19, 2023

Attention is currently required from: Anastasia Klimchuk, David Wu, Kapil Porwal, Maximilian Brune, Nikolai Artemiev, Subrata Banik. Tyler Wang has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/79088?usp=email ) Change subject: flashchips: Add GD25LQ255E ...................................................................... Patch Set 9: (1 comment) File flashchips.c: https://review.coreboot.org/c/flashrom/+/79088/comment/0b1d2293_689b3985 : PS9, Line 6595: 1695, 1950 > Just for my education: the datasheet says `Full voltage range: 1.65-2. […] I was reference GD25LQ128E settings. The datasheet says `Full voltage range: 1.65-2.0V`, which is same as GD25LQ255E. So I use the same voltage settings. Shall we set the voltage setting to `{1650, 2000}`? GD25LQ128E datasheet: https://www.gigadevice.com.cn/Public/Uploads/uploadfile/files/20230926/DS-0… -- To view, visit https://review.coreboot.org/c/flashrom/+/79088?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: I0d780255ed6772f4aa406584acf071a7ddd6da47 Gerrit-Change-Number: 79088 Gerrit-PatchSet: 9 Gerrit-Owner: Tyler Wang <tyler.wang(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: David Wu <david_wu(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com> 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: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: David Wu <david_wu(a)quanta.corp-partner.google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Tue, 19 Dec 2023 02:54:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[main]: Add Idaville platform into chipset enable list and add update IRC reg...
by Kevin Yu (Code Review) Dec. 19, 2023

Dec. 19, 2023

Attention is currently required from: Anastasia Klimchuk. Kevin Yu has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/79340?usp=email ) Change subject: Add Idaville platform into chipset enable list and add update IRC region support ...................................................................... Patch Set 6: (1 comment) Patchset: PS6: Hi Anastasia Klimchuk Can you help review this ticket? Thanks -- To view, visit https://review.coreboot.org/c/flashrom/+/79340?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: Icc526b22a9efc8071e972bb1d8cfc51d6a83651b Gerrit-Change-Number: 79340 Gerrit-PatchSet: 6 Gerrit-Owner: Kevin Yu <ykevin(a)celestica.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Tue, 19 Dec 2023 01:52:41 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in flashrom[main]: erasure_layout: Remove redundant verifications from `erase_write`
by Anastasia Klimchuk (Code Review) Dec. 18, 2023

Dec. 18, 2023

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/79354?usp=email ) Change subject: erasure_layout: Remove redundant verifications from `erase_write` ...................................................................... erasure_layout: Remove redundant verifications from `erase_write` Previously, in the worst-case scenario of erasing region content then writing new data, three rounds of verification were performed inside of the `erase_write` function through calls to: 1. `check_erased_range` when erasing with respect to region boundaries 2. `check_erased_range` for the entire erase block after the loop containing verification 1 completed 3. `verify_range` when all erases/writes were complete Verification 2 duplicated verification 1 and was orphaned by commit fa8808595a, which dropped its paired erasefn call but missed this related step. Verification 3 duplicated verification which occurs in `flashrom_image_write` based on `flashctx flags`. Now, these 2 redundant verifications are removed to improve the performance of `erase_write`. This change was tested using the linux_spi programmer to erase and write to an MT25QL512 chip. Change-Id: I638835facd9311979c4991cc4ca41a4b9e174bd5 Signed-off-by: Carly Zlabek <carlyzlabek(a)gmail.com> Signed-off-by: Vincent Fazio <vfazio(a)gmail.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/79354 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M erasure_layout.c 1 file changed, 0 insertions(+), 14 deletions(-) Approvals: Vincent Fazio: Looks good to me, but someone else must approve build bot (Jenkins): Verified Anastasia Klimchuk: Looks good to me, approved diff --git a/erasure_layout.c b/erasure_layout.c index 0c31911..c60305f 100644 --- a/erasure_layout.c +++ b/erasure_layout.c @@ -350,13 +350,6 @@ // after erase make it unselected again erase_layout[i].layout_list[j].selected = false; msg_cdbg("E(%"PRIx32":%"PRIx32")", start_addr, start_addr + block_len - 1); - // verify erase - ret = check_erased_range(flashctx, start_addr, block_len); - if (ret) { - msg_cerr("Verifying flash. Erase failed for range %#"PRIx32" : %#"PRIx32", Abort.\n", - start_addr, start_addr + block_len - 1); - goto _end; - } *all_skipped = false; } @@ -385,13 +378,6 @@ *all_skipped = false; } - // verify write - ret = verify_range(flashctx, newcontents + region_start, region_start, region_end - region_start); - if (ret) { - msg_cerr("Verifying flash. Write failed for range %#"PRIx32" : %#"PRIx32", Abort.\n", - region_start, region_end); - goto _end; - } _end: memcpy(newcontents + region_start, old_start_buf, old_start - region_start); -- To view, visit https://review.coreboot.org/c/flashrom/+/79354?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: I638835facd9311979c4991cc4ca41a4b9e174bd5 Gerrit-Change-Number: 79354 Gerrit-PatchSet: 3 Gerrit-Owner: Carly Zlabek <carlyzlabek(a)gmail.com> Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Vincent Fazio <vfazio(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[S] Change in flashrom[main]: flashchips: Add write-protect support for GD25LQ255E
by Subrata Banik (Code Review) Dec. 17, 2023

Dec. 17, 2023

Attention is currently required from: Anastasia Klimchuk, David Wu, Hsuan-ting Chen, Kapil Porwal, Lean Sheng Tan, Nikolai Artemiev, Tyler Wang. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/79460?usp=email ) Change subject: flashchips: Add write-protect support for GD25LQ255E ...................................................................... Patch Set 4: (1 comment) File flashchips.c: https://review.coreboot.org/c/flashrom/+/79460/comment/e963d0cc_2b73208f : PS1, Line 6560: TEST_OK_PREW > That's a great point! Typically `.reg_bits` are added when someone started using write-protect and tested that it works. > > Tyler, maybe you can tell us about it. If you are using write-protect for this chip then you can add the list of commands you were running [successfully] to the commit message. It is important to share the relevant testing info for each commit. Thanks! @tyler. please follow the commit msg which we did sometime back at https://review.coreboot.org/c/flashrom/+/70549/9//COMMIT_MSG -- To view, visit https://review.coreboot.org/c/flashrom/+/79460?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: I1425e931433c00caceaabc6037a79099d6d5eac5 Gerrit-Change-Number: 79460 Gerrit-PatchSet: 4 Gerrit-Owner: Tyler Wang <tyler.wang(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: David Wu <david_wu(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> 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: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: David Wu <david_wu(a)quanta.corp-partner.google.com> Gerrit-Attention: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Tyler Wang <tyler.wang(a)quanta.corp-partner.google.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Sun, 17 Dec 2023 05:13:39 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Hsuan-ting Chen <roccochen(a)google.com> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[main]: flashchips: Add write-protect support for GD25LQ255E
by Anastasia Klimchuk (Code Review) Dec. 16, 2023

Dec. 16, 2023

Attention is currently required from: David Wu, Hsuan-ting Chen, Kapil Porwal, Lean Sheng Tan, Nikolai Artemiev, Subrata Banik, Tyler Wang. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/79460?usp=email ) Change subject: flashchips: Add write-protect support for GD25LQ255E ...................................................................... Patch Set 4: (1 comment) File flashchips.c: https://review.coreboot.org/c/flashrom/+/79460/comment/593504d5_00c5374c : PS1, Line 6560: TEST_OK_PREW > If you are able to test it (e.g. run `flashrom_tester`), please update this. That's a great point! Typically `.reg_bits` are added when someone started using write-protect and tested that it works. Tyler, maybe you can tell us about it. If you are using write-protect for this chip then you can add the list of commands you were running [successfully] to the commit message. It is important to share the relevant testing info for each commit. Thanks! -- To view, visit https://review.coreboot.org/c/flashrom/+/79460?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: I1425e931433c00caceaabc6037a79099d6d5eac5 Gerrit-Change-Number: 79460 Gerrit-PatchSet: 4 Gerrit-Owner: Tyler Wang <tyler.wang(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: David Wu <david_wu(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> 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: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: David Wu <david_wu(a)quanta.corp-partner.google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Tyler Wang <tyler.wang(a)quanta.corp-partner.google.com> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Sat, 16 Dec 2023 11:21:44 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[main]: flashchips: Add GD25LQ255E
by Anastasia Klimchuk (Code Review) Dec. 16, 2023

Dec. 16, 2023

Attention is currently required from: David Wu, Kapil Porwal, Maximilian Brune, Nikolai Artemiev, Subrata Banik, Tyler Wang. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/79088?usp=email ) The change is no longer submittable: All-Comments-Resolved is unsatisfied now. Change subject: flashchips: Add GD25LQ255E ...................................................................... Patch Set 9: (3 comments) Patchset: PS9: Hello Tyler, thank you for the patch! I just have two questions to clarify. File flashchips.c: https://review.coreboot.org/c/flashrom/+/79088/comment/a8af90df_8fdf85cb : PS9, Line 6559: FEATURE_WRSR_EXT2 Also for my education: how do I find out from datasheet that FEATURE_WRSR_EXT2 is supported? Thank you! https://review.coreboot.org/c/flashrom/+/79088/comment/ea7972ba_4a3a3663 : PS9, Line 6595: 1695, 1950 Just for my education: the datasheet says `Full voltage range: 1.65-2.0V` but you are using slightly different range. Is there a reason for this? -- To view, visit https://review.coreboot.org/c/flashrom/+/79088?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: I0d780255ed6772f4aa406584acf071a7ddd6da47 Gerrit-Change-Number: 79088 Gerrit-PatchSet: 9 Gerrit-Owner: Tyler Wang <tyler.wang(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: David Wu <david_wu(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com> 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: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: David Wu <david_wu(a)quanta.corp-partner.google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Attention: Tyler Wang <tyler.wang(a)quanta.corp-partner.google.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Sat, 16 Dec 2023 10:06:47 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0