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 _spi_write256() as documented

Turns out broken erasures highlighted some of the issues
in the write256 implementation. After a fair amount of
time deciphering scarce documentation details a correct
implementation was finally derived.

V.2: Rename 'start_program() -> execute_write()' to
clarify the intention and not to overload the term
'program' since the MST actually runs a 'program'
itself.

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 &&
flashrom -p realtek_mst_i2c_spi:bus=8 -r foo &&
hexdump -C foo

Change-Id: If61ff95697f886d3301a907b76283322c39ef5c7
Signed-off-by: Edward O'Callaghan <quasisec@google.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/41080
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, 50 insertions(+), 22 deletions(-)

diff --git a/realtek_mst_i2c_spi.c b/realtek_mst_i2c_spi.c
index 682d4a6..2117c3c 100644
--- a/realtek_mst_i2c_spi.c
+++ b/realtek_mst_i2c_spi.c
@@ -33,6 +33,8 @@

#define MCU_MODE 0x6F
#define ENTER_ISP_MODE 0x80
+#define START_WRITE_XFER 0xA0
+#define WRITE_XFER_STATUS_MASK 0x20

#define MCU_DATA_PORT 0x70

@@ -94,14 +96,14 @@
return ret ? SPI_GENERIC_ERROR : 0;
}

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

if (tried == MAX_SPI_WAIT_RETRIES) {
msg_perr("%s: Time out on sending command.\n", __func__);
@@ -125,6 +127,13 @@
return ret;
}

+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);
+ return ret;
+}
+
static int realtek_mst_i2c_spi_reset_mpu(int fd)
{
int ret = 0;
@@ -252,7 +261,7 @@
if (ret)
return ret;

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

@@ -272,6 +281,21 @@
return ret ? SPI_GENERIC_ERROR : 0;
}

+static int realtek_mst_i2c_spi_write_page(int fd, uint8_t reg, const uint8_t *buf, unsigned int len)
+{
+ /**
+ * Using static buffer with maximum possible size,
+ * extra byte is needed for prefixing the data port register at index 0.
+ */
+ uint8_t wbuf[PAGE_SIZE + 1] = { MCU_DATA_PORT };
+ if (len > PAGE_SIZE)
+ return SPI_GENERIC_ERROR;
+
+ memcpy(&wbuf[1], buf, len);
+
+ return realtek_mst_i2c_spi_write_data(fd, REGISTER_ADDRESS, wbuf, len + 1);
+}
+
static int realtek_mst_i2c_spi_read(struct flashctx *flash, uint8_t *buf,
unsigned int start, unsigned int len)
{
@@ -297,7 +321,7 @@
if (ret)
return ret;

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

@@ -335,30 +359,34 @@
if (ret)
return ret;

- start--;
- ret |= realtek_mst_i2c_spi_write_register(fd, 0x60, 0x46); // **
- ret |= realtek_mst_i2c_spi_write_register(fd, 0x61, OPCODE_WRITE);
- uint8_t block_idx = start >> 16;
- uint8_t page_idx = start >> 8;
- uint8_t byte_idx = start;
- ret |= realtek_mst_i2c_spi_map_page(fd, block_idx, page_idx, byte_idx);
- ret |= realtek_mst_i2c_spi_write_register(fd, 0x6a, 0x03);
- ret |= realtek_mst_i2c_spi_write_register(fd, 0x60, 0x47); // **
- if (ret)
- goto fail;
-
- ret = realtek_mst_i2c_spi_wait_command_done(fd, 0x60, 0x01);
- if (ret)
- goto fail;
+ ret |= realtek_mst_i2c_spi_write_register(fd, 0x6D, 0x02); /* write opcode */
+ ret |= realtek_mst_i2c_spi_write_register(fd, 0x71, (PAGE_SIZE - 1)); /* fit len=256 */

for (i = 0; i < len; i += PAGE_SIZE) {
- ret |= realtek_mst_i2c_spi_write_data(fd, REGISTER_ADDRESS,
- (uint8_t *)buf + i, min(len - i, PAGE_SIZE));
+ uint16_t page_len = min(len - i, PAGE_SIZE);
+ if (len - i < PAGE_SIZE)
+ ret |= realtek_mst_i2c_spi_write_register(fd, 0x71, page_len-1);
+ uint8_t block_idx = (start + i) >> 16;
+ uint8_t page_idx = (start + i) >> 8;
+ ret |= realtek_mst_i2c_spi_map_page(fd, block_idx, page_idx, 0);
+ if (ret)
+ break;
+
+ /* Wait for empty buffer. */
+ ret |= realtek_mst_i2c_spi_wait_command_done(fd, MCU_MODE, 0x10, 0x10);
+ if (ret)
+ break;
+
+ ret |= realtek_mst_i2c_spi_write_page(fd, MCU_DATA_PORT,
+ buf + i, page_len);
+ if (ret)
+ break;
+ ret |= realtek_mst_i2c_execute_write(fd);
if (ret)
break;
}

-fail:
+
/* TODO: re-enable the write protection? */

return ret;

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

Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: If61ff95697f886d3301a907b76283322c39ef5c7
Gerrit-Change-Number: 41080
Gerrit-PatchSet: 6
Gerrit-Owner: Edward O'Callaghan <quasisec@chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com>
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-CC: Paul Menzel <paulepanter@users.sourceforge.net>
Gerrit-MessageType: merged