Edward O'Callaghan submitted this change.

View Change

Approvals: build bot (Jenkins): Verified Sam McNally: Looks good to me, approved
realtek_mst_i2c_spi.c: Fix cmd timeout issue

Chip erasures take much longer than sector and bank
erasures. Allow the wait loop helper to multiply the
max timeout in this very specific case while quickly
timeout for other ops that are expected to be shorter.

V.2: Fix nonsense fall though warn-err

BUG=b:152558985,b:148745673
BRANCH=none
TEST=flashrom -p realtek_mst_i2c_spi:bus=8 -E &&
flashrom -p realtek_mst_i2c_spi:bus=8 -w foo
(cycle)..

Change-Id: I4a36aa3101827e69eb244775d25bbb476d4bb780
Signed-off-by: Edward O'Callaghan <quasisec@google.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/41123
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Sam McNally <sammc@google.com>
---
M realtek_mst_i2c_spi.c
1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/realtek_mst_i2c_spi.c b/realtek_mst_i2c_spi.c
index 2117c3c..e9ee576 100644
--- a/realtek_mst_i2c_spi.c
+++ b/realtek_mst_i2c_spi.c
@@ -96,21 +96,22 @@
return ret ? SPI_GENERIC_ERROR : 0;
}

-static int realtek_mst_i2c_spi_wait_command_done(int fd, unsigned int offset, int mask, int target)
+static int realtek_mst_i2c_spi_wait_command_done(int fd, unsigned int offset, int mask,
+ int target, int multiplier)
{
uint8_t val;
int tried = 0;
int ret = 0;
do {
ret |= realtek_mst_i2c_spi_read_register(fd, offset, &val);
- } while (!ret && ((val & mask) != target) && ++tried < MAX_SPI_WAIT_RETRIES);
+ } while (!ret && ((val & mask) != target) && ++tried < (MAX_SPI_WAIT_RETRIES*multiplier));

if (tried == MAX_SPI_WAIT_RETRIES) {
msg_perr("%s: Time out on sending command.\n", __func__);
return -MAX_SPI_WAIT_RETRIES;
}

- return (val & mask) ? SPI_GENERIC_ERROR : ret;
+ return (val & mask) != target ? SPI_GENERIC_ERROR : ret;
}

static int realtek_mst_i2c_spi_enter_isp_mode(int fd)
@@ -130,7 +131,7 @@
static int realtek_mst_i2c_execute_write(int fd)
{
int ret = realtek_mst_i2c_spi_write_register(fd, MCU_MODE, START_WRITE_XFER);
- ret |= realtek_mst_i2c_spi_wait_command_done(fd, MCU_MODE, WRITE_XFER_STATUS_MASK, 0);
+ ret |= realtek_mst_i2c_spi_wait_command_done(fd, MCU_MODE, WRITE_XFER_STATUS_MASK, 0, 1);
return ret;
}

@@ -202,6 +203,7 @@
unsigned char *readarr)
{
unsigned i;
+ int max_timeout_mul = 1;
int ret = 0;

if (writecnt > 4 || readcnt > 3 || writecnt == 0) {
@@ -239,8 +241,10 @@
ctrl_reg_val |= (2 << 5);
break;
/* Erasures require BIT7 && BIT5 set. */
- case JEDEC_CE_60:
case JEDEC_CE_C7:
+ max_timeout_mul *= 20; /* chip erasures take much longer! */
+ /* FALLTHRU */
+ case JEDEC_CE_60:
case JEDEC_BE_52:
case JEDEC_BE_D8:
case JEDEC_BE_D7:
@@ -261,7 +265,7 @@
if (ret)
return ret;

- ret = realtek_mst_i2c_spi_wait_command_done(fd, 0x60, 0x01, 0);
+ ret = realtek_mst_i2c_spi_wait_command_done(fd, 0x60, 0x01, 0, max_timeout_mul);
if (ret)
return ret;

@@ -321,7 +325,7 @@
if (ret)
return ret;

- ret = realtek_mst_i2c_spi_wait_command_done(fd, 0x60, 0x01, 0);
+ ret = realtek_mst_i2c_spi_wait_command_done(fd, 0x60, 0x01, 0, 1);
if (ret)
return ret;

@@ -373,7 +377,7 @@
break;

/* Wait for empty buffer. */
- ret |= realtek_mst_i2c_spi_wait_command_done(fd, MCU_MODE, 0x10, 0x10);
+ ret |= realtek_mst_i2c_spi_wait_command_done(fd, MCU_MODE, 0x10, 0x10, 1);
if (ret)
break;


To view, visit change 41123. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I4a36aa3101827e69eb244775d25bbb476d4bb780
Gerrit-Change-Number: 41123
Gerrit-PatchSet: 3
Gerrit-Owner: Edward O'Callaghan <quasisec@chromium.org>
Gerrit-Reviewer: Edward Hill <ecgh@chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org>
Gerrit-Reviewer: Sam McNally <sammc@google.com>
Gerrit-Reviewer: Shiyu Sun <sshiyu@google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org>
Gerrit-MessageType: merged