- flashrom-gerrit - mail.coreboot.org

[M] Change in flashrom[master]: ichspi.c: Read chip ID and use it to populate `flash->chip`
by Nikolai Artemiev (Code Review) 06 Dec '22

06 Dec '22

Attention is currently required from: Subrata Banik, Edward O'Callaghan, Angel Pons. Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/69195 ) Change subject: ichspi.c: Read chip ID and use it to populate `flash->chip` ...................................................................... Patch Set 11: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/69195/comment/7a0a5f8c_3a0b3a9d PS2, Line 7: ichspi.c: Read chip ID and use it to identify chip > In flashrom.c the top level of the CFG is: […] I've made flash identification bail out and print a message if there's more than one chip now. Retested on dedede. -- To view, visit https://review.coreboot.org/c/flashrom/+/69195 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia408e1e45dc6f53c0934afd6558e301abfa48ee6 Gerrit-Change-Number: 69195 Gerrit-PatchSet: 11 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Wed, 07 Dec 2022 00:11:50 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: ichspi.c: Read chip ID and use it to populate `flash->chip`
by Nikolai Artemiev (Code Review) 06 Dec '22

06 Dec '22

Attention is currently required from: Subrata Banik, Angel Pons, Nikolai Artemiev. Hello build bot (Jenkins), Subrata Banik, Edward O'Callaghan, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/69195 to look at the new patch set (#11). Change subject: ichspi.c: Read chip ID and use it to populate `flash->chip` ...................................................................... ichspi.c: Read chip ID and use it to populate `flash->chip` Read the flash chip vendor/device ID using hardware sequencing, find the corresponding flashchip entry, and copy it over to `flash->chip`. Identifying the chip was not previously required as ICH hardware sequencing handles chip-level details related to read/write/erase ops. However writeprotect operations require the chip entry to be identified so that chip->reg_bits can be used to compute status register values. BUG=b:253715389,b:253713774 BRANCH=none TEST=flashrom on dedede (JSL) identifies "W25Q128.V..M" chip TEST=flashrom -{r,v} on dedede TEST=write/erase bios region on dedede: flashrom -{E,w} --layout <(echo '0x381000:0xffffff bios') -i bios Change-Id: Ia408e1e45dc6f53c0934afd6558e301abfa48ee6 Signed-off-by: Nikolai Artemiev <nartemiev(a)google.com> --- M ichspi.c 1 file changed, 95 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/95/69195/11 -- To view, visit https://review.coreboot.org/c/flashrom/+/69195 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia408e1e45dc6f53c0934afd6558e301abfa48ee6 Gerrit-Change-Number: 69195 Gerrit-PatchSet: 11 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-MessageType: newpatchset

1 0

[S] Change in flashrom[master]: flashchips.c: remove WREN from GD25Q256D enter 4BA sequence
by Nikolai Artemiev (Code Review) 06 Dec '22

06 Dec '22

