summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorSubrata Banik <subratabanik@google.com>2022-11-18 16:43:16 +0530
committerFelix Singer <felixsinger@posteo.net>2022-12-01 10:25:36 +0000
commitc37b38b7706348e6cd4e45f5f6ceb4041aea72e0 (patch)
tree0d92436b2c93bcc1cf92ee49aa08175cfbdaaff0
parent5dbd1b3608ca3057a8d4e5f8e0f58c2466fc1bbc (diff)
downloadflashrom-c37b38b7706348e6cd4e45f5f6ceb4041aea72e0.tar.gz
flashrom-c37b38b7706348e6cd4e45f5f6ceb4041aea72e0.tar.bz2
flashrom-c37b38b7706348e6cd4e45f5f6ceb4041aea72e0.zip
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) Original-Signed-off-by: Subrata Banik <subratabanik@google.com> Original-Reviewed-on: https://review.coreboot.org/c/flashrom/+/69789 Original-Reviewed-by: Angel Pons <th3fanbus@gmail.com> Original-Reviewed-by: Edward O'Callaghan <quasisec@chromium.org> Original-Reviewed-by: Nikolai Artemiev <nartemiev@google.com> Original-Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Change-Id: I5b911655649c693e576497520687d7810bbd3c54 Signed-off-by: Felix Singer <felixsinger@posteo.net> Reviewed-on: https://review.coreboot.org/c/flashrom/+/70039 Reviewed-by: Anastasia Klimchuk <aklm@chromium.org> Reviewed-by: Edward O'Callaghan <quasisec@chromium.org> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
-rw-r--r--ichspi.c12
1 files changed, 9 insertions, 3 deletions
diff --git a/ichspi.c b/ichspi.c
index 29dcd2550..dd5a2c174 100644
--- a/ichspi.c
+++ b/ichspi.c
@@ -1360,6 +1360,12 @@ static void ich_start_hwseq_xfer(const struct flashctx *flash,
/* Set up transaction parameters. */
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);
@@ -1399,7 +1405,7 @@ static int ich_hwseq_read_status(const struct flashctx *flash, enum flash_reg re
}
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;
@@ -1422,7 +1428,7 @@ static int ich_hwseq_write_status(const struct flashctx *flash, enum flash_reg r
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;
@@ -1518,7 +1524,7 @@ static int ich_hwseq_block_erase(struct flashctx *flash, unsigned int addr,
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;