September 2022 - flashrom-gerrit

[M] Change in flashrom[master]: spi.c: Add erasefn and opcodes for AT45 and S25F to lookup list
by Aarya (Code Review) 09 Sep '22

09 Sep '22

Attention is currently required from: Simon Buhrow, Thomas Heijligen, Aarya. Hello build bot (Jenkins), Simon Buhrow, Nico Huber, Thomas Heijligen, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/67354 to look at the new patch set (#6). Change subject: spi.c: Add erasefn and opcodes for AT45 and S25F to lookup list ...................................................................... spi.c: Add erasefn and opcodes for AT45 and S25F to lookup list Add opcode and erasefn for AT45 and S25F to the spi_get_[opcode/erasefn]_from[erasefn/opcode] lookup list. Also AT45CS1282 uses multiple opcodes in it's erasefn so modifying aforementioned functions and the lookup list to use a list of opcodes rather than a single opcode. The lookup list and the passed lookup list must have opcodes in sorted order and must be 0x00 terminated. Also the macro MAX_OPCODE must have be one more than the maximum opcodes used by any erasefn. Change-Id: Ief25932120f940dea0ffe161a79219deecb2e8ab Signed-off-by: Aarya Chaumal <aarya.chaumal(a)gmail.com> --- M flashrom.c M include/chipdrivers.h M sfdp.c M spi.c 4 files changed, 80 insertions(+), 32 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/54/67354/6 -- To view, visit https://review.coreboot.org/c/flashrom/+/67354 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ief25932120f940dea0ffe161a79219deecb2e8ab Gerrit-Change-Number: 67354 Gerrit-PatchSet: 6 Gerrit-Owner: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Simon Buhrow Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Simon Buhrow Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[master]: flashrom_tester: Fix cargo check and clippy warnings
by Peter Marheine (Code Review) 09 Sep '22

09 Sep '22

Attention is currently required from: Evan Benn. Peter Marheine has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67472 ) Change subject: flashrom_tester: Fix cargo check and clippy warnings ...................................................................... Patch Set 2: (3 comments) File util/flashrom_tester/flashrom/src/cmd.rs: https://review.coreboot.org/c/flashrom/+/67472/comment/6e57f1bd_b1b1b2b9 PS2, Line 89: return This can be converted to drop the `return` too? File util/flashrom_tester/src/tests.rs: https://review.coreboot.org/c/flashrom/+/67472/comment/b80266a5_997f1eb0 PS2, Line 135: let mut filter_names: Option<HashSet<String>> = if let Some(names) = test_names { I think this version is easier to understand, but if you prefer the newer version then go ahead. https://review.coreboot.org/c/flashrom/+/67472/comment/a8a498f3_b1d0e300 PS2, Line 142: .unwrap_or_else(|_| "<Unknown chip>".into()); I don't like this clippy lint; we really don't care about the cost of this allocation, and this is harder to read. -- To view, visit https://review.coreboot.org/c/flashrom/+/67472 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I50c5af61e06df1bb6956f347cb6806a7eca6ce0e Gerrit-Change-Number: 67472 Gerrit-PatchSet: 2 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Evan Benn <evanbenn(a)google.com> Gerrit-Comment-Date: Fri, 09 Sep 2022 06:21:22 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: flashrom_tester: Fix cargo check and clippy warnings
by Evan Benn (Code Review) 09 Sep '22

09 Sep '22

