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

Change in flashrom[master]: Makefile: Only enable I2C programmers on Linux
by Edward O'Callaghan (Code Review) 09 May '21

09 May '21

Attention is currently required from: Angel Pons. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/53999 ) Change subject: Makefile: Only enable I2C programmers on Linux ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/53999 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I04d25ff8f8c3097428ac8695669b1757c38f49e9 Gerrit-Change-Number: 53999 Gerrit-PatchSet: 1 Gerrit-Owner: 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-Comment-Date: Mon, 10 May 2021 03:34:36 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: libflashrom: Return progress state to the library user
by Edward O'Callaghan (Code Review) 09 May '21

09 May '21

Attention is currently required from: Richard Hughes, Angel Pons, Anastasia Klimchuk, Patrick Rudolph. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/49643 ) Change subject: libflashrom: Return progress state to the library user ...................................................................... Patch Set 7: Code-Review+1 (1 comment) Patchset: PS4: > I added some reviewers Richard to help try and make progress with libflashrom improvements. […] Any thoughts on the splitting up idea suggested above Richard? -- To view, visit https://review.coreboot.org/c/flashrom/+/49643 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7197572bb7f19e3bdb2bde855d70a0f50fd3854c Gerrit-Change-Number: 49643 Gerrit-PatchSet: 7 Gerrit-Owner: Richard Hughes <richard(a)hughsie.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Daniel Campello <campello(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Richard Hughes <hughsient(a)gmail.com> Gerrit-CC: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Richard Hughes <richard(a)hughsie.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Mon, 10 May 2021 03:33:47 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tests: Add unit test to run init/shutdown for mec1308.c, ene_lpc.c
by Anastasia Klimchuk (Code Review) 09 May '21

09 May '21

Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons, Simon Glass. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/51487 ) Change subject: tests: Add unit test to run init/shutdown for mec1308.c, ene_lpc.c ...................................................................... Patch Set 13: (2 comments) Patchset: PS13: I rebased this on the top of io_mock.h. Also I went through all the comments to summarize what's left to do / to decide on: 1) Moving hwaccess_io_unittest.h into tests/ directory? a) there is a unittest_env.h in the same situation, these two files should be moved (or not moved) together, and probably in a separate patch? b) I don't know whether I can move flashrom_test_dep into tests/meson.build because it is using srcs and deps from root meson.build. 2) Configure verbosity level for LOG_ME messages a) agree it would be cool, don't know how to do this yet 3) Need to fix compiler warning ../tests/tests.c:30:13: warning: assignment discards ‘const’ qualifier from pointer target type But the question in general is whether I am using `io_mock_register` in a right way? File tests/init_shutdown.c: https://review.coreboot.org/c/flashrom/+/51487/comment/bbe3c91f_4c759b19 PS10, Line 46: struct mec1308_test_data *test_data = current_io->test_data; > I think you should add a blank line between decls and code Done -- To view, visit https://review.coreboot.org/c/flashrom/+/51487 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3af612defe1af3850dfc1626a208d873e3a3eddc Gerrit-Change-Number: 51487 Gerrit-PatchSet: 13 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: Simon Glass <sjg(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Simon Glass <sjg(a)chromium.org> Gerrit-Comment-Date: Mon, 10 May 2021 01:51:52 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Simon Glass <sjg(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tests: Add unit test to run init/shutdown for mec1308.c, ene_lpc.c
by Anastasia Klimchuk (Code Review) 09 May '21

