October 2022 - flashrom-gerrit

[M] Change in flashrom[master]: [RFC] tests: Replace pre-processor ifdefs with weak functions
by Felix Singer (Code Review) 13 Oct '22

13 Oct '22

Attention is currently required from: Nico Huber, Thomas Heijligen, Edward O'Callaghan, Anastasia Klimchuk. Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68359 ) Change subject: [RFC] tests: Replace pre-processor ifdefs with weak functions ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: > Have you evaluated the idea to handle things in plain C? i.e. passing strings […] I'm still playing around with it. So yes :) -- To view, visit https://review.coreboot.org/c/flashrom/+/68359 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3e25c5f1afa889566960f295d8ad9fb3a8c99b22 Gerrit-Change-Number: 68359 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> 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: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> 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: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Thu, 13 Oct 2022 10:41:57 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: [RFC] tests: Replace pre-processor ifdefs with weak functions
by Nico Huber (Code Review) 13 Oct '22

13 Oct '22

Attention is currently required from: Felix Singer, Thomas Heijligen, Edward O'Callaghan, Anastasia Klimchuk. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68359 ) Change subject: [RFC] tests: Replace pre-processor ifdefs with weak functions ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: Have you evaluated the idea to handle things in plain C? i.e. passing strings instead of global programmer objects for the programmer init? -- To view, visit https://review.coreboot.org/c/flashrom/+/68359 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3e25c5f1afa889566960f295d8ad9fb3a8c99b22 Gerrit-Change-Number: 68359 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> 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: Thomas Heijligen <src(a)posteo.de> 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: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Thu, 13 Oct 2022 10:32:18 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: [WIP] Allow writing to stdout with --read
by Angel Pons (Code Review) 13 Oct '22

13 Oct '22

Attention is currently required from: Felix Singer. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68368 ) Change subject: [WIP] Allow writing to stdout with --read ...................................................................... Patch Set 1: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/68368/comment/a02bc3ae_92596e6e PS1, Line 7: [WIP] Allow writing to stdout with --read 1. What about regular logging? 2. It would be good to not spam stdout if it's not redirected anywhere. -- To view, visit https://review.coreboot.org/c/flashrom/+/68368 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If11a2fa4de843446aa831418b40df3699eddc16f Gerrit-Change-Number: 68368 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Comment-Date: Thu, 13 Oct 2022 09:52:02 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: [WIP] Allow writing to stdout with --read
by Felix Singer (Code Review) 13 Oct '22

13 Oct '22

