- flashrom-gerrit - mail.coreboot.org

[S] Change in flashrom[master]: flashrom_tester: Use tempdir crate for test data
by Evan Benn (Code Review) 24 Oct '22

24 Oct '22

Evan Benn has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/68739 ) Change subject: flashrom_tester: Use tempdir crate for test data ...................................................................... flashrom_tester: Use tempdir crate for test data Use tempdir crate to create a tempdir that is deleted when the tester closes. BUG=b:243460685 BRANCH=None TEST=/usr/bin/flashrom_tester --flashrom_binary /usr/sbin/flashrom host Change-Id: I14089ef69c3f509cb6f01cc44c7924c244ab7d73 Signed-off-by: Evan Benn <evanbenn(a)chromium.org> --- M util/flashrom_tester/Cargo.toml M util/flashrom_tester/src/tester.rs M util/flashrom_tester/src/tests.rs 3 files changed, 31 insertions(+), 8 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/39/68739/1 diff --git a/util/flashrom_tester/Cargo.toml b/util/flashrom_tester/Cargo.toml index b57f04e..364a223 100644 --- a/util/flashrom_tester/Cargo.toml +++ b/util/flashrom_tester/Cargo.toml @@ -23,6 +23,7 @@ rand = "0.6.4" serde_json = "1" sys-info = "0.9" +tempdir = "0.3.7" [build-dependencies] built = { version = "0.5", features = ["chrono"] } diff --git a/util/flashrom_tester/src/tester.rs b/util/flashrom_tester/src/tester.rs index 6ce3889..8324ade 100644 --- a/util/flashrom_tester/src/tester.rs +++ b/util/flashrom_tester/src/tester.rs @@ -46,6 +46,7 @@ use std::path::PathBuf; use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::Mutex; +use tempdir::TempDir; // type-signature comes from the return type of lib.rs workers. type TestError = Box<dyn std::error::Error>; @@ -67,6 +68,8 @@ random_data: PathBuf, /// The path to a file containing layout data. pub layout_file: PathBuf, + /// A directory for temporary test data, including the above files. + pub tmp_dir: TempDir, } impl<'a> TestEnv<'a> { @@ -76,14 +79,16 @@ print_layout: bool, ) -> Result<Self, FlashromError> { let rom_sz = cmd.get_size()?; + let tmp_dir = TempDir::new("flashrom_tester").map_err(|e| e.to_string())?; let out = TestEnv { chip_type, cmd, layout: utils::get_layout_sizes(rom_sz)?, wp: WriteProtectState::from_hardware(cmd, chip_type)?, - original_flash_contents: "/tmp/flashrom_tester_golden.bin".into(), - random_data: "/tmp/random_content.bin".into(), - layout_file: create_layout_file(rom_sz, Path::new("/tmp/"), print_layout), + original_flash_contents: tmp_dir.path().join("flashrom_tester_golden.bin"), + random_data: tmp_dir.path().join("random_content.bin"), + layout_file: create_layout_file(rom_sz, tmp_dir.path(), print_layout), + tmp_dir, }; info!("Stashing golden image for verification/recovery on completion"); diff --git a/util/flashrom_tester/src/tests.rs b/util/flashrom_tester/src/tests.rs index af34208..94fe14d 100644 --- a/util/flashrom_tester/src/tests.rs +++ b/util/flashrom_tester/src/tests.rs @@ -43,8 +43,6 @@ use std::io::BufRead; use std::sync::atomic::AtomicBool; -const ELOG_FILE: &str = "/tmp/elog.file"; - /// Iterate over tests, yielding only those tests with names matching filter_names. /// /// If filter_names is None, all tests will be run. None is distinct from Some(∅); @@ -223,15 +221,17 @@ env.ensure_golden()?; const ELOG_RW_REGION_NAME: &str = "RW_ELOG"; + let elog_file = env.tmp_dir.path().join("elog.file"); + env.cmd - .read_region_into_file(ELOG_FILE.as_ref(), ELOG_RW_REGION_NAME)?; + .read_region_into_file(&elog_file, ELOG_RW_REGION_NAME)?; // Just checking for the magic numer // TODO: improve this test to read the events - if fs::metadata(ELOG_FILE)?.len() < 4 { + if fs::metadata(&elog_file)?.len() < 4 { return Err("ELOG contained no data".into()); } - let data = fs::read(ELOG_FILE)?; + let data = fs::read(&elog_file)?; if u32::from_le_bytes(data[0..4].try_into()?) != 0x474f4c45 { return Err("ELOG had bad magic number".into()); } -- To view, visit https://review.coreboot.org/c/flashrom/+/68739 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I14089ef69c3f509cb6f01cc44c7924c244ab7d73 Gerrit-Change-Number: 68739 Gerrit-PatchSet: 1 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-MessageType: newchange