09 May '21

Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk. Hello build bot (Jenkins), Nico Huber, Edward O'Callaghan, Angel Pons, Simon Glass, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/51487 to look at the new patch set (#13). Change subject: tests: Add unit test to run init/shutdown for mec1308.c, ene_lpc.c ...................................................................... tests: Add unit test to run init/shutdown for mec1308.c, ene_lpc.c This patch includes mocks for io operations in hwaccess_x86_io.h because those are needed to test lifecycle of mec1308.c and ene_lpc.c BUG=b:181803212 TEST=builds and ninja test Change-Id: I3af612defe1af3850dfc1626a208d873e3a3eddc Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- A hwaccess_x86_io_unittest.h M meson.build M tests/init_shutdown.c M tests/io_mock.h M tests/meson.build M tests/tests.c M tests/tests.h 7 files changed, 249 insertions(+), 7 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/87/51487/13 -- To view, visit https://review.coreboot.org/c/flashrom/+/51487 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3af612defe1af3850dfc1626a208d873e3a3eddc Gerrit-Change-Number: 51487 Gerrit-PatchSet: 13 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: Simon Glass <sjg(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> 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-MessageType: newpatchset

1 0

Change in flashrom[master]: tests: Add unit test to run init/shutdown for mec1308.c, ene_lpc.c
by Anastasia Klimchuk (Code Review) 09 May '21

09 May '21

Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk. Hello build bot (Jenkins), Nico Huber, Edward O'Callaghan, Angel Pons, Simon Glass, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/51487 to look at the new patch set (#12). Change subject: tests: Add unit test to run init/shutdown for mec1308.c, ene_lpc.c ...................................................................... tests: Add unit test to run init/shutdown for mec1308.c, ene_lpc.c This patch includes mocks for io operations in hwaccess_x86_io.h because those are needed to test lifecycle of mec1308.c and ene_lpc.c BUG=b:181803212 TEST=builds and ninja test Change-Id: I3af612defe1af3850dfc1626a208d873e3a3eddc Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- A hwaccess_x86_io_unittest.h M meson.build M tests/init_shutdown.c M tests/io_mock.h M tests/meson.build M tests/tests.c M tests/tests.h 7 files changed, 248 insertions(+), 7 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/87/51487/12 -- To view, visit https://review.coreboot.org/c/flashrom/+/51487 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3af612defe1af3850dfc1626a208d873e3a3eddc Gerrit-Change-Number: 51487 Gerrit-PatchSet: 12 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: Simon Glass <sjg(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> 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-MessageType: newpatchset

1 0

Change in flashrom[master]: tests: Add unit test to run init/shutdown for mec1308.c, ene_lpc.c
by Anastasia Klimchuk (Code Review) 09 May '21

09 May '21

Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk. Hello build bot (Jenkins), Nico Huber, Edward O'Callaghan, Angel Pons, Simon Glass, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/51487 to look at the new patch set (#11). Change subject: tests: Add unit test to run init/shutdown for mec1308.c, ene_lpc.c ...................................................................... tests: Add unit test to run init/shutdown for mec1308.c, ene_lpc.c This patch includes mocks for io operations in hwaccess_x86_io.h because those are needed to test lifecycle of mec1308.c and ene_lpc.c BUG=b:181803212 TEST=builds and ninja test Change-Id: I3af612defe1af3850dfc1626a208d873e3a3eddc Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- A hwaccess_x86_io_unittest.h M meson.build M tests/init_shutdown.c M tests/io_mock.h M tests/meson.build M tests/tests.c M tests/tests.h 7 files changed, 242 insertions(+), 7 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/87/51487/11 -- To view, visit https://review.coreboot.org/c/flashrom/+/51487 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3af612defe1af3850dfc1626a208d873e3a3eddc Gerrit-Change-Number: 51487 Gerrit-PatchSet: 11 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: Simon Glass <sjg(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> 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-MessageType: newpatchset

1 0

Change in flashrom[master]: linux_mtd: move global state into programmer data field
by Anastasia Klimchuk (Code Review) 09 May '21

09 May '21

Attention is currently required from: Nikolai Artemiev. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/53947 ) Change subject: linux_mtd: move global state into programmer data field ...................................................................... Patch Set 4: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/53947 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I5ce6900e4892ed5687cfddb245dfe5461a3e2e84 Gerrit-Change-Number: 53947 Gerrit-PatchSet: 4 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Mon, 10 May 2021 00:29:15 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: WIP: Allow opaque programmers to provide status register access
by Edward O'Callaghan (Code Review) 09 May '21

