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

Change in flashrom[master]: ft2232_spi.c: Implement spi_send_multicommand()
by Nico Huber (Code Review) 20 Jun '21

20 Jun '21

Attention is currently required from: Simon Buhrow, Edward O'Callaghan, Samir Ibradžić. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/40477 ) Change subject: ft2232_spi.c: Implement spi_send_multicommand() ...................................................................... Patch Set 34: (1 comment) File ft2232_spi.c: https://review.coreboot.org/c/flashrom/+/40477/comment/40a9dbbf_acc3ed89 PS34, Line 82: I guess this space is not intentional. In any case, it should align with the surrounding style. -- 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: 34 Gerrit-Owner: Simon Buhrow Gerrit-Reviewer: Alan Green <avg(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Samir Ibradžić <sibradzic(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Patrick Georgi <pgeorgi(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Simon Buhrow Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Samir Ibradžić <sibradzic(a)gmail.com> Gerrit-Comment-Date: Sun, 20 Jun 2021 13:52:04 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: hwaccess.c: Fix PowerPC memory barrier
by Angel Pons (Code Review) 20 Jun '21

20 Jun '21

Attention is currently required from: Nico Huber, Stefan Reinauer, Edward O'Callaghan, Arthur Heymans. Angel Pons has removed a vote from this change. ( https://review.coreboot.org/c/flashrom/+/52470 ) Change subject: hwaccess.c: Fix PowerPC memory barrier ...................................................................... Removed Code-Review+2 by Edward O'Callaghan <quasisec(a)chromium.org> -- To view, visit https://review.coreboot.org/c/flashrom/+/52470 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia84fd1362a1d39aca6269eef22ac994dee68ba4b Gerrit-Change-Number: 52470 Gerrit-PatchSet: 4 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: Stefan Reinauer <stefan.reinauer(a)coreboot.org> 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: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: deleteVote

1 0

Change in flashrom[master]: hwaccess.c: Fix PowerPC memory barrier
by Angel Pons (Code Review) 20 Jun '21

20 Jun '21

Attention is currently required from: Nico Huber, Stefan Reinauer, Edward O'Callaghan, Arthur Heymans. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52470 ) Change subject: hwaccess.c: Fix PowerPC memory barrier ...................................................................... Patch Set 4: (1 comment) Patchset: PS4: > I had a thought lately about such updates with potential, hard to […] This change can wait for now. -- To view, visit https://review.coreboot.org/c/flashrom/+/52470 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia84fd1362a1d39aca6269eef22ac994dee68ba4b Gerrit-Change-Number: 52470 Gerrit-PatchSet: 4 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: Stefan Reinauer <stefan.reinauer(a)coreboot.org> 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: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Sun, 20 Jun 2021 12:39:22 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ft2232_spi: Don't lower write data chunksize
by Angel Pons (Code Review) 20 Jun '21

20 Jun '21

Attention is currently required from: Nico Huber. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/55683 ) Change subject: ft2232_spi: Don't lower write data chunksize ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/55683 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I779e24dc3f3379a98ddce02c3765062ac3241884 Gerrit-Change-Number: 55683 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Comment-Date: Sun, 20 Jun 2021 12:38:41 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ft2232_spi.c: Implement spi_send_multicommand()
by Nico Huber (Code Review) 20 Jun '21

20 Jun '21

Attention is currently required from: Simon Buhrow, Edward O'Callaghan, Samir Ibradžić. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/40477 ) Change subject: ft2232_spi.c: Implement spi_send_multicommand() ...................................................................... Patch Set 34: (1 comment) Patchset: PS34: You might also want to update the commit message, i.e. mention that the numbers were measured with flashrom inside a virtualization environment. -- 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: 34 Gerrit-Owner: Simon Buhrow Gerrit-Reviewer: Alan Green <avg(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Samir Ibradžić <sibradzic(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Patrick Georgi <pgeorgi(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Simon Buhrow Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Samir Ibradžić <sibradzic(a)gmail.com> Gerrit-Comment-Date: Sun, 20 Jun 2021 11:59:30 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ft2232_spi.c: Implement spi_send_multicommand()
by Nico Huber (Code Review) 20 Jun '21