1 0

[M] Change in flashrom[master]: flashrom_tester: Use Path type for test data
by Evan Benn (Code Review) 24 Oct '22

24 Oct '22

Attention is currently required from: Evan Benn. Hello build bot (Jenkins), Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/68643 to look at the new patch set (#2). Change subject: flashrom_tester: Use Path type for test data ...................................................................... flashrom_tester: Use Path type for test data Update may str to Path. BUG=b:243460685 BRANCH=None TEST=/usr/bin/flashrom_tester --flashrom_binary /usr/sbin/flashrom host Change-Id: I4a0b25c8c70fa6dfd682fba3db7d3bf06b2eb7ac Signed-off-by: Evan Benn <evanbenn(a)chromium.org> --- M util/flashrom_tester/flashrom/src/cmd.rs M util/flashrom_tester/flashrom/src/flashromlib.rs M util/flashrom_tester/flashrom/src/lib.rs M util/flashrom_tester/src/rand_util.rs M util/flashrom_tester/src/tester.rs M util/flashrom_tester/src/tests.rs 6 files changed, 127 insertions(+), 88 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/43/68643/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/68643 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I4a0b25c8c70fa6dfd682fba3db7d3bf06b2eb7ac Gerrit-Change-Number: 68643 Gerrit-PatchSet: 2 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Evan Benn <evanbenn(a)google.com> Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[master]: tree/: Replace NULL-case of programmer_delay() with internal_delay
by Anastasia Klimchuk (Code Review) 24 Oct '22

24 Oct '22

Attention is currently required from: Felix Singer, Edward O'Callaghan, Angel Pons, Alexander Goncharov. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68434 ) Change subject: tree/: Replace NULL-case of programmer_delay() with internal_delay ...................................................................... Patch Set 1: Code-Review+1 (2 comments) Patchset: PS1: I like this. The cherry on the top of the cake would be to rename `internal_delay` so that it won't look like a property of internal programmer. But not in this patch. File flashrom.c: https://review.coreboot.org/c/flashrom/+/68434/comment/55ed01fa_b743911c PS1, Line 282: else else branch also needs braces as per coding style (because all previous if - else if conditions are mutiline and have braces). -- To view, visit https://review.coreboot.org/c/flashrom/+/68434 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I1da230804d5e8f47a6e281feb66f381514dc6861 Gerrit-Change-Number: 68434 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Comment-Date: Sun, 23 Oct 2022 23:57:37 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: flashrom.c: Confine is_internal checks under a symbol
by Anastasia Klimchuk (Code Review) 24 Oct '22

24 Oct '22

Attention is currently required from: Felix Singer, Thomas Heijligen, Edward O'Callaghan. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67475 ) Change subject: flashrom.c: Confine is_internal checks under a symbol ...................................................................... Patch Set 5: Code-Review+1 (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/67475/comment/2fe85fd4_4fc8679c PS5, Line 7: Confine is_internal checks under a symbol This is already done in some of the earlier patches, `is_internal_programmer()` already exists in flashrom.c. May I suggest a different commit message to reflect what is happening in the latest version of this patch: flashrom.c: Pass is_internal as an argument to *_help_message functions Providing is_internal as an argument allows *_help_message to be pure functions without side-effects, and to do one job: printing a message. -- To view, visit https://review.coreboot.org/c/flashrom/+/67475 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I569811798bfe310c59b8b61afba359bef68969fb Gerrit-Change-Number: 67475 Gerrit-PatchSet: 5 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Sun, 23 Oct 2022 23:40:22 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: flashrom.c: Drop emergency_help_message() after erasure
by Anastasia Klimchuk (Code Review) 24 Oct '22

24 Oct '22

Attention is currently required from: Felix Singer, Nico Huber, Thomas Heijligen, Edward O'Callaghan, Angel Pons. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67474 ) Change subject: flashrom.c: Drop emergency_help_message() after erasure ...................................................................... Patch Set 5: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/67474/comment/8933a303_bb8af04a PS2, Line 10: agnostic > The principle idea suggested here is not at all unreasonable and had crossed my mind already however […] It seems to me there are two questions here: 1) do we need to show emergency message when erase failed? (and the FIXME from many years ago thinks No we don't need) 2) where should help messages live, in cli_classic or in flashrom.c? If the answer to 1) is No, then this patch can go ahead, and 2) can be discussed and decided completely independently of this patch. If the answer to 1) is Yes then... I made an observation that `flashrom_image_write` has 3 callsites of `emergency_help_message()`. So moving the messages to cli_classic would need to take care of `flashrom_image_write`. I would start with answering 1) Maybe instead of emergency message we can msg_gerr with the text similar to the text of FIXME? -- To view, visit https://review.coreboot.org/c/flashrom/+/67474 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ib0d390c44e7a851e684014925a25c8b259b810cd Gerrit-Change-Number: 67474 Gerrit-PatchSet: 5 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Sun, 23 Oct 2022 23:18:48 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: Makefile: Don't install git hooks automatically
by Anastasia Klimchuk (Code Review) 24 Oct '22