09 May '21

Attention is currently required from: Nikolai Artemiev. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/53948 ) Change subject: WIP: Allow opaque programmers to provide status register access ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: Nikolai, please mark this is WIP or abandon as the ichspi piece to this story downstream needs cleaning up before any of this can be presented upstream. It paints a confusing or at best an incomplete picture. The ichspi part downstream has globals and is fragile currently so needs a bit of work. Furthermore, the ignore_error part is very messy and also needs cleaning up. I believe Damien had some WIP patches for the ignore_error part for ME. In any case it is better to keep focused on the writeprotect cleanups so upstream has the complete implementation and they are in sync and not worry about this right now. -- To view, visit https://review.coreboot.org/c/flashrom/+/53948 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I11a03c84363ec68a8edcbb9b10c7ca115e222fe0 Gerrit-Change-Number: 53948 Gerrit-PatchSet: 1 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Damien Zammit Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Mon, 10 May 2021 00:26:37 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: linux_mtd: move global state into programmer data field
by Nikolai Artemiev (Code Review) 09 May '21

09 May '21

Attention is currently required from: Anastasia Klimchuk. Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/53947 ) Change subject: linux_mtd: move global state into programmer data field ...................................................................... Patch Set 4: (3 comments) File linux_mtd.c: https://review.coreboot.org/c/flashrom/+/53947/comment/cdf8a3f1_1e18a8e9 PS3, Line 365: mtd_data->dev_fp = NULL; > Is this line needed? Data is freed below anyway and won't be reused. […] Done https://review.coreboot.org/c/flashrom/+/53947/comment/44e5ca1b_13b82ada PS3, Line 421: programmer_linux_mtd.data = data; > Just to double-check, data has 6 members and none of them is initialised in init function, is this W […] `data` gets passed through to `linux_mtd_setup()` which then initializes it. I've added a comment since it's different to other programmers fully initialize `data` in the init function. We could also pass the individual fields through to `linux_mtd_setup()` and `get_mtd_info()`, but I think this is quite a bit cleaner. https://review.coreboot.org/c/flashrom/+/53947/comment/527580a3_b0365e98 PS3, Line 431: free(param); > Looking at the code, there is no reason to keep param until the very end, it can be freed just befor […] There are still two different code paths in the code that parses the `dev` parameter, so we would need to call `free(param)` in both of them, but I think that would still be nicer. As you say it's probably best to leave that for another patch. -- To view, visit https://review.coreboot.org/c/flashrom/+/53947 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I5ce6900e4892ed5687cfddb245dfe5461a3e2e84 Gerrit-Change-Number: 53947 Gerrit-PatchSet: 4 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Sun, 09 May 2021 23:49:30 +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]: linux_mtd: move global state into programmer data field
by Nikolai Artemiev (Code Review) 09 May '21

09 May '21

Attention is currently required from: Nikolai Artemiev. Hello build bot (Jenkins), Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/53947 to look at the new patch set (#4). Change subject: linux_mtd: move global state into programmer data field ...................................................................... linux_mtd: move global state into programmer data field BUG=b:161951062 BRANCH=none TEST=builds, reading /dev/mtd0 on Oak succeeds Change-Id: I5ce6900e4892ed5687cfddb245dfe5461a3e2e84 Signed-off-by: Nikolai Artemiev <nartemiev(a)google.com> --- M linux_mtd.c 1 file changed, 65 insertions(+), 45 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/47/53947/4 -- To view, visit https://review.coreboot.org/c/flashrom/+/53947 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I5ce6900e4892ed5687cfddb245dfe5461a3e2e84 Gerrit-Change-Number: 53947 Gerrit-PatchSet: 4 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-MessageType: newpatchset

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit May 2021