[flashrom-gerrit] [M] Change in flashrom[1.3.x]: ichspi: Clear Fast SPI HSFC register before HW seq operation

[Felix Singer (Code Review)]

Felix Singer has submitted this change. ( https://review.coreboot.org/c/flashrom/+/69998 )

(

1 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.
 )Change subject: ichspi: Clear Fast SPI HSFC register before HW seq operation
......................................................................

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.

Original-Signed-off-by: Subrata Banik 
Original-Reviewed-on: https://review.coreboot.org/c/flashrom/+/69788
Original-Tested-by: build bot (Jenkins) 
Original-Reviewed-by: Edward O'Callaghan 
Original-Reviewed-by: Nikolai Artemiev

Change-Id: I4cc3f24f880d1d621f1f48a6e6b276449fa46f98
Signed-off-by: Felix Singer 
Reviewed-on: https://review.coreboot.org/c/flashrom/+/69998
Reviewed-by: Anastasia Klimchuk 
Reviewed-by: Edward O'Callaghan 
Tested-by: build bot (Jenkins) 
---
M ichspi.c
1 file changed, 69 insertions(+), 4 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Edward O'Callaghan: Looks good to me, approved
  Anastasia Klimchuk: Looks good to me, but someone else must approve

diff --git a/ichspi.c b/ichspi.c
index cd6e802..29dcd25 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 */