Attention is currently required from: Edward O'Callaghan, Nikolai Artemiev. Hello build bot (Jenkins), Nico Huber, Edward O'Callaghan, Anastasia Klimchuk, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/70342 to look at the new patch set (#2). Change subject: flashchips.c: remove WREN from GD25Q256D enter 4BA sequence ...................................................................... flashchips.c: remove WREN from GD25Q256D enter 4BA sequence As noted in a comment on `commit 86fc9cf7ab221bc54ef6f10252e296fc2d7a22d2`, the GD25Q256D datasheet indicates that the chip does not require a WREN command to enter 4BA mode. Testing has confirmed that a WREN command is not required, so change the flashchip feature flags from FEATURE_4BA_WREN to FEATURE_4BA. Ticket: https://ticket.coreboot.org/issues/356 BUG=none BRANCH=none TEST=read/write/erase/verify GD25Q256D flash with FT2232H programmer Change-Id: I96e48933f33c52c0d10a0d4cb7f7e07c1fceab99 Signed-off-by: Nikolai Artemiev <nartemiev(a)google.com> --- M flashchips.c 1 file changed, 25 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/42/70342/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/70342 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I96e48933f33c52c0d10a0d4cb7f7e07c1fceab99 Gerrit-Change-Number: 70342 Gerrit-PatchSet: 2 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[master]: libflashrom.c: Invert if conditions to improve the readability
by Felix Singer (Code Review) 06 Dec '22

06 Dec '22

Attention is currently required from: Edward O'Callaghan. Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68153 ) Change subject: libflashrom.c: Invert if conditions to improve the readability ...................................................................... Patch Set 6: (1 comment) File libflashrom.c: https://review.coreboot.org/c/flashrom/+/68153/comment/17e2a7af_3968d9ab PS3, Line 107: (enum flashrom_test_state) > There are duplicate definitions for this. `test_state` in flash. […] So yes, the cast is needed for now. I'm setting this to resolved since it's unrelated. -- To view, visit https://review.coreboot.org/c/flashrom/+/68153 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I4021d8802cd041dcca29a226af0798ebd9c5a81b Gerrit-Change-Number: 68153 Gerrit-PatchSet: 6 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Tue, 06 Dec 2022 23:03:34 +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> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: libflashrom.c: Invert if conditions to improve the readability
by Felix Singer (Code Review) 06 Dec '22

06 Dec '22

Attention is currently required from: Edward O'Callaghan. Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68153 ) Change subject: libflashrom.c: Invert if conditions to improve the readability ...................................................................... Patch Set 6: (2 comments) File libflashrom.c: https://review.coreboot.org/c/flashrom/+/68153/comment/c8f0df98_93257f04 PS3, Line 103: (; > optional: while we are here could we just scope the indexer into the loop construct. […] Done https://review.coreboot.org/c/flashrom/+/68153/comment/aee63750_06fa30dc PS3, Line 107: (enum flashrom_test_state) > is this cast needed? There are duplicate definitions for this. `test_state` in flash.h and `flashrom_test_state` in libflashrom.h. The latter is only used for the functions I'm cleaning up here. -- To view, visit https://review.coreboot.org/c/flashrom/+/68153 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I4021d8802cd041dcca29a226af0798ebd9c5a81b Gerrit-Change-Number: 68153 Gerrit-PatchSet: 6 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Tue, 06 Dec 2022 22:57:22 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: libflashrom.c: Remove superfluous type casts
by Felix Singer (Code Review) 06 Dec '22

06 Dec '22

Felix Singer has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/70391 ) Change subject: libflashrom.c: Remove superfluous type casts ...................................................................... libflashrom.c: Remove superfluous type casts Change-Id: I528245511f0620a771863d3f8f7a74add7299948 Signed-off-by: Felix Singer <felixsinger(a)posteo.net> --- M libflashrom.c 1 file changed, 16 insertions(+), 12 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/91/70391/1 diff --git a/libflashrom.c b/libflashrom.c index 44f297f..c6674e3 100644 --- a/libflashrom.c +++ b/libflashrom.c @@ -102,14 +102,10 @@ for (unsigned int i = 0; i < flashchips_size; ++i) { supported_flashchips[i].vendor = flashchips[i].vendor; supported_flashchips[i].name = flashchips[i].name; - supported_flashchips[i].tested.erase = - (enum flashrom_test_state)flashchips[i].tested.erase; - supported_flashchips[i].tested.probe = - (enum flashrom_test_state)flashchips[i].tested.probe; - supported_flashchips[i].tested.read = - (enum flashrom_test_state)flashchips[i].tested.read; - supported_flashchips[i].tested.write = - (enum flashrom_test_state)flashchips[i].tested.write; + supported_flashchips[i].tested.erase = flashchips[i].tested.erase; + supported_flashchips[i].tested.probe = flashchips[i].tested.probe; + supported_flashchips[i].tested.read = flashchips[i].tested.read; + supported_flashchips[i].tested.write = flashchips[i].tested.write; supported_flashchips[i].total_size = flashchips[i].total_size; } @@ -139,8 +135,7 @@ for (int i = 0; i < boards_known_size; ++i) { supported_boards[i].vendor = binfo[i].vendor; supported_boards[i].name = binfo[i].name; - supported_boards[i].working = - (enum flashrom_test_state) binfo[i].working; + supported_boards[i].working = binfo[i].working; } return supported_boards; @@ -174,8 +169,7 @@ supported_chipsets[i].chipset = chipset[i].device_name; supported_chipsets[i].vendor_id = chipset[i].vendor_id; supported_chipsets[i].chipset_id = chipset[i].device_id; - supported_chipsets[i].status = - (enum flashrom_test_state) chipset[i].status; + supported_chipsets[i].status = chipset[i].status; } return supported_chipsets; -- To view, visit https://review.coreboot.org/c/flashrom/+/70391 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I528245511f0620a771863d3f8f7a74add7299948 Gerrit-Change-Number: 70391 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> Gerrit-MessageType: newchange

1 0

[M] Change in flashrom[master]: libflashrom.c: Invert if conditions to improve the readability
by Felix Singer (Code Review) 06 Dec '22

06 Dec '22

Attention is currently required from: Felix Singer. Hello build bot (Jenkins), Edward O'Callaghan, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/68153 to look at the new patch set (#6). Change subject: libflashrom.c: Invert if conditions to improve the readability ...................................................................... libflashrom.c: Invert if conditions to improve the readability Invert some if conditions to improve the readability of the code. Instead of running some code if the specific condition applies, error out early and reduce the indentation levels. Also, while at it, move the initializers for these for-loops iterators into their constructs. They are only used by them. Change-Id: I4021d8802cd041dcca29a226af0798ebd9c5a81b Signed-off-by: Felix Singer <felixsinger(a)posteo.net> --- M libflashrom.c 1 file changed, 53 insertions(+), 36 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/53/68153/6 -- To view, visit https://review.coreboot.org/c/flashrom/+/68153 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I4021d8802cd041dcca29a226af0798ebd9c5a81b Gerrit-Change-Number: 68153 Gerrit-PatchSet: 6 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[master]: libflashrom.c: Invert if conditions to improve the readability
by Felix Singer (Code Review) 06 Dec '22

06 Dec '22

