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

Change in flashrom[master]: s25f.c: Fix mismatched function definitions
by Angel Pons (Code Review) 23 Apr '21

23 Apr '21

Attention is currently required from: Edward O'Callaghan. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52473 ) Change subject: s25f.c: Fix mismatched function definitions ...................................................................... Patch Set 1: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/52473/comment/f7fb3e79_0732db0b PS1, Line 8: : This was missed because `uint32_t` is `unsigned int` in most cases. : However, it is not the case for DJGPP 6.1.0 for some reason. > WTF? Edward, any updates? -- To view, visit https://review.coreboot.org/c/flashrom/+/52473 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I656a72b85d4c70b57f6ff9268186a4a60933f8a9 Gerrit-Change-Number: 52473 Gerrit-PatchSet: 1 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> 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: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Fri, 23 Apr 2021 15:31:06 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: mec1308.c: Separate shutdown from failed init cleanup
by Nico Huber (Code Review) 23 Apr '21

23 Apr '21

Attention is currently required from: Edward O'Callaghan, Anastasia Klimchuk. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52597 ) Change subject: mec1308.c: Separate shutdown from failed init cleanup ...................................................................... Patch Set 2: Code-Review+1 (2 comments) File mec1308.c: https://review.coreboot.org/c/flashrom/+/52597/comment/563f50bb_3d74365e PS2, Line 496: goto mec1308_init_exit; It's becoming more and more obvious that the original cleanup procedure was insufficient. If we get here, we already disable "ACPI" (whatever that means, I guess SCIs). So we should try to clean that up too, shouldn't we? We could either run the full cleanup, or add more labels (we'd need to revert to PS1, sorry). If you decide for the former, that could be a follow-up commit. https://review.coreboot.org/c/flashrom/+/52597/comment/c5b0b7a9_f7541409 PS2, Line 519: failed_init_cleanup: : mec1308_shutdown(ctx_data); : return 1; : : mec1308_init_exit: Would be nice to align the label names somehow. I first thought "aren't both about failed init?", but there's (almost, see other comment) a difference: In one case we actually changed hardware state. So how about failed_init_cleanup: failed_prep_cleanup: ? -- To view, visit https://review.coreboot.org/c/flashrom/+/52597 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6d62d43dd8b6ebc595f9fd747e0f4cd80f3c10da Gerrit-Change-Number: 52597 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: Victor Ding <victording(a)google.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Fri, 23 Apr 2021 15:09:59 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: mec1308.c: Untangle successful vs failed init paths
by Nico Huber (Code Review) 23 Apr '21

23 Apr '21

Attention is currently required from: Anastasia Klimchuk. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52596 ) Change subject: mec1308.c: Untangle successful vs failed init paths ...................................................................... Patch Set 2: Code-Review+1 (1 comment) File mec1308.c: https://review.coreboot.org/c/flashrom/+/52596/comment/9ad938d2_50d13057 PS1, Line 519: mec1308_init_exit I would probably rename it, e.g. `mec1308_init_err`. It's not "the" exit anymore. Could be done in the follow up, too. -- To view, visit https://review.coreboot.org/c/flashrom/+/52596 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ibf35335501e59636c544af124ad7a04a186790b4 Gerrit-Change-Number: 52596 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: Victor Ding <victording(a)google.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Fri, 23 Apr 2021 14:57:22 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

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

23 Apr '21

Attention is currently required from: Anastasia Klimchuk. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52595 ) Change subject: mec1308.c: Extract params check into a function ...................................................................... Patch Set 2: Code-Review+1 (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/52595/comment/7bc037d5_ba374fc0 PS1, Line 9: This allows char *p to become a local variable in > done (and also for other patches in this chain). It's literally a limit, though. There is no recommendation or even rule to make text columns that wide. Generally, narrower columns are easier to read (with some limit too, ofc, having only one word per line wouldn't help readability). To me, the original looked better. -- To view, visit https://review.coreboot.org/c/flashrom/+/52595 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If5be7709e93233a2e7ea9133de50382d2524a55f Gerrit-Change-Number: 52595 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: Victor Ding <victording(a)google.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Fri, 23 Apr 2021 14:55:42 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes 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]: linux_spi.c: Drop some unnecessary initialisations and checks
by Nico Huber (Code Review) 23 Apr '21

23 Apr '21

Nico Huber has submitted this change. ( https://review.coreboot.org/c/flashrom/+/52492 ) Change subject: linux_spi.c: Drop some unnecessary initialisations and checks ...................................................................... linux_spi.c: Drop some unnecessary initialisations and checks In previous patches 52283, 52284, 52285 there were some unresolved comments left, resolving [and replying to] all of that here. TEST=builds and ninja test from 51487 BUG=b:185191942 Change-Id: I27a718b515fc474f63b3e61be58a6f9302527559 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/52492 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> Reviewed-by: Nico Huber <nico.h(a)gmx.de> --- M linux_spi.c 1 file changed, 7 insertions(+), 17 deletions(-) Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved Angel Pons: Looks good to me, approved diff --git a/linux_spi.c b/linux_spi.c index 541ca65..4668e7f 100644 --- a/linux_spi.c +++ b/linux_spi.c @@ -68,10 +68,8 @@ static int linux_spi_shutdown(void *data) { - struct linux_spi_data *spi_data = (struct linux_spi_data *) data; - if (spi_data->fd != -1) - close(spi_data->fd); - + struct linux_spi_data *spi_data = data; + close(spi_data->fd); free(spi_data); return 0; } @@ -127,7 +125,8 @@ }; /* Read max buffer size from sysfs, or use page size as fallback. */ -static size_t get_max_kernel_buf_size() { +static size_t get_max_kernel_buf_size() +{ size_t result = 0; FILE *fp; fp = fopen(BUF_SIZE_FROM_SYSFS, "r"); @@ -174,10 +173,9 @@ /* SPI mode 0 (beware this also includes: MSB first, CS active low and others */ const uint8_t mode = SPI_MODE_0; const uint8_t bits = 8; - int fd = -1; + int fd; size_t max_kernel_buf_size; struct linux_spi_data *spi_data; - int ret = 0; p = extract_programmer_param("spispeed"); if (p && strlen(p)) { @@ -214,7 +212,6 @@ if (ioctl(fd, SPI_IOC_WR_MAX_SPEED_HZ, &speed_hz) == -1) { msg_perr("%s: failed to set speed to %"PRIu32"Hz: %s\n", __func__, speed_hz, strerror(errno)); - ret = 1; goto init_err; } msg_pdbg("Using %"PRIu32"kHz clock\n", speed_hz / 1000); @@ -222,14 +219,12 @@ if (ioctl(fd, SPI_IOC_WR_MODE, &mode) == -1) { msg_perr("%s: failed to set SPI mode to 0x%02x: %s\n", __func__, mode, strerror(errno)); - ret = 1; goto init_err; } if (ioctl(fd, SPI_IOC_WR_BITS_PER_WORD, &bits) == -1) { msg_perr("%s: failed to set the number of bits per SPI word to %u: %s\n", __func__, bits == 0 ? 8 : bits, strerror(errno)); - ret = 1; goto init_err; } @@ -239,7 +234,6 @@ spi_data = calloc(1, sizeof(*spi_data)); if (!spi_data) { msg_perr("Unable to allocated space for SPI master data\n"); - ret = SPI_GENERIC_ERROR; goto init_err; } spi_data->fd = fd; @@ -248,18 +242,14 @@ if (register_shutdown(linux_spi_shutdown, spi_data)) { free(spi_data); - ret = 1; goto init_err; } register_spi_master(&spi_master_linux); return 0; init_err: - if (fd != -1) { - close(fd); - fd = -1; - } - return ret; + close(fd); + return 1; } #endif // CONFIG_LINUX_SPI == 1 -- To view, visit https://review.coreboot.org/c/flashrom/+/52492 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I27a718b515fc474f63b3e61be58a6f9302527559 Gerrit-Change-Number: 52492 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: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Sam McNally <sammc(a)google.com> Gerrit-MessageType: merged

1 0

Change in flashrom[master]: linux_spi.c: Drop some unnecessary initialisations and checks
by Nico Huber (Code Review) 23 Apr '21

23 Apr '21

Attention is currently required from: Edward O'Callaghan, Anastasia Klimchuk. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52492 ) Change subject: linux_spi.c: Drop some unnecessary initialisations and checks ...................................................................... Patch Set 2: Code-Review+2 (1 comment) Patchset: PS2: Small hint: If you change the commit message without rebasing, the review scores will be maintained. -- To view, visit https://review.coreboot.org/c/flashrom/+/52492 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I27a718b515fc474f63b3e61be58a6f9302527559 Gerrit-Change-Number: 52492 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: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Sam McNally <sammc(a)google.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Fri, 23 Apr 2021 13:18:26 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ft2232_spi.c - Pack read cmd and nCS in one ftdi_write_data() call
by Simon Buhrow (Code Review) 23 Apr '21

23 Apr '21

Simon Buhrow has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/40748 ) Change subject: ft2232_spi.c - Pack read cmd and nCS in one ftdi_write_data() call ...................................................................... ft2232_spi.c - Pack read cmd and nCS in one ftdi_write_data() call Every ftdi_write_data() call is quite time consuming as the ftdi-chip takes 2-3ms to respond. As the comment already says: Minimize USB transfers by packing as many commands as possible together. As I cannot see any reason to put read cmd and deassertion of nCS into seperated ftdi calls, I packed them together to save programming time. Signed-off-by: Simon Buhrow <simon.buhrow(a)posteo.de> Change-Id: I3f76da3dd0124a0deec7f937dbc4b7dc488c8004 --- M ft2232_spi.c 1 file changed, 38 insertions(+), 40 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/48/40748/1 diff --git a/ft2232_spi.c b/ft2232_spi.c index 9f4c7f0..b2f5e40 100644 --- a/ft2232_spi.c +++ b/ft2232_spi.c @@ -398,7 +398,10 @@ msg_perr("Unable to set latency timer (%s).\n", ftdi_get_error_string(ftdic)); } - if (ftdi_write_data_set_chunksize(ftdic, 270)) { + if (ftdi_write_data_set_chunksize(ftdic, 280)) { + /* 280 Byte = (9 Byte CMD + 1 Byte WREN) + (9 Byte CMD + 1 Byte op) + 4 Byte Addr + * + 256 Byte PageWrite-data + with op: PageProgram or Erase; CMD: FTDI-Chip commands*/ msg_perr("Unable to set chunk size (%s).\n", ftdi_get_error_string(ftdic)); } @@ -467,31 +470,26 @@ struct ftdi_context *ftdic = &ftdic_context; static unsigned char *buf = NULL; /* failed is special. We use bitwise ops, but it is essentially bool. */ - int i = 0, ret = 0, failed = 0; - size_t bufsize; - static size_t oldbufsize = 0; + static int i = 0; + int ret = 0, failed = 0; if (writecnt > 65536 || readcnt > 65536) return SPI_INVALID_LENGTH; - /* buf is not used for the response from the chip. */ - bufsize = max(writecnt + 9, 260 + 9); - /* Never shrink. realloc() calls are expensive. */ - if (!buf || bufsize > oldbufsize) { - buf = realloc(buf, bufsize); + if (!buf) { + /* set buf size to hardware buffer size of ftdi*/ + buf = realloc(buf, 4096); if (!buf) { msg_perr("Out of memory!\n"); /* TODO: What to do with buf? */ return SPI_GENERIC_ERROR; } - oldbufsize = bufsize; } /* * Minimize USB transfers by packing as many commands as possible - * together. If we're not expecting to read, we can assert CS#, write, - * and deassert CS# all in one shot. If reading, we do three separate - * operations. + * together. We can assert CS#, write, (read,) + * and deassert CS# all in one shot. */ msg_pspew("Assert CS#\n"); buf[i++] = SET_BITS_LOW; @@ -506,44 +504,44 @@ i += writecnt; } - /* - * Optionally terminate this batch of commands with a - * read command, then do the fetch of the results. - */ + /* A optionally read command */ if (readcnt) { buf[i++] = MPSSE_DO_READ; buf[i++] = (readcnt - 1) & 0xff; buf[i++] = ((readcnt - 1) >> 8) & 0xff; - ret = send_buf(ftdic, buf, i); - failed = ret; - /* We can't abort here, we still have to deassert CS#. */ - if (ret) - msg_perr("send_buf failed before read: %i\n", ret); - i = 0; - if (ret == 0) { - /* - * FIXME: This is unreliable. There's no guarantee that - * we read the response directly after sending the read - * command. We may be scheduled out etc. - */ - ret = get_buf(ftdic, readarr, readcnt); - failed |= ret; - /* We can't abort here either. */ - if (ret) - msg_perr("get_buf failed: %i\n", ret); - } } msg_pspew("De-assert CS#\n"); buf[i++] = SET_BITS_LOW; buf[i++] = cs_bits; buf[i++] = pindir; - ret = send_buf(ftdic, buf, i); - failed |= ret; - if (ret) - msg_perr("send_buf failed at end: %i\n", ret); - return failed ? -1 : 0; + if (writearr[0] == JEDEC_WREN) { + /* Return to get second op (Program or Erase) without resetting buf nor i*/ + return 0; + } else { + ret = send_buf(ftdic, buf, i); + failed |= ret; + if (ret) + msg_perr("send_buf failed: %i\n", ret); + i = 0; + + if (readcnt) { + if (ret == 0) { + /* + * FIXME: This is unreliable. There's no guarantee that + * we read the response directly after sending the read + * command. We may be scheduled out etc. + */ + ret = get_buf(ftdic, readarr, readcnt); + failed |= ret; + /* We can't abort here either. */ + if (ret) + msg_perr("get_buf failed: %i\n", ret); + } + } + return failed ? -1 : 0; + } } #endif -- To view, visit https://review.coreboot.org/c/flashrom/+/40748 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3f76da3dd0124a0deec7f937dbc4b7dc488c8004 Gerrit-Change-Number: 40748 Gerrit-PatchSet: 1 Gerrit-Owner: Simon Buhrow Gerrit-MessageType: newchange

