Felix Singer has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/69986 )
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 subratabanik@google.com Original-Change-Id: I4cc3f24f880d1d621f1f48a6e6b276449fa46f98 Original-Reviewed-on: https://review.coreboot.org/c/flashrom/+/69788 Original-Tested-by: build bot (Jenkins) no-reply@coreboot.org Original-Reviewed-by: Edward O'Callaghan quasisec@chromium.org Original-Reviewed-by: Nikolai Artemiev nartemiev@google.com
Change-Id: I1b77e563f67ad2793983b65e694be60c85f126c0 Signed-off-by: Felix Singer felixsinger@posteo.net --- M ichspi.c 1 file changed, 66 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/86/69986/1
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 */