Evan Benn has uploaded a new patch set (#2). ( https://review.coreboot.org/c/flashrom/+/67472 ) Change subject: flashrom_tester: Fix cargo check and clippy warnings ...................................................................... flashrom_tester: Fix cargo check and clippy warnings Change-Id: I50c5af61e06df1bb6956f347cb6806a7eca6ce0e Signed-off-by: Evan Benn <evanbenn(a)chromium.org> --- M util/flashrom_tester/flashrom/src/cmd.rs M util/flashrom_tester/flashrom/src/lib.rs M util/flashrom_tester/src/cros_sysinfo.rs M util/flashrom_tester/src/logger.rs M util/flashrom_tester/src/rand_util.rs M util/flashrom_tester/src/tester.rs M util/flashrom_tester/src/tests.rs M util/flashrom_tester/src/utils.rs 8 files changed, 45 insertions(+), 47 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/72/67472/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/67472 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I50c5af61e06df1bb6956f347cb6806a7eca6ce0e Gerrit-Change-Number: 67472 Gerrit-PatchSet: 2 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[master]: flashrom_tester: Fix cargo check and clippy warnings
by Evan Benn (Code Review) 09 Sep '22

09 Sep '22

Evan Benn has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/67472 ) Change subject: flashrom_tester: Fix cargo check and clippy warnings ...................................................................... flashrom_tester: Fix cargo check and clippy warnings Change-Id: I50c5af61e06df1bb6956f347cb6806a7eca6ce0e Signed-off-by: Evan Benn <evanbenn(a)chromium.org> --- M util/flashrom_tester/flashrom/src/cmd.rs M util/flashrom_tester/flashrom/src/lib.rs M util/flashrom_tester/src/cros_sysinfo.rs M util/flashrom_tester/src/logger.rs M util/flashrom_tester/src/rand_util.rs M util/flashrom_tester/src/tester.rs M util/flashrom_tester/src/tests.rs M util/flashrom_tester/src/utils.rs 8 files changed, 44 insertions(+), 46 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/72/67472/1 diff --git a/util/flashrom_tester/flashrom/src/cmd.rs b/util/flashrom_tester/flashrom/src/cmd.rs index 57ae975..eb466d8 100644 --- a/util/flashrom_tester/flashrom/src/cmd.rs +++ b/util/flashrom_tester/flashrom/src/cmd.rs @@ -67,7 +67,7 @@ pub region: Option<(&'a str, &'a str)>, // --image } -#[derive(PartialEq, Debug)] +#[derive(PartialEq, Eq, Debug)] pub struct FlashromCmd { pub path: String, pub fc: FlashChip, @@ -84,7 +84,7 @@ .last() .map(str::parse::<i64>) { - None => return Err("Found no purely-numeric lines in flashrom output".into()), + None => Err("Found no purely-numeric lines in flashrom output".into()), Some(Err(e)) => { return Err(format!("Failed to parse flashrom size output as integer: {}", e).into()) } @@ -169,7 +169,7 @@ }; let (stdout, _) = self.dispatch(opts, "wp_list")?; - if stdout.len() == 0 { + if stdout.is_empty() { return Err( "wp_list isn't supported on platforms using the Linux kernel SPI driver wp_list" .into(), @@ -208,7 +208,7 @@ let opts = FlashromOpt { wp_opt: WPOpt { - range: range, + range, enable: en, disable: !en, ..Default::default() @@ -249,7 +249,7 @@ ..Default::default() }; - let (stdout, _) = self.dispatch(opts, "read_region_into_file")?; + self.dispatch(opts, "read_region_into_file")?; Ok(()) } @@ -405,11 +405,6 @@ dut_ctrl(&args) } -pub fn dut_ctrl_servo_type() -> Result<(Vec<u8>, Vec<u8>), FlashromError> { - let args = ["servo_type"]; - dut_ctrl(&args) -} - fn dut_ctrl(args: &[&str]) -> Result<(Vec<u8>, Vec<u8>), FlashromError> { let output = match Command::new("dut-control").args(args).output() { Ok(x) => x, @@ -431,7 +426,7 @@ } fn hex_range_string(s: i64, l: i64) -> String { - format!("{:#08X},{:#08X}", s, l).to_string() + format!("{:#08X},{:#08X}", s, l) } /// Get a flash vendor and name from the first matching line of flashrom output. diff --git a/util/flashrom_tester/flashrom/src/lib.rs b/util/flashrom_tester/flashrom/src/lib.rs index 7326391..7e245ae 100644 --- a/util/flashrom_tester/flashrom/src/lib.rs +++ b/util/flashrom_tester/flashrom/src/lib.rs @@ -49,7 +49,7 @@ FLASHROM_MSG_INFO, FLASHROM_MSG_SPEW, FLASHROM_MSG_WARN, }; -#[derive(Copy, Clone, PartialEq, Debug)] +#[derive(Copy, Clone, PartialEq, Eq, Debug)] pub enum FlashChip { EC, HOST, @@ -59,23 +59,21 @@ impl FlashChip { pub fn from(s: &str) -> Result<FlashChip, &str> { - let r = match s { + match s { "ec" => Ok(FlashChip::EC), "host" => Ok(FlashChip::HOST), "servo" => Ok(FlashChip::SERVO), "dediprog" => Ok(FlashChip::DEDIPROG), _ => Err("cannot convert str to enum"), - }; - return r; + } } pub fn to(fc: FlashChip) -> &'static str { - let r = match fc { + match fc { FlashChip::EC => "ec", FlashChip::HOST => "host", FlashChip::SERVO => "ft2231_spi:type=servo-v2", FlashChip::DEDIPROG => "dediprog", - }; - return r; + } } /// Return the programmer string and optional programmer options @@ -97,7 +95,7 @@ } } -#[derive(Debug, PartialEq)] +#[derive(Debug, PartialEq, Eq)] pub struct FlashromError { msg: String, } diff --git a/util/flashrom_tester/src/cros_sysinfo.rs b/util/flashrom_tester/src/cros_sysinfo.rs index 7073edd..e047f8e 100644 --- a/util/flashrom_tester/src/cros_sysinfo.rs +++ b/util/flashrom_tester/src/cros_sysinfo.rs @@ -34,7 +34,6 @@ // use std::ffi::OsStr; -use std::fmt::Debug; use std::io::Result as IoResult; use std::process::{Command, Stdio}; diff --git a/util/flashrom_tester/src/logger.rs b/util/flashrom_tester/src/logger.rs index e1c00f5..12e5495 100644 --- a/util/flashrom_tester/src/logger.rs +++ b/util/flashrom_tester/src/logger.rs @@ -129,7 +129,7 @@ for record in records { if logger.enabled(record.metadata()) { - logger.log(&record); + logger.log(record); } } } diff --git a/util/flashrom_tester/src/rand_util.rs b/util/flashrom_tester/src/rand_util.rs index 51411d0..1b0912b 100644 --- a/util/flashrom_tester/src/rand_util.rs +++ b/util/flashrom_tester/src/rand_util.rs @@ -42,9 +42,7 @@ pub fn gen_rand_testdata(path: &str, size: usize) -> std::io::Result<()> { let mut buf = BufWriter::new(File::create(path)?); - let mut a: Vec<u8> = Vec::with_capacity(size); - // Pad out array to be filled in by Rng::fill(). - a.resize(size, 0b0); + let mut a: Vec<u8> = vec![0; size]; thread_rng().fill(a.as_mut_slice()); buf.write_all(a.as_slice())?; diff --git a/util/flashrom_tester/src/tester.rs b/util/flashrom_tester/src/tester.rs index e701763..f2c3846 100644 --- a/util/flashrom_tester/src/tester.rs +++ b/util/flashrom_tester/src/tester.rs @@ -69,8 +69,8 @@ pub fn create(chip_type: FlashChip, cmd: &'a dyn Flashrom) -> Result<Self, FlashromError> { let rom_sz = cmd.get_size()?; let out = TestEnv { - chip_type: chip_type, - cmd: cmd, + 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(), @@ -462,7 +462,7 @@ } #[allow(dead_code)] -#[derive(Copy, Clone, PartialEq, Debug)] +#[derive(Copy, Clone, PartialEq, Eq, Debug)] pub enum TestConclusion { Pass, Fail, @@ -514,7 +514,7 @@ results } -#[derive(Debug, PartialEq, Clone, Copy)] +#[derive(Debug, PartialEq, Eq, Clone, Copy)] pub enum OutputFormat { Pretty, Json, diff --git a/util/flashrom_tester/src/tests.rs b/util/flashrom_tester/src/tests.rs index 7d746b5..de795fe 100644 --- a/util/flashrom_tester/src/tests.rs +++ b/util/flashrom_tester/src/tests.rs @@ -43,8 +43,8 @@ use std::io::{BufRead, Write}; use std::sync::atomic::AtomicBool; -const LAYOUT_FILE: &'static str = "/tmp/layout.file"; -const ELOG_FILE: &'static str = "/tmp/elog.file"; +const LAYOUT_FILE: &str = "/tmp/layout.file"; +const ELOG_FILE: &str = "/tmp/elog.file"; /// Iterate over tests, yielding only those tests with names matching filter_names. /// @@ -132,17 +132,14 @@ // Limit the tests to only those requested, unless none are requested // in which case all tests are included. - let mut filter_names: Option<HashSet<String>> = if let Some(names) = test_names { - Some(names.map(|s| s.to_lowercase()).collect()) - } else { - None - }; + let mut filter_names: Option<HashSet<String>> = + test_names.map(|names| names.map(|s| s.to_lowercase()).collect()); let tests = filter_tests(tests, &mut filter_names); let chip_name = cmd .name() .map(|x| format!("vendor=\"{}\" name=\"{}\"", x.0, x.1)) - .unwrap_or("<Unknown chip>".into()); + .unwrap_or_else(|_| "<Unknown chip>".into()); // ------------------------. // Run all the tests and collate the findings: @@ -153,15 +150,16 @@ warn!("No test matches filter name \"{}\"", leftover); } - let os_rel = sys_info::os_release().unwrap_or("<Unknown OS>".to_string()); - let system_info = cros_sysinfo::system_info().unwrap_or("<Unknown System>".to_string()); - let bios_info = cros_sysinfo::bios_info().unwrap_or("<Unknown BIOS>".to_string()); + let os_rel = sys_info::os_release().unwrap_or_else(|_| "<Unknown OS>".to_string()); + let system_info = + cros_sysinfo::system_info().unwrap_or_else(|_| "<Unknown System>".to_string()); + let bios_info = cros_sysinfo::bios_info().unwrap_or_else(|_| "<Unknown BIOS>".to_string()); let meta_data = tester::ReportMetaData { - chip_name: chip_name, + chip_name, os_release: os_rel, - system_info: system_info, - bios_info: bios_info, + system_info, + bios_info, }; tester::collate_all_test_runs(&results, meta_data, output_format); Ok(()) diff --git a/util/flashrom_tester/src/utils.rs b/util/flashrom_tester/src/utils.rs index e8fbb53..4e8dd7c 100644 --- a/util/flashrom_tester/src/utils.rs +++ b/util/flashrom_tester/src/utils.rs @@ -143,14 +143,14 @@ match wp_s_val { Ok(v) => { if v == 1 { - return Ok(true); + Ok(true) } else if v == 0 { - return Ok(false); + Ok(false) } else { - return Err("Unknown write protect value".into()); + Err("Unknown write protect value".into()) } } - Err(_) => return Err("Cannot parse write protect value".into()), + Err(_) => Err("Cannot parse write protect value".into()), } } -- To view, visit https://review.coreboot.org/c/flashrom/+/67472 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I50c5af61e06df1bb6956f347cb6806a7eca6ce0e Gerrit-Change-Number: 67472 Gerrit-PatchSet: 1 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-MessageType: newchange

