November 2023 - flashrom-gerrit

Change in flashrom[master]: Makefile: Check for last line only from preprocessed output
by Khem Raj (Code Review) 01 Aug '24

01 Aug '24

Khem Raj has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/43770 ) Change subject: Makefile: Check for last line only from preprocessed output ...................................................................... Makefile: Check for last line only from preprocessed output This started to fail with glibc 2.32 since glibc added additional attributes to functions in signal.h therefore existing regexp started to fail as it is not able to handle these functions e.g. extern int siginterrupt (int __sig, int __interrupt) __attribute__ ((__nothrow__ , __leaf__)) __attribute__ ((__deprecated__ ("Use sigaction with SA_RESTART instead"))); grep -v '^\#' | grep '"' | cut -f 2 -d'"' bit outside of fd_set selected Use sigaction with SA_RESTART instead arm So changing it to tail -1 | grep '"' | cut -f 2 -d'"' arm Produces the expected result, this was hidden until now Signed-off-by: Khem Raj <raj.khem(a)gmail.com> Change-Id: I123a046e142d54632f12d54e2aa09b0928c02b91 --- M Makefile 1 file changed, 3 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/70/43770/1 diff --git a/Makefile b/Makefile index 803529f..3795681 100644 --- a/Makefile +++ b/Makefile @@ -106,7 +106,7 @@ # IMPORTANT: The following line must be placed before TARGET_OS is ever used # (of course), but should come after any lines setting CC because the line # below uses CC itself. -override TARGET_OS := $(strip $(call debug_shell,$(CC) $(CPPFLAGS) -E os.h 2>/dev/null | grep -v '^\#' | grep '"' | cut -f 2 -d'"')) +override TARGET_OS := $(strip $(call debug_shell,$(CC) $(CPPFLAGS) -E os.h 2>/dev/null | tail -1 | grep '"' | cut -f 2 -d'"')) ifeq ($(TARGET_OS), Darwin) override CPPFLAGS += -I/opt/local/include -I/usr/local/include @@ -460,8 +460,8 @@ # IMPORTANT: The following line must be placed before ARCH is ever used # (of course), but should come after any lines setting CC because the line # below uses CC itself. -override ARCH := $(strip $(call debug_shell,$(CC) $(CPPFLAGS) -E archtest.c 2>/dev/null | grep -v '^\#' | grep '"' | cut -f 2 -d'"')) -override ENDIAN := $(strip $(call debug_shell,$(CC) $(CPPFLAGS) -E endiantest.c 2>/dev/null | grep -v '^\#')) +override ARCH := $(strip $(call debug_shell,$(CC) $(CPPFLAGS) -E archtest.c 2>/dev/null | tail -1 | grep '"' | cut -f 2 -d'"')) +override ENDIAN := $(strip $(call debug_shell,$(CC) $(CPPFLAGS) -E endiantest.c 2>/dev/null | tail -1)) # Disable the internal programmer on unsupported architectures (everything but x86 and mipsel) ifneq ($(ARCH)-little, $(filter $(ARCH),x86 mips)-$(ENDIAN)) -- To view, visit https://review.coreboot.org/c/flashrom/+/43770 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I123a046e142d54632f12d54e2aa09b0928c02b91 Gerrit-Change-Number: 43770 Gerrit-PatchSet: 1 Gerrit-Owner: Khem Raj Gerrit-MessageType: newchange

4 5

Change in flashrom[master]: Add -- to make clean
by Ryan O'Leary (Code Review) 01 Aug '24

01 Aug '24

Ryan O'Leary has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/37776 ) Change subject: Add -- to make clean ...................................................................... Add -- to make clean Somehow, I ended up with a file name "-c.d". I probably made it by mistake. This change lets "make clean" be a bit more resilient to files such as these. Change-Id: I2517ffac975f3df75f706350a07f189a98a11b7c Signed-off-by: Ryan O'Leary <ryanoleary(a)google.com> --- M Makefile 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/76/37776/1 diff --git a/Makefile b/Makefile index 518d41b..fa66910 100644 --- a/Makefile +++ b/Makefile @@ -1129,7 +1129,7 @@ # This includes all frontends and libflashrom. # We don't use EXEC_SUFFIX here because we want to clean everything. clean: - rm -f $(PROGRAM) $(PROGRAM).exe libflashrom.a *.o *.d $(PROGRAM).8 $(PROGRAM).8.html $(BUILD_DETAILS_FILE) + rm -f -- $(PROGRAM) $(PROGRAM).exe libflashrom.a *.o *.d $(PROGRAM).8 $(PROGRAM).8.html $(BUILD_DETAILS_FILE) @+$(MAKE) -C util/ich_descriptors_tool/ clean distclean: clean -- To view, visit https://review.coreboot.org/c/flashrom/+/37776 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2517ffac975f3df75f706350a07f189a98a11b7c Gerrit-Change-Number: 37776 Gerrit-PatchSet: 1 Gerrit-Owner: Ryan O'Leary <ryanoleary(a)google.com> Gerrit-MessageType: newchange

6 8

Change in flashrom[master]: dummyflasher.c: Use programmer logger functions
by Angel Pons (Code Review) 11 Apr '24

11 Apr '24

Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/46523 ) Change subject: dummyflasher.c: Use programmer logger functions ...................................................................... dummyflasher.c: Use programmer logger functions All but three log messages use the programmer logger. Change the three outliers that were using the chip logger accordingly. Change-Id: Ia8668e05df2da739e6bb4c7d0fddad86e8d054a3 Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> --- M dummyflasher.c 1 file changed, 3 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/23/46523/1 diff --git a/dummyflasher.c b/dummyflasher.c index 6426ad2..d233db4 100644 --- a/dummyflasher.c +++ b/dummyflasher.c @@ -610,7 +610,7 @@ } for (i = 0; i < data->spi_ignorelist_size; i++) { if (writearr[0] == data->spi_ignorelist[i]) { - msg_cdbg("Ignoring ignorelisted SPI command 0x%02x\n", + msg_pdbg("Ignoring ignorelisted SPI command 0x%02x\n", data->spi_ignorelist[i]); /* Return success because the command does not fail, * it is simply ignored. @@ -1050,7 +1050,7 @@ * Search "total_size * 1024" in code. */ flash->chip->total_size = emu_data->emu_chip_size / 1024; - msg_cdbg("%s: set flash->total_size to %dK bytes.\n", __func__, + msg_pdbg("%s: set flash->total_size to %dK bytes.\n", __func__, flash->chip->total_size); /* Update the first count of each of the block_erasers. */ @@ -1061,7 +1061,7 @@ eraser->eraseblocks[0].count = 1; eraser->eraseblocks[0].size = emu_data->emu_chip_size; - msg_cdbg("%s: eraser.size=%d, .count=%d\n", + msg_pdbg("%s: eraser.size=%d, .count=%d\n", __func__, eraser->eraseblocks[0].size, eraser->eraseblocks[0].count); } -- To view, visit https://review.coreboot.org/c/flashrom/+/46523 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia8668e05df2da739e6bb4c7d0fddad86e8d054a3 Gerrit-Change-Number: 46523 Gerrit-PatchSet: 1 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newchange

3 3

Change in flashrom[master]: it85spi.c: Kill off dead code system() hack
by Edward O'Callaghan (Code Review) 08 Dec '23

08 Dec '23

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/46547 ) Change subject: it85spi.c: Kill off dead code system() hack ...................................................................... it85spi.c: Kill off dead code system() hack This is dead code sitting under a 'if 0' and is extremely hacky. One could argue this patch perhaps should include a update to documentation to stop powerd instead of keeping this rot around? BUG=none BRANCH=none TEST=builds Change-Id: I5761dea48d553b1334eef3c9cf0ad79cedbd7806 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M it85spi.c 1 file changed, 0 insertions(+), 21 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/47/46547/1 diff --git a/it85spi.c b/it85spi.c index b998790..1d93a5b 100644 --- a/it85spi.c +++ b/it85spi.c @@ -114,15 +114,6 @@ if (it85xx_scratch_rom_reenter > 0) return; -#if 0 - /* FIXME: this a workaround for the bug that SMBus signal would - * interfere the EC firmware update. Should be removed if - * we find out the root cause. */ - ret = system("stop powerd >&2"); - if (ret) - msg_perr("Cannot stop powerd.\n"); -#endif - for (tries = 0; tries < MAX_TRY; ++tries) { /* Wait until IBF (input buffer) is not full. */ if (wait_for(KB_IBF, 0, MAX_TIMEOUT, @@ -165,9 +156,6 @@ static void it85xx_exit_scratch_rom(void) { -#if 0 - int ret; -#endif int tries; msg_pdbg("%s():%d was called ...\n", __func__, __LINE__); @@ -204,15 +192,6 @@ } else { msg_perr("%s():%d * Max try reached.\n", __func__, __LINE__); } - -#if 0 - /* FIXME: this a workaround for the bug that SMBus signal would - * interfere the EC firmware update. Should be removed if - * we find out the root cause. */ - ret = system("start powerd >&2"); - if (ret) - msg_perr("Cannot start powerd again.\n"); -#endif } static int it85xx_shutdown(void *data) -- To view, visit https://review.coreboot.org/c/flashrom/+/46547 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I5761dea48d553b1334eef3c9cf0ad79cedbd7806 Gerrit-Change-Number: 46547 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

4 8

[XS] Change in flashrom[main]: erasure_layout: Remove redundant `verify_range` call from `erase_write`
by Carly Zlabek (Code Review) 01 Dec '23

01 Dec '23

Attention is currently required from: Aarya, Anastasia Klimchuk, Vincent Fazio. Carly Zlabek has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/79354?usp=email ) Change subject: erasure_layout: Remove redundant `verify_range` call from `erase_write` ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: Testing Summary: 3 types of testing: 1. Switching from 1 bios version to another 2. Erasing the entire chip 3. Programming a bios starting from empty chip contents Averages (seconds): Note: The summation of the individual parts compared to the total may differ by a couple seconds, the timing captures were not exactly back to back. Legacy Erase Path: - The field 'Verify erase' was lumped into `Erase relevant parts` in the New Erase Path test results 1. Read previous contents: 57.025 Erase relevant parts: 27.336 Verify erase: 2.655 Write time: 9.323 Verify write: 58.808 Total execution time: 157.591 2. Read previous contents: 0 Erase chip contents: 198.692 Verify erase: 32.804 Write time: 0 Verify write: 0 Total execution time: 241.46 3. Read previous contents: 56.980 Erase chip contents: 0 Verify erase: 0 Write time: 23.563 Verify write: 58.786 Total execution time: 141.540 New Erase Path: 1. Read previous contents: 56.802 Erase relevant parts: 25.697 Write time: 10.342 Verify write: 57.510 Verify contents: 58.718 Total execution time: 209.89 2. Erase chip contents: 141.953 Write to chip: 0.091 (artifact from timing capture location) Verify contents: 57.423 Total execution time: 200.929 3. Read previous contents: 57.110 Erase relevant parts: 0 (already empty) Write time: 25.206 Verify write: 57.502 Verify contents: 58.902 Total execution time: 199.37 New Erase Path Sans Redundant Verification: 1. Read previous contents: 56.991 Erase relevant parts: 25.679 Write time: 10.323 Verify contents: 58.799 Total execution time: 153.451 2. Erase chip contents: 143.002 Write to chip: 0.088 (artifact from timing capture location) Total execution time: 143.646 3. Read previous contents: 56.939 Erase relevant parts: 0 (already empty) Write time: 25.174 Verify contents: 58.692 Total execution time: 141.396 -- To view, visit https://review.coreboot.org/c/flashrom/+/79354?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I638835facd9311979c4991cc4ca41a4b9e174bd5 Gerrit-Change-Number: 79354 Gerrit-PatchSet: 1 Gerrit-Owner: Carly Zlabek <carlyzlabek(a)gmail.com> Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Vincent Fazio <vfazio(a)gmail.com> Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Vincent Fazio <vfazio(a)gmail.com> Gerrit-Comment-Date: Thu, 30 Nov 2023 23:49:10 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[XS] Change in flashrom[main]: erasure_layout: Remove redundant `verify_range` call from `erase_write`
by Carly Zlabek (Code Review) 30 Nov '23

30 Nov '23

Attention is currently required from: Vincent Fazio. Hello Vincent Fazio, I'd like you to do a code review. Please visit https://review.coreboot.org/c/flashrom/+/79354?usp=email to review the following change. Change subject: erasure_layout: Remove redundant `verify_range` call from `erase_write` ...................................................................... erasure_layout: Remove redundant `verify_range` call from `erase_write` Previously, verification would be unconditionally completed at the end of `erase_write` with the non-legacy erase path. Verification is already handled when writing and erasing chip data; in `flashrom_image_write` based on `flashctx` options, and in the erase loop of `erase_write` via `check_erased_range` calls, respectively. This unnecessary verification added a performance penalty that was not present with the legacy implementation. Now, this call to `verify_range` has been removed to improve performance and more closely reflect the verification done with the legacy path. This change was tested using the linux_spi programmer to erase and write to an MT25QL512 chip. Change-Id: I638835facd9311979c4991cc4ca41a4b9e174bd5 Signed-off-by: Carly Zlabek <carlyzlabek(a)gmail.com> Signed-off-by: Vincent Fazio <vfazio(a)gmail.com> --- M erasure_layout.c 1 file changed, 0 insertions(+), 7 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/54/79354/1 diff --git a/erasure_layout.c b/erasure_layout.c index 0c31911..febfde1 100644 --- a/erasure_layout.c +++ b/erasure_layout.c @@ -385,13 +385,6 @@ *all_skipped = false; } - // verify write - ret = verify_range(flashctx, newcontents + region_start, region_start, region_end - region_start); - if (ret) { - msg_cerr("Verifying flash. Write failed for range %#"PRIx32" : %#"PRIx32", Abort.\n", - region_start, region_end); - goto _end; - } _end: memcpy(newcontents + region_start, old_start_buf, old_start - region_start); -- To view, visit https://review.coreboot.org/c/flashrom/+/79354?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I638835facd9311979c4991cc4ca41a4b9e174bd5 Gerrit-Change-Number: 79354 Gerrit-PatchSet: 1 Gerrit-Owner: Carly Zlabek <carlyzlabek(a)gmail.com> Gerrit-Reviewer: Vincent Fazio <vfazio(a)gmail.com> Gerrit-Attention: Vincent Fazio <vfazio(a)gmail.com> Gerrit-MessageType: newchange

1 0

[XL] Change in flashrom[main]: erasure_layout: Remove redundant `verify_range` call from `erase_write`
by Carly Zlabek (Code Review) 30 Nov '23

30 Nov '23

Attention is currently required from: Aarya, Anastasia Klimchuk. Carly Zlabek has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/79324?usp=email ) Change subject: erasure_layout: Remove redundant `verify_range` call from `erase_write` ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: Patchset 2 includes files containing test results to accompany the changes introduced in Patchset 1. -- To view, visit https://review.coreboot.org/c/flashrom/+/79324?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I638835facd9311979c4991cc4ca41a4b9e174bd5 Gerrit-Change-Number: 79324 Gerrit-PatchSet: 2 Gerrit-Owner: Carly Zlabek <carlyzlabek(a)gmail.com> Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Vincent Fazio <vfazio(a)gmail.com> Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Thu, 30 Nov 2023 20:38:01 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in flashrom[main]: pci2: add a second device private called data
by Luc Verhaegen (Code Review) 30 Nov '23

30 Nov '23

Luc Verhaegen has abandoned this change. ( https://review.coreboot.org/c/flashrom/+/29082?usp=email ) Change subject: pci2: add a second device private called data ...................................................................... Abandoned I have 0 interest in testing, supporting or maintaining this code, and i doubt whether any of the reviewers here is willing to get the hardware to properly test this code. -- To view, visit https://review.coreboot.org/c/flashrom/+/29082?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I7927773a88439ca3828d694785e58a4cf2b7795c Gerrit-Change-Number: 29082 Gerrit-PatchSet: 3 Gerrit-Owner: Luc Verhaegen <libv(a)skynet.be> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-MessageType: abandon

1 0

[M] Change in flashrom[main]: pci2: add device enable/disable
by Luc Verhaegen (Code Review) 30 Nov '23

30 Nov '23

Luc Verhaegen has abandoned this change. ( https://review.coreboot.org/c/flashrom/+/29081?usp=email ) Change subject: pci2: add device enable/disable ...................................................................... Abandoned I have 0 interest in testing, supporting or maintaining this code, and i doubt whether any of the reviewers here is willing to get the hardware to properly test this code. -- To view, visit https://review.coreboot.org/c/flashrom/+/29081?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I79bbc5c4b0f571df692e0992f43b46da7ab3bf01 Gerrit-Change-Number: 29081 Gerrit-PatchSet: 3 Gerrit-Owner: Luc Verhaegen <libv(a)skynet.be> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-MessageType: abandon

1 0

[M] Change in flashrom[main]: pci2: add mmio bar map/unmap
by Luc Verhaegen (Code Review) 30 Nov '23

30 Nov '23

Luc Verhaegen has abandoned this change. ( https://review.coreboot.org/c/flashrom/+/29079?usp=email ) Change subject: pci2: add mmio bar map/unmap ...................................................................... Abandoned I have 0 interest in testing, supporting or maintaining this code, and i doubt whether any of the reviewers here is willing to get the hardware to properly test this code. -- To view, visit https://review.coreboot.org/c/flashrom/+/29079?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I1c67d51d853f751d13271bf83316ee2118c1c476 Gerrit-Change-Number: 29079 Gerrit-PatchSet: 3 Gerrit-Owner: Luc Verhaegen <libv(a)skynet.be> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-MessageType: abandon

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit November 2023