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

Change in flashrom[master]: flashrom: initialize restore func count in correct place
by Edward O'Callaghan (Code Review) 27 Apr '22

27 Apr '22

Attention is currently required from: Angel Pons, Nikolai Artemiev. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/63881 ) Change subject: flashrom: initialize restore func count in correct place ...................................................................... Patch Set 1: Code-Review+2 (2 comments) Patchset: PS1: Thank you for tracing this down Nick! Is it possible to construct a unit-test for this too? File flashrom.c: https://review.coreboot.org/c/flashrom/+/63881/comment/8bfa5656_0fe44cb8 PS1, Line 1865: flash->chip_restore_fn_count = 0; Maybe add a short comment here? `/* Initialisation of chip_restore_fn_count before chip unlock calls. */` It alludes others to git blame and find their way to this commit if need be. -- To view, visit https://review.coreboot.org/c/flashrom/+/63881 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I4c7df424bd2ae2b5fb2a2ab6b47a3c9ff3233acf Gerrit-Change-Number: 63881 Gerrit-PatchSet: 1 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Wed, 27 Apr 2022 01:12:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: meson: add option to enable or disable tests
by Anastasia Klimchuk (Code Review) 27 Apr '22

27 Apr '22

Attention is currently required from: Thomas Heijligen, Edward O'Callaghan. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/63832 ) Change subject: meson: add option to enable or disable tests ...................................................................... Patch Set 1: (2 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/63832/comment/7a08bd85_ae75f9a6 PS1, Line 10: Run `meson -Dtests=true` to enable tests. (default) : Run `meson -Dtests=false` to disable tests. One last test scenario: does it work with no option mentioned in command line? just meson, or just meson test. File meson.build: https://review.coreboot.org/c/flashrom/+/63832/comment/c25bc98c_ba3d140a PS1, Line 522: if cmocka_dep.found() I remember that is an important thing, to check cmocka dep. We can keep both conditions I think (option present and dependency found). -- To view, visit https://review.coreboot.org/c/flashrom/+/63832 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I384c904c577b265dfe36bf46bf07c641bc29de9b Gerrit-Change-Number: 63832 Gerrit-PatchSet: 1 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-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Wed, 27 Apr 2022 00:41:51 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: meson: outsource platform specific code to `platform/meson.build`
by Anastasia Klimchuk (Code Review) 27 Apr '22

27 Apr '22

Attention is currently required from: Thomas Heijligen. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/63826 ) Change subject: meson: outsource platform specific code to `platform/meson.build` ...................................................................... Patch Set 2: (1 comment) File meson.build: https://review.coreboot.org/c/flashrom/+/63826/comment/c2bf2352_3d0e950f PS2, Line 80: host_is_x86 = ['x86', 'x86_64'].contains(host_machine.cpu_family()) I have a overall question about the idea: what is counted as "platform specific code" ? If in future if I want to add some code, how do I decide where to put it? For example `host_is_x86` is platform specific information, but it stays in root build file. Which means, maybe I don't entirely understand what is platform specific? -- To view, visit https://review.coreboot.org/c/flashrom/+/63826 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I88044a3f903f316138483dd872a6d95f8686ae69 Gerrit-Change-Number: 63826 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Comment-Date: Wed, 27 Apr 2022 00:34:29 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: flashrom: initialize restore func count in correct place
by Nikolai Artemiev (Code Review) 27 Apr '22

27 Apr '22

Nikolai Artemiev has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/63881 ) Change subject: flashrom: initialize restore func count in correct place ...................................................................... flashrom: initialize restore func count in correct place Set `flash->chip_restore_fn_count` to zero before calling the chip's unlock funciton in `prepare_flash_access()`. Previously the count was reset after calling chip->unlock(), causing the restore handler that is registered by spi_disable_blockprotect_generic() to be lost. BUG=b:228945411 BRANCH=none TEST=enable wp; flashrom -w; check wp still enabled. Change-Id: I4c7df424bd2ae2b5fb2a2ab6b47a3c9ff3233acf Signed-off-by: Nikolai Artemiev <nartemiev(a)google.com> --- M flashrom.c 1 file changed, 4 insertions(+), 4 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/81/63881/1 diff --git a/flashrom.c b/flashrom.c index 7937885..5c180f3 100644 --- a/flashrom.c +++ b/flashrom.c @@ -1860,15 +1860,15 @@ if (map_flash(flash) != 0) return 1; + flash->address_high_byte = -1; + flash->in_4ba_mode = false; + flash->chip_restore_fn_count = 0; + /* Given the existence of read locks, we want to unlock for read, erase and write. */ if (flash->chip->unlock) flash->chip->unlock(flash); - flash->address_high_byte = -1; - flash->in_4ba_mode = false; - flash->chip_restore_fn_count = 0; - /* Be careful about 4BA chips and broken masters */ if (flash->chip->total_size > 16 * 1024 && spi_master_no_4ba_modes(flash)) { /* If we can't use native instructions, bail out */ -- To view, visit https://review.coreboot.org/c/flashrom/+/63881 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I4c7df424bd2ae2b5fb2a2ab6b47a3c9ff3233acf Gerrit-Change-Number: 63881 Gerrit-PatchSet: 1 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: [WIP] meson: rework the programmer selection
by Peter Marheine (Code Review) 27 Apr '22

