Attention is currently required from: Paul Menzel, Angel Pons, Peter Marheine.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/56799 )
Change subject: realtek_mst_i2c_spi: don't always fail init
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
I got confused at first because even initial version of this file has `int ret = SPI_GENERIC_ERROR;`, so this line has always been here. But then I looked closely, the structure of get_params has changed at some point. Previously ret was assigned `ret = 0` in successful case(s), so it was working. And then at some point `ret = 0` was lost.
Peter thank you for fixing this! I assume you tested on the device? Could you please add TEST info to commit message?
--
To view, visit https://review.coreboot.org/c/flashrom/+/56799
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I0596014fc9b76b4d11de230f9f5c414c1321dff1
Gerrit-Change-Number: 56799
Gerrit-PatchSet: 1
Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org>
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: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Comment-Date: Wed, 04 Aug 2021 21:43:17 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Paul Menzel, Angel Pons, Anastasia Klimchuk, Peter Marheine.
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/56799 )
Change subject: realtek_mst_i2c_spi: don't always fail init
......................................................................
Patch Set 1:
(2 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/56799/comment/17301a9d_0a24d76f
PS1, Line 14: Change-Id: I0596014fc9b76b4d11de230f9f5c414c1321dff1
> Missing Signed-off-by line?
+1
Patchset:
PS1:
> Clearly nobody is actually using this (aside from me using it to debug things), otherwise we would h […]
well it was being used until it got re-implemented in fwupd right.
--
To view, visit https://review.coreboot.org/c/flashrom/+/56799
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I0596014fc9b76b4d11de230f9f5c414c1321dff1
Gerrit-Change-Number: 56799
Gerrit-PatchSet: 1
Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org>
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: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Comment-Date: Wed, 04 Aug 2021 14:00:44 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>
Comment-In-Reply-To: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Edward O'Callaghan, Angel Pons, Anastasia Klimchuk.
Peter Marheine has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/56799 )
Change subject: realtek_mst_i2c_spi: don't always fail init
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
Clearly nobody is actually using this (aside from me using it to debug things), otherwise we would have noticed that it can't work sooner!
--
To view, visit https://review.coreboot.org/c/flashrom/+/56799
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I0596014fc9b76b4d11de230f9f5c414c1321dff1
Gerrit-Change-Number: 56799
Gerrit-PatchSet: 1
Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org>
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: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Wed, 04 Aug 2021 07:21:40 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Peter Marheine has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/56799 )
Change subject: realtek_mst_i2c_spi: don't always fail init
......................................................................
realtek_mst_i2c_spi: don't always fail init
In some earlier refactoring the init function for this programmer was
broken such that it never succeeds, since the return value was never set
to a value other than a generic failure. Fix it to return success unless
there is a problem.
Change-Id: I0596014fc9b76b4d11de230f9f5c414c1321dff1
---
M realtek_mst_i2c_spi.c
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/99/56799/1
diff --git a/realtek_mst_i2c_spi.c b/realtek_mst_i2c_spi.c
index 22f7e7a..a45838c 100644
--- a/realtek_mst_i2c_spi.c
+++ b/realtek_mst_i2c_spi.c
@@ -445,7 +445,7 @@
static int get_params(int *reset, int *enter_isp)
{
char *reset_str = NULL, *isp_str = NULL;
- int ret = SPI_GENERIC_ERROR;
+ int ret = 0;
reset_str = extract_programmer_param("reset-mcu");
if (reset_str) {
--
To view, visit https://review.coreboot.org/c/flashrom/+/56799
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I0596014fc9b76b4d11de230f9f5c414c1321dff1
Gerrit-Change-Number: 56799
Gerrit-PatchSet: 1
Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-MessageType: newchange
Attention is currently required from: Nico Huber, Furquan Shaikh, Tim Wawrzynczak.
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/56721 )
Change subject: internal: Return early from map_flash for >16MiB on x86
......................................................................
Patch Set 3: Code-Review+2
(1 comment)
Patchset:
PS3:
Seems OK to me.
--
To view, visit https://review.coreboot.org/c/flashrom/+/56721
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I60244b6970118dbdfdbb5b8424fc0a8acd9def2e
Gerrit-Change-Number: 56721
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Comment-Date: Wed, 04 Aug 2021 06:48:28 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Furquan Shaikh, Edward O'Callaghan.
Hello build bot (Jenkins), Furquan Shaikh, Edward O'Callaghan,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/56721
to look at the new patch set (#3).
Change subject: internal: Return early from map_flash for >16MiB on x86
......................................................................
internal: Return early from map_flash for >16MiB on x86
In the case of a flash chip bigger than 16 MiB, x86 chipsets are only
able to map the topmost 16 MiB of the chip into memory. To access the
rest, flashrom can use hardware sequencing, therefore return early from
`physmap_common()` in this situation; this avoids messages like the
following in Linux kernel logs:
[ 57.715654] x86/PAT: flashrom:2805 conflicting memory types fe0000...
[ 57.726270] x86/PAT: memtype_reserve failed [mem 0xfe000000-0xffff...
BUG=b:185021901
TEST=On a system with a W25Q256.V (32MB), flashrom logs show:
$ flashrom -p host -r bios.bin
...
Found Winbond flash chip "W25Q256.V" (32768 kB, Programmer-specific) ...
Chipset unable to map >16 MiB of flash
Reading flash... done.
SUCCESS
Signed-off-by: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Change-Id: I60244b6970118dbdfdbb5b8424fc0a8acd9def2e
---
M physmap.c
1 file changed, 9 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/21/56721/3
--
To view, visit https://review.coreboot.org/c/flashrom/+/56721
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I60244b6970118dbdfdbb5b8424fc0a8acd9def2e
Gerrit-Change-Number: 56721
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-MessageType: newpatchset