Subrata Banik has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/69789 )
Change subject: ichspi: Fix number of bytes for HW seq operations ......................................................................
ichspi: Fix number of bytes for HW seq operations
This patch fixes a potential issue where the SPI controller register HSFC.FDBC (bits 24-29) value gets incorrectly calculated while passing the `len` as `0` instead of `1`.
As per Intel EDS, `0b` in the FDBC represents 1 byte while `0x3f` represents 64-bytes to be transferred. The number of bytes transferred is the value of this field plus 1.
If we would like to transfer 1 byte then we need to set `0b` in FDBC for operations like read, write, flash id as to account for the `set byte count` hence, the `len` argument should be `1`.
Additionally, as per EDS, the FDBC field is ignored for any block erase command.
BUG=b:258280679 TEST=Able to build flashrom and perform below operations on Google, Rex and Google, Kano/Taeko.
During `--wp-disable` HW seq operation that requires 1 byte data transfer.
HSFC.FDBC value while passing `len` as `0` = 0x3f (represents 64-byte)
HSFC.FDBC value while passing `len` as `1` = 0x0 (represents 1-byte)
Signed-off-by: Subrata Banik subratabanik@google.com Change-Id: I5b911655649c693e576497520687d7810bbd3c54 --- M ichspi.c 1 file changed, 45 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/89/69789/1
diff --git a/ichspi.c b/ichspi.c index a3552d3..58b74dc 100644 --- a/ichspi.c +++ b/ichspi.c @@ -1364,6 +1364,12 @@ hsfc = REGREAD16(ICH9_REG_HSFC); hsfc &= ~hwseq_data->hsfc_fcycle; /* clear operation */ hsfc |= hsfc_cycle; + /* + * The number of bytes transferred is the value of `FDBC` plus 1, hence, + * subtracted 1 from the length field. + * As per Intel EDS, `0b` in the FDBC represents 1 byte while `0x3f` + * represents 64-bytes to be transferred. + */ hsfc |= HSFC_FDBC_VAL(len - 1); hsfc |= HSFC_FGO; /* start */ prettyprint_ich9_reg_hsfc(hsfc, ich_generation); @@ -1403,7 +1409,7 @@ } msg_pdbg("Reading Status register\n");
- if (ich_exec_sync_hwseq_xfer(flash, HSFC_CYCLE_RD_STATUS, 0, len, ich_generation, + if (ich_exec_sync_hwseq_xfer(flash, HSFC_CYCLE_RD_STATUS, 1, len, ich_generation, hwseq_data->addr_mask)) { msg_perr("Reading Status register failed\n!!"); return -1; @@ -1426,7 +1432,7 @@
ich_fill_data(&value, len, ICH9_REG_FDATA0);
- if (ich_exec_sync_hwseq_xfer(flash, HSFC_CYCLE_WR_STATUS, 0, len, ich_generation, + if (ich_exec_sync_hwseq_xfer(flash, HSFC_CYCLE_WR_STATUS, 1, len, ich_generation, hwseq_data->addr_mask)) { msg_perr("Writing Status register failed\n!!"); return -1; @@ -1522,7 +1528,7 @@
msg_pdbg("Erasing %d bytes starting at 0x%06x.\n", len, addr);
- if (ich_exec_sync_hwseq_xfer(flash, HSFC_CYCLE_BLOCK_ERASE, addr, 0, ich_generation, + if (ich_exec_sync_hwseq_xfer(flash, HSFC_CYCLE_BLOCK_ERASE, addr, 1, ich_generation, hwseq_data->addr_mask)) return -1; return 0;