January 2018 - flashrom-gerrit

Change in flashrom[master]: layout: Add -i <region>[:<file>] support.
by Nico Huber (Code Review) 08 Jan '18

08 Jan '18

Nico Huber has posted comments on this change. ( https://review.coreboot.org/23021 ) Change subject: layout: Add -i <region>[:<file>] support. ...................................................................... Patch Set 5: (15 comments) https://review.coreboot.org/#/c/23021/5/cli_classic.c File cli_classic.c: https://review.coreboot.org/#/c/23021/5/cli_classic.c@61 PS5, Line 61: <: rather :< https://review.coreboot.org/#/c/23021/5/flashrom.c File flashrom.c: https://review.coreboot.org/#/c/23021/5/flashrom.c@1320 PS5, Line 1320: const char *filename, const char *size_msg) Line limit is 112 chars for flashrom. (not my first choice, though hadn't time to look at the discussion, back in 2011~2012 iirc) https://review.coreboot.org/#/c/23021/5/flashrom.c@1364 PS5, Line 1364: #ifdef __LIBPAYLOAD__ : msg_gerr("Error: No file I/O support in libpayload\n"); : return 1; : #else Seems unnecessary, now that we use read_buf_from_file(). https://review.coreboot.org/#/c/23021/5/flashrom.c@1368 PS5, Line 1368: int ret = 0, i; `ret` is not used (as a variable) anymore. https://review.coreboot.org/#/c/23021/5/flashrom.c@1379 PS5, Line 1379: } I'm a bit confused here. Do we allow such layouts at all? Will try to investigate myself... https://review.coreboot.org/#/c/23021/5/flashrom.c@1387 PS5, Line 1387: if ((entry->start > size) || (entry->end > size)) { Checking `entry->end` should suffice. https://review.coreboot.org/#/c/23021/5/flashrom.c@1391 PS5, Line 1391: } Again, do we allow such layouts? https://review.coreboot.org/#/c/23021/5/flashrom.c@1484 PS5, Line 1484: #ifdef __LIBPAYLOAD__ : msg_gerr("Error: No file I/O support in libpayload\n"); : return 1; : #else Should be kept in do_write_buf_to_file() where the actual file handling happens. It also shows that flashrom.c is not the right place for this maybe worth a follow-up. https://review.coreboot.org/#/c/23021/5/flashrom.c@1502 PS5, Line 1502: msg_gdbg("\n"); /* avoid printing in caller's output */ `i == 0` isn't always the first included region https://review.coreboot.org/#/c/23021/5/flashrom.c@2660 PS5, Line 2660: /* : * This must be done before any files specified by -i arguments are : * processed and merged into newcontents since -i files take priority. : * See http://crbug.com/263495. : */ I don't understand it. The comment seems to assume that we call both after another? Also we should catch it in the CLI in the next patch. So we have a proper error message in case both a -w filename and -i :files are given. Same for -v. https://review.coreboot.org/#/c/23021/5/layout.c File layout.c: https://review.coreboot.org/#/c/23021/5/layout.c@175 PS5, Line 175: -l/--layout This list might get very long... https://review.coreboot.org/#/c/23021/5/layout.c@189 PS5, Line 189: /* Quotes or whitespace are not allowed in region names. */ why test it here? why add it now? https://review.coreboot.org/#/c/23021/5/layout.c@202 PS5, Line 202: found++; As we bail out otherwise, `found` will always be `num_include_args` in the end? https://review.coreboot.org/#/c/23021/5/layout.c@213 PS5, Line 213: #ifdef __LIBPAYLOAD__ : msg_gerr("Error: No file I/O support in libpayload\n"); : return 1; : #else libflashrom doesn't call it anyway, does it? The original guards only exist to make it compile (i.e. hide calls to i/o stream functions). https://review.coreboot.org/#/c/23021/5/layout.c@217 PS5, Line 217: file = strdup(file); why? is it necessary? doesn't `file` point into `argv` of main()? -- To view, visit https://review.coreboot.org/23021 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic5465659605d8431d931053967b40290195cfd99 Gerrit-Change-Number: 23021 Gerrit-PatchSet: 5 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Nicola Corna <nicola(a)corna.info> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Mon, 08 Jan 2018 00:13:20 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: No

1 0

Change in flashrom[master]: [NOTFORMERGE] X200 SPI hack
by build bot (Jenkins) (Code Review) 07 Jan '18

07 Jan '18

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/23151 ) Change subject: [NOTFORMERGE] X200 SPI hack ...................................................................... Patch Set 1: Verified+1 Build Successful https://qa.coreboot.org/job/flashrom_gerrit/862/ : SUCCESS https://qa.coreboot.org/job/flashrom-customrules/1012/ : SUCCESS -- To view, visit https://review.coreboot.org/23151 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I28f2e06c5b919bc08ea33ec15713e4dc3b183edd Gerrit-Change-Number: 23151 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Sun, 07 Jan 2018 23:29:16 +0000 Gerrit-HasComments: No Gerrit-HasLabels: Yes

1 0

Change in flashrom[master]: [NOTFORMERGE] X200 SPI hack
by Nico Huber (Code Review) 07 Jan '18

07 Jan '18

Nico Huber has uploaded this change for review. ( https://review.coreboot.org/23151 Change subject: [NOTFORMERGE] X200 SPI hack ...................................................................... [NOTFORMERGE] X200 SPI hack Send a dummy READ command before any other command. Change-Id: I28f2e06c5b919bc08ea33ec15713e4dc3b183edd Signed-off-by: Nico Huber <nico.h(a)gmx.de> --- M spi.c 1 file changed, 3 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/51/23151/1 diff --git a/spi.c b/spi.c index 56f1fdf..4338124 100644 --- a/spi.c +++ b/spi.c @@ -34,6 +34,9 @@ unsigned int readcnt, const unsigned char *writearr, unsigned char *readarr) { + unsigned char dummy[256]; + const unsigned char readcmd[JEDEC_READ_OUTSIZE] = { JEDEC_READ, }; + flash->mst->spi.command(flash, sizeof(readcmd), sizeof(dummy), readcmd, dummy); return flash->mst->spi.command(flash, writecnt, readcnt, writearr, readarr); } -- To view, visit https://review.coreboot.org/23151 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I28f2e06c5b919bc08ea33ec15713e4dc3b183edd Gerrit-Change-Number: 23151 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>

1 0

Change in flashrom[master]: layout: Add -i <region>[:<file>] support.
by Nicola Corna (Code Review) 07 Jan '18

07 Jan '18

Nicola Corna has posted comments on this change. ( https://review.coreboot.org/23021 ) Change subject: layout: Add -i <region>[:<file>] support. ...................................................................... Patch Set 5: (1 comment) https://review.coreboot.org/#/c/23021/5/cli_classic.c File cli_classic.c: https://review.coreboot.org/#/c/23021/5/cli_classic.c@61 PS5, Line 61: flash read/write -- To view, visit https://review.coreboot.org/23021 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic5465659605d8431d931053967b40290195cfd99 Gerrit-Change-Number: 23021 Gerrit-PatchSet: 5 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nicola Corna <nicola(a)corna.info> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Sun, 07 Jan 2018 23:12:01 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: No

1 0

Change in flashrom[master]: Hack up test_v2.sh to work better with upstream
by David Hendricks (Code Review) 07 Jan '18

07 Jan '18

David Hendricks has restored this change. ( https://review.coreboot.org/23025 ) Change subject: Hack up test_v2.sh to work better with upstream ...................................................................... Restored switching over to development on master -- To view, visit https://review.coreboot.org/23025 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: restore Gerrit-Change-Id: I9ace99fdb8d779804fe1887fe49f6409f8aff02e Gerrit-Change-Number: 23025 Gerrit-PatchSet: 1 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

Change in flashrom[master]: test_v2.sh: Copy layout to local tempdir in local-layout mode
by David Hendricks (Code Review) 07 Jan '18

07 Jan '18

David Hendricks has restored this change. ( https://review.coreboot.org/23024 ) Change subject: test_v2.sh: Copy layout to local tempdir in local-layout mode ...................................................................... Restored switching over to development on master -- To view, visit https://review.coreboot.org/23024 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: restore Gerrit-Change-Id: Ia2328d367ee3df5ac1c481c16e4502a81d69fd98 Gerrit-Change-Number: 23024 Gerrit-PatchSet: 1 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

Change in flashrom[master]: Copy test_v2 directory from chromiumos branch
by David Hendricks (Code Review) 07 Jan '18

07 Jan '18

David Hendricks has restored this change. ( https://review.coreboot.org/23023 ) Change subject: Copy test_v2 directory from chromiumos branch ...................................................................... Restored switching over to development on master -- To view, visit https://review.coreboot.org/23023 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: restore Gerrit-Change-Id: I41bef98d32df61af400ecac46ea9571e722b2ac6 Gerrit-Change-Number: 23023 Gerrit-PatchSet: 1 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

Change in flashrom[master]: make args for -r/-w/-v non-positional and optional
by David Hendricks (Code Review) 07 Jan '18

07 Jan '18

David Hendricks has posted comments on this change. ( https://review.coreboot.org/23022 ) Change subject: make args for -r/-w/-v non-positional and optional ...................................................................... Patch Set 4: Code-Review+2 -- To view, visit https://review.coreboot.org/23022 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iefbcb7dc4fefe26f5afd1292dfd5c1687fa62803 Gerrit-Change-Number: 23022 Gerrit-PatchSet: 4 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Louis Yung-Chieh Lo <yjlou(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Sun, 07 Jan 2018 13:27:26 +0000 Gerrit-HasComments: No Gerrit-HasLabels: Yes

1 0

Change in flashrom[master]: layout: Add -i <region>[:<file>] support.
by David Hendricks (Code Review) 07 Jan '18

07 Jan '18

David Hendricks has posted comments on this change. ( https://review.coreboot.org/23021 ) Change subject: layout: Add -i <region>[:<file>] support. ...................................................................... Patch Set 5: Code-Review+2 Thanks! -- To view, visit https://review.coreboot.org/23021 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic5465659605d8431d931053967b40290195cfd99 Gerrit-Change-Number: 23021 Gerrit-PatchSet: 5 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Sun, 07 Jan 2018 13:27:19 +0000 Gerrit-HasComments: No Gerrit-HasLabels: Yes

1 0

Change in flashrom[master]: make args for -r/-w/-v non-positional and optional
by build bot (Jenkins) (Code Review) 07 Jan '18

07 Jan '18

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/23022 ) Change subject: make args for -r/-w/-v non-positional and optional ...................................................................... Patch Set 4: Verified+1 Build Successful https://qa.coreboot.org/job/flashrom_gerrit/861/ : SUCCESS https://qa.coreboot.org/job/flashrom-customrules/1011/ : SUCCESS -- To view, visit https://review.coreboot.org/23022 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iefbcb7dc4fefe26f5afd1292dfd5c1687fa62803 Gerrit-Change-Number: 23022 Gerrit-PatchSet: 4 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Louis Yung-Chieh Lo <yjlou(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Sun, 07 Jan 2018 13:04:25 +0000 Gerrit-HasComments: No Gerrit-HasLabels: Yes

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit January 2018