20 Jun '21

Attention is currently required from: Simon Buhrow, Edward O'Callaghan, Samir Ibradžić. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/40477 ) Change subject: ft2232_spi.c: Implement spi_send_multicommand() ...................................................................... Patch Set 33: Code-Review+1 (2 comments) Patchset: PS33: I am deeply sorry, I somehow tricked myself to believe that this is already merged yet. There is a conflict now and I have spotted a small error. As it was my fault that other, conflicting patches were merged first, I went ahead and did the manual rebase. The error still needs to be addressed though, see inline comment. File ft2232_spi.c: https://review.coreboot.org/c/flashrom/+/40477/comment/b63204f6_6e0abf51 PS33, Line 290: + cmd->readcnt One tiny mistake left. I also didn't realize this before: The buffer `buf` below is not used for the payload data in the `readcnt` transfer. Instead, the `readarr` buffer passed with the command is used, so we must not add `readcnt` here. It only became obvious when I tried to remove the single-command ft2232_ spi_send_command() function (CB:55684). It was still used for reads. With `readcnt` removed here, it works without ft2232_spi_send_command()! -- 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: 33 Gerrit-Owner: Simon Buhrow Gerrit-Reviewer: Alan Green <avg(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Samir Ibradžić <sibradzic(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Patrick Georgi <pgeorgi(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Simon Buhrow Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Samir Ibradžić <sibradzic(a)gmail.com> Gerrit-Comment-Date: Sun, 20 Jun 2021 11:58:28 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ft2232_spi: Don't lower write data chunksize
by Nico Huber (Code Review) 20 Jun '21

20 Jun '21

Nico Huber has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/55683 ) Change subject: ft2232_spi: Don't lower write data chunksize ...................................................................... ft2232_spi: Don't lower write data chunksize This "chunk size" limits the amount of data that is passed to libusb at once. If we had exceeded the chunk size, libftdi would have split the data into individual, synchronous bulk transfers. But the chunk size was actually chosen to avoid this. So without any known effect, setting the chunk size is useless. Drop it. Change-Id: I779e24dc3f3379a98ddce02c3765062ac3241884 Signed-off-by: Nico Huber <nico.h(a)gmx.de> --- M ft2232_spi.c 1 file changed, 0 insertions(+), 4 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/83/55683/1 diff --git a/ft2232_spi.c b/ft2232_spi.c index b5dcf5c..48f462b 100644 --- a/ft2232_spi.c +++ b/ft2232_spi.c @@ -651,10 +651,6 @@ msg_perr("Unable to set latency timer (%s).\n", ftdi_get_error_string(&ftdic)); } - if (ftdi_write_data_set_chunksize(&ftdic, 270)) { - msg_perr("Unable to set chunk size (%s).\n", ftdi_get_error_string(&ftdic)); - } - if (ftdi_set_bitmode(&ftdic, 0x00, BITMODE_BITBANG_SPI) < 0) { msg_perr("Unable to set bitmode to SPI (%s).\n", ftdi_get_error_string(&ftdic)); } -- To view, visit https://review.coreboot.org/c/flashrom/+/55683 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I779e24dc3f3379a98ddce02c3765062ac3241884 Gerrit-Change-Number: 55683 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: ft2232_spi: Drop ft2232_spi_send_command()
by Nico Huber (Code Review) 20 Jun '21

20 Jun '21

Nico Huber has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/55684 ) Change subject: ft2232_spi: Drop ft2232_spi_send_command() ...................................................................... ft2232_spi: Drop ft2232_spi_send_command() Now that ft2232_spi_send_multicommand() is implemented, we don't need the single-command version anymore. Change-Id: I2e7fa1046e260f490b881a33e02ad73d16f0a30c Signed-off-by: Nico Huber <nico.h(a)gmx.de> --- M ft2232_spi.c 1 file changed, 1 insertion(+), 90 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/84/55684/1 diff --git a/ft2232_spi.c b/ft2232_spi.c index 48f462b..696a234 100644 --- a/ft2232_spi.c +++ b/ft2232_spi.c @@ -189,95 +189,6 @@ return 0; } -/* Returns 0 upon success, a negative number upon errors. */ -static int ft2232_spi_send_command(const struct flashctx *flash, - unsigned int writecnt, unsigned int readcnt, - const unsigned char *writearr, - unsigned char *readarr) -{ - struct ft2232_data *spi_data = flash->mst->spi.data; - struct ftdi_context *ftdic = &spi_data->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; - - 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) { - 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. - */ - msg_pspew("Assert CS#\n"); - buf[i++] = SET_BITS_LOW; - buf[i++] = ~ 0x08 & spi_data->pinlvl; /* assert CS (3rd) bit only */ - buf[i++] = spi_data->pindir; - - if (writecnt) { - buf[i++] = MPSSE_DO_WRITE | MPSSE_WRITE_NEG; - buf[i++] = (writecnt - 1) & 0xff; - buf[i++] = ((writecnt - 1) >> 8) & 0xff; - memcpy(buf + i, writearr, writecnt); - i += writecnt; - } - - /* - * Optionally terminate this batch of commands with a - * read command, then do the fetch of the results. - */ - 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++] = spi_data->pinlvl; - buf[i++] = spi_data->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; -} - static bool ft2232_spi_command_fits(const struct spi_command *cmd, size_t buffer_size) { const size_t cmd_len = 3; /* same length for any ft2232 command */ @@ -369,7 +280,7 @@ .features = SPI_MASTER_4BA, .max_data_read = 64 * 1024, .max_data_write = 256, - .command = ft2232_spi_send_command, + .command = default_spi_send_command, .multicommand = ft2232_spi_send_multicommand, .read = default_spi_read, .write_256 = default_spi_write_256, -- To view, visit https://review.coreboot.org/c/flashrom/+/55684 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2e7fa1046e260f490b881a33e02ad73d16f0a30c Gerrit-Change-Number: 55684 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: ft2232_spi.c: Implement spi_send_multicommand()
by Nico Huber (Code Review) 20 Jun '21