24 Oct '22

Attention is currently required from: Felix Singer, Nico Huber, Thomas Heijligen. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68647 ) Change subject: Makefile: Don't install git hooks automatically ...................................................................... Patch Set 4: Code-Review+1 (2 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/68647/comment/9f77c5e2_2675f950 PS4, Line 16: Could you please add one more line in commit message explaining how to install git hooks? I understand that's probably `make gitconfig` but let's write in down crystal clear. Patchset: PS4: I agree on the condition that we update Development guidelines straight after this patch is merged. Which means... either me or Angel will need to update that. -- To view, visit https://review.coreboot.org/c/flashrom/+/68647 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ib83568c7ff149a8ec34ad7e92720c36a89def7bd Gerrit-Change-Number: 68647 Gerrit-PatchSet: 4 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Comment-Date: Sun, 23 Oct 2022 22:25:26 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: flash.h: extend `struct tested` with .wp field
by Anastasia Klimchuk (Code Review) 24 Oct '22

24 Oct '22

Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68179 ) Change subject: flash.h: extend `struct tested` with .wp field ...................................................................... Patch Set 6: (1 comment) Patchset: PS4: > Rebased. […] I left CB:68221 not submitted and moved the comment there. Let's get other people thoughts and see how it goes. -- To view, visit https://review.coreboot.org/c/flashrom/+/68179 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I791400889159bc6f305fb05f3e2dd9a90dbe18a4 Gerrit-Change-Number: 68179 Gerrit-PatchSet: 6 Gerrit-Owner: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Sun, 23 Oct 2022 22:04:07 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Comment-In-Reply-To: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: writeprotect.c: differentiate lack of WP from lack of implementation
by Anastasia Klimchuk (Code Review) 24 Oct '22

24 Oct '22

