Attention is currently required from: Angel Pons.
Peter Marheine has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/51967 )
Change subject: lspcon_i2c_spi: support a devpath option
......................................................................
Patch Set 3:
(4 comments)
File lspcon_i2c_spi.c:
https://review.coreboot.org/c/flashrom/+/51967/comment/67689049_f24a3d89
PS2, Line 457: __func__);
> nit: `__func__` probably fits on the previous line
Done
https://review.coreboot.org/c/flashrom/+/51967/comment/b17fd475_96290503
PS2, Line 463: __func__);
> nit: `__func__` most likely fits on the previous line
Done
https://review.coreboot.org/c/flashrom/+/51967/comment/1512b0d9_1d11c202
PS2, Line 489:
> nit: replace 8 spaces with a tab
Done
https://review.coreboot.org/c/flashrom/+/51967/comment/fb3374fa_ec8dea86
PS2, Line 492: } else if (device_path != NULL) {
> The first two cases are error paths that return an error, whereas the last two cases are the "normal […]
Done
--
To view, visit https://review.coreboot.org/c/flashrom/+/51967
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Id2adf8a307b9205ce5e5804a6c3e22f19d0c34c9
Gerrit-Change-Number: 51967
Gerrit-PatchSet: 3
Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Daniel Campello <campello(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Mon, 12 Apr 2021 00:18:19 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Peter Marheine.
Hello build bot (Jenkins), Edward O'Callaghan, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/51967
to look at the new patch set (#3).
Change subject: lspcon_i2c_spi: support a devpath option
......................................................................
lspcon_i2c_spi: support a devpath option
Some callers may find it easier to provide the path to an I2C device
at which to communicate with the device, rather than specify the bus
number- doing so might involve trying to parse a path to extract the
number only for flashrom to do the reverse, which is error-prone and
unnecessary.
This change adds support for a `devpath` option, continuing to
allow `bus` and requiring only one of them be specified.
TEST=Verified --flash-size outputs correct values with both
devpath=/dev/i2c-7 and bus=7, as well as noting that one is
required if neither is specified and only one may be specified
if both are given.
Signed-off-by: Peter Marheine <pmarheine(a)chromium.org>
Change-Id: Id2adf8a307b9205ce5e5804a6c3e22f19d0c34c9
---
M i2c_helper.h
M i2c_helper_linux.c
M lspcon_i2c_spi.c
3 files changed, 79 insertions(+), 50 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/67/51967/3
--
To view, visit https://review.coreboot.org/c/flashrom/+/51967
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Id2adf8a307b9205ce5e5804a6c3e22f19d0c34c9
Gerrit-Change-Number: 51967
Gerrit-PatchSet: 3
Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Daniel Campello <campello(a)chromium.org>
Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-MessageType: newpatchset
Jack Rosenthal has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/52215 )
Change subject: linux_mtd: prevent corruption of flash when stdout/stderr is closed
......................................................................
linux_mtd: prevent corruption of flash when stdout/stderr is closed
While it's not posixly-correct, it's possible that a user, script, or
application may attempt to start flashrom with stdout or stderr
closed. If this happens with an mtd device, it's possible that we'll
get a file descriptor of 1 or 2, and flashrom will send garbage debug
logs to the flash:
# bash -c "exec >&- flashrom ..."
Observed corruption:
43 40 45 42 45 44 00 00 00 00 00 00 01 00 00 00 |C@EBED..........|
00 02 00 00 63 65 73 73 66 75 6c 6c 79 0a 46 6f |....cessfully.Fo|
75 6e 64 20 50 72 6f 67 72 61 6d 6d 65 72 20 66 |und Programmer f|
6c 61 73 68 20 63 68 69 70 20 22 4f 70 61 71 75 |lash chip "Opaqu|
65 20 66 6c 61 73 68 20 63 68 69 70 22 20 28 38 |e flash chip" (8|
31 39 32 20 6b 42 2c 20 50 72 6f 67 72 61 6d 6d |192 kB, Programm|
65 72 2d 73 70 65 63 69 66 69 63 29 20 6d 61 70 |er-specific) map|
70 65 64 20 61 74 20 70 68 79 73 69 63 61 6c 20 |ped at physical |
61 64 64 72 65 73 73 20 30 78 30 30 30 30 30 30 |address 0x000000|
30 30 2e 0a ff ff ff ff ff ff ff ff ff ff ff ff |00..............|
ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff |................|
...
While for most applications, closing stdout or stderr would just lead
to obsure bugs, for flashrom, we should have extra safety guards, as
this could mean that we might be bricking a device instead.
Add a basic safety check.
Signed-off-by: Jack Rosenthal <jrosenth(a)chromium.org>
Change-Id: I751c9dd88ad1d30283b94bd2185b4f8f25569c8f
---
M linux_mtd.c
1 file changed, 20 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/15/52215/1
diff --git a/linux_mtd.c b/linux_mtd.c
index 22702e9..aa4fe4f 100644
--- a/linux_mtd.c
+++ b/linux_mtd.c
@@ -340,6 +340,26 @@
msg_perr("Cannot open file stream for %s\n", dev_path);
goto linux_mtd_setup_exit;
}
+
+ /*
+ * Safety guard against accidentally writing stdout or stderr
+ * to the flash. In case flashrom was started with stdout or
+ * stderr closed, it's possible that we will get fd 1 or fd 2
+ * for the flash device. We don't want to accidentally write
+ * debug messages to the flash device, corrupting the
+ * flash.
+ */
+ if (fileno(dev_fp) <= 2) {
+ fclose(dev_fp);
+ /*
+ * This message may not actually make it to the user,
+ * given stdout/stderr may be closed. But we can at
+ * least try.
+ */
+ msg_perr("stdout or stderr is closed\n");
+ goto linux_mtd_setup_exit;
+ }
+
ret = setvbuf(dev_fp, NULL, _IONBF, 0);
if (ret)
msg_pwarn("Failed to set MTD device to unbuffered: %d\n", ret);
--
To view, visit https://review.coreboot.org/c/flashrom/+/52215
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I751c9dd88ad1d30283b94bd2185b4f8f25569c8f
Gerrit-Change-Number: 52215
Gerrit-PatchSet: 1
Gerrit-Owner: Jack Rosenthal <jrosenth(a)chromium.org>
Gerrit-MessageType: newchange
Attention is currently required from: Richard Hughes, Edward O'Callaghan, Patrick Rudolph.
Daniel Campello has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/49643 )
Change subject: libflashrom: Return progress state to the library user
......................................................................
Patch Set 4: Code-Review+1
--
To view, visit https://review.coreboot.org/c/flashrom/+/49643
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I7197572bb7f19e3bdb2bde855d70a0f50fd3854c
Gerrit-Change-Number: 49643
Gerrit-PatchSet: 4
Gerrit-Owner: Richard Hughes <richard(a)hughsie.com>
Gerrit-Reviewer: Daniel Campello <campello(a)chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-CC: Richard Hughes <hughsient(a)gmail.com>
Gerrit-CC: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Richard Hughes <richard(a)hughsie.com>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Comment-Date: Fri, 09 Apr 2021 15:25:45 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Simon Buhrow, Alan Green.
Hello build bot (Jenkins), Alan Green, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/40477
to look at the new patch set (#25).
Change subject: ft2232_spi.c: Pack WREN and op in one ftdi_write_data() call
......................................................................
ft2232_spi.c: Pack WREN and op in one ftdi_write_data() call
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!
I am using ftdi-2232H chip. That is 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, 90 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/77/40477/25
--
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: 25
Gerrit-Owner: Simon Buhrow
Gerrit-Reviewer: Alan Green <avg(a)google.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(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)users.sourceforge.net>
Gerrit-Attention: Simon Buhrow
Gerrit-Attention: Alan Green <avg(a)google.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Simon Buhrow, Alan Green.
Hello build bot (Jenkins), Alan Green, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/40477
to look at the new patch set (#24).
Change subject: ft2232_spi.c: Pack WREN and op in one ftdi_write_data() call
......................................................................
ft2232_spi.c: Pack WREN and op in one ftdi_write_data() call
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!
I am using ftdi-2232H chip. That is 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, 90 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/77/40477/24
--
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: 24
Gerrit-Owner: Simon Buhrow
Gerrit-Reviewer: Alan Green <avg(a)google.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(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)users.sourceforge.net>
Gerrit-Attention: Simon Buhrow
Gerrit-Attention: Alan Green <avg(a)google.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Simon Buhrow, Alan Green.
Hello build bot (Jenkins), Alan Green, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/40477
to look at the new patch set (#23).
Change subject: ft2232_spi.c: Pack WREN and op in one ftdi_write_data() call
......................................................................
ft2232_spi.c: Pack WREN and op in one ftdi_write_data() call
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!
IŽm using ftdi-2232H chip. That is 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, 90 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/77/40477/23
--
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: 23
Gerrit-Owner: Simon Buhrow
Gerrit-Reviewer: Alan Green <avg(a)google.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(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)users.sourceforge.net>
Gerrit-Attention: Simon Buhrow
Gerrit-Attention: Alan Green <avg(a)google.com>
Gerrit-MessageType: newpatchset