1 0

[S] Change in flashrom[master]: flashrom.c: Plumb 'all_skipped' global state into func param
by Aarya (Code Review) 09 Sep '22

09 Sep '22

Attention is currently required from: Felix Singer, Thomas Heijligen, Edward O'Callaghan, Angel Pons. Aarya has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67093 ) Change subject: flashrom.c: Plumb 'all_skipped' global state into func param ...................................................................... Patch Set 5: (1 comment) File flashrom.c: https://review.coreboot.org/c/flashrom/+/67093/comment/a853e23c_80e02ce2 PS5, Line 1053: walk_eraseblocks > I created a new branch and cherry picked the patch on it, made changes committed them. […] I have uploaded the patch here https://review.coreboot.org/c/flashrom/+/67470 -- To view, visit https://review.coreboot.org/c/flashrom/+/67093 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2346c869c47b48604360b0facf9313aae086c8dd Gerrit-Change-Number: 67093 Gerrit-PatchSet: 5 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com> 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: Nikolai Artemiev <nartemiev(a)google.com> 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-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Fri, 09 Sep 2022 05:41:24 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net> Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Comment-In-Reply-To: Aarya <aarya.chaumal(a)gmail.com> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: flashrom.c: Plumb 'all_skipped' global state into func param
by Aarya (Code Review) 09 Sep '22

09 Sep '22

Aarya has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/67470 ) Change subject: flashrom.c: Plumb 'all_skipped' global state into func param ...................................................................... flashrom.c: Plumb 'all_skipped' global state into func param The 'all_skipped' global state can be made into a function parameter if one just follows though the CFG. Change-Id: Ib14d1b87958ae933aae937eb5233afcfa0f32582 Signed-off-by: Aarya Chaumal <aarya.chaumal(a)gmail.com> --- M flashrom.c 1 file changed, 23 insertions(+), 11 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/70/67470/1 diff --git a/flashrom.c b/flashrom.c index a0a7627..8f9e6a2 100644 --- a/flashrom.c +++ b/flashrom.c @@ -65,9 +65,6 @@ */ static int may_register_shutdown = 0; -/* Did we change something or was every erase/write skipped (if any)? */ -static bool all_skipped = true; - /* Register a function to be executed on programmer shutdown. * The advantage over atexit() is that you can supply a void pointer which will * be used as parameter to the registered function upon programmer shutdown. @@ -875,7 +872,7 @@ chipoff_t erase_end; }; /* returns 0 on success, 1 to retry with another erase function, 2 for immediate abort */ -typedef int (*per_blockfn_t)(struct flashctx *, const struct walk_info *, erasefn_t); +typedef int (*per_blockfn_t)(struct flashctx *, const struct walk_info *, erasefn_t, bool *); void align_region(const struct erase_layout *layout, struct flashctx *const flashctx, chipoff_t *region_start, chipoff_t *region_end); @@ -1014,7 +1011,7 @@ return layout; } -static int erase_write(struct flashctx *const flashctx, chipoff_t region_start, chipoff_t region_end, uint8_t* curcontents, uint8_t* newcontents, struct erase_layout *erase_layout) +static int erase_write(struct flashctx *const flashctx, chipoff_t region_start, chipoff_t region_end, uint8_t* curcontents, uint8_t* newcontents, struct erase_layout *erase_layout, bool *all_skipped) { size_t erasefn_count = count_usable_erasers(flashctx); int ret = 0; @@ -1063,7 +1060,7 @@ //after erase make it unselected again erase_layout[i].layout_list[j].selected = false; msg_cinfo("E(%x:%x)", start_addr, start_addr + block_len - 1); - all_skipped = false; + *all_skipped = false; } } } @@ -1081,7 +1078,7 @@ //adjust curcontents memcpy(curcontents + i, newcontents + i, flashctx->chip->page_size); msg_cinfo("W(%lx:%lx)", i, i + flashctx->chip->page_size); - all_skipped = false; + *all_skipped = false; } } @@ -1117,6 +1114,7 @@ struct erase_layout *erase_layout = create_erase_layout(flashctx); int erasefn_count = count_usable_erasers(flashctx); int ret = 0; + bool all_skipped = true; //not enough memory if (curcontents == NULL || newcontents == NULL) { @@ -1131,7 +1129,7 @@ const struct romentry *entry = NULL; while ((entry = layout_next_included(flash_layout, entry))) { - ret |= erase_write(flashctx, entry->start, entry->end, curcontents, newcontents, erase_layout); + ret |= erase_write(flashctx, entry->start, entry->end, curcontents, newcontents, erase_layout, &all_skipped); //if ret!=0 ?? assert(!ret); } @@ -1156,7 +1154,7 @@ * 1 if anything has gone wrong. */ static int write_by_layout(struct flashctx *const flashctx, - void *const curcontents, const void *const newcontents) + void *const curcontents, const void *const newcontents, bool *all_skipped) { const struct flashrom_layout *const flash_layout = get_layout(flashctx); const struct romentry *entry = NULL; @@ -1165,7 +1163,7 @@ int ret = 0; while ((entry = layout_next_included(flash_layout, entry))) { - ret = erase_write(flashctx, entry->start, entry->end, (uint8_t*)curcontents, (uint8_t*)newcontents, erase_layout); + ret = erase_write(flashctx, entry->start, entry->end, (uint8_t*)curcontents, (uint8_t*)newcontents, erase_layout, all_skipped); //if ret!=0 ?? assert(!ret); } @@ -1629,7 +1627,8 @@ msg_cinfo("done.\n"); } - if (write_by_layout(flashctx, curcontents, newcontents)) { + bool all_skipped = true; + if (write_by_layout(flashctx, curcontents, newcontents, &all_skipped)) { msg_cerr("Uh oh. Erase/write failed. "); ret = 2; if (verify_all) { -- To view, visit https://review.coreboot.org/c/flashrom/+/67470 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ib14d1b87958ae933aae937eb5233afcfa0f32582 Gerrit-Change-Number: 67470 Gerrit-PatchSet: 1 Gerrit-Owner: Aarya <aarya.chaumal(a)gmail.com> Gerrit-CC: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

1 0

[S] Change in flashrom[master]: flashrom.c: create is_internal_programmer() helper
by Anastasia Klimchuk (Code Review) 09 Sep '22

09 Sep '22

Attention is currently required from: Thomas Heijligen, Edward O'Callaghan. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/66684 ) Change subject: flashrom.c: create is_internal_programmer() helper ...................................................................... Patch Set 6: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/66684 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I43243b990192077583a9a3a95d35844923d9c158 Gerrit-Change-Number: 66684 Gerrit-PatchSet: 6 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: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Fri, 09 Sep 2022 03:35:48 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: flashrom.c: create is_internal_programmer() helper
by Edward O'Callaghan (Code Review) 09 Sep '22

09 Sep '22

Attention is currently required from: Thomas Heijligen, Anastasia Klimchuk. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/66684 ) Change subject: flashrom.c: create is_internal_programmer() helper ...................................................................... Patch Set 6: (1 comment) Patchset: PS6: > I am wondering about testing. […] It is a good question although this change in of itself does nothing more than put all the preprocessor under the one symbol. Regarding where to from here, my advice would be to make `emergency_help_message(void)` static internal to `flashrom.c` then follow up with changing both the signature's of the printer functions to `emergency_help_message(bool) && nonfatal_help_message(bool)` so they are simple printer functions then call them as follows `xxx_help_message(is_internal_programmer());` Past that requires more substantive changes where possibly `is_internal_programmer()` is done inside `programmer_init()` and the resulting state is stored for later use in the flashctx or force_boardmismatch becomes part of programmer_entry. All this is a little unclear at the minute without some unrelated changes to surface the underlying hidden structures that best encode what we want. Lastly, emulating 'broken hardware' is actually relatively easy. A EM100 could emulate a bad write op for a programmer which would exercise branches such as these. As discussed in the leadership meeting we would like to see a future where the CI bot runs precisely this kind of E2E test. -- To view, visit https://review.coreboot.org/c/flashrom/+/66684 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I43243b990192077583a9a3a95d35844923d9c158 Gerrit-Change-Number: 66684 Gerrit-PatchSet: 6 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: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Fri, 09 Sep 2022 02:50:52 +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[master]: ichspi: Add Intel Idaville HCC support
by Kevin Yu (Code Review) 09 Sep '22

09 Sep '22

Kevin Yu has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/66635 ) Change subject: ichspi: Add Intel Idaville HCC support ...................................................................... Patch Set 3: (1 comment) Patchset: PS3: I check the Merge Conflict, there are many updates to the codebase, so I have resubmitted a code commit, for this one:https://review.coreboot.org/c/flashrom/+/67438 -- To view, visit https://review.coreboot.org/c/flashrom/+/66635 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I36fe5d296ed65547fbd5130abe1a0c8dd291e920 Gerrit-Change-Number: 66635 Gerrit-PatchSet: 3 Gerrit-Owner: Kevin Yu <ykevin(a)celestica.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Fri, 09 Sep 2022 01:21:45 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: tree: Retype variable `laptop_ok` with bool
by Angel Pons (Code Review) 08 Sep '22

08 Sep '22

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/66891 ) Change subject: tree: Retype variable `laptop_ok` with bool ...................................................................... Patch Set 14: (1 comment) File board_enable.c: https://review.coreboot.org/c/flashrom/+/66891/comment/7b763304_6c207e09 PS13, Line 2295: is_laptop = 1; > I suggested to create a ticket for this, not to forget https://ticket.coreboot.org/issues/416 […] Perfect, thank you! -- To view, visit https://review.coreboot.org/c/flashrom/+/66891 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I5d9fc3516bc2d29f11b056e35b3e5e324ce93423 Gerrit-Change-Number: 66891 Gerrit-PatchSet: 14 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Thu, 08 Sep 2022 19:49:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit September 2022