January 2022 - flashrom-gerrit

Change in flashrom[master]: Makefile: check if librt is an external library
by Thomas Heijligen (Code Review) Feb. 1, 2022

Feb. 1, 2022

Attention is currently required from: Felix Singer, Nico Huber, Angel Pons. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/61523 ) Change subject: Makefile: check if librt is an external library ...................................................................... Patch Set 2: (4 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/61523/comment/05b77a1d_b3669760 PS1, Line 9: OpenBSD > e.g. […] Done https://review.coreboot.org/c/flashrom/+/61523/comment/82335864_474b74d2 PS1, Line 10: to link > linking Done https://review.coreboot.org/c/flashrom/+/61523/comment/697989d9_9ec0cafb PS1, Line 10: therefor > therefor_e_ Done https://review.coreboot.org/c/flashrom/+/61523/comment/ff3f25a4_a080ebc0 PS1, Line 10: Detect > Thus, detect [... […] Done -- To view, visit https://review.coreboot.org/c/flashrom/+/61523 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2c1668a350aa0806fccfb4e9cd8b04861f085ee9 Gerrit-Change-Number: 61523 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Mon, 31 Jan 2022 23:41:40 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: platform/pci.h: adapt NetBSD include path to pkg-config
by Thomas Heijligen (Code Review) Feb. 1, 2022

Feb. 1, 2022

Thomas Heijligen has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/61525 ) Change subject: platform/pci.h: adapt NetBSD include path to pkg-config ...................................................................... platform/pci.h: adapt NetBSD include path to pkg-config The pkg-config include path resolves the pciutils directory. So the include is only the pci.h file. Change-Id: I69dc8184d1d012fb695770cbf6f7c64e5a024453 Signed-off-by: Thomas Heijligen <thomas.heijligen(a)secunet.com> --- M platform/pci.h 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/25/61525/1 diff --git a/platform/pci.h b/platform/pci.h index f63529b..93bc163 100644 --- a/platform/pci.h +++ b/platform/pci.h @@ -15,7 +15,7 @@ #define index shadow_workaround_index #if defined (__NetBSD__) -#include <pciutils/pci.h> +#include <pci.h> #else #include <pci/pci.h> #endif -- To view, visit https://review.coreboot.org/c/flashrom/+/61525 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I69dc8184d1d012fb695770cbf6f7c64e5a024453 Gerrit-Change-Number: 61525 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: Makefile: remove CPPFLAGS and LDFLAGS to default directories
by Thomas Heijligen (Code Review) Feb. 1, 2022

Feb. 1, 2022

Thomas Heijligen has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/61526 ) Change subject: Makefile: remove CPPFLAGS and LDFLAGS to default directories ...................................................................... Makefile: remove CPPFLAGS and LDFLAGS to default directories All CPPFLAGS and LDFLAGS for dependencies are handled by pkg-config Change-Id: Ib7c11a0c8a7918562256480c4be0c95355f981c5 Signed-off-by: Thomas Heijligen <thomas.heijligen(a)secunet.com> --- M Makefile 1 file changed, 1 insertion(+), 6 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/26/61526/1 diff --git a/Makefile b/Makefile index 37af20b..d746f5a 100644 --- a/Makefile +++ b/Makefile @@ -243,7 +243,7 @@ HAS_LINUX_SPI := $(call c_compile_test, Makefile.d/linux_spi_test.c) HAS_LINUX_I2C := $(call c_compile_test, Makefile.d/linux_i2c_test.c) -ifeq ($(TARGET_OS), $(filter $(TARGET_OS), FreeBSD OpenBSD DragonFlyBSD)) +ifeq ($(TARGET_OS), $(filter $(TARGET_OS), DragonFlyBSD)) override CPPFLAGS += -I/usr/local/include override LDFLAGS += -L/usr/local/lib endif @@ -253,11 +253,6 @@ override LDFLAGS += -L/opt/local/lib -L/usr/local/lib endif -ifeq ($(TARGET_OS), NetBSD) -override CPPFLAGS += -I/usr/pkg/include -override LDFLAGS += -L/usr/pkg/lib -endif - ifeq ($(TARGET_OS), DOS) EXEC_SUFFIX := .exe # DJGPP has odd uint*_t definitions which cause lots of format string warnings. -- To view, visit https://review.coreboot.org/c/flashrom/+/61526 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ib7c11a0c8a7918562256480c4be0c95355f981c5 Gerrit-Change-Number: 61526 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: Makefile: check if librt is an external library
by Thomas Heijligen (Code Review) Feb. 1, 2022

Feb. 1, 2022