3 10

Change in flashrom[master]: ft2232_spi.c: Implement spi_send_multicommand()
by Simon Buhrow (Code Review) 23 Apr '21

23 Apr '21

Attention is currently required from: Alan Green, Nico Huber, Edward O'Callaghan, Anastasia Klimchuk, Samir Ibradžić. Simon Buhrow has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/40477 ) Change subject: ft2232_spi.c: Implement spi_send_multicommand() ...................................................................... Patch Set 29: (7 comments) Patchset: PS29: PS 29 has not been HW tested yet. File ft2232_spi.c: https://review.coreboot.org/c/flashrom/+/40477/comment/98247231_518a3413 PS25, Line 189: ft2232_spi_send_command > As mentioned earlier, this is already implemented: default_spi_send_command(). […] Ack https://review.coreboot.org/c/flashrom/+/40477/comment/1fefe787_68aaa8f4 PS25, Line 295: 9 > oh yes... I made a mistake, I didn't notice continue statement. […] Good idea Nico! It makes the code more readable! Thanks! https://review.coreboot.org/c/flashrom/+/40477/comment/1676189d_2de0812d PS25, Line 317: if (!cmds->readcnt && ((cmds + 1)->writecnt || (cmds + 1)->readcnt)){ > I think we should check here also if the next command still fits. Might […] Checking if next command fits is a very good idea! I did as you proposed. However I´m not sure if the '12' is too far from it´s origin if it´s in an extra function. But this is more a question of taste I think. More important: I think we need to reset 'i' after send_buf in case the next command does not fit into buffer? https://review.coreboot.org/c/flashrom/+/40477/comment/4fa462c3_1727eafa PS25, Line 343: msg_perr("send_buf failed: %i\n", ret); > If `ret` is non-0, we'd skip the next block, and the loop condition […] Done. Very good overview Nico! https://review.coreboot.org/c/flashrom/+/40477/comment/c7fd8af8_93455fd6 PS25, Line 348: * FIXME: This is unreliable. There's no guarantee that : * we read the response directly after sending the read : * command. We may be scheduled out etc. > Looks like you lost a space before each asterisk. […] Done https://review.coreboot.org/c/flashrom/+/40477/comment/39a45309_9e9d0786 PS25, Line 356: msg_perr("get_buf failed: %i\n", ret); > Sorry, forgot this one, we'd have to `break;` here too on error. Also […] Done -- 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: 29 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: Samir Ibradžić <sibradzic(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Patrick Georgi <pgeorgi(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Alan Green <avg(a)google.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Samir Ibradžić <sibradzic(a)gmail.com> Gerrit-Comment-Date: Fri, 23 Apr 2021 11:31:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Alan Green <avg(a)google.com> Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ft2232_spi.c: Implement spi_send_multicommand()
by Simon Buhrow (Code Review) 23 Apr '21

