[flashrom-gerrit] Change in flashrom[master]: realtek_mst_i2c_spi.c: Fix _spi_write256() as documented

[Edward O'Callaghan (Code Review)]

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/41080 )

Change subject: realtek_mst_i2c_spi.c: Fix _spi_write256() as documented
......................................................................

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 documention details a correct
implementation was finally derived.

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 
---
M realtek_mst_i2c_spi.c
1 file changed, 51 insertions(+), 22 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/80/41080/1

diff --git a/realtek_mst_i2c_spi.c b/realtek_mst_i2c_spi.c
index 682d4a6..6a3bac5 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_PROGRAM  0xA0
+#define 	PROGRAM_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_start_program(int fd)
+{
+	int ret = realtek_mst_i2c_spi_write_register(fd, MCU_MODE, START_PROGRAM);
+	ret |= realtek_mst_i2c_spi_wait_command_done(fd, MCU_MODE, PROGRAM_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 zero at index 0.
+	 */
+	uint8_t wbuf[PAGE_SIZE + 1] = { 0x70 };
+	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,35 @@
    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_start_program(fd);
+		if (ret) {
+			break;
+		}
    }

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

return ret;