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

Change in flashrom[master]: tests: Add unit test to run init/shutdown for linux_spi.c
by Nico Huber (Code Review) 22 Apr '21

22 Apr '21

Attention is currently required from: Angel Pons, Anastasia Klimchuk. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52498 ) Change subject: tests: Add unit test to run init/shutdown for linux_spi.c ...................................................................... Patch Set 3: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/52498/comment/be00bc25_cd4bb2ac PS3, Line 22: AFAICS, the current implementation tests a particular path of the init procedure. There are two ways for it to succeed: reading the buffer size from sysfs and the fallback to getpagesize(). It would be nice to mention the tested path in the commit message and as a comment in linux_spi_init_and_shutdown_test_success(). -- To view, visit https://review.coreboot.org/c/flashrom/+/52498 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I4911fbb6f04371283f0e62d2196bdd691a227584 Gerrit-Change-Number: 52498 Gerrit-PatchSet: 3 Gerrit-Owner: 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Thu, 22 Apr 2021 11:59:35 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tests: Add unit test to run init/shutdown for dummyflasher.c
by Nico Huber (Code Review) 22 Apr '21

22 Apr '21

Attention is currently required from: Edward O'Callaghan, Angel Pons, Anastasia Klimchuk. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52497 ) Change subject: tests: Add unit test to run init/shutdown for dummyflasher.c ...................................................................... Patch Set 3: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/52497 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3c0ef73397f00c1db7aabb5f9f00cb43525af29c Gerrit-Change-Number: 52497 Gerrit-PatchSet: 3 Gerrit-Owner: 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-CC: 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: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Thu, 22 Apr 2021 11:51:24 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: dummyflasher.c: Fix memory leak on shutdown
by Nico Huber (Code Review) 22 Apr '21

22 Apr '21

Nico Huber has submitted this change. ( https://review.coreboot.org/c/flashrom/+/52557 ) Change subject: dummyflasher.c: Fix memory leak on shutdown ...................................................................... dummyflasher.c: Fix memory leak on shutdown emu_data *data is allocated in init function and needs to be freed in shutdown function. BUG=b:181803212 TEST=builds and ninja test Change-Id: I36f76d84d3547d081c64857e06da23ee63cc5594 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/52557 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> --- M dummyflasher.c 1 file changed, 1 insertion(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved Edward O'Callaghan: Looks good to me, approved diff --git a/dummyflasher.c b/dummyflasher.c index 5190282..ec5060d 100644 --- a/dummyflasher.c +++ b/dummyflasher.c @@ -650,6 +650,7 @@ free(flashchip_contents); } #endif + free(data); return 0; } -- To view, visit https://review.coreboot.org/c/flashrom/+/52557 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I36f76d84d3547d081c64857e06da23ee63cc5594 Gerrit-Change-Number: 52557 Gerrit-PatchSet: 2 Gerrit-Owner: 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

Change in flashrom[master]: tests: Add logging to STDOUT when __wrap function is called
by Nico Huber (Code Review) 22 Apr '21

22 Apr '21

Attention is currently required from: Angel Pons, Anastasia Klimchuk. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52496 ) Change subject: tests: Add logging to STDOUT when __wrap function is called ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/52496 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ifcef55c9cdb7756c38dcc44fdc57cd88c3d65e70 Gerrit-Change-Number: 52496 Gerrit-PatchSet: 2 Gerrit-Owner: 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Thu, 22 Apr 2021 11:40:30 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: flashrom.c: automatic generated programmer_enum.h
by Nico Huber (Code Review) 22 Apr '21

22 Apr '21

Attention is currently required from: Sam McNally, Xiang Wang, Stefan Reinauer. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/49599 ) Change subject: flashrom.c: automatic generated programmer_enum.h ...................................................................... Patch Set 6: (1 comment) File Makefile: https://review.coreboot.org/c/flashrom/+/49599/comment/424b728f_06913680 PS4, Line 681: $(shell bash ./util/generator_programmer_enum.sh) > Thank you. I know these, but this modification will be inconsistent with meson. […] I guess you still haven't tested it. Every file that directly or indirectly #includes the generated header would (with the current version) be recompiled even if nothing changed. -- To view, visit https://review.coreboot.org/c/flashrom/+/49599 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3f4370ccae2b64da3c4178243b192700d3d205d2 Gerrit-Change-Number: 49599 Gerrit-PatchSet: 6 Gerrit-Owner: Xiang Wang <wxjstz(a)126.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Sam McNally <sammc(a)google.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-CC: Idwer Vollering <vidwer(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Shawn C <citypw(a)hardenedlinux.org> Gerrit-Attention: Sam McNally <sammc(a)google.com> Gerrit-Attention: Xiang Wang <wxjstz(a)126.com> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Comment-Date: Thu, 22 Apr 2021 11:29:49 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Xiang Wang <wxjstz(a)126.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: Revert "Makefile: Explicitly set '-std=c99'"
by Nico Huber (Code Review) 22 Apr '21

