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

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: 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: 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

Change in flashrom[master]: nic3com.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/+/55106 ) Change subject: nic3com.c: Refactor singleton states into reentrant pattern ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/55106 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I1c3e4836760cc9f4f9a0bd4294e8d2407b150566 Gerrit-Change-Number: 55106 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 03:25:23 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

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+2 -- 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 03:22:34 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

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

01 Jun '21

Anastasia Klimchuk has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/55108 ) Change subject: nicrealtek.c: Refactor singleton states into reentrant pattern ...................................................................... nicrealtek.c: Refactor singleton states into reentrant pattern Move global singleton states into a struct and store within the par_master data field for the life-time of the driver. This is one of the steps on the way to move par_master data memory management behind the initialisation API, for more context see other patches under the same topic "register_master_api". BUG=b:185191942 TEST=builds Change-Id: If0cb0fefb53b2c6bb65a85f4c8dc6f323954dd0c Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M nicrealtek.c 1 file changed, 31 insertions(+), 10 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/08/55108/1 diff --git a/nicrealtek.c b/nicrealtek.c index 788c088..544abab 100644 --- a/nicrealtek.c +++ b/nicrealtek.c @@ -24,8 +24,11 @@ #define PCI_VENDOR_ID_REALTEK 0x10ec #define PCI_VENDOR_ID_SMC1211 0x1113 -static uint32_t io_base_addr = 0; -static int bios_rom_addr, bios_rom_data; +struct nicrealtek_data { + uint32_t io_base_addr; + int bios_rom_addr; + int bios_rom_data; +}; const struct dev_entry nics_realtek[] = { {0x10ec, 0x8139, OK, "Realtek", "RTL8139/8139C/8139C+"}, @@ -37,38 +40,41 @@ static void nicrealtek_chip_writeb(const struct flashctx *flash, uint8_t val, chipaddr addr) { + struct nicrealtek_data *data = flash->mst->par.data; + /* Output addr and data, set WE to 0, set OE to 1, set CS to 0, * enable software access. */ OUTL(((uint32_t)addr & 0x01FFFF) | 0x0A0000 | (val << 24), - io_base_addr + bios_rom_addr); + data->io_base_addr + data->bios_rom_addr); /* Output addr and data, set WE to 1, set OE to 1, set CS to 1, * enable software access. */ OUTL(((uint32_t)addr & 0x01FFFF) | 0x1E0000 | (val << 24), - io_base_addr + bios_rom_addr); + data->io_base_addr + data->bios_rom_addr); } static uint8_t nicrealtek_chip_readb(const struct flashctx *flash, const chipaddr addr) { + struct nicrealtek_data *data = flash->mst->par.data; uint8_t val; /* FIXME: Can we skip reading the old data and simply use 0? */ /* Read old data. */ - val = INB(io_base_addr + bios_rom_data); + val = INB(data->io_base_addr + data->bios_rom_data); /* Output new addr and old data, set WE to 1, set OE to 0, set CS to 0, * enable software access. */ OUTL(((uint32_t)addr & 0x01FFFF) | 0x060000 | (val << 24), - io_base_addr + bios_rom_addr); + data->io_base_addr + data->bios_rom_addr); /* Read new data. */ - val = INB(io_base_addr + bios_rom_data); + val = INB(data->io_base_addr + data->bios_rom_data); /* Output addr and new data, set WE to 1, set OE to 1, set CS to 1, * enable software access. */ OUTL(((uint32_t)addr & 0x01FFFF) | 0x1E0000 | (val << 24), - io_base_addr + bios_rom_addr); + data->io_base_addr + data->bios_rom_addr); return val; } @@ -87,12 +93,16 @@ static int nicrealtek_shutdown(void *data) { /* FIXME: We forgot to disable software access again. */ + free(data); return 0; } int nicrealtek_init(void) { struct pci_dev *dev = NULL; + uint32_t io_base_addr = 0; + int bios_rom_addr; + int bios_rom_data; if (rget_io_perms()) return 1; @@ -119,10 +129,21 @@ break; } - if (register_shutdown(nicrealtek_shutdown, NULL)) + struct nicrealtek_data *data = calloc(1, sizeof(*data)); + if (!data) { + msg_perr("Unable to allocate space for PAR master data\n"); return 1; + } + data->io_base_addr = io_base_addr; + data->bios_rom_addr = bios_rom_addr; + data->bios_rom_data = bios_rom_data; - register_par_master(&par_master_nicrealtek, BUS_PARALLEL, NULL); + if (register_shutdown(nicrealtek_shutdown, data)) { + free(data); + return 1; + } + + register_par_master(&par_master_nicrealtek, BUS_PARALLEL, data); return 0; } -- 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-MessageType: newchange

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit June 2021