January 2023 - flashrom-gerrit

[S] Change in flashrom[master]: rust: Add license and other metadata to Cargo.toml
by Angel Pons (Code Review) 05 Jan '23

05 Jan '23

Attention is currently required from: Evan Benn, Peter Marheine. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71618 ) Change subject: rust: Add license and other metadata to Cargo.toml ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/71618 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ibdab16713395509be511e45c5eae946496020429 Gerrit-Change-Number: 71618 Gerrit-PatchSet: 1 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Evan Benn <evanbenn(a)google.com> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Thu, 05 Jan 2023 00:40:23 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: programmer.h: Guard against sending spi commands on non-spi mst
by Jonathon Hall (Code Review) 04 Jan '23

04 Jan '23

Attention is currently required from: Felix Singer, Edward O'Callaghan, Angel Pons. Jonathon Hall has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71269 ) Change subject: programmer.h: Guard against sending spi commands on non-spi mst ...................................................................... Patch Set 2: Code-Review+1 (1 comment) Patchset: PS2: I also ran into this issue and can confirm that this fixes it, W25Q256FV on Coffee Lake. Thanks Edward! -- To view, visit https://review.coreboot.org/c/flashrom/+/71269 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7cce4f9c032d33c01bf616e27a50b9727a40fe1b Gerrit-Change-Number: 71269 Gerrit-PatchSet: 2 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Jonathon Hall <jonathon.hall(a)puri.sm> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-CC: Subrata Banik <subratabanik(a)google.com> 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-Comment-Date: Wed, 04 Jan 2023 16:14:42 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: rust: Add license and other metadata to Cargo.toml
by Evan Benn (Code Review) 04 Jan '23

04 Jan '23

Evan Benn has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/71618 ) Change subject: rust: Add license and other metadata to Cargo.toml ...................................................................... rust: Add license and other metadata to Cargo.toml Add missing license to the Cargo.toml rust files, and some other metadata that might be useful. BUG=None BRANCH=None TEST=None Change-Id: Ibdab16713395509be511e45c5eae946496020429 Signed-off-by: Evan Benn <evanbenn(a)chromium.org> --- M bindings/rust/libflashrom-sys/Cargo.toml M bindings/rust/libflashrom/Cargo.toml M util/flashrom_tester/Cargo.toml M util/flashrom_tester/flashrom/Cargo.toml 4 files changed, 33 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/18/71618/1 diff --git a/bindings/rust/libflashrom-sys/Cargo.toml b/bindings/rust/libflashrom-sys/Cargo.toml index 54e016d..af84b8a 100644 --- a/bindings/rust/libflashrom-sys/Cargo.toml +++ b/bindings/rust/libflashrom-sys/Cargo.toml @@ -1,6 +1,12 @@ [package] name = "libflashrom-sys" version = "0.1.0" +description = "Native bindings to the libflashrom library." +readme = "../README" +homepage = "https://www.flashrom.org/" +repository = "https://review.coreboot.org/plugins/gitiles/flashrom/" +license = "GPL-2.0-only" +categories = ["external-ffi-bindings", "hardware-support"] links = "flashrom" build = "build.rs" edition = "2021" diff --git a/bindings/rust/libflashrom/Cargo.toml b/bindings/rust/libflashrom/Cargo.toml index 68636df..1d8fb52 100644 --- a/bindings/rust/libflashrom/Cargo.toml +++ b/bindings/rust/libflashrom/Cargo.toml @@ -1,10 +1,14 @@ [package] name = "libflashrom" version = "0.1.0" +description = "Bindings to the libflashrom library." +readme = "../README" +homepage = "https://www.flashrom.org/" +repository = "https://review.coreboot.org/plugins/gitiles/flashrom/" +license = "GPL-2.0-only" +categories = ["api-bindings", "hardware-support"] edition = "2021" -# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html - [dependencies] libflashrom-sys = { path = "../libflashrom-sys" } libc = "0.2.124" diff --git a/util/flashrom_tester/Cargo.toml b/util/flashrom_tester/Cargo.toml index a7b6881..948553a 100644 --- a/util/flashrom_tester/Cargo.toml +++ b/util/flashrom_tester/Cargo.toml @@ -3,6 +3,8 @@ version = "1.6.0" authors = ["Edward O'Callaghan <quasisec(a)chromium.org>", "Peter Marheine <pmarheine(a)chromium.org>"] +description = "A tool to verify flashrom and flash chip behaviour." +license = "GPL-2.0-only" edition = "2018" build = "build.rs" diff --git a/util/flashrom_tester/flashrom/Cargo.toml b/util/flashrom_tester/flashrom/Cargo.toml index a2447d0..4d4fc2f 100644 --- a/util/flashrom_tester/flashrom/Cargo.toml +++ b/util/flashrom_tester/flashrom/Cargo.toml @@ -3,6 +3,8 @@ version = "1.0.0" authors = ["Edward O'Callaghan <quasisec(a)chromium.org>", "Peter Marheine <pmarheine(a)chromium.org>"] +description = "Flashrom abstraction for the flashrom_tester tool." +license = "GPL-2.0-only" edition = "2018" [dependencies] -- To view, visit https://review.coreboot.org/c/flashrom/+/71618 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ibdab16713395509be511e45c5eae946496020429 Gerrit-Change-Number: 71618 Gerrit-PatchSet: 1 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-MessageType: newchange

