Attention is currently required from: Thomas Heijligen, Angel Pons, Anastasia Klimchuk, Nicholas Chin.
qianfan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70573 )
Change subject: ch347_spi.c: Add initial support for the WCH CH347
......................................................................
Patch Set 3:
(1 comment)
Patchset:
PS3:
I had already pushed a PR: https://review.coreboot.org/c/flashrom/+/70529 and has more clear defines of spi config structure. could you please take a look of that?
And maybe it's a good way to merge those two PR.
--
To view, visit https://review.coreboot.org/c/flashrom/+/70573
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I31b86c41076cc45d4a416a73fa1131350fb745ba
Gerrit-Change-Number: 70573
Gerrit-PatchSet: 3
Gerrit-Owner: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: qianfan <qianfanguijin(a)163.com>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Comment-Date: Mon, 12 Dec 2022 02:00:16 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/70603 )
Change subject: flashrom.c: Reduce the prog global state machine
......................................................................
flashrom.c: Reduce the prog global state machine
programmer_init() no longer needs to set a full ref to the programmer
handle. A few cases of needing the programmer name and if it is a
internal type are required however a 'const char *' and 'bool' type
are much easier to reason about as well as mitigates going backwards
with patches that attempt to use the 'programmer' state via the global.
Change-Id: I93dfe09fe877acd32c4f22665921544ccd9323f6
Signed-off-by: Edward O'Callaghan <quasisec(a)google.com>
---
M flashrom.c
1 file changed, 25 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/03/70603/1
diff --git a/flashrom.c b/flashrom.c
index 4f9bf7d..59533d2 100644
--- a/flashrom.c
+++ b/flashrom.c
@@ -36,7 +36,9 @@
const char flashrom_version[] = FLASHROM_VERSION;
-static const struct programmer_entry *programmer = NULL;
+// TODO: Remove global state.
+static const char *g_programmer_name = NULL;
+static bool g_programmer_type = false;
/*
* Programmers supporting multiple buses can have differing size limits on
@@ -132,7 +134,10 @@
msg_perr("Invalid programmer specified!\n");
return -1;
}
- programmer = prog;
+ g_programmer_name = prog->name;
+#if CONFIG_INTERNAL == 1
+ g_programmer_type = prog == &programmer_internal;
+#endif
/* Initialize all programmer specific data. */
/* Default to unlimited decode sizes. */
max_rom_decode = (const struct decode_sizes) {
@@ -996,7 +1001,7 @@
msg_cinfo("mapped at physical address 0x%0*" PRIxPTR ".\n",
PRIxPTR_WIDTH, flash->physical_memory);
else
- msg_cinfo("on %s.\n", programmer->name);
+ msg_cinfo("on %s.\n", g_programmer_name);
/* Flash registers may more likely not be mapped if the chip was forced.
* Lock info may be stored in registers, so avoid lock info printing. */
@@ -1480,11 +1485,7 @@
static bool is_internal_programmer()
{
-#if CONFIG_INTERNAL == 1
- return programmer == &programmer_internal;
-#else
- return false;
-#endif
+ return g_programmer_type;
}
static void nonfatal_help_message(void)
--
To view, visit https://review.coreboot.org/c/flashrom/+/70603
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I93dfe09fe877acd32c4f22665921544ccd9323f6
Gerrit-Change-Number: 70603
Gerrit-PatchSet: 1
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-MessageType: newchange
Attention is currently required from: Edward O'Callaghan, Angel Pons.
Hello build bot (Jenkins), Angel Pons, Anastasia Klimchuk,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/68855
to look at the new patch set (#4).
Change subject: tree/: Rename 'internal_delay()' to 'default_delay()'
......................................................................
tree/: Rename 'internal_delay()' to 'default_delay()'
The non-custom driver programmer delay implementation
'internal_delay()' is unrelated specifically to the
'internal' programmer. The delay implementation is
simply a platform-agnostic host delay implementation.
Therefore, rename to simply default_delay().
Change-Id: I5e04adf16812ceb1480992c92bca25ed80f8897a
Signed-off-by: Edward O'Callaghan <quasisec(a)google.com>
---
M amd_imc.c
M atavia.c
M bitbang_spi.c
M ch341a_spi.c
M dediprog.c
M dummyflasher.c
M flashrom.c
M ichspi.c
M include/flash.h
M include/programmer.h
M it87spi.c
M nicintel_eeprom.c
M pony_spi.c
M raiden_debug_spi.c
M serial.c
M serprog.c
M udelay.c
M wbsio_spi.c
18 files changed, 62 insertions(+), 46 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/55/68855/4
--
To view, visit https://review.coreboot.org/c/flashrom/+/68855
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I5e04adf16812ceb1480992c92bca25ed80f8897a
Gerrit-Change-Number: 68855
Gerrit-PatchSet: 4
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
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-MessageType: newpatchset
Attention is currently required from: Angel Pons, Anastasia Klimchuk.
Evan Benn has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70554 )
Change subject: tests/selfcheck.c: Fix on non-x86 machines
......................................................................
Patch Set 2:
(1 comment)
File tests/selfcheck.c:
https://review.coreboot.org/c/flashrom/+/70554/comment/6b332367_ebb879fe
PS2, Line 142: assert_true(board_matches_size > 1);
Maybe it should be `> 0` unconditionally then?
https://source.chromium.org/chromiumos/chromiumos/codesearch/+/HEAD:src/thi…
I guess I added the check to assert that we aren't accessing invalid memory on the next line, I dont know why I used 1 instead of zero though.
--
To view, visit https://review.coreboot.org/c/flashrom/+/70554
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Icbe677d3ef164e998daf898ddbea34f96246677f
Gerrit-Change-Number: 70554
Gerrit-PatchSet: 2
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Evan Benn <evanbenn(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Mon, 12 Dec 2022 00:02:45 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Edward O'Callaghan, Anastasia Klimchuk, Evan Benn.
Hello build bot (Jenkins), Anastasia Klimchuk, Evan Benn,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/70554
to look at the new patch set (#2).
Change subject: tests/selfcheck.c: Fix on non-x86 machines
......................................................................
tests/selfcheck.c: Fix on non-x86 machines
The global const of `board_matches_size` has value `1` on non-x86
machines.
Change-Id: Icbe677d3ef164e998daf898ddbea34f96246677f
Signed-off-by: Edward O'Callaghan <quasisec(a)google.com>
---
M tests/selfcheck.c
1 file changed, 17 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/54/70554/2
--
To view, visit https://review.coreboot.org/c/flashrom/+/70554
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Icbe677d3ef164e998daf898ddbea34f96246677f
Gerrit-Change-Number: 70554
Gerrit-PatchSet: 2
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Evan Benn <evanbenn(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Attention: Evan Benn <evanbenn(a)google.com>
Gerrit-MessageType: newpatchset