May 2022 - flashrom-gerrit - mail.coreboot.org

Change in flashrom[master]: dummyflasher: add SR2 and SR3 emulation harness
by Thomas Heijligen (Code Review) 10 May '22

10 May '22

Attention is currently required from: Nico Huber, Angel Pons, Anastasia Klimchuk, Sergii Dmytruk. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/59072 ) Change subject: dummyflasher: add SR2 and SR3 emulation harness ...................................................................... Patch Set 34: (1 comment) File dummyflasher.c: https://review.coreboot.org/c/flashrom/+/59072/comment/40d6eb8c_405b63f0 PS34, Line 169: assert > `assert` is used only once in flashrom beside the tests. I would prefer not to use it. […] Sorry, I haven't read your follow up before this comment. So the hardcoded values does not make sense. I would still prefer a solution without assert. Maybe separate functions for each status? -- To view, visit https://review.coreboot.org/c/flashrom/+/59072 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I177ae3f068f03380f5b3941d9996a07205672e59 Gerrit-Change-Number: 59072 Gerrit-PatchSet: 34 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: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Maciej Pijanowski <maciej.pijanowski(a)3mdeb.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> 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: Tue, 10 May 2022 06:28:14 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Thomas Heijligen <src(a)posteo.de> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: dummyflasher: add SR2 and SR3 emulation harness
by Thomas Heijligen (Code Review) 10 May '22

10 May '22

Attention is currently required from: Nico Huber, Angel Pons, Anastasia Klimchuk, Sergii Dmytruk. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/59072 ) Change subject: dummyflasher: add SR2 and SR3 emulation harness ...................................................................... Patch Set 34: (1 comment) File dummyflasher.c: https://review.coreboot.org/c/flashrom/+/59072/comment/8fd98740_25459198 PS34, Line 169: assert > Just wanted to check whether everyone is fine with `assert` in the code? It is not often used in fla […] `assert` is used only once in flashrom beside the tests. I would prefer not to use it. But I don't think we have a rule for it. How about: #define STATUS1_RO_BITMASK SPI_SW_WIP #define STATUS2_RO_BITMASK 0 #define STATUS3_RO_BITMASK 0 or const uint8_t status1_ro_bitmask = SPI_SW_WIP; const uint8_t status2_ro_bitmask = 0; const uint8_t status3_ro_bitmask = 0; ? Where is the benefit from a function over this? -- To view, visit https://review.coreboot.org/c/flashrom/+/59072 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I177ae3f068f03380f5b3941d9996a07205672e59 Gerrit-Change-Number: 59072 Gerrit-PatchSet: 34 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: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Maciej Pijanowski <maciej.pijanowski(a)3mdeb.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> 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: Tue, 10 May 2022 06:20:22 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: meson: relocate config_print_wiki & config_default_programmer_*
by Thomas Heijligen (Code Review) 10 May '22

10 May '22

Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/64029 ) Change subject: meson: relocate config_print_wiki & config_default_programmer_* ...................................................................... Patch Set 3: (1 comment) File meson.build: https://review.coreboot.org/c/flashrom/+/64029/comment/f4ce1f7c_80bb6ab8 PS2, Line 417: cargs += '-DCONFIG_DEFAULT_PROGRAMMER_ARGS="' + config_default_programmer_args + '"' > Shouldn't this line be under if condition too? There is no need for default programmer args if defau […] CONFIG_DEFAULT_PROGRAMMER_ARGS must be defined, at least as an empty string. classic_cli.c:838 I would prefer to drop the default programmer to have a unified behavior across all flashrom builds. Do you think that would be good? -- To view, visit https://review.coreboot.org/c/flashrom/+/64029 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9538b0aee31b294844d4f4ca0396334a81dfb498 Gerrit-Change-Number: 64029 Gerrit-PatchSet: 3 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Tue, 10 May 2022 05:48:45 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: meson: link flashrom binary against static libflashrom
by Peter Marheine (Code Review) 10 May '22

10 May '22

Attention is currently required from: Thomas Heijligen, Edward O'Callaghan. Peter Marheine has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/64030 ) Change subject: meson: link flashrom binary against static libflashrom ...................................................................... Patch Set 3: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/64030 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic522610f59e00299ebfa1bd29482ff92120ec52b Gerrit-Change-Number: 64030 Gerrit-PatchSet: 3 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: 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-Comment-Date: Tue, 10 May 2022 05:12:23 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tests: test write protection
by Anastasia Klimchuk (Code Review) 10 May '22

10 May '22

Attention is currently required from: Nico Huber, Sergii Dmytruk. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/59075 ) Change subject: tests: test write protection ...................................................................... Patch Set 36: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/59075 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I49af7f6d173eb4c56c22d80b01a473b8c499c0f8 Gerrit-Change-Number: 59075 Gerrit-PatchSet: 36 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-CC: Maciej Pijanowski <maciej.pijanowski(a)3mdeb.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Tue, 10 May 2022 04:04:42 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: dummyflasher: enforce write protection for W25Q128FV
by Anastasia Klimchuk (Code Review) 10 May '22

10 May '22

