January 2021 - flashrom-gerrit

Change in flashrom[master]: libflashrom: Return progress state to the library user
by Richard Hughes (Code Review) 30 Jan '21

30 Jan '21

Attention is currently required from: Richard Hughes, Edward O'Callaghan, Patrick Rudolph. Richard Hughes 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 3: (2 comments) Patchset: PS3: Okay, rebased without the floating point. File cli_output.c: https://review.coreboot.org/c/flashrom/+/49643/comment/a83988be_ebffdb51 PS2, Line 84: double > that could be slow on soft float CPUs. […] We could... but I think the msleeps between the SPI transactions are going to be orders of magnitude slower. We could of course do something like: pc = ((unsigned long long) current * 10000) / ((unsigned long long) total * 100); If this works for you I'll respin the patch with that. -- 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: 3 Gerrit-Owner: Richard Hughes <richard(a)hughsie.com> 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-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: Sat, 30 Jan 2021 16:58:36 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Patrick Rudolph <siro(a)das-labor.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: libflashrom: Return progress state to the library user
by Richard Hughes (Code Review) 30 Jan '21

30 Jan '21

Attention is currently required from: Richard Hughes, Edward O'Callaghan. Hello build bot (Jenkins), Edward O'Callaghan, Patrick Rudolph, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/49643 to look at the new patch set (#3). Change subject: libflashrom: Return progress state to the library user ...................................................................... libflashrom: Return progress state to the library user Include test for the dummy spi25 device. Change-Id: I7197572bb7f19e3bdb2bde855d70a0f50fd3854c Signed-off-by: Richard Hughes <richard(a)hughsie.com> --- M 82802ab.c M at45db.c M cli_classic.c M cli_output.c M dediprog.c M en29lv640b.c M flash.h M it87spi.c M jedec.c M libflashrom.c M libflashrom.h M linux_mtd.c M lspcon_i2c_spi.c M realtek_mst_i2c_spi.c M spi.c M spi25.c M sst28sf040.c M tests/spi25.c M tests/tests.c M tests/tests.h 20 files changed, 123 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/43/49643/3 -- 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: 3 Gerrit-Owner: Richard Hughes <richard(a)hughsie.com> 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-Attention: Richard Hughes <richard(a)hughsie.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: linux_mtd: Disable buffering on the mtd device
by Angel Pons (Code Review) 30 Jan '21

30 Jan '21

Attention is currently required from: Douglas Anderson, David Hendricks, Edward O'Callaghan. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/50155 ) Change subject: linux_mtd: Disable buffering on the mtd device ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/50155 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I989afd83a33013b2756a0090d6b08245613215c6 Gerrit-Change-Number: 50155 Gerrit-PatchSet: 2 Gerrit-Owner: Douglas Anderson <dianders(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Vadim Bendebury <vbendeb(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Douglas Anderson <dianders(a)chromium.org> Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Sat, 30 Jan 2021 11:29:10 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: libflashrom: Return progress state to the library user
by Patrick Rudolph (Code Review) 30 Jan '21

30 Jan '21

Attention is currently required from: Richard Hughes, Edward O'Callaghan. Patrick Rudolph 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 2: Code-Review+2 (1 comment) File cli_output.c: https://review.coreboot.org/c/flashrom/+/49643/comment/e5613162_d1976fad PS2, Line 84: double that could be slow on soft float CPUs. Can you use fix point division instead? -- 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: 2 Gerrit-Owner: Richard Hughes <richard(a)hughsie.com> 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-Attention: Richard Hughes <richard(a)hughsie.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Sat, 30 Jan 2021 06:51:45 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: linux_mtd: Disable buffering on the mtd device
by Patrick Rudolph (Code Review) 30 Jan '21

30 Jan '21

Attention is currently required from: Douglas Anderson, David Hendricks, Edward O'Callaghan. Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/50155 ) Change subject: linux_mtd: Disable buffering on the mtd device ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/50155 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I989afd83a33013b2756a0090d6b08245613215c6 Gerrit-Change-Number: 50155 Gerrit-PatchSet: 2 Gerrit-Owner: Douglas Anderson <dianders(a)chromium.org> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Vadim Bendebury <vbendeb(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Douglas Anderson <dianders(a)chromium.org> Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Sat, 30 Jan 2021 06:42:32 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: linux_mtd: Disable buffering on the mtd device
by Hung-Te Lin (Code Review) 30 Jan '21

30 Jan '21

Attention is currently required from: Douglas Anderson, David Hendricks, Edward O'Callaghan. Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/50155 ) Change subject: linux_mtd: Disable buffering on the mtd device ...................................................................... Patch Set 2: Code-Review+1 (1 comment) Patchset: PS2: I'm surprised why we didn't see issues until now. In the meantime, will it make more sense if we change fopen to open for MTD device files? -- To view, visit https://review.coreboot.org/c/flashrom/+/50155 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I989afd83a33013b2756a0090d6b08245613215c6 Gerrit-Change-Number: 50155 Gerrit-PatchSet: 2 Gerrit-Owner: Douglas Anderson <dianders(a)chromium.org> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Vadim Bendebury <vbendeb(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Douglas Anderson <dianders(a)chromium.org> Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Sat, 30 Jan 2021 02:11:48 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: linux_mtd: Disable buffering on the mtd device
by Douglas Anderson (Code Review) 30 Jan '21

30 Jan '21

Attention is currently required from: Hung-Te Lin, Vadim Bendebury, David Hendricks. Douglas Anderson has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/50155 ) Change subject: linux_mtd: Disable buffering on the mtd device ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: PTAL, thanks! -- To view, visit https://review.coreboot.org/c/flashrom/+/50155 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I989afd83a33013b2756a0090d6b08245613215c6 Gerrit-Change-Number: 50155 Gerrit-PatchSet: 2 Gerrit-Owner: Douglas Anderson <dianders(a)chromium.org> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Vadim Bendebury <vbendeb(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Attention: Vadim Bendebury <vbendeb(a)chromium.org> Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Comment-Date: Sat, 30 Jan 2021 00:59:14 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: linux_mtd: Disable buffering on the mtd device
by Douglas Anderson (Code Review) 30 Jan '21

30 Jan '21

Douglas Anderson has uploaded a new patch set (#2). ( https://review.coreboot.org/c/flashrom/+/50155 ) Change subject: linux_mtd: Disable buffering on the mtd device ...................................................................... linux_mtd: Disable buffering on the mtd device We open the device node for the MTD device with this: dev_fp = fopen(dev_path, "r+") In C fopen() is allowed to provide _buffered_ access to the file. That means that the standard library is allowed to read ahead and/or return cached data. That's really not what we want for something like this. Let's turn it off. This fixes a problem where flashrom would sometimes fail to "verify" that it erased the flash. The error message would look something like this: Erasing and writing flash chip... FAILED at 0x0000e220! Expected=0xff, Found=0xe9, failed byte count from 0x0000e200-0x0000e2ff: 0xdc failed byte count from 0x0000e000-0x0000efff: 0xffffffff ERASE_FAILED FAILED! Uh oh. Erase/write failed. Checking if anything changed. After the failure I could read the flash device with a new invocation of flashrom and I would see that, indeed, the erase had worked. Tracing in the kernel showed that when the failure happened we saw a pattern that looked like this: * Read 0x0b00 bytes starting at 0x0000d000 * Read 0x1000 bytes starting at 0x0000db00 * Erase 0x1000 bytes starting at 0x0000e000 ...and then there was _not_ a read after the erase. It can be assumed that, since userspace had already read 0xdb00 - 0xeaff that it was looking at old buffered data after the erase. Signed-off-by: Douglas Anderson <dianders(a)chromium.org> Change-Id: I989afd83a33013b2756a0090d6b08245613215c6 --- M linux_mtd.c 1 file changed, 4 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/55/50155/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/50155 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I989afd83a33013b2756a0090d6b08245613215c6 Gerrit-Change-Number: 50155 Gerrit-PatchSet: 2 Gerrit-Owner: Douglas Anderson <dianders(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: WIP: linux_mtd: Disable buffering on the mtd device
by Douglas Anderson (Code Review) 30 Jan '21

30 Jan '21

Douglas Anderson has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/50155 ) Change subject: WIP: linux_mtd: Disable buffering on the mtd device ...................................................................... WIP: linux_mtd: Disable buffering on the mtd device We open the device node for the MTD device with this: dev_fp = fopen(dev_path, "r+") In C fopen() is allowed to provide _buffered_ access to the file. That means that the standard library is allowed to read ahead and/or return cached data. That's really not what we want for something like this. Let's turn it off. This fixes a problem where flashrom would sometimes fail to "verify" that it erased the flash. The error message would look something like this: Erasing and writing flash chip... FAILED at 0x0000e220! Expected=0xff, Found=0xe9, failed byte count from 0x0000e200-0x0000e2ff: 0xdc failed byte count from 0x0000e000-0x0000efff: 0xffffffff ERASE_FAILED FAILED! Uh oh. Erase/write failed. Checking if anything changed. After the failure I could read the flash device with a new invocation of flashrom and I would see that, indeed, the erase had worked. Tracing in the kernel showed that when the failure happened we saw a pattern that looked like this: * Read 0x0b00 bytes starting at 0x0000d000 * Read 0x1000 bytes starting at 0x0000db00 * Erase 0x1000 bytes starting at 0x0000e000 ...and then there was _not_ a read after the erase. It can be assumed that, since userspace had already read 0xdb00 - 0xeaff that it was looking at old buffered data after the erase. BRANCH=trogdor BUG=b:178822068 TEST=Loop back and forth writing two BIOS files => no errors Signed-off-by: Douglas Anderson <dianders(a)chromium.org> Change-Id: I989afd83a33013b2756a0090d6b08245613215c6 --- M linux_mtd.c 1 file changed, 4 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/55/50155/1 diff --git a/linux_mtd.c b/linux_mtd.c index d2df95e..22702e9 100644 --- a/linux_mtd.c +++ b/linux_mtd.c @@ -340,6 +340,10 @@ msg_perr("Cannot open file stream for %s\n", dev_path); 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); + msg_pinfo("Opened %s successfully\n", dev_path); ret = 0; -- To view, visit https://review.coreboot.org/c/flashrom/+/50155 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I989afd83a33013b2756a0090d6b08245613215c6 Gerrit-Change-Number: 50155 Gerrit-PatchSet: 1 Gerrit-Owner: Douglas Anderson <dianders(a)chromium.org> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: CHROMIUM: flashrom_tester: Drop nix dependency
by Angel Pons (Code Review) 29 Jan '21

29 Jan '21

Attention is currently required from: Sam McNally, Edward O'Callaghan. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/49916 ) Change subject: CHROMIUM: flashrom_tester: Drop nix dependency ...................................................................... Patch Set 1: Code-Review+1 (1 comment) File util/flashrom_tester/src/main.rs: https://review.coreboot.org/c/flashrom/+/49916/comment/bc510ec4_fd2c9d81 PS1, Line 171: Safe because this doesn't : // modify any memory. nit: put the whole sentence in the next line? This way, the comment block is more rectangular -- To view, visit https://review.coreboot.org/c/flashrom/+/49916 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If7d1c7c7b5663a32c0a338866f7f7f9bee604cc0 Gerrit-Change-Number: 49916 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Sam McNally <sammc(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Sam McNally <sammc(a)google.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Fri, 29 Jan 2021 10:56:13 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit January 2021