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

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: Anastasia Klimchuk. 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/+/55408 to look at the new patch set (#2). Change subject: tests: Move test environment header files into tests directory ...................................................................... tests: Move test environment header files into tests directory Both of these headers are only used in test builds, so they should live in tests/ directory. No changes to meson.build and tests/meson.build files are needed because tests/meson.build adds current directory to search for include files. BUG=b:181803212 TEST=ninja test -> all tests pass nm builddir/tests/flashrom_unit_tests.p/.._it85spi.c.o -> has symbols _test_calloc, _test_free, test_inb, test_outb nm builddir/flashrom.p/it85spi.c.o -> has symbols calloc free inb outb Change-Id: Ia42773b98b1eb6c65241aa559c0c8b4926bd0814 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- R tests/hwaccess_x86_io_unittest.h R tests/unittest_env.h 2 files changed, 0 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/08/55408/2 -- 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: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: layout: Use linked list for `struct romentry`
by Peter Marheine (Code Review) 20 Jun '21

20 Jun '21

Attention is currently required from: Nico Huber, Edward O'Callaghan, Daniel Campello, Anastasia Klimchuk. Peter Marheine has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/33521 ) Change subject: layout: Use linked list for `struct romentry` ...................................................................... Patch Set 8: (1 comment) File layout.c: https://review.coreboot.org/c/flashrom/+/33521/comment/c5c597f6_0ec69e1d PS4, Line 418: entry->next = layout->head; > Ping Ack; makes sense looking at it now. -- To view, visit https://review.coreboot.org/c/flashrom/+/33521 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I60a0aa1007ebcd5eb401db116f835d129b3e9732 Gerrit-Change-Number: 33521 Gerrit-PatchSet: 8 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Daniel Campello <campello(a)chromium.org> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> 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: Daniel Campello <campello(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Sun, 20 Jun 2021 23:33:41 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Peter Marheine <pmarheine(a)chromium.org> Gerrit-MessageType: comment

1 0

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

20 Jun '21

Attention is currently required from: Alan Green, Nico Huber, Samir Ibradžić. Angel Pons 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) File ft2232_spi.c: https://review.coreboot.org/c/flashrom/+/55695/comment/8b2cc93a_66910bc7 PS1, Line 98: * TCK/SK is bit 0. : * TDI/DO is bit 1. : * TDO/DI is bit 2. : * TMS/CS is bit 3. : * GPIOL0 is bit 4. : * GPIOL1 is bit 5. : * GPIOL2 is bit 6. : * GPIOL3 is bit 7. > I was thinking of using these in the code where we handle the `type` programmer param. […] For the GPIOL stuff, one could use something like this: #define PIN_GPIOL(x) (1 << (4 + (x))) -- 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: 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: Sun, 20 Jun 2021 15:12:42 +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> Gerrit-MessageType: comment

1 0

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

20 Jun '21

Attention is currently required from: Alan Green, Nico Huber, Samir Ibradžić. Angel Pons 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) File ft2232_spi.c: https://review.coreboot.org/c/flashrom/+/55695/comment/ebf7e590_7901d2a1 PS1, Line 98: * TCK/SK is bit 0. : * TDI/DO is bit 1. : * TDO/DI is bit 2. : * TMS/CS is bit 3. : * GPIOL0 is bit 4. : * GPIOL1 is bit 5. : * GPIOL2 is bit 6. : * GPIOL3 is bit 7. > I don't know, the identifiers wouldn't be used in the code. It's mostly […] I was thinking of using these in the code where we handle the `type` programmer param. Still, it's something that can be discussed later. -- 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: 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: Sun, 20 Jun 2021 15:11:14 +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> Gerrit-MessageType: comment

1 0

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

20 Jun '21

Attention is currently required from: Alan Green, Angel Pons, Samir Ibradžić. Nico Huber 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) File ft2232_spi.c: https://review.coreboot.org/c/flashrom/+/55695/comment/949fe212_6a244b8b PS1, Line 98: * TCK/SK is bit 0. : * TDI/DO is bit 1. : * TDO/DI is bit 2. : * TMS/CS is bit 3. : * GPIOL0 is bit 4. : * GPIOL1 is bit 5. : * GPIOL2 is bit 6. : * GPIOL3 is bit 7. > Shouldn't this be code, e.g. macros or an enum? […] I don't know, the identifiers wouldn't be used in the code. It's mostly reading a number from the UI and shifting that into a register value. Or we could write less condensed code, e.g. switch (input) { case 0: cs_bits |= PIN_GPIOL0; ... break; or const int bits[] = { PIN_GPIOL0, PIN_GPIOL1, PIN_GPIOL2, PIN_GPIOL3 }; cs_bits |= bits[input]; ... Not sure if I'd want to write it either way. What do you think? -- 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: Alan Green <avg(a)google.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Samir Ibradžić <sibradzic(a)gmail.com> Gerrit-Comment-Date: Sun, 20 Jun 2021 15:08:05 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ft2232_spi: Normalize error paths in ft2232_shutdown()
by Angel Pons (Code Review) 20 Jun '21

20 Jun '21

Attention is currently required from: Alan Green, Nico Huber, Anastasia Klimchuk. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/55696 ) Change subject: ft2232_spi: Normalize error paths in ft2232_shutdown() ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/55696 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I5158d06127a9a8934b083e48b69d29c4d5a11831 Gerrit-Change-Number: 55696 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-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Sun, 20 Jun 2021 15:05:28 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ft2232_spi: Revise error message
by Angel Pons (Code Review) 20 Jun '21

20 Jun '21

Attention is currently required from: Alan Green, Nico Huber, Anastasia Klimchuk. Angel Pons 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-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Sun, 20 Jun 2021 15:04:44 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ft2232_spi: Revise error message
by Nico Huber (Code Review) 20 Jun '21

20 Jun '21

Attention is currently required from: Alan Green, Anastasia Klimchuk. Hello Alan Green, Anastasia Klimchuk, I'd like you to do a code review. Please visit https://review.coreboot.org/c/flashrom/+/55697 to review the following change. Change subject: ft2232_spi: Revise error message ...................................................................... ft2232_spi: Revise error message Reword the message and drop the error string from libftdi. It is already printed in send_buf(). Change-Id: I125ae9ec0d5487fc26d588a7fd6c54da4ebd0d70 Signed-off-by: Nico Huber <nico.h(a)gmx.de> --- M ft2232_spi.c 1 file changed, 1 insertion(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/97/55697/1 diff --git a/ft2232_spi.c b/ft2232_spi.c index fbea0f9..1404214 100644 --- a/ft2232_spi.c +++ b/ft2232_spi.c @@ -180,8 +180,7 @@ buf[1] = 0; /* Output byte ignored */ buf[2] = 0; /* Pin direction: all inputs */ if (send_buf(ftdic, buf, 3)) { - msg_perr("Unable to set pins back inputs: (%s)\n", - ftdi_get_error_string(ftdic)); + msg_perr("Unable to set pins back to inputs.\n"); ret = 1; } -- 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-Attention: Alan Green <avg(a)google.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: ft2232_spi: Normalize error paths in ft2232_shutdown()
by Nico Huber (Code Review) 20 Jun '21

20 Jun '21

Attention is currently required from: Alan Green, Anastasia Klimchuk. Hello Alan Green, Anastasia Klimchuk, I'd like you to do a code review. Please visit https://review.coreboot.org/c/flashrom/+/55696 to review the following change. Change subject: ft2232_spi: Normalize error paths in ft2232_shutdown() ...................................................................... ft2232_spi: Normalize error paths in ft2232_shutdown() We missed to `free(spi_data)` on one path. It also seems odd to leak the return code of a locally used library into our common infrastruc- ture, so normalize all error paths to return 1. Change-Id: I5158d06127a9a8934b083e48b69d29c4d5a11831 Signed-off-by: Nico Huber <nico.h(a)gmx.de> --- M ft2232_spi.c 1 file changed, 7 insertions(+), 5 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/96/55696/1 diff --git a/ft2232_spi.c b/ft2232_spi.c index e546f6c..fbea0f9 100644 --- a/ft2232_spi.c +++ b/ft2232_spi.c @@ -170,10 +170,10 @@ static int ft2232_shutdown(void *data) { - int f; struct ft2232_data *spi_data = (struct ft2232_data *) data; struct ftdi_context *ftdic = &spi_data->ftdic_context; unsigned char buf[3]; + int ret = 0; msg_pdbg("Releasing I/Os\n"); buf[0] = SET_BITS_LOW; @@ -182,16 +182,18 @@ if (send_buf(ftdic, buf, 3)) { msg_perr("Unable to set pins back inputs: (%s)\n", ftdi_get_error_string(ftdic)); + ret = 1; } - if ((f = ftdi_usb_close(ftdic)) < 0) { - msg_perr("Unable to close FTDI device: %d (%s)\n", f, + const int close_ret = ftdi_usb_close(ftdic); + if (close_ret < 0) { + msg_perr("Unable to close FTDI device: %d (%s)\n", close_ret, ftdi_get_error_string(ftdic)); - return f; + ret = 1; } free(spi_data); - return 0; + return ret; } static bool ft2232_spi_command_fits(const struct spi_command *cmd, size_t buffer_size) -- To view, visit https://review.coreboot.org/c/flashrom/+/55696 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I5158d06127a9a8934b083e48b69d29c4d5a11831 Gerrit-Change-Number: 55696 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-Attention: Alan Green <avg(a)google.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newchange

1 0

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

20 Jun '21

Attention is currently required from: Alan Green, Nico Huber, Samir Ibradžić. Angel Pons 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: Code-Review+2 (1 comment) File ft2232_spi.c: https://review.coreboot.org/c/flashrom/+/55695/comment/520560ac_04601d62 PS1, Line 98: * TCK/SK is bit 0. : * TDI/DO is bit 1. : * TDO/DI is bit 2. : * TMS/CS is bit 3. : * GPIOL0 is bit 4. : * GPIOL1 is bit 5. : * GPIOL2 is bit 6. : * GPIOL3 is bit 7. Shouldn't this be code, e.g. macros or an enum? #define PIN_TCK_SK (1 << 0) #define PIN_TDI_DO (1 << 1) #define PIN_TDO_DI (1 << 2) #define PIN_TMS_CS (1 << 3) #define PIN_GPIOL0 (1 << 4) #define PIN_GPIOL1 (1 << 5) #define PIN_GPIOL2 (1 << 6) #define PIN_GPIOL3 (1 << 7) -- 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: 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: Sun, 20 Jun 2021 14:24:41 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit June 2021