- flashrom-gerrit - mail.coreboot.org

Change in flashrom[refs/meta/config]: flashrom: keep Code-Review votes on trivial rebase and no-co...
by Patrick Georgi (Code Review) 13 Jun '17

13 Jun '17

Patrick Georgi has uploaded this change for review. ( https://review.coreboot.org/20182 Change subject: flashrom: keep Code-Review votes on trivial rebase and no-code changes ...................................................................... flashrom: keep Code-Review votes on trivial rebase and no-code changes Also avoid sending mails to flashrom-gerrit@ for draft CLs. Change-Id: I9a76f9646269e38556f3c4f766cbf52a720a1d1d Signed-off-by: Patrick Georgi <pgeorgi(a)google.com> --- M project.config 1 file changed, 14 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/82/20182/2 diff --git a/project.config b/project.config index 390f702..7f041b1 100644 --- a/project.config +++ b/project.config @@ -12,6 +12,20 @@ [access "refs/tags/*"] push = +force group flashrom developers +[label "Code-Review"] + function = MaxWithBlock + abbreviation = R + copyMinScore = true + value = -2 Do not submit + value = -1 I would prefer that you didn't submit this + value = 0 No score + value = +1 Looks good to me, but someone else must approve + value = +2 Looks good to me, approved + copyAllScoresOnTrivialRebase = true + copyAllScoresIfNoCodeChange = true + defaultValue = 0 + [notify "flashrom-gerrit"] email = flashrom-gerrit(a)flashrom.org header = to + filter = -is:draft \ No newline at end of file -- To view, visit https://review.coreboot.org/20182 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: refs/meta/config Gerrit-MessageType: newchange Gerrit-Change-Id: I9a76f9646269e38556f3c4f766cbf52a720a1d1d Gerrit-Change-Number: 20182 Gerrit-PatchSet: 2 Gerrit-Owner: Patrick Georgi <pgeorgi(a)google.com>

1 0

Change in flashrom[staging]: chipset_enable: Add support for Intel Skylake / Kabylake
by Youness Alaoui (Code Review) 13 Jun '17

13 Jun '17

Youness Alaoui has posted comments on this change. ( https://review.coreboot.org/18925 ) Change subject: chipset_enable: Add support for Intel Skylake / Kabylake ...................................................................... Patch Set 5: One issue I'm having with this code is that it will segfault at the end. I've tracked it down to the rpci_write_byte in enable_flash_ich_bios_cntl_common which calls register_undo_pci_write_byte which saves a copy of the pci device, but the copy is wrong (pci accessor is somehow set to -1) and it segfaults when it tries to restore the pci register in the shutdown handlers. I'm not yet sure how to properly fix this. -- To view, visit https://review.coreboot.org/18925 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: comment Gerrit-Change-Id: I000819aff25fbe9764f33df85f040093b82cd948 Gerrit-Change-Number: 18925 Gerrit-PatchSet: 5 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Stefan Tauner <stefan.tauner(a)gmx.at> Gerrit-Reviewer: Youness Alaoui <snifikino(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Mon, 12 Jun 2017 22:24:45 +0000 Gerrit-HasComments: No

1 0

Change in flashrom[staging]: cli_classic: Remove layout-for-write-only check
by build bot (Jenkins) (Code Review) 09 Jun '17

09 Jun '17

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/17951 ) Change subject: cli_classic: Remove layout-for-write-only check ...................................................................... Patch Set 15: Verified+1 Build Successful https://qa.coreboot.org/job/flashrom_gerrit/354/ : SUCCESS https://qa.coreboot.org/job/flashrom-customrules/358/ : SUCCESS -- To view, visit https://review.coreboot.org/17951 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: comment Gerrit-Change-Id: I0f5e9623ca75bc4503aeb45ae346d7573c0fef2c Gerrit-Change-Number: 17951 Gerrit-PatchSet: 15 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Fri, 09 Jun 2017 12:01:24 +0000 Gerrit-HasComments: No

1 0

Change in flashrom[staging]: cli_classic: Add option (-N, --noverify-all)
by Nico Huber (Code Review) 09 Jun '17

09 Jun '17

Nico Huber has submitted this change and it was merged. ( https://review.coreboot.org/17950 ) Change subject: cli_classic: Add option (-N, --noverify-all) ...................................................................... cli_classic: Add option (-N, --noverify-all) This option specifies to verify included regions only after a write. It also reduces the data read before the write. v2: o Changed short option name to `-N`. o Added section in the manual page. Change-Id: I40b5983f56d62821d17b827b88b73d1d41a30bd7 Signed-off-by: Nico Huber <nico.huber(a)secunet.com> Reviewed-on: https://review.coreboot.org/17950 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Stefan Reinauer <stefan.reinauer(a)coreboot.org> --- M cli_classic.c M flashrom.8.tmpl 2 files changed, 26 insertions(+), 5 deletions(-) Approvals: build bot (Jenkins): Verified Stefan Reinauer: Looks good to me, approved diff --git a/cli_classic.c b/cli_classic.c index 00baf49..391fc5a 100644 --- a/cli_classic.c +++ b/cli_classic.c @@ -42,7 +42,7 @@ "-z|" #endif "-p <programmername>[:<parameters>] [-c <chipname>]\n" - "[-E|(-r|-w|-v) <file>] [-l <layoutfile> [-i <imagename>]...] [-n] [-f]]\n" + "[-E|(-r|-w|-v) <file>] [-l <layoutfile> [-i <imagename>]...] [-n] [-N] [-f]]\n" "[-V[V[V]]] [-o <logfile>]\n\n", name); printf(" -h | --help print this help text\n" @@ -55,6 +55,7 @@ " -c | --chip <chipname> probe only for specified flash chip\n" " -f | --force force specific operations (see man page)\n" " -n | --noverify don't auto-verify\n" + " -N | --noverify-all verify included regions only (cf. -i)\n" " -l | --layout <layoutfile> read ROM layout from <layoutfile>\n" " -i | --image <name> only flash image <name> from flash layout\n" " -o | --output <logfile> log output to <logfile>\n" @@ -103,17 +104,18 @@ int list_supported_wiki = 0; #endif int read_it = 0, write_it = 0, erase_it = 0, verify_it = 0; - int dont_verify_it = 0, list_supported = 0, operation_specified = 0; + int dont_verify_it = 0, dont_verify_all = 0, list_supported = 0, operation_specified = 0; enum programmer prog = PROGRAMMER_INVALID; int ret = 0; - static const char optstring[] = "r:Rw:v:nVEfc:l:i:p:Lzho:"; + static const char optstring[] = "r:Rw:v:nNVEfc:l:i:p:Lzho:"; static const struct option long_options[] = { {"read", 1, NULL, 'r'}, {"write", 1, NULL, 'w'}, {"erase", 0, NULL, 'E'}, {"verify", 1, NULL, 'v'}, {"noverify", 0, NULL, 'n'}, + {"noverify-all", 0, NULL, 'N'}, {"chip", 1, NULL, 'c'}, {"verbose", 0, NULL, 'V'}, {"force", 0, NULL, 'f'}, @@ -189,6 +191,9 @@ cli_classic_abort_usage(); } dont_verify_it = 1; + break; + case 'N': + dont_verify_all = 1; break; case 'c': chip_to_probe = strdup(optarg); @@ -536,7 +541,7 @@ flashrom_flag_set(fill_flash, FLASHROM_FLAG_FORCE, !!force); flashrom_flag_set(fill_flash, FLASHROM_FLAG_FORCE_BOARDMISMATCH, !!force_boardmismatch); flashrom_flag_set(fill_flash, FLASHROM_FLAG_VERIFY_AFTER_WRITE, !dont_verify_it); - flashrom_flag_set(fill_flash, FLASHROM_FLAG_VERIFY_WHOLE_CHIP, true); + flashrom_flag_set(fill_flash, FLASHROM_FLAG_VERIFY_WHOLE_CHIP, !dont_verify_all); /* FIXME: We should issue an unconditional chip reset here. This can be * done once we have a .reset function in struct flashchip. diff --git a/flashrom.8.tmpl b/flashrom.8.tmpl index 34e1fe7..34e7f02 100644 --- a/flashrom.8.tmpl +++ b/flashrom.8.tmpl @@ -48,7 +48,8 @@ \fB\-p\fR <programmername>[:<parameters>] [\fB\-E\fR|\fB\-r\fR <file>|\fB\-w\fR <file>|\fB\-v\fR <file>] \ [\fB\-c\fR <chipname>] - [\fB\-l\fR <file> [\fB\-i\fR <image>]] [\fB\-n\fR] [\fB\-f\fR]] + [\fB\-l\fR <file> [\fB\-i\fR <image>]] [\fB\-n\fR] [\fB\-N\fR] \ +[\fB\-f\fR]] [\fB\-V\fR[\fBV\fR[\fBV\fR]]] [\fB-o\fR <logfile>] .SH DESCRIPTION .B flashrom @@ -112,6 +113,21 @@ This option is only useful in combination with .BR \-\-write . .TP +.B "\-N, \-\-noverify-all" +Skip not included regions during automatic verification after writing (cf. +.BR "\-l " "and " "\-i" ). +You should only use this option if you are sure that communication with +the flash chip is reliable (e.g. when using the +.BR internal +programmer). Even if flashrom is instructed not to touch parts of the +flash chip, their contents could be damaged (e.g. due to misunderstood +erase commands). +.sp +This option is required to flash an Intel system with locked ME flash +region using the +.BR internal +programmer. It may be enabled by default in this case in the future. +.TP .B "\-v, \-\-verify <file>" Verify the flash ROM contents against the given .BR <file> . -- To view, visit https://review.coreboot.org/17950 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: merged Gerrit-Change-Id: I40b5983f56d62821d17b827b88b73d1d41a30bd7 Gerrit-Change-Number: 17950 Gerrit-PatchSet: 15 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Stefan Tauner <stefan.tauner(a)gmx.at> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

Change in flashrom[staging]: Enable writes with active ME
by build bot (Jenkins) (Code Review) 09 Jun '17

09 Jun '17

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/18740 ) Change subject: Enable writes with active ME ...................................................................... Patch Set 15: Verified+1 Build Successful https://qa.coreboot.org/job/flashrom-customrules/356/ : SUCCESS -- To view, visit https://review.coreboot.org/18740 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: comment Gerrit-Change-Id: I94a5e7074b845c227e43d76d04dd1a71082a1cef Gerrit-Change-Number: 18740 Gerrit-PatchSet: 15 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Fri, 09 Jun 2017 11:27:18 +0000 Gerrit-HasComments: No

1 0

Change in flashrom[staging]: Enable writes with active ME
by Nico Huber (Code Review) 09 Jun '17

09 Jun '17

Hello Paul Menzel, David Hendricks, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/18740 to look at the new patch set (#15). Change subject: Enable writes with active ME ...................................................................... Enable writes with active ME At least with the -N,--noverify-all switch, this warning isn't true any more. Maybe we want to force -N in that case? Change-Id: I94a5e7074b845c227e43d76d04dd1a71082a1cef Signed-off-by: Nico Huber <nico.huber(a)secunet.com> --- M flashrom.8.tmpl M ichspi.c 2 files changed, 1 insertion(+), 37 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/40/18740/15 -- To view, visit https://review.coreboot.org/18740 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: newpatchset Gerrit-Change-Id: I94a5e7074b845c227e43d76d04dd1a71082a1cef Gerrit-Change-Number: 18740 Gerrit-PatchSet: 15 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

Change in flashrom[staging]: Free board_vendor and board_model vars before returning
by Stefan Reinauer (Code Review) 07 Jun '17

07 Jun '17

Stefan Reinauer has posted comments on this change. ( https://review.coreboot.org/18207 ) Change subject: Free board_vendor and board_model vars before returning ...................................................................... Patch Set 3: -Code-Review Sorry, +2ed by accident. I don't think free()ing here is a good idea. (Not that it actually really matters ) -- To view, visit https://review.coreboot.org/18207 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: comment Gerrit-Change-Id: I825f53702803292277b44dcb03fdc49b5dd410a8 Gerrit-Change-Number: 18207 Gerrit-PatchSet: 3 Gerrit-Owner: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Stefan Tauner <stefan.tauner(a)gmx.at> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Wed, 07 Jun 2017 21:29:25 +0000 Gerrit-HasComments: No

1 0

Change in flashrom[staging]: Free board_vendor and board_model vars before returning
by Nico Huber (Code Review) 07 Jun '17

07 Jun '17

Nico Huber has posted comments on this change. ( https://review.coreboot.org/18207 ) Change subject: Free board_vendor and board_model vars before returning ...................................................................... Patch Set 3: Code-Review-2 See previous comment. The variable `ret` is undeclared on big-endian. -- To view, visit https://review.coreboot.org/18207 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: comment Gerrit-Change-Id: I825f53702803292277b44dcb03fdc49b5dd410a8 Gerrit-Change-Number: 18207 Gerrit-PatchSet: 3 Gerrit-Owner: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Stefan Tauner <stefan.tauner(a)gmx.at> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Wed, 07 Jun 2017 21:18:37 +0000 Gerrit-HasComments: No

1 0

Change in flashrom[staging]: Free board_vendor and board_model vars before returning
by Stefan Reinauer (Code Review) 07 Jun '17

07 Jun '17

Stefan Reinauer has posted comments on this change. ( https://review.coreboot.org/18207 ) Change subject: Free board_vendor and board_model vars before returning ...................................................................... Patch Set 3: Code-Review+2 (2 comments) https://review.coreboot.org/#/c/18207/3/internal.c File internal.c: https://review.coreboot.org/#/c/18207/3/internal.c@170 PS3, Line 170: #if IS_X86 || IS_ARM We should assume coreboot is available on all architectures https://review.coreboot.org/#/c/18207/3/internal.c@222 PS3, Line 222: oard_parse_parameter(arg, &board_vendor, &board_model Would it make more sense to rewrite this function to avoid the layering issue that this patch is trying to fix? Or maybe alloca() the space with a reasonable size and have board_parse_parameter copy the data instead of overwriting the pointer. -- To view, visit https://review.coreboot.org/18207 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: comment Gerrit-Change-Id: I825f53702803292277b44dcb03fdc49b5dd410a8 Gerrit-Change-Number: 18207 Gerrit-PatchSet: 3 Gerrit-Owner: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Stefan Tauner <stefan.tauner(a)gmx.at> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Wed, 07 Jun 2017 21:01:35 +0000 Gerrit-HasComments: Yes

1 0

Change in flashrom[staging]: spi25: Fix layering violation in probe_spi_rdid4()
by Nico Huber (Code Review) 07 Jun '17

07 Jun '17

Nico Huber has posted comments on this change. ( https://review.coreboot.org/19420 ) Change subject: spi25: Fix layering violation in probe_spi_rdid4() ...................................................................... Patch Set 2: Code-Review-2 (1 comment) Will update this (sometime). https://review.coreboot.org/#/c/19420/2/wbsio_spi.c File wbsio_spi.c: https://review.coreboot.org/#/c/19420/2/wbsio_spi.c@73 PS2, Line 73: .max_data_read = 3, > The table above wbsio_spi_send_command() implies that this is 4. It seems y Letting wbsio_spi_send_command() handle it sounds way better than what I did, thanks! I first thought the code in spi25.c was there to make sure we return 0 on this error, but that seems to be handled deeper down already. -- To view, visit https://review.coreboot.org/19420 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: comment Gerrit-Change-Id: Idd21d20465cb214f3ff5bf3267b9014f8beee3f3 Gerrit-Change-Number: 19420 Gerrit-PatchSet: 2 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Wed, 07 Jun 2017 20:17:20 +0000 Gerrit-HasComments: Yes

1 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit