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

Change in flashrom[master]: ft2232_spi.c - Pack WREN and op in one ftdi_write_data() call to save...
by Simon Buhrow (Code Review) 27 Apr '20

27 Apr '20

Simon Buhrow has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/40477 ) Change subject: ft2232_spi.c - Pack WREN and op in one ftdi_write_data() call to save programming time ...................................................................... Patch Set 6: (1 comment) > Patch Set 3: > > (1 comment) https://review.coreboot.org/c/flashrom/+/40477/2/ft2232_spi.c File ft2232_spi.c: https://review.coreboot.org/c/flashrom/+/40477/2/ft2232_spi.c@549 PS2, Line 549: return 0; > Well, in the usual use case with maximum buf size equal to normal page program operations, I agree. […] Done in patchset 6: Set buf to fix size to fix realloc issue. -- 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: 6 Gerrit-Owner: Simon Buhrow Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Patrick Georgi <pgeorgi(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Mon, 27 Apr 2020 08:29:58 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Simon Buhrow Comment-In-Reply-To: Patrick Georgi <pgeorgi(a)google.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ft2232_spi.c - Pack WREN and op in one ftdi_write_data() call to save...
by Simon Buhrow (Code Review) 27 Apr '20

27 Apr '20

Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/40477 to look at the new patch set (#6). Change subject: ft2232_spi.c - Pack WREN and op in one ftdi_write_data() call to save programming time ...................................................................... ft2232_spi.c - Pack WREN and op in one ftdi_write_data() call to save programming time 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! See timings WREN in extra ftdi_write_data() call: https://ibb.co/0c1J25d WREN packed with op: https://ibb.co/7G1hLkj I´m using ftdi-2232H chip. That´s why I put it at this place. If anyone has a good overview about all programmers: This could be implemented in spi_write_cmd() in case that it is compatible to all programmers or this principle could be transfered to other programmers which act in a similar way. Signed-off-by: Simon Buhrow <simon.buhrow(a)posteo.de> Change-Id: Ie4a07499ec5ef0af23818593f45dc427285a9e8a --- M ft2232_spi.c 1 file changed, 18 insertions(+), 11 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/77/40477/6 -- 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: 6 Gerrit-Owner: Simon Buhrow Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Patrick Georgi <pgeorgi(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: Set buffers size fix to 4096 Signed-off-by: Simon Buhrow <simon.buhro...
by Simon Buhrow (Code Review) 27 Apr '20

27 Apr '20

Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/40477 to look at the new patch set (#5). Change subject: Set buffers size fix to 4096 Signed-off-by: Simon Buhrow <simon.buhrow(a)posteo.de> Change-Id: Ie4a07499ec5ef0af23818593f45dc427285a9e8a ...................................................................... Set buffers size fix to 4096 Signed-off-by: Simon Buhrow <simon.buhrow(a)posteo.de> Change-Id: Ie4a07499ec5ef0af23818593f45dc427285a9e8a --- M ft2232_spi.c 1 file changed, 18 insertions(+), 11 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/77/40477/5 -- 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: 5 Gerrit-Owner: Simon Buhrow Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Patrick Georgi <pgeorgi(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: ft2232_spi.c - Pack WREN and op in one ftdi_write_data() call to save...
by Simon Buhrow (Code Review) 27 Apr '20

27 Apr '20

Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/40477 to look at the new patch set (#4). Change subject: ft2232_spi.c - Pack WREN and op in one ftdi_write_data() call to save programming time ...................................................................... ft2232_spi.c - Pack WREN and op in one ftdi_write_data() call to save programming time 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! See timings WREN in extra ftdi_write_data() call: https://ibb.co/0c1J25d WREN packed with op: https://ibb.co/7G1hLkj I´m using ftdi-2232H chip. That´s why I put it at this place. If anyone has a good overview about all programmers: This could be implemented in spi_write_cmd() in case that it is compatible to all programmers or this principle could be transfered to other programmers which act in a similar way. Signed-off-by: Simon Buhrow <simon.buhrow(a)posteo.de> Change-Id: Ie4a07499ec5ef0af23818593f45dc427285a9e8a --- M ft2232_spi.c 1 file changed, 19 insertions(+), 4 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/77/40477/4 -- 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: 4 Gerrit-Owner: Simon Buhrow Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Patrick Georgi <pgeorgi(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: ft2232_spi.c: Improve handling of static buffer
by Angel Pons (Code Review) 25 Apr '20

25 Apr '20

Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/39975 ) Change subject: ft2232_spi.c: Improve handling of static buffer ...................................................................... ft2232_spi.c: Improve handling of static buffer If `buf` became NULL because of an error, subsequent calls to the `ft2232_spi_send_command` function with a smaller buffer size will result in a null pointer dereference. Add an additional null check before using `buf` to prevent that. Moreover, use `size_t` for the `bufsize` and `oldbufsize` variables, as it's what `realloc` uses. Change-Id: Idc4237ddca94c42ce2a930e6d00fd2d14e4f125c Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> --- M ft2232_spi.c 1 file changed, 3 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/75/39975/1 diff --git a/ft2232_spi.c b/ft2232_spi.c index 1a5b2fe..84aebb3 100644 --- a/ft2232_spi.c +++ b/ft2232_spi.c @@ -468,8 +468,8 @@ static unsigned char *buf = NULL; /* failed is special. We use bitwise ops, but it is essentially bool. */ int i = 0, ret = 0, failed = 0; - int bufsize; - static int oldbufsize = 0; + size_t bufsize; + static size_t oldbufsize = 0; if (writecnt > 65536 || readcnt > 65536) return SPI_INVALID_LENGTH; @@ -477,7 +477,7 @@ /* buf is not used for the response from the chip. */ bufsize = max(writecnt + 9, 260 + 9); /* Never shrink. realloc() calls are expensive. */ - if (bufsize > oldbufsize) { + if (!buf || bufsize > oldbufsize) { buf = realloc(buf, bufsize); if (!buf) { msg_perr("Out of memory!\n"); -- To view, visit https://review.coreboot.org/c/flashrom/+/39975 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Idc4237ddca94c42ce2a930e6d00fd2d14e4f125c Gerrit-Change-Number: 39975 Gerrit-PatchSet: 1 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newchange

3 3

Change in flashrom[master]: spi25.c: Add a null check
by Angel Pons (Code Review) 25 Apr '20

25 Apr '20

Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/39974 ) Change subject: spi25.c: Add a null check ...................................................................... spi25.c: Add a null check Change-Id: I5fcc23d81b8404af90768afa2954509bf334ab2c Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> --- M spi25.c 1 file changed, 2 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/74/39974/1 diff --git a/spi25.c b/spi25.c index 2a1d492..0a939e7 100644 --- a/spi25.c +++ b/spi25.c @@ -427,6 +427,8 @@ msg_cerr("%s called for too long a write\n", __func__); return 1; } + if (!out_bytes && out_len > 0) + return 1; memcpy(cmd + 1 + addr_len, out_bytes, out_len); cmds[1].writecnt = 1 + addr_len + out_len; -- To view, visit https://review.coreboot.org/c/flashrom/+/39974 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I5fcc23d81b8404af90768afa2954509bf334ab2c Gerrit-Change-Number: 39974 Gerrit-PatchSet: 1 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newchange

