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

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

24 May '21

Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52958 ) Change subject: buspirate_spi.c: Refactor singleton states into reentrant pattern ...................................................................... Patch Set 3: (2 comments) File buspirate_spi.c: https://review.coreboot.org/c/flashrom/+/52958/comment/bf5dbd9e_23229ca3 PS2, Line 54: bp_ > The `bp_` prefix seems unnecessary now. Yes sure, I usually do this as separate patch - so now there is CB:54886 on the top of the chain. https://review.coreboot.org/c/flashrom/+/52958/comment/428cd28f_28fc3f97 PS2, Line 167: struct buspirate_spi_data *buspirate_data = data; > Thank you so much for detailed explanation, and I am really sorry for delay from my side! During las […] After some more thinking, I did this: I really wanted to keep as much optimisations from the Author as possible, but also reduce noise and diffs because that was the suggestion which started this thread. So bp_data is still spi_master data (keeps optimisations), but I use a shortcut local variable in most of functions (reduce noise and diffs). -- To view, visit https://review.coreboot.org/c/flashrom/+/52958 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I418bbfff15fb126b042fbc9be09dbf59f4d243b8 Gerrit-Change-Number: 52958 Gerrit-PatchSet: 3 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: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Mon, 24 May 2021 22:29:43 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: buspirate_spi.c: Drop bp_ prefix for spi data struct members
by Anastasia Klimchuk (Code Review) 24 May '21

24 May '21

Anastasia Klimchuk has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/54886 ) Change subject: buspirate_spi.c: Drop bp_ prefix for spi data struct members ...................................................................... buspirate_spi.c: Drop bp_ prefix for spi data struct members The name of the struct type already contains bp_ prefix, so prefix doesn't need to be repeated in members name. BUG=b:185191942 TEST=builds Change-Id: Ia463e3f58497e4b10be072bdd29244f4868098e7 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M buspirate_spi.c 1 file changed, 15 insertions(+), 15 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/86/54886/1 diff --git a/buspirate_spi.c b/buspirate_spi.c index c201d38..440d12d 100644 --- a/buspirate_spi.c +++ b/buspirate_spi.c @@ -51,8 +51,8 @@ #endif struct bp_spi_data { - unsigned char *bp_commbuf; - int bp_commbufsize; + unsigned char *commbuf; + int commbufsize; }; static int buspirate_commbuf_grow(int bufsize, unsigned char **bp_commbuf, int *bp_commbufsize) @@ -165,7 +165,7 @@ static int buspirate_spi_shutdown(void *data) { struct bp_spi_data *bp_data = data; - unsigned char *const bp_commbuf = bp_data->bp_commbuf; + unsigned char *const bp_commbuf = bp_data->commbuf; int ret = 0, ret2 = 0; /* No need to allocate a buffer here, we know that bp_commbuf is at least DEFAULT_BUFSIZE big. */ @@ -215,10 +215,10 @@ return SPI_INVALID_LENGTH; /* 3 bytes extra for CS#, len, CS#. */ - if (buspirate_commbuf_grow(writecnt + readcnt + 3, &bp_data->bp_commbuf, &bp_data->bp_commbufsize)) + if (buspirate_commbuf_grow(writecnt + readcnt + 3, &bp_data->commbuf, &bp_data->commbufsize)) return ERROR_OOM; - unsigned char *const bp_commbuf = bp_data->bp_commbuf; + unsigned char *const bp_commbuf = bp_data->commbuf; /* Assert CS# */ bp_commbuf[i++] = 0x02; @@ -272,10 +272,10 @@ /* 5 bytes extra for command, writelen, readlen. * 1 byte extra for Ack/Nack. */ - if (buspirate_commbuf_grow(max(writecnt + 5, readcnt + 1), &bp_data->bp_commbuf, &bp_data->bp_commbufsize)) + if (buspirate_commbuf_grow(max(writecnt + 5, readcnt + 1), &bp_data->commbuf, &bp_data->commbufsize)) return ERROR_OOM; - unsigned char *const bp_commbuf = bp_data->bp_commbuf; + unsigned char *const bp_commbuf = bp_data->commbuf; /* Combined SPI write/read. */ bp_commbuf[i++] = 0x04; @@ -401,8 +401,8 @@ free(bp_commbuf); return 1; } - bp_data->bp_commbuf = bp_commbuf; - bp_data->bp_commbufsize = bp_commbufsize; + bp_data->commbuf = bp_commbuf; + bp_data->commbufsize = bp_commbufsize; /* This is the brute force version, but it should work. * It is likely to fail if a previous flashrom run was aborted during a write with the new SPI commands @@ -504,8 +504,8 @@ ret = ERROR_OOM; goto init_err_cleanup_exit; } - bp_data->bp_commbuf = bp_commbuf; - bp_data->bp_commbufsize = bp_commbufsize; + bp_data->commbuf = bp_commbuf; + bp_data->commbufsize = bp_commbufsize; spi_master_buspirate.max_data_read = 2048; spi_master_buspirate.max_data_write = 256; spi_master_buspirate.command = buspirate_spi_send_command_v2; @@ -518,8 +518,8 @@ ret = ERROR_OOM; goto init_err_cleanup_exit; } - bp_data->bp_commbuf = bp_commbuf; - bp_data->bp_commbufsize = bp_commbufsize; + bp_data->commbuf = bp_commbuf; + bp_data->commbufsize = bp_commbufsize; spi_master_buspirate.max_data_read = 12; spi_master_buspirate.max_data_write = 12; spi_master_buspirate.command = buspirate_spi_send_command_v1; @@ -693,8 +693,8 @@ } /* Storing the latest values into data, commbuf migth have grown during init. */ - bp_data->bp_commbuf = bp_commbuf; - bp_data->bp_commbufsize = bp_commbufsize; + bp_data->commbuf = bp_commbuf; + bp_data->commbufsize = bp_commbufsize; if (register_shutdown(buspirate_spi_shutdown, bp_data) != 0) { ret = 1; goto init_err_cleanup_exit; -- To view, visit https://review.coreboot.org/c/flashrom/+/54886 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia463e3f58497e4b10be072bdd29244f4868098e7 Gerrit-Change-Number: 54886 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newchange

1 0

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

24 May '21

Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons. Hello build bot (Jenkins), Nico Huber, Edward O'Callaghan, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/52958 to look at the new patch set (#3). Change subject: buspirate_spi.c: Refactor singleton states into reentrant pattern ...................................................................... buspirate_spi.c: Refactor singleton states into reentrant pattern Move global singleton states into a struct and store within the spi_master data field for the life-time of the driver. This is one of the steps on the way to move spi_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: I418bbfff15fb126b042fbc9be09dbf59f4d243b8 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M buspirate_spi.c 1 file changed, 45 insertions(+), 19 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/58/52958/3 -- To view, visit https://review.coreboot.org/c/flashrom/+/52958 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I418bbfff15fb126b042fbc9be09dbf59f4d243b8 Gerrit-Change-Number: 52958 Gerrit-PatchSet: 3 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: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: buspirate_spi.c: Separate shutdown from failed init cleanup
by Anastasia Klimchuk (Code Review) 24 May '21

24 May '21

Attention is currently required from: Angel Pons, Anastasia Klimchuk. Hello build bot (Jenkins), Nico Huber, Edward O'Callaghan, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/52877 to look at the new patch set (#2). Change subject: buspirate_spi.c: Separate shutdown from failed init cleanup ...................................................................... buspirate_spi.c: Separate shutdown from failed init cleanup Shutdown function was covering two different jobs here: 1) the actual shutdown which is run at the end of the driver's lifecycle and 2) cleanup in cases when initialisation failed. Now, shutdown is only doing its main job (#1), and the driver itself is doing cleanup when init fails (#2). The good thing is that now resources are released/closed immediately in cases when init fails (vs shutdown function which was run at some point later), and the driver leaves clean space after itself if init fails. And very importantly this unlocks API change which plans to move register_shutdown inside register master API, see https://review.coreboot.org/c/flashrom/+/51761 BUG=b:185191942 TEST=builds Change-Id: I04d57e2552d23a1a4a906c68539f0ccefc13e8a0 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M buspirate_spi.c 1 file changed, 71 insertions(+), 49 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/77/52877/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/52877 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I04d57e2552d23a1a4a906c68539f0ccefc13e8a0 Gerrit-Change-Number: 52877 Gerrit-PatchSet: 2 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: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: par_masters: Reshuffle to remove forward declarations
by Anastasia Klimchuk (Code Review) 24 May '21

24 May '21

Attention is currently required from: Edward O'Callaghan, Angel Pons. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/54873 ) Change subject: par_masters: Reshuffle to remove forward declarations ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/54873 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I88e89992380195fee7c9de7ec57502ab980ec5df Gerrit-Change-Number: 54873 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Mon, 24 May 2021 20:43:17 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: dummyflasher.c: Move 'flashchip_contents' into emu_data
by Edward O'Callaghan (Code Review) 24 May '21

24 May '21

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/54722 ) Change subject: dummyflasher.c: Move 'flashchip_contents' into emu_data ...................................................................... dummyflasher.c: Move 'flashchip_contents' into emu_data Move 'flashchip_contents' out of global scope and into the emu_data reentrent struct. BUG=none BRANCH=none TEST=builds Change-Id: I11dfe713dd2fecfd3981ab50e31c9215d00bc787 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/54722 Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M dummyflasher.c 1 file changed, 20 insertions(+), 20 deletions(-) Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved Anastasia Klimchuk: Looks good to me, but someone else must approve diff --git a/dummyflasher.c b/dummyflasher.c index 2657efb..2f39c98 100644 --- a/dummyflasher.c +++ b/dummyflasher.c @@ -37,7 +37,6 @@ #endif #if EMULATE_CHIP -static uint8_t *flashchip_contents = NULL; enum emu_chip { EMULATE_NONE, EMULATE_ST_M25P10_RES, @@ -71,6 +70,7 @@ unsigned int spi_ignorelist_size; unsigned int spi_write_256_chunksize; + uint8_t *flashchip_contents; }; #if EMULATE_SPI_CHIP @@ -353,14 +353,14 @@ /* Truncate to emu_chip_size. */ offs %= data->emu_chip_size; if (readcnt > 0) - memcpy(readarr, flashchip_contents + offs, readcnt); + memcpy(readarr, data->flashchip_contents + offs, readcnt); break; case JEDEC_READ_4BA: offs = writearr[1] << 24 | writearr[2] << 16 | writearr[3] << 8 | writearr[4]; /* Truncate to emu_chip_size. */ offs %= data->emu_chip_size; if (readcnt > 0) - memcpy(readarr, flashchip_contents + offs, readcnt); + memcpy(readarr, data->flashchip_contents + offs, readcnt); break; case JEDEC_BYTE_PROGRAM: offs = writearr[1] << 16 | writearr[2] << 8 | writearr[3]; @@ -374,7 +374,7 @@ msg_perr("Max BYTE PROGRAM size exceeded!\n"); return 1; } - memcpy(flashchip_contents + offs, writearr + 4, writecnt - 4); + memcpy(data->flashchip_contents + offs, writearr + 4, writecnt - 4); data->emu_modified = 1; break; case JEDEC_BYTE_PROGRAM_4BA: @@ -389,7 +389,7 @@ msg_perr("Max BYTE PROGRAM size exceeded!\n"); return 1; } - memcpy(flashchip_contents + offs, writearr + 5, writecnt - 5); + memcpy(data->flashchip_contents + offs, writearr + 5, writecnt - 5); data->emu_modified = 1; break; case JEDEC_AAI_WORD_PROGRAM: @@ -411,7 +411,7 @@ writearr[3]; /* Truncate to emu_chip_size. */ aai_offs %= data->emu_chip_size; - memcpy(flashchip_contents + aai_offs, writearr + 4, 2); + memcpy(data->flashchip_contents + aai_offs, writearr + 4, 2); aai_offs += 2; } else { if (writecnt < JEDEC_AAI_WORD_PROGRAM_CONT_OUTSIZE) { @@ -424,7 +424,7 @@ "too long!\n"); return 1; } - memcpy(flashchip_contents + aai_offs, writearr + 1, 2); + memcpy(data->flashchip_contents + aai_offs, writearr + 1, 2); aai_offs += 2; } data->emu_modified = 1; @@ -448,7 +448,7 @@ if (offs & (data->emu_jedec_se_size - 1)) msg_pdbg("Unaligned SECTOR ERASE 0x20: 0x%x\n", offs); offs &= ~(data->emu_jedec_se_size - 1); - memset(flashchip_contents + offs, 0xff, data->emu_jedec_se_size); + memset(data->flashchip_contents + offs, 0xff, data->emu_jedec_se_size); data->emu_modified = 1; break; case JEDEC_BE_52: @@ -466,7 +466,7 @@ if (offs & (data->emu_jedec_be_52_size - 1)) msg_pdbg("Unaligned BLOCK ERASE 0x52: 0x%x\n", offs); offs &= ~(data->emu_jedec_be_52_size - 1); - memset(flashchip_contents + offs, 0xff, data->emu_jedec_be_52_size); + memset(data->flashchip_contents + offs, 0xff, data->emu_jedec_be_52_size); data->emu_modified = 1; break; case JEDEC_BE_D8: @@ -484,7 +484,7 @@ if (offs & (data->emu_jedec_be_d8_size - 1)) msg_pdbg("Unaligned BLOCK ERASE 0xd8: 0x%x\n", offs); offs &= ~(data->emu_jedec_be_d8_size - 1); - memset(flashchip_contents + offs, 0xff, data->emu_jedec_be_d8_size); + memset(data->flashchip_contents + offs, 0xff, data->emu_jedec_be_d8_size); data->emu_modified = 1; break; case JEDEC_CE_60: @@ -500,7 +500,7 @@ } /* JEDEC_CE_60_OUTSIZE is 1 (no address) -> no offset. */ /* emu_jedec_ce_60_size is emu_chip_size. */ - memset(flashchip_contents, 0xff, data->emu_jedec_ce_60_size); + memset(data->flashchip_contents, 0xff, data->emu_jedec_ce_60_size); data->emu_modified = 1; break; case JEDEC_CE_C7: @@ -516,7 +516,7 @@ } /* JEDEC_CE_C7_OUTSIZE is 1 (no address) -> no offset. */ /* emu_jedec_ce_c7_size is emu_chip_size. */ - memset(flashchip_contents, 0xff, data->emu_jedec_ce_c7_size); + memset(data->flashchip_contents, 0xff, data->emu_jedec_ce_c7_size); data->emu_modified = 1; break; case JEDEC_SFDP: @@ -643,13 +643,13 @@ if (emu_data->emu_chip != EMULATE_NONE) { if (emu_data->emu_persistent_image && emu_data->emu_modified) { msg_pdbg("Writing %s\n", emu_data->emu_persistent_image); - write_buf_to_file(flashchip_contents, + write_buf_to_file(emu_data->flashchip_contents, emu_data->emu_chip_size, emu_data->emu_persistent_image); free(emu_data->emu_persistent_image); emu_data->emu_persistent_image = NULL; } - free(flashchip_contents); + free(emu_data->flashchip_contents); } #endif free(data); @@ -968,8 +968,8 @@ } free(tmp); - flashchip_contents = malloc(data->emu_chip_size); - if (!flashchip_contents) { + data->flashchip_contents = malloc(data->emu_chip_size); + if (!data->flashchip_contents) { msg_perr("Out of memory!\n"); return 1; } @@ -993,7 +993,7 @@ msg_pdbg("Filling fake flash chip with 0x%02x, size %i\n", data->erase_to_zero ? 0x00 : 0xff, data->emu_chip_size); - memset(flashchip_contents, data->erase_to_zero ? 0x00 : 0xff, data->emu_chip_size); + memset(data->flashchip_contents, data->erase_to_zero ? 0x00 : 0xff, data->emu_chip_size); /* Will be freed by shutdown function if necessary. */ data->emu_persistent_image = extract_programmer_param("image"); @@ -1009,10 +1009,10 @@ if ((uintmax_t)image_stat.st_size == data->emu_chip_size) { msg_pdbg("matches.\n"); msg_pdbg("Reading %s\n", data->emu_persistent_image); - if (read_buf_from_file(flashchip_contents, data->emu_chip_size, + if (read_buf_from_file(data->flashchip_contents, data->emu_chip_size, data->emu_persistent_image)) { msg_perr("Unable to read %s\n", data->emu_persistent_image); - free(flashchip_contents); + free(data->flashchip_contents); return 1; } } else { @@ -1023,7 +1023,7 @@ dummy_init_out: if (register_shutdown(dummy_shutdown, data)) { - free(flashchip_contents); + free(data->flashchip_contents); free(data); return 1; } -- To view, visit https://review.coreboot.org/c/flashrom/+/54722 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I11dfe713dd2fecfd3981ab50e31c9215d00bc787 Gerrit-Change-Number: 54722 Gerrit-PatchSet: 3 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> 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: Namyoon Woo <namyoon(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

