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

Change in flashrom[master]: layout: Add -i <region>[:<file>] support
by Edward O'Callaghan (Code Review) 27 Apr '21

27 Apr '21

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/23021 ) Change subject: layout: Add -i <region>[:<file>] support ...................................................................... layout: Add -i <region>[:<file>] support Add an optional sub-parameter to the -i parameter to allow building the image to be written from multiple files. This will also allow regions to be read from flash and written to separate image files. This is a rebase of a patch that was ported from chromiumos. A lot of things have changed, but the idea is the same. Original patch by Louis Yung-Chieh Lo <yjlou(a)chromium.org>: Summary: Support -i partition:file feature for both read and write. Commit: 9c7525f Review URL: http://codereview.chromium.org/6611015 Ported version by Stefan Tauner <stefan.tauner(a)student.tuwien.ac.at> and Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net>: Summary: [PATCH 2/6] layout: Add -i <region>[:<file>] support. Review URL: https://mail.coreboot.org/pipermail/flashrom/2013-October/011729.html Change-Id: Ic5465659605d8431d931053967b40290195cfd99 Signed-off-by: David Hendricks <dhendrix(a)chromium.org> Signed-off-by: Stefan Tauner <stefan.tauner(a)student.tuwien.ac.at> Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net> Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> Signed-off-by: Nico Huber <nico.huber(a)secunet.com> Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> Signed-off-by: Daniel Campello <campello(a)chromium.org> Co-Authored-by: Edward O'Callaghan <quasisec(a)google.com> Co-Authored-by: Daniel Campello <campello(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/23021 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Sam McNally <sammc(a)google.com> Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> --- M cli_classic.c M flash.h M flashrom.8.tmpl M flashrom.c M layout.c M layout.h 6 files changed, 234 insertions(+), 37 deletions(-) Approvals: build bot (Jenkins): Verified Edward O'Callaghan: Looks good to me, approved Sam McNally: Looks good to me, approved diff --git a/cli_classic.c b/cli_classic.c index 3160d76..a22d642 100644 --- a/cli_classic.c +++ b/cli_classic.c @@ -41,7 +41,7 @@ "\n\t-p <programmername>[:<parameters>] [-c <chipname>]\n" "\t\t(--flash-name|--flash-size|\n" "\t\t [-E|(-r|-w|-v) <file>]\n" - "\t\t [(-l <layoutfile>|--ifd| --fmap|--fmap-file <file>) [-i <imagename>]...]\n" + "\t\t [(-l <layoutfile>|--ifd| --fmap|--fmap-file <file>) [-i <region>[:<file>]]...]\n" "\t\t [-n] [-N] [-f])]\n" "\t[-V[V[V]]] [-o <logfile>]\n\n", name); @@ -67,7 +67,8 @@ " --fmap read ROM layout from fmap embedded in ROM\n" " --fmap-file <fmapfile> read ROM layout from fmap in <fmapfile>\n" " --ifd read layout from an Intel Firmware Descriptor\n" - " -i | --image <name> only flash image <name> from flash layout\n" + " -i | --image <region>[:<file>] only read/write image <region> from layout\n" + " (optionally with data from <file>)\n" " -o | --output <logfile> log output to <logfile>\n" " --flash-contents <ref-file> assume flash contents to be <ref-file>\n" " -L | --list-supported print supported devices\n" @@ -337,11 +338,8 @@ fmap = 1; break; case 'i': - tempstr = strdup(optarg); - if (register_include_arg(&include_args, tempstr)) { - free(tempstr); + if (register_include_arg(&include_args, optarg)) cli_classic_abort_usage(NULL); - } break; case OPTION_FLASH_CONTENTS: if (referencefile) diff --git a/flash.h b/flash.h index 883b6f4..c0e5c27 100644 --- a/flash.h +++ b/flash.h @@ -417,7 +417,7 @@ #define msg_cspew(...) print(FLASHROM_MSG_SPEW, __VA_ARGS__) /* chip debug spew */ /* layout.c */ -int register_include_arg(struct layout_include_args **args, char *name); +int register_include_arg(struct layout_include_args **args, const char *arg); int read_romlayout(const char *name); int normalize_romentries(const struct flashctx *flash); void layout_cleanup(struct layout_include_args **args); diff --git a/flashrom.8.tmpl b/flashrom.8.tmpl index 02f60dc..4634539 100644 --- a/flashrom.8.tmpl +++ b/flashrom.8.tmpl @@ -48,7 +48,8 @@ \fB\-p\fR <programmername>[:<parameters>] [\fB\-c\fR <chipname>] (\fB\-\-flash\-name\fR|\fB\-\-flash\-size\fR| [\fB\-E\fR|\fB\-r\fR <file>|\fB\-w\fR <file>|\fB\-v\fR <file>] - [(\fB\-l\fR <file>|\fB\-\-ifd|\fB \-\-fmap\fR|\fB\-\-fmap-file\fR <file>) [\fB\-i\fR <image>]] + [(\fB\-l\fR <file>|\fB\-\-ifd|\fB \-\-fmap\fR|\fB\-\-fmap-file\fR <file>) + [\fB\-i\fR <image>[:<file>]]] [\fB\-n\fR] [\fB\-N\fR] [\fB\-f\fR])] [\fB\-V\fR[\fBV\fR[\fBV\fR]]] [\fB-o\fR <logfile>] .SH DESCRIPTION @@ -239,10 +240,45 @@ gbe gigabit ethernet firmware pd platform specific data .TP -.B "\-i, \-\-image <imagename>" -Only flash region/image -.B <imagename> -from flash layout. +.B "\-i, \-\-include <region>[:<file>]" +Read or write only +.B <region> +to or from ROM. +The +.B "\-i" +option may be used multiple times if the user wishes to read or write +multiple regions using a single command. +.sp +The user may optionally specify a corresponding +.B <file> +for any region they wish to read or write. A read operation will read the +corresponding regions from ROM and write individual files for each one. A write +option will read file(s) and write to the corresponding region(s) in ROM. +.sp +For write operations, files specified using +.B "\-i" +take precedence over content from the argument to +.B "\-w." +.sp +Examples: +.sp + To read regions named +.BR "foo " and " bar" +in layout file +.B <layout> +into region-sized files +.BR "foo.bin " and " bar.bin" ", run: +.sp +.B " flashrom \-p prog \-l <layout> \-i foo:foo.bin -i bar:bar.bin -r rom.bin +.sp + To write files +.BR "foo.bin " and " bar.bin" +into regions named +.BR "foo " and " bar" " in layout file +.BR <layout> +to the ROM, run: +.sp +.B " flashrom \-p prog \-l <layout> \-i foo:foo.bin -i bar:bar.bin -w rom.bin .TP .B "\-\-flash\-name" Prints out the detected flash chips name. diff --git a/flashrom.c b/flashrom.c index 056fc57..3a733df 100644 --- a/flashrom.c +++ b/flashrom.c @@ -1360,7 +1360,7 @@ goto out; } if (image_stat.st_size != (intmax_t)size) { - msg_gerr("Error: Image size (%jd B) doesn't match the flash chip's size (%lu B)!\n", + msg_gerr("Error: Image size (%jd B) doesn't match the expected size (%lu B)!\n", (intmax_t)image_stat.st_size, size); ret = 1; goto out; @@ -1378,6 +1378,51 @@ #endif } +/** + * @brief Reads content to buffer from one or more files. + * + * Reads content to supplied buffer from files. If a filename is specified for + * individual regions using the partial read syntax ('-i <region>[:<filename>]') + * then this will read file data into the corresponding region in the + * supplied buffer. + * + * @param flashctx Flash context to be used. + * @param buf Chip-sized buffer to write data to + * @return 0 on success + */ +static int read_buf_from_include_args(const struct flashctx *const flash, + unsigned char *buf) +{ + const struct flashrom_layout *const layout = get_layout(flash); + const struct romentry *entry = NULL; + + /* + * Content will be read from -i args, so they must not overlap since + * we need to know exactly what content to write to the ROM. + */ + if (included_regions_overlap(layout)) { + msg_gerr("Error: Included regions must not overlap when writing.\n"); + return 1; + } + + while ((entry = layout_next_included(layout, entry))) { + if (!entry->file) + continue; + if (read_buf_from_file(buf + entry->start, + entry->end - entry->start + 1, entry->file)) + return 1; + } + return 0; +} + +/** + * @brief Writes passed data buffer into a file + * + * @param buf Buffer with data to write + * @param size Size of buffer + * @param filename File path to write to + * @return 0 on success + */ int write_buf_to_file(const unsigned char *buf, unsigned long size, const char *filename) { #ifdef __LIBPAYLOAD__ @@ -1430,6 +1475,35 @@ #endif } +/** + * @brief Writes content from buffer to one or more files. + * + * Writes content from supplied buffer to files. If a filename is specified for + * individual regions using the partial read syntax ('-i <region>[:<filename>]') + * then this will write files using data from the corresponding region in the + * supplied buffer. + * + * @param flashctx Flash context to be used. + * @param buf Chip-sized buffer to read data from + * @return 0 on success + */ +static int write_buf_to_include_args(const struct flashctx *const flash, + unsigned char *buf) +{ + const struct flashrom_layout *const layout = get_layout(flash); + const struct romentry *entry = NULL; + + while ((entry = layout_next_included(layout, entry))) { + if (!entry->file) + continue; + if (write_buf_to_file(buf + entry->start, + entry->end - entry->start + 1, entry->file)) + return 1; + } + + return 0; +} + static int read_by_layout(struct flashctx *, uint8_t *); int read_flash_to_file(struct flashctx *flash, const char *filename) { @@ -1453,6 +1527,10 @@ ret = 1; goto out_free; } + if (write_buf_to_include_args(flash, buf)) { + ret = 1; + goto out_free; + } ret = write_buf_to_file(buf, size, filename); out_free: @@ -2643,8 +2721,15 @@ goto _free_ret; } + /* Read '-w' argument first... */ if (read_buf_from_file(newcontents, flash_size, filename)) goto _free_ret; + /* + * ... then update newcontents with contents from files provided to '-i' + * args if needed. + */ + if (read_buf_from_include_args(flash, newcontents)) + goto _free_ret; if (referencefile) { if (read_buf_from_file(refcontents, flash_size, referencefile)) @@ -2670,8 +2755,15 @@ goto _free_ret; } + /* Read '-v' argument first... */ if (read_buf_from_file(newcontents, flash_size, filename)) goto _free_ret; + /* + * ... then update newcontents with contents from files provided to '-i' + * args if needed. + */ + if (read_buf_from_include_args(flash, newcontents)) + goto _free_ret; ret = flashrom_image_verify(flash, newcontents, flash_size); diff --git a/layout.c b/layout.c index 272912c..d0d2607 100644 --- a/layout.c +++ b/layout.c @@ -52,7 +52,7 @@ romlayout = fopen(name, "r"); if (!romlayout) { - msg_gerr("ERROR: Could not open ROM layout (%s).\n", + msg_gerr("ERROR: Could not open layout file (%s).\n", name); return -1; } @@ -82,6 +82,7 @@ layout->entries[layout->num_entries].start = strtol(tstr1, (char **)NULL, 16); layout->entries[layout->num_entries].end = strtol(tstr2, (char **)NULL, 16); layout->entries[layout->num_entries].included = false; + layout->entries[layout->num_entries].file = NULL; layout->entries[layout->num_entries].name = strdup(tempname); if (!layout->entries[layout->num_entries].name) { msg_gerr("Error adding layout entry: %s\n", strerror(errno)); @@ -105,44 +106,76 @@ #endif /* register an include argument (-i) for later processing */ -int register_include_arg(struct layout_include_args **args, char *name) +int register_include_arg(struct layout_include_args **args, const char *arg) { struct layout_include_args *tmp; - if (name == NULL) { + char *colon; + char *name; + char *file; + + if (arg == NULL) { msg_gerr("<NULL> is a bad region name.\n"); return 1; } - tmp = *args; - while (tmp) { + /* -i <image>[:<file>] */ + colon = strchr(arg, ':'); + if (colon && !colon[1]) { + msg_gerr("Missing filename parameter in %s\n", arg); + return 1; + } + name = colon ? strndup(arg, colon - arg) : strdup(arg); + file = colon ? strdup(colon + 1) : NULL; + + for (tmp = *args; tmp; tmp = tmp->next) { if (!strcmp(tmp->name, name)) { msg_gerr("Duplicate region name: \"%s\".\n", name); - return 1; + goto error; } - tmp = tmp->next; } tmp = malloc(sizeof(struct layout_include_args)); if (tmp == NULL) { msg_gerr("Could not allocate memory"); - return 1; + goto error; } tmp->name = name; + tmp->file = file; tmp->next = *args; *args = tmp; - return 0; + +error: + free(name); + free(file); + return 1; +} + +/* returns 0 to indicate success, 1 to indicate failure */ +static int include_region(struct flashrom_layout *const l, const char *name, + const char *file) +{ + size_t i; + for (i = 0; i < l->num_entries; ++i) { + if (!strcmp(l->entries[i].name, name)) { + l->entries[i].included = true; + if (file) + l->entries[i].file = strdup(file); + return 0; + } + } + return 1; } /* returns -1 if an entry is not found, 0 if found. */ -static int find_romentry(struct flashrom_layout *const l, char *name) +static int find_romentry(struct flashrom_layout *const l, char *name, char *file) { if (l->num_entries == 0) return -1; msg_gspew("Looking for region \"%s\"... ", name); - if (flashrom_layout_include_region(l, name)) { + if (include_region(l, name, file)) { msg_gspew("not found.\n"); return -1; } @@ -161,7 +194,7 @@ if (args == NULL) return 0; - /* User has specified an area, but no layout file is loaded. */ + /* User has specified an include argument, but no layout is loaded. */ if (l->num_entries == 0) { msg_gerr("Region requested (with -i \"%s\"), " "but no layout data is available.\n", @@ -171,7 +204,7 @@ tmp = args; while (tmp) { - if (find_romentry(l, tmp->name) < 0) { + if (find_romentry(l, tmp->name, tmp->file) < 0) { msg_gerr("Invalid region specified: \"%s\".\n", tmp->name); return 1; @@ -180,10 +213,14 @@ found++; } - msg_ginfo("Using region%s:", found > 1 ? "s" : ""); + msg_ginfo("Using region%s: ", found > 1 ? "s" : ""); tmp = args; while (tmp) { - msg_ginfo(" \"%s\"%s", tmp->name, found > 1 ? "," : ""); + msg_ginfo("\"%s\"", tmp->name); + if (tmp->file) + msg_ginfo(":\"%s\"", tmp->file); + if (found > 1) + msg_ginfo(", "); found--; tmp = tmp->next; } @@ -191,6 +228,39 @@ return 0; } +/* returns boolean 1 if any regions overlap, 0 otherwise */ +int included_regions_overlap(const struct flashrom_layout *const l) +{ + size_t i; + int overlap_detected = 0; + + for (i = 0; i < l->num_entries; i++) { + size_t j; + + if (!l->entries[i].included) + continue; + + for (j = i + 1; j < l->num_entries; j++) { + if (!l->entries[j].included) + continue; + + if (l->entries[i].start > l->entries[j].end) + continue; + + if (l->entries[i].end < l->entries[j].start) + continue; + + msg_gdbg("Regions %s [0x%08x-0x%08x] and " + "%s [0x%08x-0x%08x] overlap\n", + l->entries[i].name, l->entries[i].start, + l->entries[i].end, l->entries[j].name, + l->entries[j].start, l->entries[j].end); + overlap_detected = 1; + } + } + return overlap_detected; +} + void layout_cleanup(struct layout_include_args **args) { struct flashrom_layout *const layout = get_global_layout(); @@ -199,12 +269,15 @@ while (*args) { tmp = (*args)->next; + free((*args)->name); + free((*args)->file); free(*args); *args = tmp; } for (i = 0; i < layout->num_entries; i++) { free(layout->entries[i].name); + free(layout->entries[i].file); layout->entries[i].included = false; } layout->num_entries = 0; @@ -287,14 +360,7 @@ */ int flashrom_layout_include_region(struct flashrom_layout *const layout, const char *name) { - size_t i; - for (i = 0; i < layout->num_entries; ++i) { - if (!strcmp(layout->entries[i].name, name)) { - layout->entries[i].included = true; - return 0; - } - } - return 1; + return include_region(layout, name, NULL); } /** @@ -309,8 +375,10 @@ if (!layout || layout == get_global_layout()) return; - for (i = 0; i < layout->num_entries; ++i) + for (i = 0; i < layout->num_entries; ++i) { free(layout->entries[i].name); + free(layout->entries[i].file); + } free(layout); } diff --git a/layout.h b/layout.h index cceedd1..b11c432 100644 --- a/layout.h +++ b/layout.h @@ -40,6 +40,7 @@ chipoff_t end; bool included; char *name; + char *file; }; struct flashrom_layout { @@ -56,6 +57,7 @@ struct layout_include_args { char *name; + char *file; struct layout_include_args *next; }; @@ -66,5 +68,6 @@ int process_include_args(struct flashrom_layout *l, const struct layout_include_args *const args); const struct romentry *layout_next_included_region(const struct flashrom_layout *, chipoff_t); const struct romentry *layout_next_included(const struct flashrom_layout *, const struct romentry *); +int included_regions_overlap(const struct flashrom_layout *const flashrom_layout); #endif /* !__LAYOUT_H__ */ -- To view, visit https://review.coreboot.org/c/flashrom/+/23021 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic5465659605d8431d931053967b40290195cfd99 Gerrit-Change-Number: 23021 Gerrit-PatchSet: 37 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: 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: Jack Rosenthal <jrosenth(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Nicola Corna <nicola(a)corna.info> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Sam McNally <sammc(a)google.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

Change in flashrom[master]: ft2232_spi.c: Implement spi_send_multicommand()
by Alan Green (Code Review) 27 Apr '21

27 Apr '21

Attention is currently required from: Simon Buhrow, Edward O'Callaghan, Samir Ibradžić. Alan Green has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/40477 ) Change subject: ft2232_spi.c: Implement spi_send_multicommand() ...................................................................... Patch Set 30: Code-Review+1 -- 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: 30 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: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Samir Ibradžić <sibradzic(a)gmail.com> Gerrit-Comment-Date: Tue, 27 Apr 2021 21:54:46 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

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

27 Apr '21

Attention is currently required from: Simon Buhrow, Alan Green, 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 30: Code-Review+1 -- 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: 30 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: Alan Green <avg(a)google.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Samir Ibradžić <sibradzic(a)gmail.com> Gerrit-Comment-Date: Tue, 27 Apr 2021 21:00:57 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ene_lpc.c: Extract params check into a function
by Nico Huber (Code Review) 27 Apr '21

27 Apr '21

Attention is currently required from: Angel Pons, Anastasia Klimchuk. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52684 ) Change subject: ene_lpc.c: Extract params check into a function ...................................................................... Patch Set 1: -Code-Review (1 comment) File ene_lpc.c: https://review.coreboot.org/c/flashrom/+/52684/comment/e636ecf7_ab84fee0 PS1, Line 519: = NULL > nit: initialisation is not necessary heh, I knew I missed something :) it's not a nit IMHO, static analysis tools could complain about it. What I usually do to avoid such situations: declare it inline with the code and `const`, e.g. char *const p = extract_programmer_param("type"); This way it's clear that it can never be used unitialized. -- To view, visit https://review.coreboot.org/c/flashrom/+/52684 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7c3b6dea0edbc7547f0b307a0508c7d2b2a6d370 Gerrit-Change-Number: 52684 Gerrit-PatchSet: 1 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: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-CC: Victor Ding <victording(a)google.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Tue, 27 Apr 2021 14:35:48 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ene_lpc.c: Untangle successful vs failed init paths
by Angel Pons (Code Review) 27 Apr '21

27 Apr '21

Attention is currently required from: Anastasia Klimchuk. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52685 ) Change subject: ene_lpc.c: Untangle successful vs failed init paths ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/52685 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iac295f1353785cd73d7cb2f19e4a8cbb69beb576 Gerrit-Change-Number: 52685 Gerrit-PatchSet: 1 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: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-CC: Victor Ding <victording(a)google.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Tue, 27 Apr 2021 13:31:50 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ene_lpc.c: Extract params check into a function
by Angel Pons (Code Review) 27 Apr '21

27 Apr '21

Attention is currently required from: Anastasia Klimchuk. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52684 ) Change subject: ene_lpc.c: Extract params check into a function ...................................................................... Patch Set 1: Code-Review+2 (1 comment) File ene_lpc.c: https://review.coreboot.org/c/flashrom/+/52684/comment/ea3be74b_48038b12 PS1, Line 519: = NULL nit: initialisation is not necessary -- To view, visit https://review.coreboot.org/c/flashrom/+/52684 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7c3b6dea0edbc7547f0b307a0508c7d2b2a6d370 Gerrit-Change-Number: 52684 Gerrit-PatchSet: 1 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: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-CC: Victor Ding <victording(a)google.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Tue, 27 Apr 2021 13:31:37 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: [RFC] tests: Abstract port I/O and factor mec1308 specifics out
by Nico Huber (Code Review) 27 Apr '21

27 Apr '21

Attention is currently required from: Anastasia Klimchuk. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52604 ) Change subject: [RFC] tests: Abstract port I/O and factor mec1308 specifics out ...................................................................... Patch Set 1: (2 comments) File tests/init_shutdown.c: https://review.coreboot.org/c/flashrom/+/52604/comment/cc11e331_0aa7f976 PS1, Line 63: .outb = mec1308_outb, > I think, maybe I could get away with a default implementation of outx functions, and only customise […] I'm actually not familiar with our port-i/o using programmer drivers. Without looking into them, it could work or not. It boils down to what the return value of an inb/inw/inl call would depend on. If it's more than the very last outb/outw/outl, we would obviously have to cache more information than that. Thinking really far ahead: For more than just testing initialization, e.g. test flashing procedures as well, we may have to implement more sophisticated state machines to mock the programmers. That would very likely need the outb/outw/outl pointers and a private data structure. tl;dr I can imagine we may need all the pointers. File tests/tests.c: https://review.coreboot.org/c/flashrom/+/52604/comment/694b35dd_5b34c7da PS1, Line 23: const struct io_mock *current_io = NULL; > Do you think it would work if I try to swap this line with extern definition in io_mock. […] I guess I'm just used to specify `extern` in header files because it has some implications with incomplete types. We can also add `extern` here. But I would keep the `= NULL` here because this compilation unit is where it is consumed and the initialization actually matters. A good rule to live by: Any global should be either declared `static` or be used somewhere else. If something is not `static`, you have to expect it to be referenced elsewhere. Alternative: Make it `static` and add a setter function to be called from the other compilation units. -- To view, visit https://review.coreboot.org/c/flashrom/+/52604 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia3a424478456a7c968b3c736ba371d755c494f30 Gerrit-Change-Number: 52604 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Tue, 27 Apr 2021 13:30:58 +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]: ene_lpc.c: Untangle successful vs failed init paths
by Edward O'Callaghan (Code Review) 27 Apr '21

