October 2022 - flashrom-gerrit

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

13 Oct '22

Attention is currently required from: Nico Huber, Anastasia Klimchuk. Sergii Dmytruk 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 3: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/68179/comment/47c659b8_84587c57 PS3, Line 11: TICKET > Is this a convention that I missed? Why not the common tag form with […] Anastasia asked to add this line, I don't know the origins of things like "TEST="/"BUG="/"BRANCH=", I guess Google uses it. -- 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: 3 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: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Thu, 13 Oct 2022 15:37:35 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: comment

1 0

[S] 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 3: Code-Review+1 (2 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/68179/comment/fd560a32_58c17f85 PS2, Line 11: systems, that also prevents running commands. > Changed to B, I like it more than X. Nice, thank you! :) Commit Message: https://review.coreboot.org/c/flashrom/+/68179/comment/5d0041f2_1e582e51 PS3, Line 11: TICKET Is this a convention that I missed? Why not the common tag form with a colon, i.e. `Ticket: `? -- 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: 3 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 12:41:25 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> 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 5: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/68221/comment/a9ea3546_618521d2 PS4, Line 10: preventing confusion > I guess it depends on the meaning of `NT`. I took it as "unknown" which can also include `NA`. […] Wrt. if this prevents confusion, I don't think it matters how we define `NT`. What causes or prevents confusion is what we print, i.e. "WP operations are not implemented for this chip" vs. "this chip doesn't support WP". I'm not sure if that is enough. We would still print the former in the NT case, right? Which is the most common and most confusing state, IMHO. And if we'd change the text, how could we distinguish "unknown" from "known WP support but not implemented"? I'm not against this change, just not sure if it changes something (wrt. usability). -- 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: 5 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 12:36:29 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-MessageType: comment

1 0

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

13 Oct '22

Attention is currently required from: Nico Huber, Anastasia Klimchuk. Sergii Dmytruk 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/7ab80d4e_aec58d6e PS2, Line 11: systems, that also prevents running commands. > How about B for block protection? I assume that's what we're going to […] Changed to B, I like it more than X. -- 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: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Thu, 13 Oct 2022 12:01:52 +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]: flashchips.c: mark WP of 9 entries as tested
by Sergii Dmytruk (Code Review) 13 Oct '22

13 Oct '22

Attention is currently required from: Nico Huber, Kamil Pokornicki, Sergii Dmytruk. Hello build bot (Jenkins), Nico Huber, Maciej Pijanowski, Kamil Pokornicki, Edward O'Callaghan, Angel Pons, Nikolai Artemiev, Anastasia Klimchuk, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/68180 to look at the new patch set (#6). Change subject: flashchips.c: mark WP of 9 entries as tested ...................................................................... flashchips.c: mark WP of 9 entries as tested This is based on information from: * commit a850fd0aa8054a1125a9231fa3317428f15900f4 - GD25LQ128C/GD25LQ128D/GD25LQ128E - GD25LQ64(B) - GD25Q127C/GD25Q128C - GD25Q256D/GD25Q256E - GD25Q64(B) * commit a8204dd34d90ac9ab2783e1dd486ec781d4c0dba - GD25Q32(B) * commit 7b4c4f36113c4b7ed5c985d4cf51733639e69bf8 - W25Q64BV/W25Q64CV/W25Q64FV * https://github.com/Dasharo/dasharo-issues/issues/67 - W25Q128.V..M * https://github.com/Dasharo/flashrom/pull/8 - W25Q64.W Change-Id: I090188bad568885f78778e7fc7d8dbe20fb2445f Signed-off-by: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Tested-by: Nikolai Artemiev <nartemiev(a)google.com> Tested-by: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Tested-by: Kamil Pokornicki <kamil.pokornicki(a)3mdeb.com> Tested-by: Przemyslaw Banasiak <przemyslaw.banasiak(a)3mdeb.com> Tested-by: Maciej Pijanowski <maciej.pijanowski(a)3mdeb.com> Signed-off-by: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> --- M flashchips.c 1 file changed, 41 insertions(+), 9 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/80/68180/6 -- To view, visit https://review.coreboot.org/c/flashrom/+/68180 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I090188bad568885f78778e7fc7d8dbe20fb2445f Gerrit-Change-Number: 68180 Gerrit-PatchSet: 6 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: Kamil Pokornicki <kamil.pokornicki(a)3mdeb.com> Gerrit-Reviewer: Maciej Pijanowski <maciej.pijanowski(a)3mdeb.com> 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: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Kamil Pokornicki <kamil.pokornicki(a)3mdeb.com> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-MessageType: newpatchset

1 0

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

13 Oct '22

Attention is currently required from: Anastasia Klimchuk, Sergii Dmytruk. Hello build bot (Jenkins), Nico Huber, Edward O'Callaghan, Angel Pons, Nikolai Artemiev, Anastasia Klimchuk, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/68179 to look at the new patch set (#3). Change subject: flash.h: extend `struct tested` with .wp field ...................................................................... flash.h: extend `struct tested` with .wp field Using "B" letter for "block protection" in TEST_* macros. TICKET=https://ticket.coreboot.org/issues/377 Change-Id: I791400889159bc6f305fb05f3e2dd9a90dbe18a4 Signed-off-by: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> --- M cli_common.c M include/flash.h M include/libflashrom.h 3 files changed, 37 insertions(+), 12 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/79/68179/3 -- 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: 3 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-MessageType: newpatchset

1 0

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

13 Oct '22

