2 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.


ichspi: Clear Fast SPI HSFC register before HW seq operation

This patch fixes a regression introduced with
commit 7ed1337309d3fe74f5af09520970f0f1d417399a (ichspi: Factor out
common hwseq_xfer logic into helpers).

The reason for the regression is ignoring the fact that the Fast SPI
controller MMIO register HSFC (0x06) might not hold the default zero
value before initiating the HW sequencing operation.

Having a `1b` value in the HSFC.FDBC (bits 24-29) field would represent
a byte that needs to be transfered.

While debugging the regression, we have observed that the default value
in the FDBC (prior to initiate any operation) is 0x3f (instead of
zero) which represents 64-byte transfer.

localhost ~ # iotools mmio_read32 0x92d16006
0x3f00

<Fast SPI MMIO BAR: 0x92d16000 and HSFC offset: 0x06>

FDBC offset during `--wp-disable` operation represents higher numbers of
bytes than the actual and eventually results in the error.

Additionally, dropped unused variable (struct hwseq_data *hwseq_data).

BUG=b:258280679
TEST=Able to build flashrom and perform below operations on Google, Rex
and Google, Kano/Taeko.

Without this patch:

HSFC register value inside ich_start_hwseq_xfer() before initiating
the HW seq operations: 0x3f00
HSFC register value inside ich_start_hwseq_xfer() during the HW seq
operations (Read Status): 0x3f11

With this patch:

HSFC register value inside ich_start_hwseq_xfer() before initiating
the HW seq operations: 0x0
HSFC register value inside ich_start_hwseq_xfer() during the HW seq
operations (Read Status): 0x11

Additionally, verified other HW sequencing operations (like read, write,
erase, read status, write status, read ID) working fine without any
error.

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

diff --git a/ichspi.c b/ichspi.c
index cd330e3..0e2a15b 100644
--- a/ichspi.c
+++ b/ichspi.c
@@ -1349,8 +1349,8 @@
 		uint32_t hsfc_cycle, uint32_t flash_addr, size_t len,
 		uint32_t addr_mask)
 {
-	uint16_t hsfc;
-	struct hwseq_data *hwseq_data = get_hwseq_data_from_context(flash);
+	/* make sure HSFC register is cleared before initiate any operation */
+	uint16_t hsfc = 0;
 
 	/* Sets flash_addr in FADDR */
 	ich_hwseq_set_addr(flash_addr, addr_mask);
@@ -1359,8 +1359,6 @@
 	REGWRITE16(ICH9_REG_HSFS, REGREAD16(ICH9_REG_HSFS));
 
 	/* Set up transaction parameters. */
-	hsfc = REGREAD16(ICH9_REG_HSFC);
-	hsfc &= ~hwseq_data->hsfc_fcycle; /* clear operation */
 	hsfc |= hsfc_cycle;
 	hsfc |= HSFC_FDBC_VAL(len - 1);
 	hsfc |= HSFC_FGO; /* start */

