Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons, Nikolai Artemiev, Sergii Dmytruk.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58479 )
Change subject: libflashrom,writeprotect: add flashrom_wp_{read,write}_chip_config()
......................................................................
Patch Set 16: Code-Review+1
(1 comment)
File writeprotect.c:
https://review.coreboot.org/c/flashrom/+/58479/comment/9c12bade_02e68c6d
PS16, Line 113: }
Loop body is one line so braces around loop body are not needed. And same for the loop right below. Sorry if I missed this in earlier patchsets.
--
To view, visit https://review.coreboot.org/c/flashrom/+/58479
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I3ad25708c3321b8fb0216c3eaf6ffc07616537ad
Gerrit-Change-Number: 58479
Gerrit-PatchSet: 16
Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.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: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Comment-Date: Mon, 29 Nov 2021 23:22:28 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk.
Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/59529 )
Change subject: spi25_statusreg: delete read_status_register()
......................................................................
Patch Set 6:
(3 comments)
File it87spi.c:
https://review.coreboot.org/c/flashrom/+/59529/comment/7fd84c15_ca22204d
PS4, Line 136: while (true)
> I am curious, can this ever become an infinite loop? I understand it was like this before ;)
It can, though without knowing more about the programmer I'm not sure what a reasonable timeout would be. Probably safest to leave it for now but I'll add a comment.
https://review.coreboot.org/c/flashrom/+/59529/comment/e5f86707_0a190c6c
PS4, Line 139: return 1;
> Can we propagate error code from spi_read_register?
Done
https://review.coreboot.org/c/flashrom/+/59529/comment/73388e5e_65903566
PS4, Line 288: return 1;
> Looks like another opportunity to propagate error code!
Done
--
To view, visit https://review.coreboot.org/c/flashrom/+/59529
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I146b4b5439872e66c5d33e156451a729d248c7da
Gerrit-Change-Number: 59529
Gerrit-PatchSet: 6
Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.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: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Mon, 29 Nov 2021 05:26:49 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk.
Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/59528 )
Change subject: spi25_statusreg: inline spi_write_register_flag()
......................................................................
Patch Set 5:
(1 comment)
File spi25_statusreg.c:
https://review.coreboot.org/c/flashrom/+/59528/comment/f168b243_1292f4d2
PS3, Line 82: }
> As per coding style, if braces are needed in one of the branches, they should be in all branches. […]
Done
--
To view, visit https://review.coreboot.org/c/flashrom/+/59528
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I4996b0848d0ed09032bad2ab13ab1f40bbfc0304
Gerrit-Change-Number: 59528
Gerrit-PatchSet: 5
Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.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: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Mon, 29 Nov 2021 05:26:39 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Paul Menzel, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, Sergii Dmytruk.
Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58738 )
Change subject: cli_classic: add writeprotect CLI
......................................................................
Patch Set 14:
(3 comments)
File cli_classic.c:
https://review.coreboot.org/c/flashrom/+/58738/comment/36f2bac5_9e5228ba
PS7, Line 63: mode
> Since permanent and powercycle protection are very rarely supported and not very useful, I think we […]
Resolving
File cli_classic.c:
https://review.coreboot.org/c/flashrom/+/58738/comment/aad5221e_b715a1fa
PS12, Line 190: wp_cli
> I remember there was a conversation about cli vs libflashrom in some other patch in this chain, I ha […]
There is some non-cli logic here (mostly for verifying configuration after writing), but moving it into libflashrom makes it more difficult to implement properly since we often need to write things to stdout/stderr and we shouldn't do that from libflashrom.
The number of libflashrom calls required to do any one operation is already quite small so I don't think we save much by moving more of the code.
https://review.coreboot.org/c/flashrom/+/58738/comment/178f63e5_af0e9ed9
PS12, Line 890: ret = wp_cli(
: fill_flash,
: enable_wp,
: disable_wp,
: print_wp_status,
: print_available_ranges,
: set_wp_range,
: wp_start,
: wp_len,
: wp_mode_opt
: );
> Does this fit into 112 chars? If yes let's have it one line?
It would be around 150 chars, it's probably quite a bit more readable with each parameter on its own line.
--
To view, visit https://review.coreboot.org/c/flashrom/+/58738
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I499f521781ee8999921996517802c0c0c641d869
Gerrit-Change-Number: 58738
Gerrit-PatchSet: 14
Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.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: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Comment-Date: Mon, 29 Nov 2021 05:26:34 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nikolai Artemiev <nartemiev(a)google.com>
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, Sergii Dmytruk.
Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58479 )
Change subject: libflashrom,writeprotect: add flashrom_wp_{read,write}_chip_config()
......................................................................
Patch Set 15:
(1 comment)
File libflashrom.h:
https://review.coreboot.org/c/flashrom/+/58479/comment/038a1685_c2573adc
PS11, Line 4: 2010
> I saw in other places a list of years, so maybe "2010,2021" is a solution here.
I think the best option is probably to just keep the first copyright year, since we also have git for the exact history. We are adding most of the lines after 2010 but just the first date should be sufficient for the copyright notice.
--
To view, visit https://review.coreboot.org/c/flashrom/+/58479
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I3ad25708c3321b8fb0216c3eaf6ffc07616537ad
Gerrit-Change-Number: 58479
Gerrit-PatchSet: 15
Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.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: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Comment-Date: Mon, 29 Nov 2021 05:26:13 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Comment-In-Reply-To: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-MessageType: comment