1 0

[S] Change in flashrom[master]: programmer.h: Guard against sending spi commands on non-spi mst
by Edward O'Callaghan (Code Review) 04 Jan '23

04 Jan '23

Attention is currently required from: Felix Singer, Angel Pons. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71269 ) Change subject: programmer.h: Guard against sending spi commands on non-spi mst ...................................................................... Patch Set 2: (1 comment) File flashrom.c: https://review.coreboot.org/c/flashrom/+/71269/comment/79edb5d5_14e8c830 PS1, Line 1937: spi_master_no_4ba_modes > Wouldn't it make more sense to add the bustype check inside this function? sort of but not really. This is a chip bustype check not a master bustype check. Adding a predicate to `spi_master_no_4ba_modes()` is not related to preventing the subsequent calls to `spi_{enter,exit}_4ba()`. I think what you are asking for is a `spi_chip_4ba()` func which I have now done? -- To view, visit https://review.coreboot.org/c/flashrom/+/71269 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7cce4f9c032d33c01bf616e27a50b9727a40fe1b Gerrit-Change-Number: 71269 Gerrit-PatchSet: 2 Gerrit-Owner: Edward O'Callaghan <quasisec(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-CC: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-CC: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Wed, 04 Jan 2023 01:28:26 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: programmer.h: Guard against sending spi commands on non-spi mst
by Edward O'Callaghan (Code Review) 04 Jan '23

04 Jan '23