22 Apr '21

Nico Huber has submitted this change. ( https://review.coreboot.org/c/flashrom/+/52425 ) Change subject: Revert "Makefile: Explicitly set '-std=c99'" ...................................................................... Revert "Makefile: Explicitly set '-std=c99'" This reverts commit 5e5c3f15efe262ff23642dd543faf6a9dbb3bbdb. Reason for revert: Breaks building on PowerPC, NetBSD, DJGPP, and it is possible that SPARC fails to build as well. Change-Id: I57b5125207de3fd156dface67cba605da893d6aa Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/52425 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> Reviewed-by: Nico Huber <nico.h(a)gmx.de> --- M Makefile M dmi.c 2 files changed, 1 insertion(+), 3 deletions(-) Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved Edward O'Callaghan: Looks good to me, approved Anastasia Klimchuk: Looks good to me, but someone else must approve diff --git a/Makefile b/Makefile index d496a9a..dd19e14 100644 --- a/Makefile +++ b/Makefile @@ -30,7 +30,7 @@ DIFF = diff PREFIX ?= /usr/local MANDIR ?= $(PREFIX)/share/man -CFLAGS ?= -std=c99 -D_DEFAULT_SOURCE -D_POSIX_C_SOURCE=200809L -D_BSD_SOURCE -Os -Wall -Wextra -Wno-unused-parameter -Wshadow -Wmissing-prototypes -Wwrite-strings +CFLAGS ?= -Os -Wall -Wextra -Wno-unused-parameter -Wshadow -Wmissing-prototypes -Wwrite-strings EXPORTDIR ?= . RANLIB ?= ranlib PKG_CONFIG ?= pkg-config diff --git a/dmi.c b/dmi.c index 55edb03..e449cd9 100644 --- a/dmi.c +++ b/dmi.c @@ -19,9 +19,7 @@ /* strnlen is in POSIX but was a GNU extension up to glibc 2.10 */ #if (__GLIBC__ == 2 && __GLIBC_MINOR__ < 10) || __GLIBC__ < 2 -#ifndef _GNU_SOURCE #define _GNU_SOURCE -#endif /* !GNU_SOURCE */ #else #define _POSIX_C_SOURCE 200809L #endif -- To view, visit https://review.coreboot.org/c/flashrom/+/52425 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I57b5125207de3fd156dface67cba605da893d6aa Gerrit-Change-Number: 52425 Gerrit-PatchSet: 4 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> 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: Sam McNally <sammc(a)google.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Idwer Vollering <vidwer(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

Change in flashrom[master]: ft2232_spi.c: Implement spi_send_multicommand()
by Simon Buhrow (Code Review) 22 Apr '21

22 Apr '21

Attention is currently required from: Alan Green, Nico Huber, Samir Ibradžić. Simon Buhrow has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/40477 ) Change subject: ft2232_spi.c: Implement spi_send_multicommand() ...................................................................... Patch Set 28: (5 comments) Patchset: PS28: Thank you very much for the comments! I´ll do the left changes next time I get some more idle time. File ft2232_spi.c: https://review.coreboot.org/c/flashrom/+/40477/comment/76c687d3_9a814477 PS25, Line 278: struct spi_command *cmds) > This is very oddly indented. Tabs and spaces but still not aligned […] Done https://review.coreboot.org/c/flashrom/+/40477/comment/f01839fa_9fefe416 PS25, Line 283: int i = 0,ret = 0, failed = 0; > missing space after first comma. Done https://review.coreboot.org/c/flashrom/+/40477/comment/d05ec628_5ae95ca2 PS25, Line 304: 0 & ~pinlvl; > The respective expression in ft2232_spi_send_command() got a fix in […] Done https://review.coreboot.org/c/flashrom/+/40477/comment/d317a6e5_218938c1 PS25, Line 326: optionally > Either […] Done -- To view, visit https://review.coreboot.org/c/flashrom/+/40477 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie4a07499ec5ef0af23818593f45dc427285a9e8a Gerrit-Change-Number: 40477 Gerrit-PatchSet: 28 Gerrit-Owner: Simon Buhrow Gerrit-Reviewer: Alan Green <avg(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: Samir Ibradžić <sibradzic(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Patrick Georgi <pgeorgi(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Alan Green <avg(a)google.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Samir Ibradžić <sibradzic(a)gmail.com> Gerrit-Comment-Date: Thu, 22 Apr 2021 11:23:58 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: linux_spi.c: Resolve comments left from previous patches
by Nico Huber (Code Review) 22 Apr '21