Felix Singer has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/68368 ) Change subject: [WIP] Allow writing to stdout with --read ...................................................................... [WIP] Allow writing to stdout with --read Signed-off-by: Felix Singer <felixsinger(a)posteo.net> Change-Id: If11a2fa4de843446aa831418b40df3699eddc16f --- M helpers_fileio.c 1 file changed, 17 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/68/68368/1 diff --git a/helpers_fileio.c b/helpers_fileio.c index cb7675d..43d39af 100644 --- a/helpers_fileio.c +++ b/helpers_fileio.c @@ -92,7 +92,13 @@ msg_gerr("No filename specified.\n"); return 1; } - if ((image = fopen(filename, "wb")) == NULL) { + + if (!strcmp(filename, "-")) + image = fdopen(fileno(stdout), "wb"); + else + image = fopen(filename, "wb"); + + if (!image) { msg_gerr("Error: opening file \"%s\" failed: %s\n", filename, strerror(errno)); return 1; } -- To view, visit https://review.coreboot.org/c/flashrom/+/68368 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If11a2fa4de843446aa831418b40df3699eddc16f Gerrit-Change-Number: 68368 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> Gerrit-MessageType: newchange

1 0

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

13 Oct '22

Attention is currently required from: Nikolai Artemiev, Sergii Dmytruk. Angel Pons 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 4: Code-Review+1 -- 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: 4 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: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Thu, 13 Oct 2022 09:48:39 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

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

13 Oct '22

Attention is currently required from: Nikolai Artemiev, Sergii Dmytruk. Nico Huber 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 4: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/68221/comment/30433c00_3d6b54e5 PS4, Line 10: preventing confusion (I only had a quick look at the code, so sorry if I'm off the rails here.) Doesn't this need an update to the chip database? i.e. explicitly setting the `NA` for all chips that don't have any WP? -- 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: 4 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: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Thu, 13 Oct 2022 09:47:10 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

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

13 Oct '22

Attention is currently required from: Anastasia Klimchuk, Sergii Dmytruk. Nico Huber 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 2: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/68179/comment/7b73bcb5_6f23b69e PS2, Line 11: systems, that also prevents running commands. How about B for block protection? I assume that's what we're going to test? If we'd add other protection modes in the future, those could get their own test status. -- 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: 2 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: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Thu, 13 Oct 2022 09:31:36 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: debug_raiden_spi: Remove fixme with explanation
by Angel Pons (Code Review) 13 Oct '22

13 Oct '22

Attention is currently required from: Edward O'Callaghan, Liam Flaherty, Nikolai Artemiev. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68364 ) Change subject: debug_raiden_spi: Remove fixme with explanation ...................................................................... Patch Set 1: Code-Review+1 (1 comment) File raiden_debug_spi.c: https://review.coreboot.org/c/flashrom/+/68364/comment/770a5942_8765a5bd PS1, Line 354: // rather than looking for a device with a specific vid:pid. > I have two thoughts on how to address the FIXME. […] Either approach seems fine. But if you want to go with `1.`, please use C-style comments for consistency with the rest of the file: ``` /* * Table is empty as raiden_debug_spi matches against connected USB * devices, rather than looking for a device with a specific vid:pid. */ ``` Also changed the line wrapping so that both lines have about the same length. It's not a rule/guideline in our coding style, but it looks better 😄 -- To view, visit https://review.coreboot.org/c/flashrom/+/68364 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I43e364c02f42dd499d3c9ca3e0a03ead673da3e6 Gerrit-Change-Number: 68364 Gerrit-PatchSet: 1 Gerrit-Owner: Liam Flaherty <liamflaherty(a)chromium.org> 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: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Liam Flaherty <liamflaherty(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Thu, 13 Oct 2022 08:59:33 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Liam Flaherty <liamflaherty(a)chromium.org> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: debug_raiden_spi: Remove fixme with explanation
by Anastasia Klimchuk (Code Review) 13 Oct '22

13 Oct '22

Attention is currently required from: Edward O'Callaghan, Liam Flaherty, Nikolai Artemiev. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68364 ) Change subject: debug_raiden_spi: Remove fixme with explanation ...................................................................... Patch Set 1: (2 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/68364/comment/7195d73e_5ff3438a PS1, Line 7: debug_raiden_spi raiden_debug_spi (should be the programmer name) https://review.coreboot.org/c/flashrom/+/68364/comment/52ef327c_e45d1aaa PS1, Line 9: The raiden_debug_spi programmer will query the connected USB devices and : match against criteria other than the pid rather than iterate through the : vid:pid table. Max width of commit message is 72 chars (see https://www.flashrom.org/Development_Guidelines#Commit_message) So you need to wrap -- To view, visit https://review.coreboot.org/c/flashrom/+/68364 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I43e364c02f42dd499d3c9ca3e0a03ead673da3e6 Gerrit-Change-Number: 68364 Gerrit-PatchSet: 1 Gerrit-Owner: Liam Flaherty <liamflaherty(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Liam Flaherty <liamflaherty(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Thu, 13 Oct 2022 03:27:02 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: debug_raiden_spi: Remove fixme with explanation
by Liam Flaherty (Code Review) 13 Oct '22

13 Oct '22

Attention is currently required from: Edward O'Callaghan, Anastasia Klimchuk, Nikolai Artemiev. Liam Flaherty has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68364 ) Change subject: debug_raiden_spi: Remove fixme with explanation ...................................................................... Patch Set 1: (1 comment) File raiden_debug_spi.c: https://review.coreboot.org/c/flashrom/+/68364/comment/c61c7680_d3df8e7e PS1, Line 354: // rather than looking for a device with a specific vid:pid. I have two thoughts on how to address the FIXME. 1. Leave the code as is and simply leave the dev_entry table empty with a comment, as it appears here. 2. Populate the `revs_raiden` table to include all of the vid:pid pairs for all the devices that use this programmer. This would also involve a change to the `raiden_debug_spi_init` method to read this table rather than the current matching logic. Thoughts? -- To view, visit https://review.coreboot.org/c/flashrom/+/68364 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I43e364c02f42dd499d3c9ca3e0a03ead673da3e6 Gerrit-Change-Number: 68364 Gerrit-PatchSet: 1 Gerrit-Owner: Liam Flaherty <liamflaherty(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Thu, 13 Oct 2022 02:04:56 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit October 2022