October 2022 - flashrom-gerrit

[M] Change in flashrom[master]: flashchips.c: mark WP of 9 entries as tested
by Maciej Pijanowski (Code Review) 12 Oct '22

12 Oct '22

Attention is currently required from: Nico Huber, Kamil Pokornicki, Sergii Dmytruk. Maciej Pijanowski has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68180 ) Change subject: flashchips.c: mark WP of 9 entries as tested ...................................................................... Patch Set 5: Code-Review+1 (1 comment) Patchset: PS5: > No, in these changes those chips don't have WP definitions. https://review.coreboot. […] Got it, thanks. -- 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: 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: 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-Comment-Date: Wed, 12 Oct 2022 07:32:45 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Maciej Pijanowski <maciej.pijanowski(a)3mdeb.com> Comment-In-Reply-To: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-MessageType: comment

1 0

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

12 Oct '22

Attention is currently required from: Nico Huber, Angel Pons. Felix Singer 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: (1 comment) Patchset: PS3: > > Is that file not in a standard format? I wouldn't know, is it python? […] Just squashed CB:68248 into this one. -- 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: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Wed, 12 Oct 2022 05:59:53 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net> Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: comment

1 0

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

12 Oct '22

Attention is currently required from: Nico Huber, Angel Pons. Hello build bot (Jenkins), Nico Huber, Thomas Heijligen, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/68068 to look at the new patch set (#4). Change subject: [WIP] test_build.sh: Rework programmer selection using Meson ...................................................................... [WIP] test_build.sh: Rework programmer selection using Meson Signed-off-by: Felix Singer <felixsinger(a)posteo.net> Change-Id: I9f41ce2ff219c70c3c05a90134291b01a084c859 --- M meson.build M test_build.sh 2 files changed, 40 insertions(+), 11 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/68/68068/4 -- 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: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

[S] Change in flashrom[master]: meson: Move programmer test sources into programmer definition
by Felix Singer (Code Review) 12 Oct '22

12 Oct '22

Attention is currently required from: Thomas Heijligen, Angel Pons, Anastasia Klimchuk. Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68162 ) Change subject: meson: Move programmer test sources into programmer definition ...................................................................... Patch Set 11: (1 comment) File tests/meson.build: https://review.coreboot.org/c/flashrom/+/68162/comment/34b4b9dd_16b48326 PS10, Line 32: p_data += { : 'testsrc' : p_data.get('testsrc', []), : } : srcs += p_data.get('testsrc') > I came up with one question, wanted to clarify. At the moment we need all test files to be included in the build, regardless of whether programmer is enabled or not. Does this work like that? Yes, all test files need to be included currently. Just the implementation of the methods differs depending on the programmer configuration (actual test code vs skip test dummy code). > why https://review.coreboot.org/c/flashrom/+/67664/5 (PS5, *not* the recent patchset) passed the jenkins? I would assume it should not, because line 43 in PS5 skips the wrong test? > > I checked test_build script, and I see it is running tests in the loop, each time one programmer enabled. In my mind, the tests where ch341a_spi is disabled should fail to build (to link) on 67664/5 but that didn't happen? I am confused :) and please tell me if I am writing into a wrong place! This place seems somewhat relevant. You are right. I was confused first too, buuuut actually Jenkins did its job as expected :) We need to look more closely: Patchset 5 was pushed on 23th September. I checked it out locally and looked at test_build.sh. At that time we only iterated over all programmers using the Make build system. For the Meson build system we just used the default configuration, since probably the Meson refactoring patch wasn't merged yet. meson $builddir -Dtests=enabled ninja -C $builddir ninja -C $builddir test So we didn't iterate over all programmers yet and built them individually, which means CH341A was part of the default programmer selection and so the actual tests were compiled in and executed by Jenkins. That's why the issue was hidden :) Meanwhile we also iterate over all programmers using the Meson build system, but the programmer list in test_build.sh needs to be refactored as well. See CB:68068. -- To view, visit https://review.coreboot.org/c/flashrom/+/68162 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I307faaf8a9f7ae3c54bd96e7d871a3abb8aadea3 Gerrit-Change-Number: 68162 Gerrit-PatchSet: 11 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> 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: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Wed, 12 Oct 2022 05:33:31 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

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

12 Oct '22

Attention is currently required from: Nico Huber, Nikolai Artemiev, Sergii Dmytruk. Anastasia Klimchuk 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 3: (1 comment) File writeprotect.c: https://review.coreboot.org/c/flashrom/+/68221/comment/f56aa9ed_ee34469c PS3, Line 425: bool It's not bool anymore? -- 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: 3 Gerrit-Owner: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> 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-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Wed, 12 Oct 2022 04:52:24 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: flashchips.c: mark WP of 9 entries as tested
by Nikolai Artemiev (Code Review) 12 Oct '22

