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

Change in flashrom[master]: pony_spi.c: Extract out get_params to simplify init
by Edward O'Callaghan (Code Review) April 30, 2022

April 30, 2022

Attention is currently required from: Nico Huber, Paul Menzel, Edward O'Callaghan, Angel Pons, Light. Hello Felix Singer, build bot (Jenkins), Light, Anastasia Klimchuk, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/63130 to look at the new patch set (#6). Change subject: pony_spi.c: Extract out get_params to simplify init ...................................................................... pony_spi.c: Extract out get_params to simplify init In light of `commit caa0335114a81`, extract out the get_param logic to its own function to simplify the number of cleanup paths. BUG=none TEST=builds Change-Id: I364febc05c870683cbad114583762b0c006f4bac Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M pony_spi.c 1 file changed, 55 insertions(+), 43 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/30/63130/6 -- To view, visit https://review.coreboot.org/c/flashrom/+/63130 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I364febc05c870683cbad114583762b0c006f4bac Gerrit-Change-Number: 63130 Gerrit-PatchSet: 6 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Light <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Light <aarya.chaumal(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: raiden_debug_spi.c: Document where raiden_debug_spi_requests are handled
by Edward O'Callaghan (Code Review) April 30, 2022

April 30, 2022

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/63902 ) Change subject: raiden_debug_spi.c: Document where raiden_debug_spi_requests are handled ...................................................................... raiden_debug_spi.c: Document where raiden_debug_spi_requests are handled Because the enum is a different name to the one within cr50 it was entirely unclear where these values were handled at the end-point and what a untargetted REQ_ENABLE actually did. Comment to avoid needing to chase this again. BUG=b:224358254 TEST=builds Change-Id: I8cc78a85660190c9f750bf67994f0d59d69592d8 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/63902 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Sam McNally <sammc(a)google.com> Reviewed-by: Peter Marheine <pmarheine(a)chromium.org> --- M raiden_debug_spi.c 1 file changed, 8 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Sam McNally: Looks good to me, approved Peter Marheine: Looks good to me, but someone else must approve diff --git a/raiden_debug_spi.c b/raiden_debug_spi.c index 521539f..f9ab3e0 100644 --- a/raiden_debug_spi.c +++ b/raiden_debug_spi.c @@ -399,6 +399,10 @@ USB_SPI_UNKNOWN_ERROR = 0x8000, }; +/* Corresponds with 'enum usb_spi_request' in, + * platform/cr50/chip/g/usb_spi.h and, + * platform/ec/chip/stm32/usb_spi.h. + */ enum raiden_debug_spi_request { RAIDEN_DEBUG_SPI_REQ_ENABLE = 0x0000, RAIDEN_DEBUG_SPI_REQ_DISABLE = 0x0001, @@ -1445,6 +1449,10 @@ static int get_target(void) { + /** + * REQ_ENABLE doesn't specify a target bus, and will be rejected + * by adapters that support more than one target. + */ int request_enable = RAIDEN_DEBUG_SPI_REQ_ENABLE; char *target_str = extract_programmer_param("target"); -- To view, visit https://review.coreboot.org/c/flashrom/+/63902 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I8cc78a85660190c9f750bf67994f0d59d69592d8 Gerrit-Change-Number: 63902 Gerrit-PatchSet: 4 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Sam McNally <sammc(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

Change in flashrom[master]: tests: test write protection
by Sergii Dmytruk (Code Review) April 29, 2022

April 29, 2022

Attention is currently required from: Nico Huber, Sergii Dmytruk. Hello build bot (Jenkins), Nico Huber, Angel Pons, Nikolai Artemiev, Anastasia Klimchuk, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/59075 to look at the new patch set (#35). Change subject: tests: test write protection ...................................................................... tests: test write protection Tests both WP implementation and its emulation in dummy programmer. Change-Id: I49af7f6d173eb4c56c22d80b01a473b8c499c0f8 Signed-off-by: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> --- A tests/chip_wp.c M tests/meson.build M tests/tests.c M tests/tests.h 4 files changed, 338 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/75/59075/35 -- 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: 35 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: 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-MessageType: newpatchset

1 0

Change in flashrom[master]: Makefile: Align BITBANG_SPI with the other DEPENDS_ and USE_ configs
by Nico Huber (Code Review) April 29, 2022

April 29, 2022

Nico Huber has submitted this change. ( https://review.coreboot.org/c/flashrom/+/63886 ) Change subject: Makefile: Align BITBANG_SPI with the other DEPENDS_ and USE_ configs ...................................................................... Makefile: Align BITBANG_SPI with the other DEPENDS_ and USE_ configs Drop not needed `-DCONFIG_BITBANG_SPI=1` Change-Id: I2a519c929f7747af70568284823b51678751e285 Signed-off-by: Thomas Heijligen <thomas.heijligen(a)secunet.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/63886 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Nico Huber <nico.h(a)gmx.de> --- M Makefile 1 file changed, 5 insertions(+), 13 deletions(-) Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved diff --git a/Makefile b/Makefile index e4cfede..513ac82 100644 --- a/Makefile +++ b/Makefile @@ -539,14 +539,6 @@ $(eval $(var)=yes))) endif -# Bitbanging SPI infrastructure, default off unless needed. - -ifneq ($(call filter_deps,$(DEPENDS_ON_BITBANG_SPI)), ) -override CONFIG_BITBANG_SPI = yes -else -CONFIG_BITBANG_SPI ?= no -endif - ############################################################################### # Handle CONFIG_* variables that depend on others set (and verified) above. @@ -616,11 +608,6 @@ PROGRAMMER_OBJS += pony_spi.o endif -ifeq ($(CONFIG_BITBANG_SPI), yes) -FEATURE_FLAGS += -D'CONFIG_BITBANG_SPI=1' -PROGRAMMER_OBJS += bitbang_spi.o -endif - ifeq ($(CONFIG_NIC3COM), yes) FEATURE_FLAGS += -D'CONFIG_NIC3COM=1' PROGRAMMER_OBJS += nic3com.o @@ -786,6 +773,11 @@ PROGRAMMER_OBJS += ni845x_spi.o endif +USE_BITBANG_SPI := $(if $(call filter_deps,$(DEPENDS_ON_BITBANG_SPI)),yes,no) +ifeq ($(USE_BITBANG_SPI), yes) +LIB_OBJS += bitbang_spi.o +endif + USE_LINUX_I2C := $(if $(call filter_deps,$(DEPENDS_ON_LINUX_I2C)),yes,no) ifeq ($(USE_LINUX_I2C), yes) LIB_OBJS += i2c_helper_linux.o -- To view, visit https://review.coreboot.org/c/flashrom/+/63886 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2a519c929f7747af70568284823b51678751e285 Gerrit-Change-Number: 63886 Gerrit-PatchSet: 3 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

Change in flashrom[master]: Makefile: Align BITBANG_SPI with the other DEPENDS_ and USE_ configs
by Nico Huber (Code Review) April 29, 2022

April 29, 2022

Attention is currently required from: Thomas Heijligen, Angel Pons. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/63886 ) Change subject: Makefile: Align BITBANG_SPI with the other DEPENDS_ and USE_ configs ...................................................................... Patch Set 2: Code-Review+2 (1 comment) Patchset: PS1: > This gets dropped with CB:63724. I think the async is ok for a while. Ack -- To view, visit https://review.coreboot.org/c/flashrom/+/63886 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2a519c929f7747af70568284823b51678751e285 Gerrit-Change-Number: 63886 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.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-Comment-Date: Fri, 29 Apr 2022 10:20:59 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Thomas Heijligen <src(a)posteo.de> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: Introduce an `include` directory for header files
by Nico Huber (Code Review) April 29, 2022

April 29, 2022

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58622 ) Change subject: Introduce an `include` directory for header files ...................................................................... Patch Set 8: (1 comment) Patchset: PS7: > I downloaded this patch on the top of the tree locally, it got applied automatically, no conflict resolutions were needed. I also checked that flash.h in latest patchset is up-to-date (contains SR3 support). You can also use Gerrit's Rebase feature. What you did is exactly what "Rebase on top of the master branch (breaks relation chain)" does. If that fails, nothing happens, if it works, you know the patch applied cleanly (and you, and other, can still verify things in Gerrit). No need to do so, I guess I got used to it because it saves me one switch to the command line and back when I "only wanted to merge something". -- To view, visit https://review.coreboot.org/c/flashrom/+/58622 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iaddd6bbfa0624b166d422f665877f096983bf4cf Gerrit-Change-Number: 58622 Gerrit-PatchSet: 8 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: 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Martin L Roth <martinroth(a)google.com> Gerrit-CC: Peter Marheine <pmarheine(a)chromium.org> Gerrit-CC: Thomas Heijligen <src(a)posteo.de> Gerrit-Comment-Date: Fri, 29 Apr 2022 10:09:10 +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]: raiden_debug_spi.c: Document where raiden_debug_spi_requests are handled
by Peter Marheine (Code Review) April 29, 2022

April 29, 2022

Attention is currently required from: Edward O'Callaghan. Peter Marheine has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/63902 ) Change subject: raiden_debug_spi.c: Document where raiden_debug_spi_requests are handled ...................................................................... Patch Set 3: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/63902 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I8cc78a85660190c9f750bf67994f0d59d69592d8 Gerrit-Change-Number: 63902 Gerrit-PatchSet: 3 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Sam McNally <sammc(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Fri, 29 Apr 2022 06:45:27 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: Makefile: Align BITBANG_SPI with the other DEPENDS_ and USE_ configs
by Thomas Heijligen (Code Review) April 29, 2022

April 29, 2022

Attention is currently required from: Nico Huber, Angel Pons. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/63886 ) Change subject: Makefile: Align BITBANG_SPI with the other DEPENDS_ and USE_ configs ...................................................................... Patch Set 2: (2 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/63886/comment/a0dee0bd_515d5b0f PS1, Line 9: D > typo: F Done Patchset: PS1: > There's another `-DCONFIG_BITBANG_SPI=1` in `meson.build`. This gets dropped with CB:63724. I think the async is ok for a while. -- To view, visit https://review.coreboot.org/c/flashrom/+/63886 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2a519c929f7747af70568284823b51678751e285 Gerrit-Change-Number: 63886 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.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: Fri, 29 Apr 2022 05:49:40 +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> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: Makefile: Align BITBANG_SPI with the other DEPENDS_ and USE_ configs
by Thomas Heijligen (Code Review) April 29, 2022

April 29, 2022

Attention is currently required from: Thomas Heijligen. Hello build bot (Jenkins), Nico Huber, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/63886 to look at the new patch set (#2). Change subject: Makefile: Align BITBANG_SPI with the other DEPENDS_ and USE_ configs ...................................................................... Makefile: Align BITBANG_SPI with the other DEPENDS_ and USE_ configs Drop not needed `-DCONFIG_BITBANG_SPI=1` Change-Id: I2a519c929f7747af70568284823b51678751e285 Signed-off-by: Thomas Heijligen <thomas.heijligen(a)secunet.com> --- M Makefile 1 file changed, 5 insertions(+), 13 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/86/63886/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/63886 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2a519c929f7747af70568284823b51678751e285 Gerrit-Change-Number: 63886 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: meson: Treat warnings as errors
by Anastasia Klimchuk (Code Review) April 29, 2022

April 29, 2022

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/63852 ) Change subject: meson: Treat warnings as errors ...................................................................... meson: Treat warnings as errors Makefile treats warnings as errors, so enable the same option for meson. Makefile has an option to disable "warnings as errors" option from command line. The same can be achieved with meson, see scenario 2 below. TEST=I verified the following scenarious 1) check that warnings treated as errors by enforcing higher warning level `meson setup --warnlevel=3 --wipe` `ninja test` shows tons of errors because warning level 3 is pedantic, build fails 2) disable "warning as errors" option from command line `meson configure -Dwerror=false` `ninja test` Now all warnings are non-blocking for build, so tests pass 3) back home `meson configure -Dwerror=true` `meson setup --warnlevel=2 --wipe` `ninja test` No warnings and tests pass 4) defaults: create new directory and run meson will all default options No warnings, all tests pass. Change-Id: I06c72abf1c861f9b35736e02ef7f4a4fb6909b97 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/63852 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Thomas Heijligen <src(a)posteo.de> Reviewed-by: Peter Marheine <pmarheine(a)chromium.org> --- M meson.build 1 file changed, 1 insertion(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Thomas Heijligen: Looks good to me, approved Peter Marheine: Looks good to me, but someone else must approve diff --git a/meson.build b/meson.build index 80ccfb3..222f7c1 100644 --- a/meson.build +++ b/meson.build @@ -5,6 +5,7 @@ default_options : [ 'warning_level=2', 'c_std=c99', + 'werror=true', 'optimization=s', 'debug=false'], ) 1 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. -- 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: 4 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: 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-MessageType: merged

1 0