23 Apr '21

Attention is currently required from: Simon Buhrow, Alan Green, Edward O'Callaghan, Samir Ibradžić. Hello build bot (Jenkins), Alan Green, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, Samir Ibradžić, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/40477 to look at the new patch set (#29). Change subject: ft2232_spi.c: Implement spi_send_multicommand() ...................................................................... ft2232_spi.c: Implement spi_send_multicommand() Every ftdi_write_data() call is quite time consuming as the ftdi-chips seems to take always 2-3ms to respond. This leads to what the comment already says: Minimize USB transfers by packing as many commands as possible together. So I packed the WREN command together with the following operation which can be program or erase operation. This saves about 1 minute when programming a 128MBit Flash with my config! Signed-off-by: Simon Buhrow <simon.buhrow(a)posteo.de> Change-Id: Ie4a07499ec5ef0af23818593f45dc427285a9e8a --- M ft2232_spi.c 1 file changed, 87 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/77/40477/29 -- 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: 29 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: Samir Ibradžić <sibradzic(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> 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-MessageType: newpatchset

1 0

Change in flashrom[master]: tests: Add unit test to run init/shutdown for linux_spi.c
by Angel Pons (Code Review) 23 Apr '21

23 Apr '21

Attention is currently required from: Nico Huber, Anastasia Klimchuk. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52498 ) Change subject: tests: Add unit test to run init/shutdown for linux_spi.c ...................................................................... Patch Set 4: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/52498 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I4911fbb6f04371283f0e62d2196bdd691a227584 Gerrit-Change-Number: 52498 Gerrit-PatchSet: 4 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: Fri, 23 Apr 2021 08:52:50 +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