Attention is currently required from: Nico Huber, Nikolai Artemiev, Sergii Dmytruk. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68221 ) Change subject: writeprotect.c: differentiate lack of WP from lack of implementation ...................................................................... Patch Set 7: Code-Review+2 (1 comment) Patchset: PS7: Moving here the comment from patch owner: > However, CB:68221 is probably better be abandoned. Without a reliable way of chip identification (e.g., by hashing SFDP) there can be no precise diagnostics on the status of WP for a chip as one chip record can cover unknown number of chips (including those that don't yet exist). Do we all need to have another look? -- To view, visit https://review.coreboot.org/c/flashrom/+/68221 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6e75b124c21ccab51a9b5e1cd344b5310d384187 Gerrit-Change-Number: 68221 Gerrit-PatchSet: 7 Gerrit-Owner: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Sun, 23 Oct 2022 22:02:42 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: flashchips.c: mark WP of 9 entries as tested
by Anastasia Klimchuk (Code Review) 24 Oct '22

24 Oct '22

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/68180 ) ( 7 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: flashchips.c: mark WP of 9 entries as tested ...................................................................... flashchips.c: mark WP of 9 entries as tested This is based on information from: * commit a850fd0aa8054a1125a9231fa3317428f15900f4 - GD25LQ128C/GD25LQ128D/GD25LQ128E - GD25LQ64(B) - GD25Q127C/GD25Q128C - GD25Q256D/GD25Q256E - GD25Q64(B) * commit a8204dd34d90ac9ab2783e1dd486ec781d4c0dba - GD25Q32(B) * commit 7b4c4f36113c4b7ed5c985d4cf51733639e69bf8 - W25Q64BV/W25Q64CV/W25Q64FV * https://github.com/Dasharo/dasharo-issues/issues/67 - W25Q128.V..M * https://github.com/Dasharo/flashrom/pull/8 - W25Q64.W Change-Id: I090188bad568885f78778e7fc7d8dbe20fb2445f Signed-off-by: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Tested-by: Nikolai Artemiev <nartemiev(a)google.com> Tested-by: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Tested-by: Kamil Pokornicki <kamil.pokornicki(a)3mdeb.com> Tested-by: Przemyslaw Banasiak <przemyslaw.banasiak(a)3mdeb.com> Tested-by: Maciej Pijanowski <maciej.pijanowski(a)3mdeb.com> Signed-off-by: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/68180 Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M flashchips.c 1 file changed, 44 insertions(+), 9 deletions(-) Approvals: build bot (Jenkins): Verified Anastasia Klimchuk: Looks good to me, approved diff --git a/flashchips.c b/flashchips.c index 47a37ee..41e4c42 100644 --- a/flashchips.c +++ b/flashchips.c @@ -6335,7 +6335,7 @@ .page_size = 256, /* OTP: 1024B total, 256B reserved; read 0x48; write 0x42, erase 0x44 */ .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_WRSR_EXT2, - .tested = TEST_OK_PREW, + .tested = TEST_OK_PREWB, .probe = probe_spi_rdid, .probe_timing = TIMING_ZERO, .block_erasers = @@ -6501,7 +6501,7 @@ .page_size = 256, /* OTP: 1024B total, 256B reserved; read 0x48; write 0x42, erase 0x44 */ .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_WRSR_EXT2, - .tested = TEST_OK_PREW, + .tested = TEST_OK_PREWB, .probe = probe_spi_rdid, .probe_timing = TIMING_ZERO, .block_erasers = @@ -6628,7 +6628,7 @@ /* OTP: 1536B total; read 0x48; write 0x42, erase 0x44 */ /* QPI: enable 0x38, disable 0xFF */ .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_QPI | FEATURE_WRSR2, - .tested = TEST_OK_PREW, + .tested = TEST_OK_PREWB, .probe = probe_spi_rdid, .probe_timing = TIMING_ZERO, .block_erasers = @@ -6755,7 +6755,7 @@ .page_size = 256, .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_4BA_WREN | FEATURE_WRSR_EXT2 | FEATURE_WRSR2 | FEATURE_WRSR3, - .tested = TEST_OK_PREW, + .tested = TEST_OK_PREWB, .probe = probe_spi_rdid, .probe_timing = TIMING_ZERO, .block_erasers = @@ -6811,7 +6811,7 @@ .page_size = 256, /* OTP: 1024B total, 256B reserved; read 0x48; write 0x42, erase 0x44 */ .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_WRSR2, - .tested = TEST_OK_PREW, + .tested = TEST_OK_PREWB, .probe = probe_spi_rdid, .probe_timing = TIMING_ZERO, .block_erasers = @@ -6933,7 +6933,7 @@ .page_size = 256, /* OTP: 1024B total, 256B reserved; read 0x48; write 0x42, erase 0x44 */ .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_WRSR2, - .tested = TEST_OK_PREW, + .tested = TEST_OK_PREWB, .probe = probe_spi_rdid, .probe_timing = TIMING_ZERO, .block_erasers = @@ -17359,7 +17359,7 @@ .total_size = 16384, .page_size = 256, .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_QPI | FEATURE_WRSR2, - .tested = TEST_OK_PREW, + .tested = TEST_OK_PREWB, .probe = probe_spi_rdid, .probe_timing = TIMING_ZERO, .block_erasers = @@ -18202,7 +18202,7 @@ /* supports SFDP */ /* OTP: 1024B total, 256B reserved; read 0x48; write 0x42, erase 0x44, read ID 0x4B */ .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_WRSR2, - .tested = TEST_OK_PREW, + .tested = TEST_OK_PREWB, .probe = probe_spi_rdid, .probe_timing = TIMING_ZERO, .block_erasers = @@ -18344,7 +18344,7 @@ /* OTP: 256B total; read 0x48; write 0x42, erase 0x44, read ID 0x4B */ /* QPI enable 0x38, disable 0xFF */ .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_QPI | FEATURE_WRSR2, - .tested = TEST_OK_PREW, + .tested = TEST_OK_PREWB, .probe = probe_spi_rdid, .probe_timing = TIMING_ZERO, .block_erasers = -- To view, visit https://review.coreboot.org/c/flashrom/+/68180 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I090188bad568885f78778e7fc7d8dbe20fb2445f Gerrit-Change-Number: 68180 Gerrit-PatchSet: 9 Gerrit-Owner: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Kamil Pokornicki <kamil.pokornicki(a)3mdeb.com> Gerrit-Reviewer: Maciej Pijanowski <maciej.pijanowski(a)3mdeb.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[M] Change in flashrom[master]: flash.h: extend `struct tested` with .wp field
by Anastasia Klimchuk (Code Review) 24 Oct '22

24 Oct '22

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/68179 ) ( 4 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: flash.h: extend `struct tested` with .wp field ...................................................................... flash.h: extend `struct tested` with .wp field Using "B" letter for "block protection" in TEST_* macros. Ticket: https://ticket.coreboot.org/issues/377 Change-Id: I791400889159bc6f305fb05f3e2dd9a90dbe18a4 Signed-off-by: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/68179 Reviewed-by: Nico Huber <nico.h(a)gmx.de> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M cli_common.c M include/flash.h M include/libflashrom.h 3 files changed, 40 insertions(+), 12 deletions(-) Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, but someone else must approve Anastasia Klimchuk: Looks good to me, approved diff --git a/cli_common.c b/cli_common.c index 9375859..5373585 100644 --- a/cli_common.c +++ b/cli_common.c @@ -35,12 +35,14 @@ if ((chip->tested.probe == BAD) || (chip->tested.probe == NT) || (chip->tested.read == BAD) || (chip->tested.read == NT) || (chip->tested.erase == BAD) || (chip->tested.erase == NT) || - (chip->tested.write == BAD) || (chip->tested.write == NT)){ + (chip->tested.write == BAD) || (chip->tested.write == NT) || + (chip->tested.wp == BAD) || (chip->tested.wp == NT)){ msg_cinfo("===\n"); if ((chip->tested.probe == BAD) || (chip->tested.read == BAD) || (chip->tested.erase == BAD) || - (chip->tested.write == BAD)) { + (chip->tested.write == BAD) || + (chip->tested.wp == BAD)) { msg_cinfo("This flash part has status NOT WORKING for operations:"); if (chip->tested.probe == BAD) msg_cinfo(" PROBE"); @@ -50,12 +52,15 @@ msg_cinfo(" ERASE"); if (chip->tested.write == BAD) msg_cinfo(" WRITE"); + if (chip->tested.wp == BAD) + msg_cinfo(" WP"); msg_cinfo("\n"); } if ((chip->tested.probe == NT) || (chip->tested.read == NT) || (chip->tested.erase == NT) || - (chip->tested.write == NT)) { + (chip->tested.write == NT) || + (chip->tested.wp == NT)) { msg_cinfo("This flash part has status UNTESTED for operations:"); if (chip->tested.probe == NT) msg_cinfo(" PROBE"); @@ -65,6 +70,8 @@ msg_cinfo(" ERASE"); if (chip->tested.write == NT) msg_cinfo(" WRITE"); + if (chip->tested.wp == NT) + msg_cinfo(" WP"); msg_cinfo("\n"); } msg_cinfo("The test status of this chip may have been updated in the latest development\n" diff --git a/include/flash.h b/include/flash.h index cd7f994..633601d 100644 --- a/include/flash.h +++ b/include/flash.h @@ -165,17 +165,19 @@ NA, /* Not applicable (e.g. write support on ROM chips) */ }; -#define TEST_UNTESTED (struct tested){ .probe = NT, .read = NT, .erase = NT, .write = NT } +#define TEST_UNTESTED (struct tested){ .probe = NT, .read = NT, .erase = NT, .write = NT, .wp = NT } -#define TEST_OK_PROBE (struct tested){ .probe = OK, .read = NT, .erase = NT, .write = NT } -#define TEST_OK_PR (struct tested){ .probe = OK, .read = OK, .erase = NT, .write = NT } -#define TEST_OK_PRE (struct tested){ .probe = OK, .read = OK, .erase = OK, .write = NT } -#define TEST_OK_PREW (struct tested){ .probe = OK, .read = OK, .erase = OK, .write = OK } +#define TEST_OK_PROBE (struct tested){ .probe = OK, .read = NT, .erase = NT, .write = NT, .wp = NT } +#define TEST_OK_PR (struct tested){ .probe = OK, .read = OK, .erase = NT, .write = NT, .wp = NT } +#define TEST_OK_PRE (struct tested){ .probe = OK, .read = OK, .erase = OK, .write = NT, .wp = NT } +#define TEST_OK_PREW (struct tested){ .probe = OK, .read = OK, .erase = OK, .write = OK, .wp = NT } +#define TEST_OK_PREWB (struct tested){ .probe = OK, .read = OK, .erase = OK, .write = OK, .wp = OK } -#define TEST_BAD_PROBE (struct tested){ .probe = BAD, .read = NT, .erase = NT, .write = NT } -#define TEST_BAD_PR (struct tested){ .probe = BAD, .read = BAD, .erase = NT, .write = NT } -#define TEST_BAD_PRE (struct tested){ .probe = BAD, .read = BAD, .erase = BAD, .write = NT } -#define TEST_BAD_PREW (struct tested){ .probe = BAD, .read = BAD, .erase = BAD, .write = BAD } +#define TEST_BAD_PROBE (struct tested){ .probe = BAD, .read = NT, .erase = NT, .write = NT, .wp = NT } +#define TEST_BAD_PR (struct tested){ .probe = BAD, .read = BAD, .erase = NT, .write = NT, .wp = NT } +#define TEST_BAD_PRE (struct tested){ .probe = BAD, .read = BAD, .erase = BAD, .write = NT, .wp = NT } +#define TEST_BAD_PREW (struct tested){ .probe = BAD, .read = BAD, .erase = BAD, .write = BAD, .wp = NT } +#define TEST_BAD_PREWB (struct tested){ .probe = BAD, .read = BAD, .erase = BAD, .write = BAD, .wp = BAD } struct flashrom_flashctx; #define flashctx flashrom_flashctx /* TODO: Agree on a name and convert all occurrences. */ @@ -235,6 +237,7 @@ enum test_state read; enum test_state erase; enum test_state write; + enum test_state wp; } tested; /* diff --git a/include/libflashrom.h b/include/libflashrom.h index 21a5785..9bbdcc5 100644 --- a/include/libflashrom.h +++ b/include/libflashrom.h @@ -121,6 +121,7 @@ enum flashrom_test_state read; enum flashrom_test_state erase; enum flashrom_test_state write; + enum flashrom_test_state wp; } tested; }; -- To view, visit https://review.coreboot.org/c/flashrom/+/68179 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I791400889159bc6f305fb05f3e2dd9a90dbe18a4 Gerrit-Change-Number: 68179 Gerrit-PatchSet: 6 Gerrit-Owner: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit