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

Change in flashrom[master]: cli_classic: replace enum programmer by programmer_entry*
by Thomas Heijligen (Code Review) 01 Jun '21

01 Jun '21

Thomas Heijligen has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/55122 ) Change subject: cli_classic: replace enum programmer by programmer_entry* ...................................................................... cli_classic: replace enum programmer by programmer_entry* Change-Id: I4c45f278addeea0d486a316435e8dc15d93cbd70 Signed-off-by: Thomas Heijligen <thomas.heijligen(a)secunet.de> --- M cli_classic.c 1 file changed, 12 insertions(+), 8 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/22/55122/1 diff --git a/cli_classic.c b/cli_classic.c index ba6885c..4537e1e 100644 --- a/cli_classic.c +++ b/cli_classic.c @@ -172,7 +172,8 @@ int read_it = 0, extract_it = 0, write_it = 0, erase_it = 0, verify_it = 0; int dont_verify_it = 0, dont_verify_all = 0, list_supported = 0, operation_specified = 0; struct flashrom_layout *layout = NULL; - enum programmer prog = PROGRAMMER_INVALID; + // enum programmer prog = PROGRAMMER_INVALID; + static const struct programmer_entry *prog = NULL; enum { OPTION_IFD = 0x0100, OPTION_FMAP, @@ -399,15 +400,16 @@ #endif break; case 'p': - if (prog != PROGRAMMER_INVALID) { + if (prog != NULL) { cli_classic_abort_usage("Error: --programmer specified " "more than once. You can separate " "multiple\nparameters for a programmer " "with \",\". Please see the man page " "for details.\n"); } - for (prog = 0; prog < programmer_table_size; prog++) { - name = programmer_table[prog]->name; + size_t p; + for (p = 0; p < programmer_table_size; p++) { + name = programmer_table[p]->name; namelen = strlen(name); if (strncmp(optarg, name, namelen) == 0) { switch (optarg[namelen]) { @@ -417,8 +419,10 @@ free(pparam); pparam = NULL; } + prog = programmer_table[p]; break; case '\0': + prog = programmer_table[p]; break; default: /* The continue refers to the @@ -431,7 +435,7 @@ break; } } - if (prog == PROGRAMMER_INVALID) { + if (prog == NULL) { fprintf(stderr, "Error: Unknown programmer \"%s\". Valid choices are:\n", optarg); list_programmers_linebreak(0, 80, 0); @@ -539,9 +543,9 @@ /* Keep chip around for later usage in case a forced read is requested. */ } - if (prog == PROGRAMMER_INVALID) { + if (prog == NULL) { if (CONFIG_DEFAULT_PROGRAMMER != PROGRAMMER_INVALID) { - prog = CONFIG_DEFAULT_PROGRAMMER; + prog = programmer_table[CONFIG_DEFAULT_PROGRAMMER]; /* We need to strdup here because we free(pparam) unconditionally later. */ pparam = strdup(CONFIG_DEFAULT_PROGRAMMER_ARGS); msg_pinfo("Using default programmer \"%s\" with arguments \"%s\".\n", @@ -562,7 +566,7 @@ /* FIXME: Delay calibration should happen in programmer code. */ myusec_calibrate_delay(); - if (programmer_init(programmer_table[prog], pparam)) { + if (programmer_init(prog, pparam)) { msg_perr("Error: Programmer initialization failed.\n"); ret = 1; goto out_shutdown; -- To view, visit https://review.coreboot.org/c/flashrom/+/55122 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I4c45f278addeea0d486a316435e8dc15d93cbd70 Gerrit-Change-Number: 55122 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: flashrom.c: replace enum programmer by struct programmer_entry*
by Thomas Heijligen (Code Review) 01 Jun '21

01 Jun '21

Thomas Heijligen has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/55119 ) Change subject: flashrom.c: replace enum programmer by struct programmer_entry* ...................................................................... flashrom.c: replace enum programmer by struct programmer_entry* Change-Id: I478c56354d5f482010bfe1560489700bc889717a Signed-off-by: Thomas Heijligen <thomas.heijligen(a)secunet.de> --- M flashrom.c 1 file changed, 12 insertions(+), 12 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/19/55119/1 diff --git a/flashrom.c b/flashrom.c index bb4dd31..d9220a3 100644 --- a/flashrom.c +++ b/flashrom.c @@ -43,7 +43,7 @@ const char flashrom_version[] = FLASHROM_VERSION; const char *chip_to_probe = NULL; -static enum programmer programmer = PROGRAMMER_INVALID; +static const struct programmer_entry *programmer = NULL; static const char *programmer_param = NULL; /* @@ -138,7 +138,7 @@ msg_perr("Invalid programmer specified!\n"); return -1; } - programmer = prog; + programmer = programmer_table[prog]; /* Initialize all programmer specific data. */ /* Default to unlimited decode sizes. */ max_rom_decode = (const struct decode_sizes) { @@ -155,8 +155,8 @@ programmer_may_write = 1; programmer_param = param; - msg_pdbg("Initializing %s programmer\n", programmer_table[programmer]->name); - ret = programmer_table[programmer]->init(); + msg_pdbg("Initializing %s programmer\n", programmer->name); + ret = programmer->init(); if (programmer_param && strlen(programmer_param)) { if (ret != 0) { /* It is quite possible that any unhandled programmer parameter would have been valid, @@ -200,7 +200,7 @@ void *programmer_map_flash_region(const char *descr, uintptr_t phys_addr, size_t len) { - void *ret = programmer_table[programmer]->map_flash_region(descr, phys_addr, len); + void *ret = programmer->map_flash_region(descr, phys_addr, len); msg_gspew("%s: mapping %s from 0x%0*" PRIxPTR " to 0x%0*" PRIxPTR "\n", __func__, descr, PRIxPTR_WIDTH, phys_addr, PRIxPTR_WIDTH, (uintptr_t) ret); return ret; @@ -208,7 +208,7 @@ void programmer_unmap_flash_region(void *virt_addr, size_t len) { - programmer_table[programmer]->unmap_flash_region(virt_addr, len); + programmer->unmap_flash_region(virt_addr, len); msg_gspew("%s: unmapped 0x%0*" PRIxPTR "\n", __func__, PRIxPTR_WIDTH, (uintptr_t)virt_addr); } @@ -256,7 +256,7 @@ void programmer_delay(unsigned int usecs) { if (usecs > 0) - programmer_table[programmer]->delay(usecs); + programmer->delay(usecs); } int read_memmapped(struct flashctx *flash, uint8_t *buf, unsigned int start, @@ -840,12 +840,12 @@ flash->chip->vendor, flash->chip->name, flash->chip->total_size, tmp); free(tmp); #if CONFIG_INTERNAL == 1 - if (programmer_table[programmer]->map_flash_region == physmap) + if (programmer->map_flash_region == physmap) msg_cinfo("mapped at physical address 0x%0*" PRIxPTR ".\n", PRIxPTR_WIDTH, flash->physical_memory); else #endif - msg_cinfo("on %s.\n", programmer_table[programmer]->name); + msg_cinfo("on %s.\n", programmer->name); /* Flash registers may more likely not be mapped if the chip was forced. * Lock info may be stored in registers, so avoid lock info printing. */ @@ -1548,7 +1548,7 @@ { msg_gerr("Good, writing to the flash chip apparently didn't do anything.\n"); #if CONFIG_INTERNAL == 1 - if (programmer == PROGRAMMER_INTERNAL) + if (programmer == &programmer_internal) msg_gerr("This means we have to add special support for your board, programmer or flash\n" "chip. Please report this on IRC at chat.freenode.net (channel #flashrom) or\n" "mail flashrom(a)flashrom.org, thanks!\n" @@ -1566,7 +1566,7 @@ { msg_gerr("Your flash chip is in an unknown state.\n"); #if CONFIG_INTERNAL == 1 - if (programmer == PROGRAMMER_INTERNAL) + if (programmer == &programmer_internal) msg_gerr("Get help on IRC at chat.freenode.net (channel #flashrom) or\n" "mail flashrom(a)flashrom.org with the subject \"FAILED: <your board name>\"!\n" "-------------------------------------------------------------------------------\n" @@ -2057,7 +2057,7 @@ } #if CONFIG_INTERNAL == 1 - if (programmer == PROGRAMMER_INTERNAL && cb_check_image(newcontents, flash_size) < 0) { + if (programmer == &programmer_internal && cb_check_image(newcontents, flash_size) < 0) { if (flashctx->flags.force_boardmismatch) { msg_pinfo("Proceeding anyway because user forced us to.\n"); } else { -- To view, visit https://review.coreboot.org/c/flashrom/+/55119 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I478c56354d5f482010bfe1560489700bc889717a Gerrit-Change-Number: 55119 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: enum programmer: replace by int
by Thomas Heijligen (Code Review) 01 Jun '21

01 Jun '21

Thomas Heijligen has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/55120 ) Change subject: enum programmer: replace by int ...................................................................... enum programmer: replace by int Change-Id: Id2ad9630fbc41e98d182768b553788e069fa5095 Signed-off-by: Thomas Heijligen <thomas.heijligen(a)secunet.de> --- M flashrom.c M libflashrom.c 2 files changed, 2 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/20/55120/1 diff --git a/flashrom.c b/flashrom.c index d9220a3..65fa559 100644 --- a/flashrom.c +++ b/flashrom.c @@ -1582,7 +1582,7 @@ const char *pname; int pnamelen; int remaining = 0, firstline = 1; - enum programmer p; + int p; int i; for (p = 0; p < programmer_table_size; p++) { diff --git a/libflashrom.c b/libflashrom.c index 2685433..29a5d2c 100644 --- a/libflashrom.c +++ b/libflashrom.c @@ -119,7 +119,7 @@ */ const char **flashrom_supported_programmers(void) { - enum programmer p = 0; + int p = 0; const char **supported_programmers = malloc((programmer_table_size + 1) * sizeof(char*)); if (supported_programmers != NULL) { -- To view, visit https://review.coreboot.org/c/flashrom/+/55120 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id2ad9630fbc41e98d182768b553788e069fa5095 Gerrit-Change-Number: 55120 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: programmer_table: replace PROGRAMMER_INVALID to programmer_table_size...
by Thomas Heijligen (Code Review) 01 Jun '21

01 Jun '21

Thomas Heijligen has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/55117 ) Change subject: programmer_table: replace PROGRAMMER_INVALID to programmer_table_size in itterations ...................................................................... programmer_table: replace PROGRAMMER_INVALID to programmer_table_size in itterations Change-Id: Icaaeefe001de604df9d7fdd06f05a5ed39fdbd84 Signed-off-by: Thomas Heijligen <thomas.heijligen(a)secunet.de> --- M cli_classic.c M flashrom.c M libflashrom.c M print.c M print_wiki.c 5 files changed, 14 insertions(+), 14 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/17/55117/1 diff --git a/cli_classic.c b/cli_classic.c index bc2d40d..f3ae123 100644 --- a/cli_classic.c +++ b/cli_classic.c @@ -406,7 +406,7 @@ "with \",\". Please see the man page " "for details.\n"); } - for (prog = 0; prog < PROGRAMMER_INVALID; prog++) { + for (prog = 0; prog < programmer_table_size; prog++) { name = programmer_table[prog]->name; namelen = strlen(name); if (strncmp(optarg, name, namelen) == 0) { diff --git a/flashrom.c b/flashrom.c index d6b35c6..bb4dd31 100644 --- a/flashrom.c +++ b/flashrom.c @@ -134,7 +134,7 @@ { int ret; - if (prog >= PROGRAMMER_INVALID) { + if (prog >= programmer_table_size) { msg_perr("Invalid programmer specified!\n"); return -1; } @@ -1585,7 +1585,7 @@ enum programmer p; int i; - for (p = 0; p < PROGRAMMER_INVALID; p++) { + for (p = 0; p < programmer_table_size; p++) { pname = programmer_table[p]->name; pnamelen = strlen(pname); if (remaining - pnamelen - 2 < 0) { @@ -1606,7 +1606,7 @@ } msg_ginfo("%s", pname); remaining -= pnamelen; - if (p < PROGRAMMER_INVALID - 1) { + if (p < programmer_table_size - 1) { msg_ginfo(","); remaining--; } else { @@ -1717,7 +1717,7 @@ msg_gerr("Programmer table miscompilation!\n"); ret = 1; } - for (i = 0; i < PROGRAMMER_INVALID; i++) { + for (i = 0; i < programmer_table_size; i++) { const struct programmer_entry *p = programmer_table[i]; if (p->name == NULL) { msg_gerr("All programmers need a valid name, but the one with index %d does not!\n", i); diff --git a/libflashrom.c b/libflashrom.c index 09a4e4d..2685433 100644 --- a/libflashrom.c +++ b/libflashrom.c @@ -120,10 +120,10 @@ const char **flashrom_supported_programmers(void) { enum programmer p = 0; - const char **supported_programmers = malloc((PROGRAMMER_INVALID + 1) * sizeof(char*)); + const char **supported_programmers = malloc((programmer_table_size + 1) * sizeof(char*)); if (supported_programmers != NULL) { - for (; p < PROGRAMMER_INVALID; ++p) { + for (; p < programmer_table_size; ++p) { supported_programmers[p] = programmer_table[p]->name; } } else { @@ -278,11 +278,11 @@ { unsigned prog; - for (prog = 0; prog < PROGRAMMER_INVALID; prog++) { + for (prog = 0; prog < programmer_table_size; prog++) { if (strcmp(prog_name, programmer_table[prog]->name) == 0) break; } - if (prog >= PROGRAMMER_INVALID) { + if (prog >= programmer_table_size) { msg_ginfo("Error: Unknown programmer \"%s\". Valid choices are:\n", prog_name); list_programmers_linebreak(0, 80, 0); return 1; diff --git a/print.c b/print.c index fbf4882..49c1aa1 100644 --- a/print.c +++ b/print.c @@ -482,7 +482,7 @@ msg_ginfo("\n"); print_supported_boards_helper(laptops_known, "mobile devices"); #endif - for (i = 0; i < PROGRAMMER_INVALID; i++) { + for (i = 0; i < programmer_table_size; i++) { const struct programmer_entry *prog = programmer_table[i]; switch (prog->type) { case USB: diff --git a/print_wiki.c b/print_wiki.c index 4088d9f..8ca0e1e 100644 --- a/print_wiki.c +++ b/print_wiki.c @@ -380,7 +380,7 @@ unsigned int usb_count = 0; unsigned int i; - for (i = 0; i < PROGRAMMER_INVALID; i++) { + for (i = 0; i < programmer_table_size; i++) { const struct programmer_entry *prog = programmer_table[i]; switch (prog->type) { case USB: @@ -399,7 +399,7 @@ "Total amount of supported PCI devices flashrom can use as a programmer: '''%d'''\n\n" "{%s%s", pci_count, th_start, programmer_th); - for (i = 0; i < PROGRAMMER_INVALID; i++) { + for (i = 0; i < programmer_table_size; i++) { const struct programmer_entry *prog = programmer_table[i]; if (prog->type == PCI) { print_supported_devs_wiki_helper(prog); @@ -411,7 +411,7 @@ "Total amount of supported USB devices flashrom can use as a programmer: '''%d'''\n\n" "{%s%s", usb_count, th_start, programmer_th); - for (i = 0; i < PROGRAMMER_INVALID; i++) { + for (i = 0; i < programmer_table_size; i++) { const struct programmer_entry *prog = programmer_table[i]; if (prog->type == USB) { print_supported_devs_wiki_helper(prog); @@ -424,7 +424,7 @@ printf("! align=\"left\" | Programmer\n" "! align=\"left\" | Note\n\n"); - for (i = 0; i < PROGRAMMER_INVALID; i++) { + for (i = 0; i < programmer_table_size; i++) { static int c = 0; const struct programmer_entry *prog = programmer_table[i]; if (prog->type == OTHER && prog->devs.note != NULL) { -- To view, visit https://review.coreboot.org/c/flashrom/+/55117 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Icaaeefe001de604df9d7fdd06f05a5ed39fdbd84 Gerrit-Change-Number: 55117 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: programmer_table: add table size variable
by Thomas Heijligen (Code Review) 01 Jun '21

01 Jun '21

Thomas Heijligen has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/55116 ) Change subject: programmer_table: add table size variable ...................................................................... programmer_table: add table size variable Change-Id: Idacb4b7b0dd98af8bc7cbecf567d33daafb4b24d Signed-off-by: Thomas Heijligen <thomas.heijligen(a)secunet.de> --- M flashrom.c M programmer.h 2 files changed, 4 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/16/55116/1 diff --git a/flashrom.c b/flashrom.c index 65f7e73..43a5599 100644 --- a/flashrom.c +++ b/flashrom.c @@ -534,6 +534,8 @@ #endif }; +const size_t programmer_table_size = ARRAY_SIZE(programmer_table); + #define SHUTDOWN_MAXFN 32 static int shutdown_fn_count = 0; /** @private */ @@ -2189,7 +2191,7 @@ /* Safety check. Instead of aborting after the first error, check * if more errors exist. */ - if (ARRAY_SIZE(programmer_table) != PROGRAMMER_INVALID) { + if (programmer_table_size != PROGRAMMER_INVALID) { msg_gerr("Programmer table miscompilation!\n"); ret = 1; } diff --git a/programmer.h b/programmer.h index 790fcc8..d3226d6 100644 --- a/programmer.h +++ b/programmer.h @@ -176,6 +176,7 @@ }; extern const struct programmer_entry programmer_table[]; +extern const size_t programmer_table_size; int programmer_init(enum programmer prog, const char *param); int programmer_shutdown(void); -- To view, visit https://review.coreboot.org/c/flashrom/+/55116 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Idacb4b7b0dd98af8bc7cbecf567d33daafb4b24d Gerrit-Change-Number: 55116 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: programmer_table: print.c use fixed fixed name instead of reference
by Thomas Heijligen (Code Review) 01 Jun '21

01 Jun '21

Thomas Heijligen has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/55118 ) Change subject: programmer_table: print.c use fixed fixed name instead of reference ...................................................................... programmer_table: print.c use fixed fixed name instead of reference Change-Id: If2cf95c71425efdd864457e213dd34b929fe8805 Signed-off-by: Thomas Heijligen <thomas.heijligen(a)secunet.de> --- M print.c 1 file changed, 1 insertion(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/18/55118/1 diff --git a/print.c b/print.c index 49c1aa1..1a01eab 100644 --- a/print.c +++ b/print.c @@ -474,8 +474,7 @@ list_programmers_linebreak(0, 80, 0); msg_ginfo("\n"); #if CONFIG_INTERNAL == 1 - msg_ginfo("\nSupported devices for the %s programmer:\n\n", - programmer_table[PROGRAMMER_INTERNAL]->name); + msg_ginfo("\nSupported devices for the internal programmer:\n\n"); print_supported_chipsets(); msg_ginfo("\n"); print_supported_boards_helper(boards_known, "mainboards"); -- To view, visit https://review.coreboot.org/c/flashrom/+/55118 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If2cf95c71425efdd864457e213dd34b929fe8805 Gerrit-Change-Number: 55118 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: programmer_table: remove NULL termination
by Thomas Heijligen (Code Review) 01 Jun '21

01 Jun '21

Thomas Heijligen has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/55115 ) Change subject: programmer_table: remove NULL termination ...................................................................... programmer_table: remove NULL termination The {0} Object at the end of programmer_table which correspond to PROGRAMMER_INVALID has no use in currend code Change-Id: Ib63c2d2941f23a0788e26e5a5feb25d8669acb42 Signed-off-by: Thomas Heijligen <thomas.heijligen(a)secunet.de> --- M flashrom.c 1 file changed, 1 insertion(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/15/55115/1 diff --git a/flashrom.c b/flashrom.c index 0af5057..65f7e73 100644 --- a/flashrom.c +++ b/flashrom.c @@ -532,8 +532,6 @@ .delay = internal_delay, }, #endif - - {0}, /* This entry corresponds to PROGRAMMER_INVALID. */ }; #define SHUTDOWN_MAXFN 32 @@ -2191,7 +2189,7 @@ /* Safety check. Instead of aborting after the first error, check * if more errors exist. */ - if (ARRAY_SIZE(programmer_table) - 1 != PROGRAMMER_INVALID) { + if (ARRAY_SIZE(programmer_table) != PROGRAMMER_INVALID) { msg_gerr("Programmer table miscompilation!\n"); ret = 1; } -- To view, visit https://review.coreboot.org/c/flashrom/+/55115 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ib63c2d2941f23a0788e26e5a5feb25d8669acb42 Gerrit-Change-Number: 55115 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: nic3com.c: Allocate data and register shutdown at the end of init
by Edward O'Callaghan (Code Review) 01 Jun '21

01 Jun '21

Attention is currently required from: Nico Huber, Angel Pons, Anastasia Klimchuk. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/55107 ) Change subject: nic3com.c: Allocate data and register shutdown at the end of init ...................................................................... Patch Set 1: -Code-Review (1 comment) File nic3com.c: https://review.coreboot.org/c/flashrom/+/55107/comment/b6379f3c_f06483f6 PS1, Line 151: data->id = id; just put the above line below this line and squash into the previous patch. -- To view, visit https://review.coreboot.org/c/flashrom/+/55107 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9834b82650cd070556cf82207796bc6bd6b31b28 Gerrit-Change-Number: 55107 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-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Tue, 01 Jun 2021 13:56:53 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: sb600spi.c: Remove sb600_spibar out of global state
by Edward O'Callaghan (Code Review) 01 Jun '21

01 Jun '21

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/48064 ) Change subject: sb600spi.c: Remove sb600_spibar out of global state ...................................................................... sb600spi.c: Remove sb600_spibar out of global state Make helper functions pure by taking the spibar as an argument and put the base address pointer within the internal spi master state structure to make the spi driver re-enterent. BUG=none BRANCH=none TEST=builds Change-Id: I2e692b230a77f3902d91d0ad36db0d668b7e9ffa Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M sb600spi.c 1 file changed, 34 insertions(+), 31 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/64/48064/1 diff --git a/sb600spi.c b/sb600spi.c index df1184a..a1089c5 100644 --- a/sb600spi.c +++ b/sb600spi.c @@ -40,7 +40,6 @@ *}; */ -static uint8_t *sb600_spibar = NULL; enum amd_chipset { CHIPSET_AMD_UNKNOWN, CHIPSET_SB6XX, @@ -57,6 +56,7 @@ struct sb600spi_data { bool map_flash_req; + uint8_t *sb600_spibar; }; static int find_smbus_dev_rev(uint16_t vendor, uint16_t device) @@ -140,7 +140,7 @@ return CHIPSET_AMD_UNKNOWN; } -static void reset_internal_fifo_pointer(void) +static void reset_internal_fifo_pointer(uint8_t *sb600_spibar) { mmio_writeb(mmio_readb(sb600_spibar + 2) | 0x10, sb600_spibar + 2); @@ -149,7 +149,7 @@ msg_pspew("reset\n"); } -static int compare_internal_fifo_pointer(uint8_t want) +static int compare_internal_fifo_pointer(uint8_t *sb600_spibar, uint8_t want) { uint8_t have = mmio_readb(sb600_spibar + 0xd) & 0x07; want %= FIFO_SIZE_OLD; @@ -183,7 +183,7 @@ return 0; } -static void execute_command(void) +static void execute_command(uint8_t *sb600_spibar) { msg_pspew("Executing... "); mmio_writeb(mmio_readb(sb600_spibar + 2) | 1, sb600_spibar + 2); @@ -197,11 +197,12 @@ const unsigned char *writearr, unsigned char *readarr) { + struct sb600spi_data * data = (struct sb600spi_data *)flash->mst->spi.data; /* First byte is cmd which can not be sent through the FIFO. */ unsigned char cmd = *writearr++; writecnt--; msg_pspew("%s, cmd=0x%02x, writecnt=%d, readcnt=%d\n", __func__, cmd, writecnt, readcnt); - mmio_writeb(cmd, sb600_spibar + 0); + mmio_writeb(cmd, data->sb600_spibar + 0); int ret = check_readwritecnt(flash, writecnt, readcnt); if (ret != 0) @@ -215,26 +216,26 @@ */ unsigned int readoffby1 = (writecnt > 0) ? 0 : 1; uint8_t readwrite = (readcnt + readoffby1) << 4 | (writecnt); - mmio_writeb(readwrite, sb600_spibar + 1); + mmio_writeb(readwrite, data->sb600_spibar + 1); - reset_internal_fifo_pointer(); + reset_internal_fifo_pointer(data->sb600_spibar); msg_pspew("Filling FIFO: "); unsigned int count; for (count = 0; count < writecnt; count++) { msg_pspew("[%02x]", writearr[count]); - mmio_writeb(writearr[count], sb600_spibar + 0xC); + mmio_writeb(writearr[count], data->sb600_spibar + 0xC); } msg_pspew("\n"); - if (compare_internal_fifo_pointer(writecnt)) + if (compare_internal_fifo_pointer(data->sb600_spibar, writecnt)) return SPI_PROGRAMMER_ERROR; /* * We should send the data in sequence, which means we need to reset * the FIFO pointer to the first byte we want to send. */ - reset_internal_fifo_pointer(); - execute_command(); - if (compare_internal_fifo_pointer(writecnt + readcnt)) + reset_internal_fifo_pointer(data->sb600_spibar); + execute_command(data->sb600_spibar); + if (compare_internal_fifo_pointer(data->sb600_spibar, writecnt + readcnt)) return SPI_PROGRAMMER_ERROR; /* @@ -248,27 +249,27 @@ * the opcode, the FIFO already stores the response from the chip. * Usually, the chip will respond with 0x00 or 0xff. */ - reset_internal_fifo_pointer(); + reset_internal_fifo_pointer(data->sb600_spibar); /* Skip the bytes we sent. */ msg_pspew("Skipping: "); for (count = 0; count < writecnt; count++) { - msg_pspew("[%02x]", mmio_readb(sb600_spibar + 0xC)); + msg_pspew("[%02x]", mmio_readb(data->sb600_spibar + 0xC)); } msg_pspew("\n"); - if (compare_internal_fifo_pointer(writecnt)) + if (compare_internal_fifo_pointer(data->sb600_spibar, writecnt)) return SPI_PROGRAMMER_ERROR; msg_pspew("Reading FIFO: "); for (count = 0; count < readcnt; count++) { - readarr[count] = mmio_readb(sb600_spibar + 0xC); + readarr[count] = mmio_readb(data->sb600_spibar + 0xC); msg_pspew("[%02x]", readarr[count]); } msg_pspew("\n"); - if (compare_internal_fifo_pointer(writecnt+readcnt)) + if (compare_internal_fifo_pointer(data->sb600_spibar, writecnt+readcnt)) return SPI_PROGRAMMER_ERROR; - if (mmio_readb(sb600_spibar + 1) != readwrite) { + if (mmio_readb(data->sb600_spibar + 1) != readwrite) { msg_perr("Unexpected change in AMD SPI read/write count!\n"); msg_perr("Something else is accessing the flash chip and causes random corruption.\n" "Please stop all applications and drivers and IPMI which access the flash chip.\n"); @@ -283,33 +284,34 @@ const unsigned char *writearr, unsigned char *readarr) { + struct sb600spi_data * data = (struct sb600spi_data *)flash->mst->spi.data; /* First byte is cmd which can not be sent through the buffer. */ unsigned char cmd = *writearr++; writecnt--; msg_pspew("%s, cmd=0x%02x, writecnt=%d, readcnt=%d\n", __func__, cmd, writecnt, readcnt); - mmio_writeb(cmd, sb600_spibar + 0); + mmio_writeb(cmd, data->sb600_spibar + 0); int ret = check_readwritecnt(flash, writecnt, readcnt); if (ret != 0) return ret; /* Use the extended TxByteCount and RxByteCount registers. */ - mmio_writeb(writecnt, sb600_spibar + 0x48); - mmio_writeb(readcnt, sb600_spibar + 0x4b); + mmio_writeb(writecnt, data->sb600_spibar + 0x48); + mmio_writeb(readcnt, data->sb600_spibar + 0x4b); msg_pspew("Filling buffer: "); unsigned int count; for (count = 0; count < writecnt; count++) { msg_pspew("[%02x]", writearr[count]); - mmio_writeb(writearr[count], sb600_spibar + 0x80 + count); + mmio_writeb(writearr[count], data->sb600_spibar + 0x80 + count); } msg_pspew("\n"); - execute_command(); + execute_command(data->sb600_spibar); msg_pspew("Reading buffer: "); for (count = 0; count < readcnt; count++) { - readarr[count] = mmio_readb(sb600_spibar + 0x80 + (writecnt + count) % FIFO_SIZE_YANGTZE); + readarr[count] = mmio_readb(data->sb600_spibar + 0x80 + (writecnt + count) % FIFO_SIZE_YANGTZE); msg_pspew("[%02x]", readarr[count]); } msg_pspew("\n"); @@ -344,7 +346,8 @@ "Fast Read", }; -static int set_speed(struct pci_dev *dev, enum amd_chipset amd_gen, uint8_t speed) +static int set_speed(struct pci_dev *dev, uint8_t *sb600_spibar, + enum amd_chipset amd_gen, uint8_t speed) { bool success = false; @@ -367,7 +370,7 @@ return 0; } -static int set_mode(struct pci_dev *dev, uint8_t mode) +static int set_mode(struct pci_dev *dev, uint8_t *sb600_spibar, uint8_t mode) { msg_pdbg("Setting SPI read mode to %s (%i)... ", spireadmodes[mode], mode); uint32_t tmp = mmio_readl(sb600_spibar + 0x00); @@ -382,7 +385,7 @@ return 0; } -static int handle_speed(struct pci_dev *dev, enum amd_chipset amd_gen) +static int handle_speed(struct pci_dev *dev, uint8_t *sb600_spibar, enum amd_chipset amd_gen) { uint32_t tmp; int16_t spispeed_idx = -1; @@ -450,7 +453,7 @@ msg_perr("Warning: spireadmode not set, " "leaving spireadmode unchanged."); } - else if (set_mode(dev, spireadmode_idx) != 0) { + else if (set_mode(dev, sb600_spibar, spireadmode_idx) != 0) { return 1; } @@ -493,7 +496,7 @@ msg_perr("Warning: spispeed not set, leaving spispeed unchanged."); return 0; } - return set_speed(dev, amd_gen, spispeed_idx); + return set_speed(dev, sb600_spibar, amd_gen, spispeed_idx); } static int handle_imc(struct pci_dev *dev, enum amd_chipset amd_gen) @@ -604,7 +607,7 @@ return 0; /* Physical memory has to be mapped at page (4k) boundaries. */ - sb600_spibar = rphysmap("SB600 SPI registers", tmp & 0xfffff000, 0x1000); + uint8_t *sb600_spibar = rphysmap("SB600 SPI registers", tmp & 0xfffff000, 0x1000); if (sb600_spibar == ERROR_PTR) return ERROR_FATAL; @@ -747,7 +750,7 @@ return 0; } - if (handle_speed(dev, amd_gen) != 0) + if (handle_speed(dev, sb600_spibar, amd_gen) != 0) return ERROR_FATAL; if (handle_imc(dev, amd_gen) != 0) -- To view, visit https://review.coreboot.org/c/flashrom/+/48064 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2e692b230a77f3902d91d0ad36db0d668b7e9ffa Gerrit-Change-Number: 48064 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

2 6

Change in flashrom[master]: nicrealtek.c: Refactor singleton states into reentrant pattern
by Edward O'Callaghan (Code Review) 01 Jun '21

01 Jun '21

Attention is currently required from: Nico Huber, Angel Pons, Anastasia Klimchuk. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/55108 ) Change subject: nicrealtek.c: Refactor singleton states into reentrant pattern ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/55108 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If0cb0fefb53b2c6bb65a85f4c8dc6f323954dd0c Gerrit-Change-Number: 55108 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-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Tue, 01 Jun 2021 04:23:38 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit June 2021