12 Oct '22

Attention is currently required from: Nico Huber, Maciej Pijanowski, Kamil Pokornicki, Sergii Dmytruk. Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68180 ) Change subject: flashchips.c: mark WP of 9 entries as tested ...................................................................... Patch Set 5: Code-Review+1 -- 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: 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: 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: Maciej Pijanowski <maciej.pijanowski(a)3mdeb.com> Gerrit-Attention: Kamil Pokornicki <kamil.pokornicki(a)3mdeb.com> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Wed, 12 Oct 2022 04:39:31 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: util: add bash completion script
by Felix Singer (Code Review) 12 Oct '22

12 Oct '22

Attention is currently required from: Thomas Heijligen, Anastasia Klimchuk, Alexander Goncharov. Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68247 ) Change subject: util: add bash completion script ...................................................................... Patch Set 4: (1 comment) File Makefile: https://review.coreboot.org/c/flashrom/+/68247/comment/f879a9cf_47acd8e4 PS4, Line 33: bash-completion > I am wondering, would `bash-comp` be sufficient? is it understandable? no, because that's a somewhat standard path where the bash-completion tool expects the completion extensions. -- 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: 4 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-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: Wed, 12 Oct 2022 04:35:49 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: util: add bash completion script
by Anastasia Klimchuk (Code Review) 12 Oct '22

12 Oct '22

Attention is currently required from: Thomas Heijligen, Alexander Goncharov. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68247 ) Change subject: util: add bash completion script ...................................................................... Patch Set 4: (2 comments) File Makefile: https://review.coreboot.org/c/flashrom/+/68247/comment/f85fc30f_582c5ffb PS4, Line 33: bash-completion I am wondering, would `bash-comp` be sufficient? is it understandable? File util/flashrom.bash-completion: https://review.coreboot.org/c/flashrom/+/68247/comment/3f5865ea_5eef1e69 PS4, Line 36: atahpt Maybe it's better to have this list vertically, same as the other one? It takes more lines, but then it's more readable. -- 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: 4 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-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Comment-Date: Wed, 12 Oct 2022 02:36:48 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: meson: Move programmer test sources into programmer definition
by Anastasia Klimchuk (Code Review) 12 Oct '22

12 Oct '22

Attention is currently required from: Felix Singer, Thomas Heijligen, Angel Pons. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68162 ) Change subject: meson: Move programmer test sources into programmer definition ...................................................................... Patch Set 11: (1 comment) File tests/meson.build: https://review.coreboot.org/c/flashrom/+/68162/comment/b5af3edd_2019ccf2 PS10, Line 32: p_data += { : 'testsrc' : p_data.get('testsrc', []), : } : srcs += p_data.get('testsrc') > Just rethinking.. […] I came up with one question, wanted to clarify. At the moment we need all test files to be included in the build, regardless of whether programmer is enabled or not. Does this work like that? Another thought, relevant to this patch and also another one: why https://review.coreboot.org/c/flashrom/+/67664/5 (PS5, *not* the recent patchset) passed the jenkins? I would assume it should not, because line 43 in PS5 skips the wrong test? I checked test_build script, and I see it is running tests in the loop, each time one programmer enabled. In my mind, the tests where ch341a_spi is disabled should fail to build (to link) on 67664/5 but that didn't happen? I am confused :) and please tell me if I am writing into a wrong place! This place seems somewhat relevant. -- To view, visit https://review.coreboot.org/c/flashrom/+/68162 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I307faaf8a9f7ae3c54bd96e7d871a3abb8aadea3 Gerrit-Change-Number: 68162 Gerrit-PatchSet: 11 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> 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: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Wed, 12 Oct 2022 01:53:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: tests/meson.build: Turn file list into list of file objects
by Anastasia Klimchuk (Code Review) 12 Oct '22

12 Oct '22

Attention is currently required from: Felix Singer. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68197 ) Change subject: tests/meson.build: Turn file list into list of file objects ...................................................................... Patch Set 4: Code-Review+2 (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/68197/comment/6fa86147_539f5706 PS3, Line 12: as it is done elsewhere > Right :) I have updated the commit message. […] Done -- To view, visit https://review.coreboot.org/c/flashrom/+/68197 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0b9144a6b76c1772833817b4e6873818dcf36b05 Gerrit-Change-Number: 68197 Gerrit-PatchSet: 4 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> 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-Comment-Date: Wed, 12 Oct 2022 01:29:36 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit October 2022