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

Change in flashrom[master]: ft2232_spi: Revise error message
by Anastasia Klimchuk (Code Review) 21 Jun '21

21 Jun '21

Attention is currently required from: Alan Green, Nico Huber. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/55697 ) Change subject: ft2232_spi: Revise error message ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/55697 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I125ae9ec0d5487fc26d588a7fd6c54da4ebd0d70 Gerrit-Change-Number: 55697 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Alan Green <avg(a)google.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Comment-Date: Mon, 21 Jun 2021 06:55:32 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tests: Do not run a test if its driver is not built
by Anastasia Klimchuk (Code Review) 21 Jun '21

21 Jun '21

Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/55295 ) Change subject: tests: Do not run a test if its driver is not built ...................................................................... Patch Set 3: Code-Review-1 (2 comments) Patchset: PS3: I think the latest version "patchset 3" does not handle disabled config options properly, I need to have another look. File tests/init_shutdown.c: https://review.coreboot.org/c/flashrom/+/55295/comment/f2e6a6ae_11a8f68d PS1, Line 35: #if CONFIG_DUMMY == 1 : void dummy_init_and_shutdown_test_success(void **state) : { : run_lifecycle(state, PROGRAMMER_DUMMY, "bus=parallel+lpc+fwh+spi"); : } : #endif /* CONFIG_DUMMY */ > I added `if (CONFIG_xxx)` and it seems to work. […] Actually, this needs more work, I tried as an experiment to use a config option that is not enabled by default and I get: "error: ‘CONFIG_JLINK_SPI’ undeclared" Because, my understanding Makefile has yes/no for config options but meson has 1/nothing at all. Maybe this will need to return back to #if, but I will have another look. -- To view, visit https://review.coreboot.org/c/flashrom/+/55295 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic1c48e41f658045a608f46636071f478ba646f77 Gerrit-Change-Number: 55295 Gerrit-PatchSet: 3 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> 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: build bot (Jenkins) <no-reply(a)coreboot.org> 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: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Mon, 21 Jun 2021 03:24:00 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Anastasia Klimchuk <aklm(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 Edward O'Callaghan (Code Review) 21 Jun '21

21 Jun '21

Attention is currently required from: Anastasia Klimchuk. Edward O'Callaghan 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 19: (1 comment) File hwaccess_x86_io_unittest.h: https://review.coreboot.org/c/flashrom/+/51487/comment/d4ad2043_4ae6c8f5 PS19, Line 32: This header needs guards for x86 as it's inclusion will result in attempts by the toolchain to resolve `#include <sys/io.h>` that will not exist on other platforms. -- 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: 19 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: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Mon, 21 Jun 2021 02:40:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tests: Do not run a test if its driver is not built
by Anastasia Klimchuk (Code Review) 21 Jun '21

21 Jun '21

Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/55295 ) Change subject: tests: Do not run a test if its driver is not built ...................................................................... Patch Set 3: (1 comment) File tests/init_shutdown.c: https://review.coreboot.org/c/flashrom/+/55295/comment/051c6e81_7af70880 PS1, Line 35: #if CONFIG_DUMMY == 1 : void dummy_init_and_shutdown_test_success(void **state) : { : run_lifecycle(state, PROGRAMMER_DUMMY, "bus=parallel+lpc+fwh+spi"); : } : #endif /* CONFIG_DUMMY */ > Ok, great, I will rebase and try - and so marking comment unresolved until this is done. […] I added `if (CONFIG_xxx)` and it seems to work. I did not change any linker flags, but looks like it is enabled by default already, I see `--gc-sections` is present in command line when running ninja test. One thing that worries me: Makefile uses yes/no values for config options that are enabled/disabled, meson uses 1 instead of yes (and I don't know what it uses instead of no). The conditional `if (CONFIG_xxx)` works when CONFIG_xxx is 1 or 0, but it doesn't compile with yes/no. I was using `-DCONFIG_xxx=0` in tests/meson.build for testing, but I am not sure it represents real-life scenario. Specifically, would it work when CONFIG_xxx is disabled and how do I test it. -- To view, visit https://review.coreboot.org/c/flashrom/+/55295 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic1c48e41f658045a608f46636071f478ba646f77 Gerrit-Change-Number: 55295 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-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: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Mon, 21 Jun 2021 01:50:30 +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> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tests: Do not run a test if its driver is not built
by Anastasia Klimchuk (Code Review) 21 Jun '21

21 Jun '21

Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons. Hello build bot (Jenkins), Nico Huber, Edward O'Callaghan, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/55295 to look at the new patch set (#3). Change subject: tests: Do not run a test if its driver is not built ...................................................................... tests: Do not run a test if its driver is not built For all tests that exist as of today, drivers are built by default, however config options can be disabled and in that case test should not be run. Technically, this is done by skipping the test scenario, so the test itself is running empty. BUG=b:181803212 TEST=Tested by adding into tests/meson.build -DCONFIG_xxx=0 4 times (for every driver with test), and then running ninja test Result: corresponding test is running empty, all other tests are running fine Also running ninja test with default config settings (everything is enabled, no overriding in test meson). Result: all tests are running. Change-Id: Ic1c48e41f658045a608f46636071f478ba646f77 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M tests/init_shutdown.c 1 file changed, 10 insertions(+), 5 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/95/55295/3 -- To view, visit https://review.coreboot.org/c/flashrom/+/55295 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic1c48e41f658045a608f46636071f478ba646f77 Gerrit-Change-Number: 55295 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-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: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: ft2232_spi: Revise comments about output pin states
by Alan Green (Code Review) 21 Jun '21

21 Jun '21

Attention is currently required from: Nico Huber, Samir Ibradžić. Alan Green has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/55695 ) Change subject: ft2232_spi: Revise comments about output pin states ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: > Alan, Samir, I'm sorry about the reverts. I think it's best to […] Hi Nico, Thanks for your time and attention on this. I think I understand what's going on here: the intent of the csgpiol parameter was to specify additional GPIOL pins that should mirror the CS pin. My changes were attempting to use the same machinery as csgpiol, but to control a general GPIO mechanism. Is that right? (For unfortunate reasons, this worked fine with the ad-hoc programmer we are using, and I didn't notice the change in behavior it caused for other programmers). It's probably still useful to have a mechanism to set the GPIO pins, but it would be best for to start again, so I'm OK with the reverts. btw, I'm going to be away for the next few weeks, so responses may be delayed. Thanks, Alan -- To view, visit https://review.coreboot.org/c/flashrom/+/55695 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2b84ede01759c80f69d5ad17e43783d09ecd1107 Gerrit-Change-Number: 55695 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Alan Green <avg(a)google.com> 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-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Samir Ibradžić <sibradzic(a)gmail.com> Gerrit-Comment-Date: Mon, 21 Jun 2021 01:21:23 +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]: buspirate: Add psus option
by Jeremy Kerr (Code Review) 21 Jun '21

21 Jun '21

Attention is currently required from: Nico Huber. Jeremy Kerr has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/54887 ) Change subject: buspirate: Add psus option ...................................................................... Patch Set 2: (1 comment) Patchset: PS1: > Hi Jeremy. The code change looks good, please also update the manpage. Super, thanks! New patchset incoming. -- To view, visit https://review.coreboot.org/c/flashrom/+/54887 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I8a7d4b40c0f7f04f6976f6757f05b61f2c9958f9 Gerrit-Change-Number: 54887 Gerrit-PatchSet: 2 Gerrit-Owner: Jeremy Kerr <jk(a)ozlabs.org> 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-Comment-Date: Mon, 21 Jun 2021 01:08:16 +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]: buspirate: Add psus option
by Jeremy Kerr (Code Review) 21 Jun '21

21 Jun '21

Attention is currently required from: Jeremy Kerr. Hello build bot (Jenkins), Nico Huber, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/54887 to look at the new patch set (#2). Change subject: buspirate: Add psus option ...................................................................... buspirate: Add psus option This change adds a 'psus=<on|off>' option, to control the external Vcc state of the bus pirate, allowing hardware where the SPI flash chip is powered by the 3V3/5V lines directly. Change-Id: I8a7d4b40c0f7f04f6976f6757f05b61f2c9958f9 Signed-off-by: Jeremy Kerr <jk(a)codeconstruct.com.au> --- M buspirate_spi.c M flashrom.8.tmpl 2 files changed, 31 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/87/54887/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/54887 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I8a7d4b40c0f7f04f6976f6757f05b61f2c9958f9 Gerrit-Change-Number: 54887 Gerrit-PatchSet: 2 Gerrit-Owner: Jeremy Kerr <jk(a)ozlabs.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jeremy Kerr <jk(a)ozlabs.org> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: ft2232_spi.c: Implement spi_send_multicommand()
by Anastasia Klimchuk (Code Review) 20 Jun '21

20 Jun '21

Attention is currently required from: Simon Buhrow, Nico Huber, Edward O'Callaghan, Samir Ibradžić. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/40477 ) Change subject: ft2232_spi.c: Implement spi_send_multicommand() ...................................................................... Patch Set 34: Code-Review+1 (1 comment) File ft2232_spi.c: https://review.coreboot.org/c/flashrom/+/40477/comment/61b67166_866b9f73 PS34, Line 283: const size_t cmd_len = 3; /* same length for any ft2232 command */ If it is always the same, maybe it can be #define above? -- 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: 34 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: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Samir Ibradžić <sibradzic(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Patrick Georgi <pgeorgi(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Simon Buhrow Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Samir Ibradžić <sibradzic(a)gmail.com> Gerrit-Comment-Date: Sun, 20 Jun 2021 23:56:33 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tests: Move test environment header files into tests directory
by Anastasia Klimchuk (Code Review) 20 Jun '21

20 Jun '21

Attention is currently required from: Angel Pons. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/55408 ) Change subject: tests: Move test environment header files into tests directory ...................................................................... Patch Set 2: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/55408/comment/5af51e06_9e96f100 PS1, Line 8: > I agree with the change, but I'd appreciate if you could add a sentence to explain why this is a goo […] Done -- To view, visit https://review.coreboot.org/c/flashrom/+/55408 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia42773b98b1eb6c65241aa559c0c8b4926bd0814 Gerrit-Change-Number: 55408 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-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Sun, 20 Jun 2021 23:39:03 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No 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 June 2021