Edward O'Callaghan submitted this change.

View Change

Approvals: build bot (Jenkins): Verified Edward O'Callaghan: Looks good to me, approved
ichspi.c: Check SPI Cycle In-Progress prior start HW Seq

As per EDS, SPI controller sets the HSFSTS.bit5 (SCIP) when software
sets the Flash Cycle Go (FGO) bit in the Hardware Sequencing Flash
Control register.

This bit remains set until the cycle completes on the SPI interface.
Hardware automatically sets and clears this bit. Software must initiate
the next SPI transaction when this bit is 0.

Platform Setup: Alder Lake based ChromeOS devices (Brya variants)

Replication Steps: Accepting and running firmware Auto Update (AU) on
the Brya variants (dogfooder system) is seeing `flashrom` getting timed
out.

Problem Statement:
Evidencing AU (Auto Update) failure while performing firmware update on
the Alder Lake based ChromeOS devices.

Observation:
Based on the initial understanding from the failure log/pattern, it
seems like the platform is evidencing multiple `flashrom` access from
different source, for example: `futility` accesses flashrom for erase,
write and read operation, `crossystem` uses flashrom for updating VBNV,
additionally, `set_fw_good` script also uses `crossystem` to update the
fw status.

Solution:
Without this SCIP check being implemented in flashrom, there is no
way to ensure multiple instances of flashrom performing different SPI
operations are not cancelling each other and running into below error:

Erasing and writing flash chip... Timeout error between offset
0x0061c000 and 0x0061c03f (= 0x0061c000 + 63)! FAILED!
Uh oh. Erase/write failed. Checking if anything has changed.

TEST=Able to flash coreboot image on Alder Lake (Brya variants), Tiger
Lake (Volteer variants), Kaby Lake (Eve system), Comet Lake (Hatch
variants) and Ivybridge without any failure.

Signed-off-by: Subrata Banik <subratabanik@google.com>
Change-Id: Ib9265cc20513fd00f32f8fa22e28c312903ca484
Reviewed-on: https://review.coreboot.org/c/flashrom/+/61854
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Edward O'Callaghan <quasisec@chromium.org>
---
M ichspi.c
1 file changed, 17 insertions(+), 0 deletions(-)

diff --git a/ichspi.c b/ichspi.c
index 84ac035..9f45ec2 100644
--- a/ichspi.c
+++ b/ichspi.c
@@ -1402,6 +1402,11 @@
/* make sure FDONE, FCERR, AEL are cleared by writing 1 to them */
REGWRITE16(ICH9_REG_HSFS, REGREAD16(ICH9_REG_HSFS));

+ if (REGREAD8(ICH9_REG_HSFS) & HSFS_SCIP) {
+ msg_perr("Error: SCIP bit is unexpectedly set.\n");
+ return -1;
+ }
+
hsfc = REGREAD16(ICH9_REG_HSFC);
hsfc &= ~hwseq_data.hsfc_fcycle; /* clear operation */
hsfc |= (0x3 << HSFC_FCYCLE_OFF); /* set erase operation */
@@ -1439,6 +1444,12 @@
block_len = min(block_len, 256 - (addr & 0xFF));

ich_hwseq_set_addr(addr);
+
+ if (REGREAD8(ICH9_REG_HSFS) & HSFS_SCIP) {
+ msg_perr("Error: SCIP bit is unexpectedly set.\n");
+ return -1;
+ }
+
hsfc = REGREAD16(ICH9_REG_HSFC);
hsfc &= ~hwseq_data.hsfc_fcycle; /* set read operation */
hsfc &= ~HSFC_FDBC; /* clear byte count */
@@ -1480,6 +1491,12 @@
/* as well as flash chip page borders as demanded in the Intel datasheets. */
block_len = min(block_len, 256 - (addr & 0xFF));
ich_fill_data(buf, block_len, ICH9_REG_FDATA0);
+
+ if (REGREAD8(ICH9_REG_HSFS) & HSFS_SCIP) {
+ msg_perr("Error: SCIP bit is unexpectedly set.\n");
+ return -1;
+ }
+
hsfc = REGREAD16(ICH9_REG_HSFC);
hsfc &= ~hwseq_data.hsfc_fcycle; /* clear operation */
hsfc |= (0x2 << HSFC_FCYCLE_OFF); /* set write operation */

To view, visit change 61854. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ib9265cc20513fd00f32f8fa22e28c312903ca484
Gerrit-Change-Number: 61854
Gerrit-PatchSet: 17
Gerrit-Owner: Subrata Banik <subratabanik@google.com>
Gerrit-Reviewer: Alex Levin <levinale@chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com>
Gerrit-Reviewer: Boris Mittelberg <bmbm@google.com>
Gerrit-Reviewer: Caveh Jalali <caveh@chromium.org>
Gerrit-Reviewer: Damien Zammit
Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org>
Gerrit-Reviewer: Martin Roth <martinroth@google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro@google.com>
Gerrit-Reviewer: Nico Huber <nico.h@gmx.de>
Gerrit-Reviewer: Patrick Georgi <patrick@coreboot.org>
Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi@intel.com>
Gerrit-Reviewer: Sridhar Siricilla <sridhar.siricilla@intel.com>
Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer@coreboot.org>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak@chromium.org>
Gerrit-Reviewer: YH Lin <yueherngl@chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org>
Gerrit-CC: David Hendricks <david.hendricks@gmail.com>
Gerrit-CC: Paul Menzel <paulepanter@mailbox.org>
Gerrit-MessageType: merged