Attention is currently required from: Nico Huber, Nikolai Artemiev. Sergii Dmytruk 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/c60c949e_e6df73be PS4, Line 10: preventing confusion > (I only had a quick look at the code, so sorry if I'm off the rails here.) […] I guess it depends on the meaning of `NT`. I took it as "unknown" which can also include `NA`. Setting `NA` correctly for all the chips would also produce lots of new chip entries. I would specify `NA` where it's well known to have no WP and left "chip unsupported" message for the rest. If you find this problematic, can drop this change. -- 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: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Thu, 13 Oct 2022 11:55:37 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: libflashrom: Allow getting the progress_state from the flashctx
by Nico Huber (Code Review) 13 Oct '22

13 Oct '22

Attention is currently required from: Felix Singer, Richard Hughes, Edward O'Callaghan, Angel Pons, Daniel Campello, Anastasia Klimchuk. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/64663 ) Change subject: libflashrom: Allow getting the progress_state from the flashctx ...................................................................... Patch Set 2: (1 comment) File include/libflashrom.h: https://review.coreboot.org/c/flashrom/+/64663/comment/baf5b056_0183b674 PS2, Line 85: typedef void(flashrom_progress_callback)(struct flashrom_flashctx *flashctx); > Nico, if we understand you correctly, flashctx should be kept internal, and the API should have a di […] Yes about `flashctx`. But with `user_data` I mean the generic (opaque) object passed around as void pointer. This is a common concept for callback functions. Beside the function, the API user passes any information that it likes to associate with the callback function calls. The implementation behind the API, that calls the callback, has no idea about this data, hence just a void pointer, no struct. The `flashctx` may or may not be needed by the callback as well. We can either have it as parameter (which would probably be unused by callback implementations), or let the API user handle it: they already have it, and can store it in their `user_data` object if needed, or have things simple if they don't need it. Quite generally, I think these APIs are best to be discussed with their users (to reach a reasoable dependency inversion [1]). Looking at [2] for instance, I see no need for `flashctx`. Other use cases may differ, though. [1] https://en.wikipedia.org/wiki/Dependency_inversion_principle (not necessarily worth reading more than the basic idea) [2] https://github.com/fwupd/fwupd/pull/4675 -- To view, visit https://review.coreboot.org/c/flashrom/+/64663 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I322bf56ff92f7b4d0ffc92768e9f0cdf7cb82010 Gerrit-Change-Number: 64663 Gerrit-PatchSet: 2 Gerrit-Owner: Richard Hughes <richard(a)hughsie.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Daniel Campello <campello(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Richard Hughes <richard(a)hughsie.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Daniel Campello <campello(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Thu, 13 Oct 2022 11:37:41 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> 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

[M] Change in flashrom[master]: tree: provide flashrom context into programmer_delay()
by Edward O'Callaghan (Code Review) 13 Oct '22

13 Oct '22

Attention is currently required from: Felix Singer, Thomas Heijligen, Anastasia Klimchuk, Alexander Goncharov. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/66373 ) Change subject: tree: provide flashrom context into programmer_delay() ...................................................................... Patch Set 11: (1 comment) File bitbang_spi.c: https://review.coreboot.org/c/flashrom/+/66373/comment/e77e5c28_eac9ecdf PS11, Line 80: flash, A lot of lines changed in this patch with some plumbing actually don't need that much of a change, you can just pass `NULL`. The only two programmers that have special delays hooked are: ``` ch341a_spi.c: .delay = ch341a_spi_delay, serprog.c: .delay = serprog_delay, ``` so all other programmers can just use `NULL` as the context which allows them to default to 'internal_delay()' and only core logic requires the plumbing like for spi25.c, spi25_statusreg.c, s25f.c, jedec.c && flashrom.c A subsequent patch could be prepared that greps for all the `programmer_delay(NULL, [..])` instances and just does a find and replace with `internal_delay([..])`. Almost all programmers do not need such indirection where they are simply just trying to call `internal_delay()`. -- To view, visit https://review.coreboot.org/c/flashrom/+/66373 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ibb0bce26ce2052853ee52158d7ba742967a9e229 Gerrit-Change-Number: 66373 Gerrit-PatchSet: 11 Gerrit-Owner: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(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-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Comment-Date: Thu, 13 Oct 2022 11:17:09 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: [WIP] test_build.sh: Rework programmer selection using Meson
by Nico Huber (Code Review) 13 Oct '22

13 Oct '22

Attention is currently required from: Felix Singer, Angel Pons. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68068 ) Change subject: [WIP] test_build.sh: Rework programmer selection using Meson ...................................................................... Patch Set 4: (3 comments) File meson.build: https://review.coreboot.org/c/flashrom/+/68068/comment/55553da6_3c5c0548 PS4, Line 641: programmers_available = [] Is it possible to concatenate a string instead? so that we don't have to parse things like the colons later in the shell? File test_build.sh: https://review.coreboot.org/c/flashrom/+/68068/comment/3c883fb2_d4cf34af PS4, Line 49: env Does it run in a shell? Meson docs don't seem to specify this :-/ If it does, we could make this much shorter, like `echo \$FLASHROM_ALL_PROGRAMMERS`. If it doesn't, we could still squash the rest of the pipe into a single sed call, e.g. `sed -n -e 's/:/ /g' -e 's/^FLASHROM_ALL_PROGRAMMERS=//p'`. https://review.coreboot.org/c/flashrom/+/68068/comment/d2e20453_3ac23c3c PS4, Line 53: echo ${programmer_list} We don't need this indirection with the variable. As there is no other output expected, we can let the meson command / pipe above print directly. -- To view, visit https://review.coreboot.org/c/flashrom/+/68068 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9f41ce2ff219c70c3c05a90134291b01a084c859 Gerrit-Change-Number: 68068 Gerrit-PatchSet: 4 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> 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: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Thu, 13 Oct 2022 10:51:51 +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