1 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.


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>
---
M ichspi.c
1 file changed, 56 insertions(+), 3 deletions(-)

diff --git a/ichspi.c b/ichspi.c
index 29dcd25..dd5a2c1 100644
--- a/ichspi.c
+++ b/ichspi.c
@@ -1360,6 +1360,12 @@
 
 	/* 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 @@
 	}
 	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 @@
 
 	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 @@
 
 	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;