27 Apr '22

Attention is currently required from: Thomas Heijligen, Edward O'Callaghan, Anastasia Klimchuk. Peter Marheine has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/63724 ) Change subject: [WIP] meson: rework the programmer selection ...................................................................... Patch Set 7: (2 comments) File meson.build: https://review.coreboot.org/c/flashrom/+/63724/comment/48d9819a_60ecaba3 PS2, Line 98: # 'deps' : list[dep], # fallback: [] > It's possible but quite complicated to put an optional external dependency together with an source f […] Ack https://review.coreboot.org/c/flashrom/+/63724/comment/10ffb954_264d326a PS2, Line 427: elif selected and available > I found non nested if statements easier. […] Ack -- 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: 7 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: 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: Wed, 27 Apr 2022 00:11:38 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Thomas Heijligen <src(a)posteo.de> Comment-In-Reply-To: Peter Marheine <pmarheine(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: meson: outsource platform specific code to `platform/meson.build`
by Peter Marheine (Code Review) 27 Apr '22

27 Apr '22

Attention is currently required from: Thomas Heijligen, Anastasia Klimchuk. Peter Marheine has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/63826 ) Change subject: meson: outsource platform specific code to `platform/meson.build` ...................................................................... Patch Set 2: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/63826 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I88044a3f903f316138483dd872a6d95f8686ae69 Gerrit-Change-Number: 63826 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Wed, 27 Apr 2022 00:10:28 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: meson: add option to enable or disable tests
by Peter Marheine (Code Review) 27 Apr '22

27 Apr '22

Attention is currently required from: Thomas Heijligen, Edward O'Callaghan, Anastasia Klimchuk. Peter Marheine has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/63832 ) Change subject: meson: add option to enable or disable tests ...................................................................... Patch Set 1: (1 comment) File meson_options.txt: https://review.coreboot.org/c/flashrom/+/63832/comment/8b8bf504_8d6a0418 PS1, Line 45: and run I think tests will only be run via `meson test`, so this option only controls whether they're built (and they can't be run if they're not built). -- To view, visit https://review.coreboot.org/c/flashrom/+/63832 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I384c904c577b265dfe36bf46bf07c641bc29de9b Gerrit-Change-Number: 63832 Gerrit-PatchSet: 1 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-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: Wed, 27 Apr 2022 00:10:05 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: meson: Treat warnings as errors
by Peter Marheine (Code Review) 27 Apr '22

27 Apr '22

Attention is currently required from: Anastasia Klimchuk. Peter Marheine has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/63852 ) Change subject: meson: Treat warnings as errors ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/63852 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I06c72abf1c861f9b35736e02ef7f4a4fb6909b97 Gerrit-Change-Number: 63852 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Wed, 27 Apr 2022 00:06:03 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: meson: ich_descriptors_tool remove unneeded dependencies
by Anastasia Klimchuk (Code Review) 26 Apr '22

26 Apr '22

Attention is currently required from: Thomas Heijligen, Angel Pons. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/63757 ) Change subject: meson: ich_descriptors_tool remove unneeded dependencies ...................................................................... Patch Set 4: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/63757/comment/fdd45599_b40befa5 PS1, Line 7: meson: ich_descriptors_tool remove unneeded dependencies > Done It looks like you accidentally returned back old commit message from the first patchset? -- To view, visit https://review.coreboot.org/c/flashrom/+/63757 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ice1437cb294729b6af0e24f0a02692459b7a1412 Gerrit-Change-Number: 63757 Gerrit-PatchSet: 4 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: Peter Marheine <pmarheine(a)chromium.org> 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-Comment-Date: Tue, 26 Apr 2022 23:26:19 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Thomas Heijligen <src(a)posteo.de> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: meson: relocate add_project_arguments() for IS_WINDOWS
by Anastasia Klimchuk (Code Review) 26 Apr '22

26 Apr '22

Attention is currently required from: Thomas Heijligen. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/63827 ) Change subject: meson: relocate add_project_arguments() for IS_WINDOWS ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/63827 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6afb65fabf858449f2706bf250588225a94c1871 Gerrit-Change-Number: 63827 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Comment-Date: Tue, 26 Apr 2022 23:21:59 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit April 2022