Change in flashrom[master]: dummyflasher.c: Move 'spi_write_256_chunksize' into emu_data
by Edward O'Callaghan (Code Review) 24 May '21

24 May '21

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/54721 ) Change subject: dummyflasher.c: Move 'spi_write_256_chunksize' into emu_data ...................................................................... dummyflasher.c: Move 'spi_write_256_chunksize' into emu_data Move 'spi_write_256_chunksize' out of global scope and into the emu_data reentrent struct. BUG=none BRANCH=none TEST=builds Change-Id: I633f4df4bd47e661cd69801f21910b667899d505 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/54721 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> --- M dummyflasher.c 1 file changed, 17 insertions(+), 14 deletions(-) Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved diff --git a/dummyflasher.c b/dummyflasher.c index cd46f2c..2657efb 100644 --- a/dummyflasher.c +++ b/dummyflasher.c @@ -69,6 +69,8 @@ unsigned char spi_ignorelist[256]; unsigned int spi_blacklist_size; unsigned int spi_ignorelist_size; + + unsigned int spi_write_256_chunksize; }; #if EMULATE_SPI_CHIP @@ -101,9 +103,18 @@ #endif #endif -static unsigned int spi_write_256_chunksize = 256; static enum chipbustype dummy_buses_supported = BUS_NONE; +static struct emu_data* get_data_from_context(const struct flashctx *flash) +{ + if (dummy_buses_supported & BUS_NONSPI) + return (struct emu_data *)flash->mst->par.data; + else if (dummy_buses_supported & BUS_SPI) + return (struct emu_data *)flash->mst->spi.data; + + return NULL; /* buses was set to BUS_NONE. */ +} + void *dummy_map(const char *descr, uintptr_t phys_addr, size_t len) { msg_pspew("%s: Mapping %s, 0x%zx bytes at 0x%0*" PRIxPTR "\n", @@ -118,8 +129,9 @@ static int dummy_spi_write_256(struct flashctx *flash, const uint8_t *buf, unsigned int start, unsigned int len) { + struct emu_data *emu_data = get_data_from_context(flash); return spi_write_chunked(flash, buf, start, len, - spi_write_256_chunksize); + emu_data->spi_write_256_chunksize); } static void dummy_chip_writeb(const struct flashctx *flash, uint8_t val, chipaddr addr) @@ -173,16 +185,6 @@ return; } -static struct emu_data* get_data_from_context(const struct flashctx *flash) -{ - if (dummy_buses_supported & BUS_NONSPI) - return (struct emu_data *)flash->mst->par.data; - else if (dummy_buses_supported & BUS_SPI) - return (struct emu_data *)flash->mst->spi.data; - - return NULL; /* buses was set to BUS_NONE. */ -} - #if EMULATE_SPI_CHIP static int emulate_spi_chip_response(unsigned int writecnt, unsigned int readcnt, @@ -674,6 +676,7 @@ } data->emu_chip = EMULATE_NONE; data->delay_us = 0; + data->spi_write_256_chunksize = 256; msg_pspew("%s\n", __func__); @@ -707,9 +710,9 @@ tmp = extract_programmer_param("spi_write_256_chunksize"); if (tmp) { - spi_write_256_chunksize = atoi(tmp); + data->spi_write_256_chunksize = atoi(tmp); free(tmp); - if (spi_write_256_chunksize < 1) { + if (data->spi_write_256_chunksize < 1) { msg_perr("invalid spi_write_256_chunksize\n"); return 1; } -- To view, visit https://review.coreboot.org/c/flashrom/+/54721 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I633f4df4bd47e661cd69801f21910b667899d505 Gerrit-Change-Number: 54721 Gerrit-PatchSet: 3 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> 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: Namyoon Woo <namyoon(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