Attention is currently required from: Felix Singer. Hello build bot (Jenkins), Edward O'Callaghan, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/68153 to look at the new patch set (#5). Change subject: libflashrom.c: Invert if conditions to improve the readability ...................................................................... libflashrom.c: Invert if conditions to improve the readability Invert some if conditions to improve the readability of the code. Instead of running some code if the specific condition applies, error out early and reduce the indentation levels. Also, while at it, move the initializers for these for-loops iterators into their constructs. They are only used by them. Change-Id: I4021d8802cd041dcca29a226af0798ebd9c5a81b Signed-off-by: Felix Singer <felixsinger(a)posteo.net> --- M libflashrom.c 1 file changed, 53 insertions(+), 36 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/53/68153/5 -- To view, visit https://review.coreboot.org/c/flashrom/+/68153 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I4021d8802cd041dcca29a226af0798ebd9c5a81b Gerrit-Change-Number: 68153 Gerrit-PatchSet: 5 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[master]: libflashrom.c: Invert if conditions to improve the readability
by Felix Singer (Code Review) 06 Dec '22

06 Dec '22

Attention is currently required from: Felix Singer. Hello build bot (Jenkins), Edward O'Callaghan, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/68153 to look at the new patch set (#4). Change subject: libflashrom.c: Invert if conditions to improve the readability ...................................................................... libflashrom.c: Invert if conditions to improve the readability Invert some if conditions to improve the readability of the code. Instead of running some code if the specific condition applies, error out early and reduce the indentation levels. Also, while at it, move the initializers for these for-loops iterators into their constructs. They are only used by them. Change-Id: I4021d8802cd041dcca29a226af0798ebd9c5a81b Signed-off-by: Felix Singer <felixsinger(a)posteo.net> --- M libflashrom.c 1 file changed, 53 insertions(+), 36 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/53/68153/4 -- To view, visit https://review.coreboot.org/c/flashrom/+/68153 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I4021d8802cd041dcca29a226af0798ebd9c5a81b Gerrit-Change-Number: 68153 Gerrit-PatchSet: 4 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[master]: util: add bash completion script
by Alexander Goncharov (Code Review) 06 Dec '22

06 Dec '22

Attention is currently required from: Felix Singer, Thomas Heijligen. Alexander Goncharov has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68247 ) Change subject: util: add bash completion script ...................................................................... Patch Set 10: (7 comments) Patchset: PS10: As part of rebasing, I replaced --image (this option is deprecated) with --include File Makefile: https://review.coreboot.org/c/flashrom/+/68247/comment/ce44ac9b_5eb3e208 PS9, Line 589: ACTIVE_PROGRAMMERS += internal > It took a little until I understood this programmer list is needed for the substitution. […] Yes, it should be mentioned, thank you. Done. https://review.coreboot.org/c/flashrom/+/68247/comment/c604bafa_2b814ac7 PS9, Line 1032: $(PROGRAM).8 $(PROGRAM).8.html $(PROGRAM).bash $(BUILD_DETAILS_FILE) > One tab too much What do you mean? I just added `$(PROGRAM).bash` https://review.coreboot.org/c/flashrom/+/68247/comment/2ed7f8fe_81105ed9 PS9, Line 1041: .bash > That's just the file in the source directory. […] From [Bash Completion FAQ](https://github.com/scop/bash-completion/blob/master/README.md#faq): ``` The completion filename for command `foo` in this directory should be either `foo`, or `foo.bash`. ``` So, we only have two naming options available. I cannot use a name without the suffix due to a name conflict (As Felix mentioned above, the flashrom binary has the same name). Unfortunately, a custom prefix isn't supported, so we have no choice. Felix, your idea is what I wanted to do initially. But there's a problem with meson, please have a look at another Thomas comment #646. File meson.build: https://review.coreboot.org/c/flashrom/+/68247/comment/e4d7faf8_5c6d71e0 PS9, Line 624: if get_option('classic_cli').disabled() > When `get_option('bash_completion').auto()` and `get_option('classic_cli'). […] Thanks for catching it! https://review.coreboot.org/c/flashrom/+/68247/comment/697a8222_540120ac PS9, Line 646: .bash > same here No way. If the output file is `flashrom`, the bash-completion file will be overwritten by the executable. `install` command (in the makefile) lets me choose DESTINATION, and meson only copies the file in one place - `install_dir`/`output`. File util/flashrom.bash-completion.tmpl: https://review.coreboot.org/c/flashrom/+/68247/comment/37b51484_679828be PS9, Line 43: OPTS="--help > Missing \ at the end of each line. It makes no difference for this task. Also, those `\` at the end of lines look ugly :( -- To view, visit https://review.coreboot.org/c/flashrom/+/68247 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie68bc91c3cea4de2ffdbeffd07e48edd8d5590e1 Gerrit-Change-Number: 68247 Gerrit-PatchSet: 10 Gerrit-Owner: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> 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: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Comment-Date: Tue, 06 Dec 2022 21:30:11 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net> Comment-In-Reply-To: Thomas Heijligen <src(a)posteo.de> Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit