May 2020 - flashrom-gerrit - mail.coreboot.org

Change in flashrom[master]: pci2: add device list printing support
by Angel Pons (Code Review) May 11, 2020

May 11, 2020

Angel Pons has uploaded a new patch set (#3) to the change originally created by Luc Verhaegen. ( https://review.coreboot.org/c/flashrom/+/29083 ) Change subject: pci2: add device list printing support ...................................................................... pci2: add device list printing support Change-Id: I3eea9edfcdf2ae1c35f4c935dca97fa93eaded58 Signed-off-by: Luc Verhaegen <libv(a)skynet.be> --- M flashrom.c M print.c M programmer.h 3 files changed, 15 insertions(+), 7 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/83/29083/3 -- To view, visit https://review.coreboot.org/c/flashrom/+/29083 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3eea9edfcdf2ae1c35f4c935dca97fa93eaded58 Gerrit-Change-Number: 29083 Gerrit-PatchSet: 3 Gerrit-Owner: Luc Verhaegen <libv(a)skynet.be> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: ati: initial commit
by Angel Pons (Code Review) May 11, 2020

May 11, 2020

Angel Pons has uploaded a new patch set (#4) to the change originally created by Luc Verhaegen. ( https://review.coreboot.org/c/flashrom/+/29085 ) Change subject: ati: initial commit ...................................................................... ati: initial commit Change-Id: Ia5f755ae7a0723372933216516625bc156b1f4bc Signed-off-by: Luc Verhaegen <libv(a)skynet.be> --- M Makefile A ati_spi.c M programmer.h 3 files changed, 58 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/85/29085/4 -- To view, visit https://review.coreboot.org/c/flashrom/+/29085 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia5f755ae7a0723372933216516625bc156b1f4bc Gerrit-Change-Number: 29085 Gerrit-PatchSet: 4 Gerrit-Owner: Luc Verhaegen <libv(a)skynet.be> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: ati: add r600_spi_command and r600_spi_master
by Angel Pons (Code Review) May 11, 2020

May 11, 2020

Angel Pons has uploaded a new patch set (#4) to the change originally created by Luc Verhaegen. ( https://review.coreboot.org/c/flashrom/+/29090 ) Change subject: ati: add r600_spi_command and r600_spi_master ...................................................................... ati: add r600_spi_command and r600_spi_master Change-Id: I3d0bc682898f8eadadfe1f6db0d1383eb7639054 Signed-off-by: Luc Verhaegen <libv(a)skynet.be> --- M ati_spi.c M programmer.h 2 files changed, 121 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/90/29090/4 -- To view, visit https://review.coreboot.org/c/flashrom/+/29090 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3d0bc682898f8eadadfe1f6db0d1383eb7639054 Gerrit-Change-Number: 29090 Gerrit-PatchSet: 4 Gerrit-Owner: Luc Verhaegen <libv(a)skynet.be> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: spi25: Debug flashrom crash when Write Protect is ON
by Edward O'Callaghan (Code Review) May 10, 2020

May 10, 2020

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/40468 ) Change subject: spi25: Debug flashrom crash when Write Protect is ON ...................................................................... spi25: Debug flashrom crash when Write Protect is ON When hardware write protect is applied, flashrom crashed and generate coredump. spi_disable_blockprotect_generic() calls flash->chip->printlock() method when disable was failed, but this method is optional, can be NULL depends on type of flashrom chip. NULL pointer check before call is added to avoid crash. BRANCH=none BUG=b:129083894 TEST=Run on Mistral P2 (On CR50 console, run "wp disable") flashrom --wp-range 0 0x400000 flashrom --wp-enable (On CR50 console, run "wp enable") flashrom -r /tmp/test.bin Verify "Block protection could not be disabled!" is shown, but flash read completes. Signed-off-by: Yuji Sasaki <sasakiy(a)chromium.org> Change-Id: I81094ab5f16a85871fc9869a2e285eddbbbdec4e Reviewed-on: https://chromium-review.googlesource.com/1535140 Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator(a)appspot.gserviceaccount.com> Tested-by: Edward O'Callaghan <quasisec(a)chromium.org> Reviewed-by: Stefan Reinauer <reinauer(a)google.com> Reviewed-by: SANTHOSH JANARDHANA HASSAN <sahassan(a)google.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/40468 Reviewed-by: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M spi25_statusreg.c 1 file changed, 2 insertions(+), 1 deletion(-) Approvals: build bot (Jenkins): Verified Stefan Reinauer: Looks good to me, approved diff --git a/spi25_statusreg.c b/spi25_statusreg.c index 4b9b2a9..34f9ad4 100644 --- a/spi25_statusreg.c +++ b/spi25_statusreg.c @@ -168,7 +168,8 @@ status = spi_read_status_register(flash); if ((status & bp_mask) != 0) { msg_cerr("Block protection could not be disabled!\n"); - flash->chip->printlock(flash); + if (flash->chip->printlock) + flash->chip->printlock(flash); return 1; } msg_cdbg("disabled.\n"); -- To view, visit https://review.coreboot.org/c/flashrom/+/40468 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I81094ab5f16a85871fc9869a2e285eddbbbdec4e Gerrit-Change-Number: 40468 Gerrit-PatchSet: 5 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: SANTHOSH JANARDHANA HASSAN <sahassan(a)google.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Yuji Sasaki <sasakiy(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-CC: Yuji Sasaki <sasakiy(a)google.com> Gerrit-MessageType: merged

1 0

Change in flashrom[master]: realtek_mst_i2c_spi.c: Show MCU running fw version
by Edward O'Callaghan (Code Review) May 8, 2020

May 8, 2020

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/41158 ) Change subject: realtek_mst_i2c_spi.c: Show MCU running fw version ...................................................................... realtek_mst_i2c_spi.c: Show MCU running fw version NOT-FOR-MERGE yet. Change-Id: I85eafd95833f7a280c46947e280f967489d1b8d1 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M realtek_mst_i2c_spi.c 1 file changed, 26 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/58/41158/1 diff --git a/realtek_mst_i2c_spi.c b/realtek_mst_i2c_spi.c index 5c1b901..68ee1a7 100644 --- a/realtek_mst_i2c_spi.c +++ b/realtek_mst_i2c_spi.c @@ -146,6 +146,28 @@ return ret; } +static int realtek_mst_i2c_spi_version(int fd) +{ + int ret = 0; + + uint8_t command[] = { 0x70, 0x04 }; + ret = realtek_mst_i2c_spi_write_data(fd, REGISTER_ADDRESS, command, 2); + uint8_t buf[3]; + ret |= realtek_mst_i2c_spi_read_data(fd, REGISTER_ADDRESS, buf, 3); + printf("(flash): projectID = %d\n", ((buf[1] & 0xC0) >> 6) + (buf[0] << 2)); + printf("(flash): version = %d.%d\n", (buf[1] & 0x3F), buf[2]); + + uint8_t x0, x1, x2; + ret |= realtek_mst_i2c_spi_read_register(fd, 0x03, &x0); + ret |= realtek_mst_i2c_spi_read_register(fd, 0x04, &x1); + ret |= realtek_mst_i2c_spi_read_register(fd, 0x07, &x2); + + printf("(reg): projectID = %d\n", ((x1 & 0xC0) >> 6) + (x0 << 2)); + printf("(reg): version = %d.%d\n", (x1 & 0x3F), x2); + + return ret; +} + static int realtek_mst_i2c_spi_toggle_protection(int fd, int enable) { int ret = 0; @@ -461,6 +483,10 @@ if (ret) return ret; + ret |= realtek_mst_i2c_spi_version(fd); + if (ret) + return ret; + struct realtek_mst_i2c_spi_data *data = calloc(1, sizeof(struct realtek_mst_i2c_spi_data)); if (!data) { msg_perr("Unable to allocate space for extra SPI master data.\n"); -- To view, visit https://review.coreboot.org/c/flashrom/+/41158 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I85eafd95833f7a280c46947e280f967489d1b8d1 Gerrit-Change-Number: 41158 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

1 1

Change in flashrom[master]: Subject: raiden_debug_spi.c: Disable USB retry during some error codes
by Brian Nemec (Code Review) May 8, 2020

May 8, 2020

Hello Brian Nemec, I'd like you to do a code review. Please visit https://review.coreboot.org/c/flashrom/+/41152 to review the following change. Change subject: Subject: raiden_debug_spi.c: Disable USB retry during some error codes ...................................................................... Subject: raiden_debug_spi.c: Disable USB retry during some error codes Enables the USB SPI transfer retry mechanism when the write count error code is returned. This error code can indicate a recoverable transfer failure. BUG=b:153887087 BRANCH=none TEST=Manual testing of ServoMicro and Flashrom when performing reads, writes, and verifications of the EC firmware on Nami. TEST=Modified servo micro to produce periodic errors when reading the packet length to verify the recovery is successful. Change-Id: I9e6b2ccec0b06aab0d6920f1bddf108058e5d6b1 --- M raiden_debug_spi.c 1 file changed, 7 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/52/41152/1 diff --git a/raiden_debug_spi.c b/raiden_debug_spi.c index 2127f69..ac74c0b 100644 --- a/raiden_debug_spi.c +++ b/raiden_debug_spi.c @@ -188,9 +188,13 @@ static bool retry_recovery(int error_code) { if (error_code < 0x10000) { - /* Handle error codes returned from the device. */ - if (USB_SPI_WRITE_COUNT_INVALID <= error_code && - error_code <= USB_SPI_DISABLED) { + /* + * Handle error codes returned from the device. USB_SPI_TIMEOUT, + * USB_SPI_BUSY, and USB_SPI_WRITE_COUNT_INVALID have been observed + * during transfer errors to the device and can be recovered. + */ + if (USB_SPI_READ_COUNT_INVALID <= error_code && + error_code <= USB_SPI_DISABLED) { return false; } } else if (usb_device_is_libusb_error(error_code)) { -- To view, visit https://review.coreboot.org/c/flashrom/+/41152 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9e6b2ccec0b06aab0d6920f1bddf108058e5d6b1 Gerrit-Change-Number: 41152 Gerrit-PatchSet: 1 Gerrit-Owner: Brian Nemec <bnemec(a)google.com> Gerrit-Reviewer: Brian Nemec <bnemec(a)chromium.org> Gerrit-MessageType: newchange

3 6

Change in flashrom[master]: realtek_mst_i2c_spi.c: Remove dead code
by Edward O'Callaghan (Code Review) May 7, 2020

May 7, 2020

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/41124 ) Change subject: realtek_mst_i2c_spi.c: Remove dead code ...................................................................... realtek_mst_i2c_spi.c: Remove dead code Turns out the MST likely doesn't need these so-called defaults to be written for the purposes of spi flashing. BUG=b:152558985,b:148745673 BRANCH=none TEST=builds Change-Id: Ieb938cf0805b22692d61db23795208c9be962b60 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M realtek_mst_i2c_spi.c 1 file changed, 0 insertions(+), 33 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/24/41124/1 diff --git a/realtek_mst_i2c_spi.c b/realtek_mst_i2c_spi.c index 863ace5..64458dd 100644 --- a/realtek_mst_i2c_spi.c +++ b/realtek_mst_i2c_spi.c @@ -145,33 +145,6 @@ return ret; } -#if 0 -static int realtek_mst_i2c_spi_set_defaults(int fd) -{ - // 0xFF1B = 0x02; - int ret = realtek_mst_i2c_spi_write_register(fd, 0x1B, 0x02); - ret = realtek_mst_i2c_spi_write_register(fd, 0x1C, 0x30); - ret = realtek_mst_i2c_spi_write_register(fd, 0x1D, 0x1C); - ret = realtek_mst_i2c_spi_write_register(fd, 0x1E, 0x02); - ret = realtek_mst_i2c_spi_write_register(fd, 0x1F, 0x00); - - ret = realtek_mst_i2c_spi_write_register(fd, 0x20, 0x1C); - ret = realtek_mst_i2c_spi_write_register(fd, 0x2C, 0x02); - ret = realtek_mst_i2c_spi_write_register(fd, 0x2D, 0x00); - ret = realtek_mst_i2c_spi_write_register(fd, 0x2E, 0x1C); - - ret = realtek_mst_i2c_spi_write_register(fd, 0x62, 0x06); - ret = realtek_mst_i2c_spi_write_register(fd, 0x6A, 0x03); - ret = realtek_mst_i2c_spi_write_register(fd, 0x6B, 0x0B); - ret = realtek_mst_i2c_spi_write_register(fd, 0x6C, 0x00); - - ret = realtek_mst_i2c_spi_write_register(fd, 0xED, 0x88); - ret = realtek_mst_i2c_spi_write_register(fd, 0xEE, 0x04); - - return ret; -} -#endif - static int realtek_mst_i2c_spi_disable_protection(int fd) { int ret = 0; @@ -484,12 +457,6 @@ ret |= realtek_mst_i2c_spi_enter_isp_mode(fd); if (ret) return ret; -// XXX: maybe make into a mode:defaults cli param? -#if 0 - ret |= realtek_mst_i2c_spi_set_defaults(fd); - if (ret) - return ret; -#endif struct realtek_mst_i2c_spi_data *data = calloc(1, sizeof(struct realtek_mst_i2c_spi_data)); if (!data) { -- To view, visit https://review.coreboot.org/c/flashrom/+/41124 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ieb938cf0805b22692d61db23795208c9be962b60 Gerrit-Change-Number: 41124 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

2 2

Change in flashrom[master]: realtek_mst_i2c_spi.c: Fix cmd timeout issue
by Edward O'Callaghan (Code Review) May 7, 2020

May 7, 2020

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/41123 ) Change subject: realtek_mst_i2c_spi.c: Fix cmd timeout issue ...................................................................... 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. 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(a)google.com> --- M realtek_mst_i2c_spi.c 1 file changed, 11 insertions(+), 8 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/23/41123/1 diff --git a/realtek_mst_i2c_spi.c b/realtek_mst_i2c_spi.c index 2117c3c..863ace5 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,9 @@ 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! */ + case JEDEC_CE_60: case JEDEC_BE_52: case JEDEC_BE_D8: case JEDEC_BE_D7: @@ -261,7 +264,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 +324,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 +376,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 https://review.coreboot.org/c/flashrom/+/41123 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I4a36aa3101827e69eb244775d25bbb476d4bb780 Gerrit-Change-Number: 41123 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

2 3

Change in flashrom[master]: realtek_mst_i2c_spi.c: Fix wp gpio toggle
by Edward O'Callaghan (Code Review) May 7, 2020

May 7, 2020

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/41125 ) Change subject: realtek_mst_i2c_spi.c: Fix wp gpio toggle ...................................................................... realtek_mst_i2c_spi.c: Fix wp gpio toggle Allow for toggling the GPIO pin 108 that is routed to the WP pin in the reference design. This need not always apply however is largely harmless for a better out-of-the-box experience. BUG=b:152558985,b:148745673 BRANCH=none TEST=builds && A double cycle of erase+write works. Change-Id: I825df3179cdd16f7c7ecf556b5ca29bfe6394346 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M realtek_mst_i2c_spi.c 1 file changed, 8 insertions(+), 6 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/25/41125/1 diff --git a/realtek_mst_i2c_spi.c b/realtek_mst_i2c_spi.c index 64458dd..6cb87fe 100644 --- a/realtek_mst_i2c_spi.c +++ b/realtek_mst_i2c_spi.c @@ -34,6 +34,7 @@ #define MCU_MODE 0x6F #define ENTER_ISP_MODE 0x80 #define START_WRITE_XFER 0xA0 +#define BUF_XFER_STATUS_MASK 0x10 #define WRITE_XFER_STATUS_MASK 0x20 #define MCU_DATA_PORT 0x70 @@ -145,7 +146,7 @@ return ret; } -static int realtek_mst_i2c_spi_disable_protection(int fd) +static int realtek_mst_i2c_spi_toggle_protection(int fd, int enable) { int ret = 0; uint8_t val = 0; @@ -164,8 +165,9 @@ ret |= realtek_mst_i2c_spi_write_register(fd, 0xF5, (val & 0xF8) | 0x01); /* Set pin value to high, 0xFFD7[0] = 1. */ + uint8_t en = enable ? 0x00 : 0x01; ret |= realtek_mst_i2c_spi_read_register(fd, 0xD7, &val); - ret |= realtek_mst_i2c_spi_write_register(fd, 0xD7, (val & 0xFE) | 0x01); + ret |= realtek_mst_i2c_spi_write_register(fd, 0xD7, (val & 0xFE) | en); return ret; } @@ -331,7 +333,7 @@ if (fd < 0) return SPI_GENERIC_ERROR; - ret = realtek_mst_i2c_spi_disable_protection(fd); + ret = realtek_mst_i2c_spi_toggle_protection(fd, 0); if (ret) return ret; @@ -349,7 +351,8 @@ break; /* Wait for empty buffer. */ - ret |= realtek_mst_i2c_spi_wait_command_done(fd, MCU_MODE, 0x10, 0x10, 1); + ret |= realtek_mst_i2c_spi_wait_command_done(fd, MCU_MODE, + BUF_XFER_STATUS_MASK, 0x10, 1); if (ret) break; @@ -362,8 +365,7 @@ break; } - - /* TODO: re-enable the write protection? */ + ret |= realtek_mst_i2c_spi_toggle_protection(fd, 1); return ret; } -- To view, visit https://review.coreboot.org/c/flashrom/+/41125 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I825df3179cdd16f7c7ecf556b5ca29bfe6394346 Gerrit-Change-Number: 41125 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

2 2

Change in flashrom[master]: realtek_mst_i2c_spi.c: Fix _spi_write256() as documented
by Edward O'Callaghan (Code Review) May 6, 2020

May 6, 2020

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 <quasisec(a)google.com> --- 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; -- To view, visit https://review.coreboot.org/c/flashrom/+/41080 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If61ff95697f886d3301a907b76283322c39ef5c7 Gerrit-Change-Number: 41080 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

3 11