From 5dbd1b3608ca3057a8d4e5f8e0f58c2466fc1bbc Mon Sep 17 00:00:00 2001 From: Subrata Banik Date: Thu, 17 Nov 2022 21:20:13 +0530 Subject: 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 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) --- ichspi.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/ichspi.c b/ichspi.c index cd6e802a..29dcd255 100644 --- a/ichspi.c +++ b/ichspi.c @@ -1349,8 +1349,8 @@ static void ich_start_hwseq_xfer(const struct flashctx *flash, 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 @@ static void ich_start_hwseq_xfer(const struct flashctx *flash, 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 */ -- cgit v1.2.1