Attention is currently required from: Nico Huber, Angel Pons, Sergii Dmytruk. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/59074 ) Change subject: dummyflasher: enforce write protection for W25Q128FV ...................................................................... Patch Set 34: Code-Review+1 (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/59074/comment/5e6d714a_66a3e244 PS34, Line 18: Thank you for listing test scenarios! I have one thing to ask: it would be great to have one scenarios with hwwp=no explicitly set in programmer param. To make sure that branch of code executes as expected. Very possible that you've done that already, so maybe that's just a matter of adding info into commit message. Thanks! -- To view, visit https://review.coreboot.org/c/flashrom/+/59074 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9fd1417f941186391bd213bd355530143c8f04a0 Gerrit-Change-Number: 59074 Gerrit-PatchSet: 34 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-CC: Maciej Pijanowski <maciej.pijanowski(a)3mdeb.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Tue, 10 May 2022 03:59:32 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: dummyflasher: emulate SR2 and SR3 for W25Q128FV
by Anastasia Klimchuk (Code Review) 10 May '22

10 May '22

Attention is currently required from: Angel Pons, Sergii Dmytruk. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/59073 ) Change subject: dummyflasher: emulate SR2 and SR3 for W25Q128FV ...................................................................... Patch Set 34: Code-Review+1 (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/59073/comment/09aae0e0_2f786cfa PS34, Line 11: As I commented on previous patch: It would be great to have 3 test scenarios listed in commit message, 3 times `flashrom -p dummy:spi_status=content` with content 8bits, 16bits and 24bits. You most likely already did those scenarios when testing the patch, then just list them. -- To view, visit https://review.coreboot.org/c/flashrom/+/59073 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I79f9b4a0b604663d3288ad70dcbe3ea4075dede5 Gerrit-Change-Number: 59073 Gerrit-PatchSet: 34 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-CC: Maciej Pijanowski <maciej.pijanowski(a)3mdeb.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Tue, 10 May 2022 03:24:41 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: dummyflasher: add SR2 and SR3 emulation harness
by Anastasia Klimchuk (Code Review) 10 May '22

10 May '22

Attention is currently required from: Nico Huber, Angel Pons, Sergii Dmytruk. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/59072 ) Change subject: dummyflasher: add SR2 and SR3 emulation harness ...................................................................... Patch Set 34: Code-Review+1 (3 comments) File dummyflasher.c: https://review.coreboot.org/c/flashrom/+/59072/comment/047eff4d_254cf665 PS16, Line 347: other chips might differ > Updated comment. Just checking: you are saying "updated comment" but the comment is removed, is it as expected? File dummyflasher.c: https://review.coreboot.org/c/flashrom/+/59072/comment/aa177688_f9ea9fe6 PS34, Line 169: assert Just wanted to check whether everyone is fine with `assert` in the code? It is not often used in flashrom code, I don't remember whether I saw it ever. On the other hand I see that it makes code easier, and allows using return value as a result (not as error code). File flashrom.8.tmpl: https://review.coreboot.org/c/flashrom/+/59072/comment/42170e18_b3a9ae97 PS34, Line 817: is a hexadecimal value of up to 24 bits. For example, 0x332211 assigns 0x11 to : SR1, 0x22 to SR2 and 0x33 to SR3. Shorter value is padded to 24 bits with : zeroes on the left. See datasheet for chosen chip for details about the : registers content. Based on this , it would be great to have 3 test scenarios listed in commit message, 3 times `flashrom -p dummy:spi_status=content` with content 8bits, 16bits and 24bits. You most likely already did those scenarios when testing the patch, then just list them. -- To view, visit https://review.coreboot.org/c/flashrom/+/59072 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I177ae3f068f03380f5b3941d9996a07205672e59 Gerrit-Change-Number: 59072 Gerrit-PatchSet: 34 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-CC: Maciej Pijanowski <maciej.pijanowski(a)3mdeb.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Tue, 10 May 2022 03:10:18 +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

Change in flashrom[master]: tests: test write protection
by Edward O'Callaghan (Code Review) 10 May '22

10 May '22

Attention is currently required from: Nico Huber, Sergii Dmytruk. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/59075 ) Change subject: tests: test write protection ...................................................................... Patch Set 36: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/59075 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I49af7f6d173eb4c56c22d80b01a473b8c499c0f8 Gerrit-Change-Number: 59075 Gerrit-PatchSet: 36 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-CC: Maciej Pijanowski <maciej.pijanowski(a)3mdeb.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Tue, 10 May 2022 02:22:08 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: [WIP] meson: rework the programmer selection
by Thomas Heijligen (Code Review) 09 May '22

09 May '22

Attention is currently required from: Edward O'Callaghan, Anastasia Klimchuk, Peter Marheine. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/63724 ) Change subject: [WIP] meson: rework the programmer selection ...................................................................... Patch Set 16: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/63724/comment/8f4658a6_3f7cf895 PS11, Line 10: has some known and unknown bugs. > I think for now we can go with bugfixes. probe_variable_size() is more important to throw time on. […] The -Werror=unused-function and undefined reference to `programmer_dummy' are now fixed. CB:64224 and CB:64031 Now only a programmer of the group_usb is needed. Tested at the end of chain. (The next commit makes testing this lot easier ;-) -- To view, visit https://review.coreboot.org/c/flashrom/+/63724 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ib44b26e3748fc71f116184082b4aed0bb208b4c1 Gerrit-Change-Number: 63724 Gerrit-PatchSet: 16 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Peter Marheine <pmarheine(a)chromium.org> Gerrit-CC: Richard Hughes <richard(a)hughsie.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Mon, 09 May 2022 18:00:46 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Thomas Heijligen <src(a)posteo.de> 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 May 2022