20 Jun '21

Attention is currently required from: Simon Buhrow, Edward O'Callaghan, Samir Ibradžić. Nico Huber has uploaded a new patch set (#34) to the change originally created by Simon Buhrow. ( https://review.coreboot.org/c/flashrom/+/40477 ) 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, 90 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/77/40477/34 -- 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: 34 Gerrit-Owner: Simon Buhrow Gerrit-Reviewer: Alan Green <avg(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Samir Ibradžić <sibradzic(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Patrick Georgi <pgeorgi(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Simon Buhrow Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Samir Ibradžić <sibradzic(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: buspirate: Add psus option
by Nico Huber (Code Review) 20 Jun '21

20 Jun '21

Attention is currently required from: Jeremy Kerr. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/54887 ) Change subject: buspirate: Add psus option ...................................................................... Patch Set 1: Code-Review+1 (1 comment) Patchset: PS1: Hi Jeremy. The code change looks good, please also update the manpage. -- To view, visit https://review.coreboot.org/c/flashrom/+/54887 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I8a7d4b40c0f7f04f6976f6757f05b61f2c9958f9 Gerrit-Change-Number: 54887 Gerrit-PatchSet: 1 Gerrit-Owner: Jeremy Kerr <jk(a)ozlabs.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jeremy Kerr <jk(a)ozlabs.org> Gerrit-Comment-Date: Sun, 20 Jun 2021 11:01:36 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit June 2021