Attention is currently required from: Nico Huber, Thomas Heijligen, Angel Pons. Hello Felix Singer, Nico Huber, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/61523 to look at the new patch set (#2). Change subject: Makefile: check if librt is an external library ...................................................................... Makefile: check if librt is an external library Some systems, e.g. OpenBSD, have clock_gettime / librt build into the libc and therefore fail linking against it with -lrt. Thus, detect this and link only if needed. Change-Id: I2c1668a350aa0806fccfb4e9cd8b04861f085ee9 Signed-off-by: Thomas Heijligen <thomas.heijligen(a)secunet.com> --- M Makefile M Makefile.include 2 files changed, 8 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/23/61523/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/61523 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2c1668a350aa0806fccfb4e9cd8b04861f085ee9 Gerrit-Change-Number: 61523 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: Makefile: use pkg-config --libs --static for LDFLAGS
by Thomas Heijligen (Code Review) Feb. 1, 2022

Feb. 1, 2022

Attention is currently required from: Felix Singer, Nico Huber, Angel Pons. Hello Felix Singer, Nico Huber, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/61524 to look at the new patch set (#2). Change subject: Makefile: use pkg-config --libs --static for LDFLAGS ...................................................................... Makefile: use pkg-config --libs --static for LDFLAGS The --static flag of pkg-config returns also the LDFLAGS which are required to link the library static. Use this flag to successfully link against static libraries when the shared variant is not available. This is the case in OpenBSD with libpci. Change-Id: I6029a096c1ceca625789d18c88119d912d79bc0e Signed-off-by: Thomas Heijligen <thomas.heijligen(a)secunet.com> --- M Makefile M Makefile.include 2 files changed, 1 insertion(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/24/61524/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/61524 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6029a096c1ceca625789d18c88119d912d79bc0e Gerrit-Change-Number: 61524 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: Makefile: check if librt is an external library
by Felix Singer (Code Review) Feb. 1, 2022

Feb. 1, 2022

Attention is currently required from: Nico Huber, Thomas Heijligen, Angel Pons. Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/61523 ) Change subject: Makefile: check if librt is an external library ...................................................................... Patch Set 1: (4 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/61523/comment/8aec8ccc_cfa91ebd PS1, Line 9: OpenBSD e.g. OpenBSD https://review.coreboot.org/c/flashrom/+/61523/comment/4975a795_795f0141 PS1, Line 10: Detect Thus, detect [...] https://review.coreboot.org/c/flashrom/+/61523/comment/dea4711d_88de2772 PS1, Line 10: to link linking https://review.coreboot.org/c/flashrom/+/61523/comment/b47314fb_bc45ff34 PS1, Line 10: therefor therefor_e_ -- To view, visit https://review.coreboot.org/c/flashrom/+/61523 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2c1668a350aa0806fccfb4e9cd8b04861f085ee9 Gerrit-Change-Number: 61523 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Mon, 31 Jan 2022 23:04:35 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: Add mediatek_i2c_spi interface
by Neill Corlett (Code Review) Jan. 31, 2022

Jan. 31, 2022

Attention is currently required from: Nico Huber, Angel Pons. Neill Corlett has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/61288 ) Change subject: Add mediatek_i2c_spi interface ...................................................................... Patch Set 3: (4 comments) Patchset: PS1: > Thank you very much! it helps a lot to know what we are dealing with here. […] Will address in a follow-up change as needed File mediatek_i2c_spi.c: https://review.coreboot.org/c/flashrom/+/61288/comment/337dcca9_c0fcce64 PS1, Line 242: GPIO line > At the very least, I'd add an option to enable WP# control. Will address in a follow-up change as needed https://review.coreboot.org/c/flashrom/+/61288/comment/ae9bb281_f759f7a9 PS1, Line 249: 0x426, 7 > Having seen these kinds of datasheets before I know what you mean Neill lol. […] Ack https://review.coreboot.org/c/flashrom/+/61288/comment/2896e5f3_ea71de24 PS1, Line 255: 0x428, 7 > What are these magic numbers? See line #249 -- To view, visit https://review.coreboot.org/c/flashrom/+/61288 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I24adb14e7b4f7160e1c3ff941774064d5a81e820 Gerrit-Change-Number: 61288 Gerrit-PatchSet: 3 Gerrit-Owner: Neill Corlett <corlett(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-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-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Mon, 31 Jan 2022 17:48:43 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Neill Corlett <corlett(a)google.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: libflashrom,writeprotect: add functions for reading/writing WP settings
by Nikolai Artemiev (Code Review) Jan. 31, 2022

Jan. 31, 2022

Attention is currently required from: Nico Huber, Edward O'Callaghan, Anastasia Klimchuk, Sergii Dmytruk. Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58479 ) Change subject: libflashrom,writeprotect: add functions for reading/writing WP settings ...................................................................... Patch Set 28: (1 comment) File writeprotect.c: https://review.coreboot.org/c/flashrom/+/58479/comment/0893c62a_71d034a8 PS22, Line 56: struct wp_chip_config *cfg > Done Undoing done -- To view, visit https://review.coreboot.org/c/flashrom/+/58479 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3ad25708c3321b8fb0216c3eaf6ffc07616537ad Gerrit-Change-Number: 58479 Gerrit-PatchSet: 28 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.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: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Mon, 31 Jan 2022 06:16:29 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: libflashrom,writeprotect: add functions for reading/writing WP settings
by Nikolai Artemiev (Code Review) Jan. 31, 2022

Jan. 31, 2022

Attention is currently required from: Nico Huber, Edward O'Callaghan, Anastasia Klimchuk, Sergii Dmytruk. Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58479 ) Change subject: libflashrom,writeprotect: add functions for reading/writing WP settings ...................................................................... Patch Set 28: (5 comments) Patchset: PS22: > Sorry for the silence. I think you made good progress on your own! The abstract […] That sounds good. I'm working on implementing it now. File libflashrom.h: https://review.coreboot.org/c/flashrom/+/58479/comment/29939841_b3c4fc52 PS27, Line 133: }; > Having a struct in the API means we can't ever change the struct without […] That sounds good to me, it's good to address ABI stability problems early on. I've been going through and updating the patches, one thing I'm not sure about is how the `flashrom_wp_get_ranges()` function (CB:58481) should be handled. It currently allocates an output buffer of `struct wp_range` elements, but that has the same ABI problem. We could have something like a ```flashrom_wp_get_ranges(size_t **starts, size_t **lengths, size_t *counts, flashrom_flashctx *)``` function, though it's a bit messy compared to just returning an array of structures. Alternatively we could have an opaque wp_range_collection or something like that and an accessor function: ``` get_wp_range_from_collection(struct wp_range_collection *, size_t index) ``` https://review.coreboot.org/c/flashrom/+/58479/comment/689828b6_cbf40989 PS27, Line 138: #define FLASHROM_WP_ERR_VERIFY_FAILED 4 > This could be an enum, maybe even without specific numbers? I'm open to using an enum, though we would probably want to explicitly assign values to ensure API stability right? File libflashrom.h: https://review.coreboot.org/c/flashrom/+/58479/comment/4df583fb_daaf2d28 PS22, Line 125: software > If it's software protection, why not name it like that? I've removed the comment now, mentioning software protection is confusing/inaccurate since software protection may not be active and can easily be disabled if it is. File writeprotect.c: https://review.coreboot.org/c/flashrom/+/58479/comment/233d2333_f8e81846 PS22, Line 56: struct wp_chip_config *cfg > Only realized this now: this requires the caller to know the size of […] Done -- To view, visit https://review.coreboot.org/c/flashrom/+/58479 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3ad25708c3321b8fb0216c3eaf6ffc07616537ad Gerrit-Change-Number: 58479 Gerrit-PatchSet: 28 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.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: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Mon, 31 Jan 2022 06:13:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: Add mediatek_i2c_spi interface
by Nico Huber (Code Review) Jan. 27, 2022

Jan. 27, 2022

Attention is currently required from: Angel Pons, Neill Corlett. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/61288 ) Change subject: Add mediatek_i2c_spi interface ...................................................................... Patch Set 2: (1 comment) Patchset: PS1: > > Ah, hmmm, I'm not sure but I think there are SMBus controller drivers that could […] Thank you very much! it helps a lot to know what we are dealing with here. I think the cannonical flashrom way would be to add this as part of the `internal` programmer which handles on-board flashing. We have all the infrastructure there to detect a specific board, handle what is necessary to enable writes and eventually decide what driver will be used. Board detection and lifting write-protection is usually done in `board_enable.c`. From there one can call the initialization of a driver like `mstarddc_spi` (in this case with a hint if I2C or SMBus transfers should be used?). -- To view, visit https://review.coreboot.org/c/flashrom/+/61288 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I24adb14e7b4f7160e1c3ff941774064d5a81e820 Gerrit-Change-Number: 61288 Gerrit-PatchSet: 2 Gerrit-Owner: Neill Corlett <corlett(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-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-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Neill Corlett <corlett(a)google.com> Gerrit-Comment-Date: Thu, 27 Jan 2022 13:30:49 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Neill Corlett <corlett(a)google.com> Gerrit-MessageType: comment

1 0