Attention is currently required from: Felix Singer, Edward O'Callaghan. Hello Felix Singer, build bot (Jenkins), Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/71269 to look at the new patch set (#2). Change subject: programmer.h: Guard against sending spi commands on non-spi mst ...................................................................... programmer.h: Guard against sending spi commands on non-spi mst Validate (flash->chip->bustype == BUS_SPI) as ich copies the chip flags in the opaque master and tries incorrectly to issue 4BA commands which results in failure. The issue was detected only in the case of chips >16MB, in this case 'W25Q256FV' that has the feature bits: ``` .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_4BA_ENTER_WREN | FEATURE_4BA_EAR_C5C8 | FEATURE_4BA_READ | FEATURE_4BA_FAST_READ | FEATURE_WRSR2, ``` The regression was noticed from, commit 0741727925b841c2479b993204ce58c5eb75185a ichspi.c: Read chip ID and use it to populate `flash->chip` TEST=In the case of 'W25Q256FV' on TigerLake. Change-Id: I7cce4f9c032d33c01bf616e27a50b9727a40fe1b Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M flashrom.c M include/programmer.h 2 files changed, 33 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/69/71269/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/71269 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7cce4f9c032d33c01bf616e27a50b9727a40fe1b Gerrit-Change-Number: 71269 Gerrit-PatchSet: 2 Gerrit-Owner: Edward O'Callaghan <quasisec(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-CC: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-CC: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[master]: sb600spi.c: Move promontory code into a mmap_read=yes mode
by Edward O'Callaghan (Code Review) 04 Jan '23

04 Jan '23

Attention is currently required from: Thomas Heijligen, Arthur Heymans. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71565 ) Change subject: sb600spi.c: Move promontory code into a mmap_read=yes mode ...................................................................... Patch Set 2: Code-Review+2 (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/71565/comment/70f0f78d_8675d2de PS2, Line 10: This is not : hardware specific I believe it is hardware specific to controllers after a certain generation. i.e., supported on SPI100 controllers (Zen etc) however not supported on pre-SPI100 controllers was my understanding. -- To view, visit https://review.coreboot.org/c/flashrom/+/71565 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I13eb5a646d3569170b3911ae7b3127cd3e6022aa Gerrit-Change-Number: 71565 Gerrit-PatchSet: 2 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> 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: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Tue, 03 Jan 2023 23:51:23 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: chipset_enable.c: Add TL UP3 and UP4/Y id's
by Angel Pons (Code Review) 03 Jan '23

03 Jan '23

Attention is currently required from: Sam McNally, Edward O'Callaghan. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71576 ) Change subject: chipset_enable.c: Add TL UP3 and UP4/Y id's ...................................................................... Patch Set 1: (1 comment) File chipset_enable.c: https://review.coreboot.org/c/flashrom/+/71576/comment/acd6174a_b5a5ac11 PS1, Line 2083: {0x8086, 0xa083, B_S, DEP, "Intel", "Tiger Lake U Premium 3", enable_flash_pch500}, > As far as I can tell, 0xa083 isn't used after all, so we don't need to add this. Let's keep it, but mark it as untested (change `DEP` to `NT`). -- To view, visit https://review.coreboot.org/c/flashrom/+/71576 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I77120612f7a770ae1319f4cd82913fa465f355f5 Gerrit-Change-Number: 71576 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Reviewer: Sam McNally <sammc(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Sam McNally <sammc(a)google.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Tue, 03 Jan 2023 06:23:50 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Sam McNally <sammc(a)google.com> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: chipset_enable.c: Add TL UP3 and UP4/Y id's
by Sam McNally (Code Review) 03 Jan '23

03 Jan '23

Attention is currently required from: Edward O'Callaghan. Sam McNally has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71576 ) Change subject: chipset_enable.c: Add TL UP3 and UP4/Y id's ...................................................................... Patch Set 1: (2 comments) File chipset_enable.c: https://review.coreboot.org/c/flashrom/+/71576/comment/339e3db0_c567540c PS1, Line 2083: {0x8086, 0xa083, B_S, DEP, "Intel", "Tiger Lake U Premium 3", enable_flash_pch500}, As far as I can tell, 0xa083 isn't used after all, so we don't need to add this. https://review.coreboot.org/c/flashrom/+/71576/comment/d1f3ec48_db6bf680 PS1, Line 2084: U Premium 4/Y If we follow the final naming scheme this should be "Tiger Lake Premium UP4". If we follow earlier platforms, and the coreboot scheme, then "Tiger Lake Y Premium". -- To view, visit https://review.coreboot.org/c/flashrom/+/71576 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I77120612f7a770ae1319f4cd82913fa465f355f5 Gerrit-Change-Number: 71576 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Reviewer: Sam McNally <sammc(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Tue, 03 Jan 2023 03:50:54 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: internal.c: Do cb_parse_table() in preinit
by Angel Pons (Code Review) 02 Jan '23

02 Jan '23

Attention is currently required from: Edward O'Callaghan. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71624 ) Change subject: internal.c: Do cb_parse_table() in preinit ...................................................................... Patch Set 1: (1 comment) File include/programmer.h: https://review.coreboot.org/c/flashrom/+/71624/comment/2a1d035a_2b63228f PS1, Line 35: struct { : const char *cb_vendor; : const char *cb_model; : } internal; This should be a programmer-defined opaque struct, pointed to by `struct programmer_cfg`. -- To view, visit https://review.coreboot.org/c/flashrom/+/71624 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7f4fbb4a8bc362c4ac27a5ab2472551a5467e240 Gerrit-Change-Number: 71624 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Mon, 02 Jan 2023 13:11:44 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: board_enables: Couple prog cfg with board cfg
by Angel Pons (Code Review) 02 Jan '23

02 Jan '23

Attention is currently required from: Edward O'Callaghan. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71623 ) Change subject: board_enables: Couple prog cfg with board cfg ...................................................................... Patch Set 1: (2 comments) Patchset: PS1: It's hard to say if all the plumbing is needed, but no idea how to make it better. File internal.c: https://review.coreboot.org/c/flashrom/+/71623/comment/e1b8040f_e61d8481 PS1, Line 270: board_handle_before_superio((struct programmer_cfg *)cfg); Ugh, it'd be better to avoid casting away const... Quick solution would be to make a mutable local copy: struct programmer_cfg mut_cfg = *cfg; Then use `&mut_cfg` as argument instead of `(struct programmer_cfg *)cfg`. -- To view, visit https://review.coreboot.org/c/flashrom/+/71623 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7058a693e714a6966a842ae97cc8da7296e63e5e Gerrit-Change-Number: 71623 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Mon, 02 Jan 2023 13:08:39 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit January 2023