Change in flashrom[master]: dummyflasher.c: Make entry struct names consistent
by Edward O'Callaghan (Code Review) 24 May '21

24 May '21

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/54720 ) Change subject: dummyflasher.c: Make entry struct names consistent ...................................................................... dummyflasher.c: Make entry struct names consistent Just make spi_master_dummyflasher and par_master_dummyflasher identifiers consistently named for easier parsing. BUG=none BRANCH=none TEST=builds Change-Id: Ib8ade96c47b4e4d358ba1fe34d06f22de326c60a Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/54720 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> --- M dummyflasher.c 1 file changed, 2 insertions(+), 2 deletions(-) Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved diff --git a/dummyflasher.c b/dummyflasher.c index d549607..cd46f2c 100644 --- a/dummyflasher.c +++ b/dummyflasher.c @@ -622,7 +622,7 @@ .write_aai = default_spi_write_aai, }; -static const struct par_master par_master_dummy = { +static const struct par_master par_master_dummyflasher = { .chip_readb = dummy_chip_readb, .chip_readw = dummy_chip_readw, .chip_readl = dummy_chip_readl, @@ -1025,7 +1025,7 @@ return 1; } if (dummy_buses_supported & BUS_NONSPI) - register_par_master(&par_master_dummy, + register_par_master(&par_master_dummyflasher, dummy_buses_supported & BUS_NONSPI, data); if (dummy_buses_supported & BUS_SPI) -- To view, visit https://review.coreboot.org/c/flashrom/+/54720 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ib8ade96c47b4e4d358ba1fe34d06f22de326c60a Gerrit-Change-Number: 54720 Gerrit-PatchSet: 3 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> 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: Namyoon Woo <namyoon(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

Change in flashrom[master]: serprog.c: Separate shutdown from failed init cleanup
by Edward O'Callaghan (Code Review) 24 May '21

24 May '21

Attention is currently required from: Nico Huber, Anastasia Klimchuk. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/54861 ) Change subject: serprog.c: Separate shutdown from failed init cleanup ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/54861 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Idf4ed62c19667e18cc807913180c48cb8c978805 Gerrit-Change-Number: 54861 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: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Mon, 24 May 2021 10:43:16 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: serprog.c: Use braces in both branches of conditional statement
by Edward O'Callaghan (Code Review) 24 May '21

24 May '21

Attention is currently required from: Nico Huber, Anastasia Klimchuk. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/54862 ) Change subject: serprog.c: Use braces in both branches of conditional statement ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/54862 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I4fdccbd66f0351af98811bf7c2d8e15f0a99d852 Gerrit-Change-Number: 54862 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: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Mon, 24 May 2021 10:42:42 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit May 2021