27 Apr '21

Attention is currently required from: Angel Pons, Anastasia Klimchuk. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52685 ) Change subject: ene_lpc.c: Untangle successful vs failed init paths ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/52685 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iac295f1353785cd73d7cb2f19e4a8cbb69beb576 Gerrit-Change-Number: 52685 Gerrit-PatchSet: 1 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: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-CC: Victor Ding <victording(a)google.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Tue, 27 Apr 2021 13:23:30 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ene_lpc.c: Extract params check into a function
by Edward O'Callaghan (Code Review) 27 Apr '21

27 Apr '21

Attention is currently required from: Angel Pons, Anastasia Klimchuk. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52684 ) Change subject: ene_lpc.c: Extract params check into a function ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/52684 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7c3b6dea0edbc7547f0b307a0508c7d2b2a6d370 Gerrit-Change-Number: 52684 Gerrit-PatchSet: 1 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: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-CC: Victor Ding <victording(a)google.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Tue, 27 Apr 2021 13:22:30 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: layout: Add -i <region>[:<file>] support
by Edward O'Callaghan (Code Review) 27 Apr '21

27 Apr '21

Attention is currently required from: Nico Huber, Nicola Corna, Jack Rosenthal, Paul Menzel, Stefan Reinauer, David Hendricks, Daniel Campello, Arthur Heymans, Carl-Daniel Hailfinger. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/23021 ) Change subject: layout: Add -i <region>[:<file>] support ...................................................................... Patch Set 36: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/23021 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic5465659605d8431d931053967b40290195cfd99 Gerrit-Change-Number: 23021 Gerrit-PatchSet: 36 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: 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: Jack Rosenthal <jrosenth(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Nicola Corna <nicola(a)corna.info> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Sam McNally <sammc(a)google.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Nicola Corna <nicola(a)corna.info> Gerrit-Attention: Jack Rosenthal <jrosenth(a)chromium.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Attention: Daniel Campello <campello(a)chromium.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net> Gerrit-Comment-Date: Tue, 27 Apr 2021 13:21:38 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit April 2021