22 Apr '21

Attention is currently required from: Anastasia Klimchuk. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52492 ) Change subject: linux_spi.c: Resolve comments left from previous patches ...................................................................... Patch Set 1: Code-Review+2 (2 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/52492/comment/b14c57c2_3454f5aa PS1, Line 7: linux_spi.c: Resolve comments left from previous patches The summary should be about the changes, not what led to them :) Maybe something like this: linux_spi: Drop some unnecessary initialisations and checks Patchset: PS1: Thanks for the quick reaction! I don't see it as your responsibility to fix things up after a patch is merged btw., but it's really nice that you did :) -- To view, visit https://review.coreboot.org/c/flashrom/+/52492 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I27a718b515fc474f63b3e61be58a6f9302527559 Gerrit-Change-Number: 52492 Gerrit-PatchSet: 1 Gerrit-Owner: 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Sam McNally <sammc(a)google.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Thu, 22 Apr 2021 11:23:40 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ft2232_spi.c: Implement spi_send_multicommand()
by Simon Buhrow (Code Review) 22 Apr '21

22 Apr '21

Attention is currently required from: Simon Buhrow, Alan Green, Nico Huber, Samir Ibradžić. Hello build bot (Jenkins), Alan Green, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, Samir Ibradžić, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/40477 to look at the new patch set (#28). Change subject: ft2232_spi.c: Implement spi_send_multicommand() ...................................................................... ft2232_spi.c: Implement spi_send_multicommand() Every ftdi_write_data() call is quite time consuming as the ftdi-chips seems to take always 2-3ms to respond. This leads to what the comment already says: Minimize USB transfers by packing as many commands as possible together. So I packed the WREN command together with the following operation which can be program or erase operation. This saves about 1 minute when programming a 128MBit Flash with my config! Signed-off-by: Simon Buhrow <simon.buhrow(a)posteo.de> Change-Id: Ie4a07499ec5ef0af23818593f45dc427285a9e8a --- M ft2232_spi.c 1 file changed, 90 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/77/40477/28 -- To view, visit https://review.coreboot.org/c/flashrom/+/40477 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie4a07499ec5ef0af23818593f45dc427285a9e8a Gerrit-Change-Number: 40477 Gerrit-PatchSet: 28 Gerrit-Owner: Simon Buhrow Gerrit-Reviewer: Alan Green <avg(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: Samir Ibradžić <sibradzic(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Patrick Georgi <pgeorgi(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Simon Buhrow Gerrit-Attention: Alan Green <avg(a)google.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Samir Ibradžić <sibradzic(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: s25f.c: Fix mismatched function definitions
by Angel Pons (Code Review) 22 Apr '21

22 Apr '21

Attention is currently required from: Edward O'Callaghan, Arthur Heymans. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52473 ) Change subject: s25f.c: Fix mismatched function definitions ...................................................................... Patch Set 1: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/52473/comment/7bea141b_7d20031a PS1, Line 8: : This was missed because `uint32_t` is `unsigned int` in most cases. : However, it is not the case for DJGPP 6.1.0 for some reason. > Edward, any updates? Edward, any updates? -- To view, visit https://review.coreboot.org/c/flashrom/+/52473 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I656a72b85d4c70b57f6ff9268186a4a60933f8a9 Gerrit-Change-Number: 52473 Gerrit-PatchSet: 1 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Thu, 22 Apr 2021 11:09:27 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit April 2021