3 3

Change in flashrom[master]: lspcon_i2c_spi.c: Trivial style fix
by Edward O'Callaghan (Code Review) 24 Apr '20

24 Apr '20

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/40679 ) Change subject: lspcon_i2c_spi.c: Trivial style fix ...................................................................... lspcon_i2c_spi.c: Trivial style fix Change-Id: I0675b467d7b0d24626a336033668bf80bfeeb815 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M lspcon_i2c_spi.c 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/79/40679/1 diff --git a/lspcon_i2c_spi.c b/lspcon_i2c_spi.c index 1ee5f37..8de2c14 100644 --- a/lspcon_i2c_spi.c +++ b/lspcon_i2c_spi.c @@ -151,7 +151,7 @@ int ret = 0; do { ret |= lspcon_i2c_spi_read_register(fd, offset, &val); - } while(!ret && (val & mask) && ++tried < MAX_SPI_WAIT_RETRIES); + } while (!ret && (val & mask) && ++tried < MAX_SPI_WAIT_RETRIES); if (tried == MAX_SPI_WAIT_RETRIES) { msg_perr("Error: Time out on sending command.\n"); -- To view, visit https://review.coreboot.org/c/flashrom/+/40679 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0675b467d7b0d24626a336033668bf80bfeeb815 Gerrit-Change-Number: 40679 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

4 4

Change in flashrom[master]: stlinkv3_spi: Avoid div-by-zero
by Patrick Georgi (Code Review) 24 Apr '20

24 Apr '20

Patrick Georgi has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/40653 ) Change subject: stlinkv3_spi: Avoid div-by-zero ...................................................................... stlinkv3_spi: Avoid div-by-zero Change-Id: I08c0612f3fea59add9bde2fb3cc5c4b5c3756516 Found-by: Coverity Scan #1412744 Signed-off-by: Patrick Georgi <pgeorgi(a)google.com> --- M stlinkv3_spi.c 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/53/40653/1 diff --git a/stlinkv3_spi.c b/stlinkv3_spi.c index 9e621c2..6297861 100644 --- a/stlinkv3_spi.c +++ b/stlinkv3_spi.c @@ -491,7 +491,7 @@ speed_str = extract_programmer_param("spispeed"); if (speed_str) { sck_freq_kHz = strtoul(speed_str, &endptr, 0); - if (*endptr) { + if (*endptr || sck_freq_kHz == 0) { msg_perr("The spispeed parameter passed with invalid format: %s\n", speed_str); msg_perr("Please pass the parameter with a simple number in kHz\n"); -- To view, visit https://review.coreboot.org/c/flashrom/+/40653 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I08c0612f3fea59add9bde2fb3cc5c4b5c3756516 Gerrit-Change-Number: 40653 Gerrit-PatchSet: 1 Gerrit-Owner: Patrick Georgi <pgeorgi(a)google.com> Gerrit-MessageType: newchange

5 7

Change in flashrom[master]: stlinkv3_spi: Fix memory leak
by Patrick Georgi (Code Review) 24 Apr '20

24 Apr '20

Patrick Georgi has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/40652 ) Change subject: stlinkv3_spi: Fix memory leak ...................................................................... stlinkv3_spi: Fix memory leak Change-Id: Ic650b43395c64b1677f6e114b0faf42a3b7b3759 Found-by: Coverity Scan #1415214 Signed-off-by: Patrick Georgi <pgeorgi(a)google.com> --- M stlinkv3_spi.c 1 file changed, 1 insertion(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/52/40652/1 diff --git a/stlinkv3_spi.c b/stlinkv3_spi.c index 7338911..9e621c2 100644 --- a/stlinkv3_spi.c +++ b/stlinkv3_spi.c @@ -495,6 +495,7 @@ msg_perr("The spispeed parameter passed with invalid format: %s\n", speed_str); msg_perr("Please pass the parameter with a simple number in kHz\n"); + free(speed_str); return -1; } free(speed_str); -- To view, visit https://review.coreboot.org/c/flashrom/+/40652 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic650b43395c64b1677f6e114b0faf42a3b7b3759 Gerrit-Change-Number: 40652 Gerrit-PatchSet: 1 Gerrit-Owner: Patrick Georgi <pgeorgi(a)google.com> Gerrit-MessageType: newchange

5 7

Change in flashrom[master]: usb_device: Fix memory leak
by Patrick Georgi (Code Review) 24 Apr '20

24 Apr '20

Patrick Georgi has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/40651 ) Change subject: usb_device: Fix memory leak ...................................................................... usb_device: Fix memory leak Change-Id: I19c91ae881895ecc4ea85dcfd40a69bb58289a60 Found-by: Coverity Scan #1420203 Signed-off-by: Patrick Georgi <pgeorgi(a)google.com> --- M usb_device.c 1 file changed, 2 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/51/40651/1 diff --git a/usb_device.c b/usb_device.c index 48833fb..344c4cb 100644 --- a/usb_device.c +++ b/usb_device.c @@ -226,6 +226,8 @@ &descriptor)); if (ret != 0) { msg_perr("USB: Failed to get device descriptor"); + free(*devices); + *devices = NULL; return ret; } -- To view, visit https://review.coreboot.org/c/flashrom/+/40651 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I19c91ae881895ecc4ea85dcfd40a69bb58289a60 Gerrit-Change-Number: 40651 Gerrit-PatchSet: 1 Gerrit-Owner: Patrick Georgi <pgeorgi(a)google.com> Gerrit-MessageType: newchange

5 5

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit April 2020