flashrom-gerrit October 2020

flashrom-gerrit@flashrom.org

1 participants
86 discussions

Change in flashrom[master]: realtek_mst_i2c_spi.c: Update wp disable function and add param
by Shiyu Sun (Code Review) 14 Oct '20

14 Oct '20

Shiyu Sun has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/46089 ) Change subject: realtek_mst_i2c_spi.c: Update wp disable function and add param ...................................................................... realtek_mst_i2c_spi.c: Update wp disable function and add param The write protection trigger has been updated by the vendor so update here as well. Also added parameter to allower user to decide turn off protection or not before an operation. Change-Id: I1aed0086f917e31bebb51857ad5cce138158fe82 Signed-off-by: Shiyu Sun <sshiyu(a)chromium.org> --- M realtek_mst_i2c_spi.c 1 file changed, 43 insertions(+), 16 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/89/46089/1 diff --git a/realtek_mst_i2c_spi.c b/realtek_mst_i2c_spi.c index d1f7227..2e5872e 100644 --- a/realtek_mst_i2c_spi.c +++ b/realtek_mst_i2c_spi.c @@ -146,27 +146,36 @@ return ret; } -static int realtek_mst_i2c_spi_disable_protection(int fd) +static int realtek_mst_i2c_spi_disable_hw_protection(int fd) { int ret = 0; uint8_t val = 0; - // 0xAB[2:0] = b001 + // Configure Pin to Push-Pull GPIO ret |= realtek_mst_i2c_spi_write_register(fd, 0xF4, 0x9F); ret |= realtek_mst_i2c_spi_write_register(fd, 0xF5, 0x10); - ret |= realtek_mst_i2c_spi_write_register(fd, 0xF4, 0xAB); + ret |= realtek_mst_i2c_spi_write_register(fd, 0xF4, 0x4F); ret |= realtek_mst_i2c_spi_read_register(fd, 0xF5, &val); ret |= realtek_mst_i2c_spi_write_register(fd, 0xF4, 0x9F); ret |= realtek_mst_i2c_spi_write_register(fd, 0xF5, 0x10); - ret |= realtek_mst_i2c_spi_write_register(fd, 0xF4, 0xAB); + ret |= realtek_mst_i2c_spi_write_register(fd, 0xF4, 0x4F); - ret |= realtek_mst_i2c_spi_write_register(fd, 0xF5, (val & 0xF8) | 0x01); + ret |= realtek_mst_i2c_spi_write_register(fd, 0xF5, (val & 0xF0) | 0x01); - /* Set pin value to high, 0xFFD7[0] = 1. */ - ret |= realtek_mst_i2c_spi_read_register(fd, 0xD7, &val); - ret |= realtek_mst_i2c_spi_write_register(fd, 0xD7, (val & 0xFE) | 0x01); + // Set Pin Value to High + ret |= realtek_mst_i2c_spi_write_register(fd, 0xF4, 0x9F); + ret |= realtek_mst_i2c_spi_write_register(fd, 0xF5, 0xFE); + ret |= realtek_mst_i2c_spi_write_register(fd, 0xF4, 0x3F); + + ret |= realtek_mst_i2c_spi_read_register(fd, 0xF5, &val); + + ret |= realtek_mst_i2c_spi_write_register(fd, 0xF4, 0x9F); + ret |= realtek_mst_i2c_spi_write_register(fd, 0xF5, 0xFE); + ret |= realtek_mst_i2c_spi_write_register(fd, 0xF4, 0x3F); + + ret |= realtek_mst_i2c_spi_write_register(fd, 0xF5, (val & 0xFE) | 0x01); return ret; } @@ -333,10 +342,6 @@ if (fd < 0) return SPI_GENERIC_ERROR; - ret = realtek_mst_i2c_spi_disable_protection(fd); - if (ret) - return ret; - 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 */ @@ -404,9 +409,9 @@ return ret; } -static int get_params(int *i2c_bus, int *reset) +static int get_params(int *i2c_bus, int *reset, int *wp_disable) { - char *bus_str = NULL, *reset_str = NULL; + char *bus_str = NULL, *reset_str = NULL, *wp_str = NULL; int ret = SPI_GENERIC_ERROR; bus_str = extract_programmer_param("bus"); @@ -450,6 +455,20 @@ *reset = 0; /* Default behaviour is no MCU reset on tear-down. */ free(reset_str); + wp_str = extract_programmer_param("wp-disable"); + if (wp_str) { + if (wp_str[0] == '1') + *wp_disable = 1; + else if (wp_str[0] == '0') + *wp_disable = 0; + else { + msg_perr("%s: Incorrect param format, wp-disable=1 or 0.\n", __func__); + ret = SPI_GENERIC_ERROR; + } + } else + *wp_disable = 0; /* Default behaviour is not have hardware write protect disabled. */ + free(wp_str); + _get_params_failed: if (bus_str) free(bus_str); @@ -460,9 +479,9 @@ int realtek_mst_i2c_spi_init(void) { int ret = 0; - int i2c_bus = 0, reset = 0; + int i2c_bus = 0, reset = 0, wp_disable = 0; - if (get_params(&i2c_bus, &reset)) + if (get_params(&i2c_bus, &reset, &wp_disable)) return SPI_GENERIC_ERROR; int fd = i2c_open(i2c_bus, REGISTER_ADDRESS, 0); @@ -473,6 +492,14 @@ if (ret) return ret; + if (wp_disable) { + ret |= realtek_mst_i2c_spi_disable_hw_protection(fd); + if (ret != 0) { + msg_perr("%s: failed to disable the write protection.\n", __func__); + 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/+/46089 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I1aed0086f917e31bebb51857ad5cce138158fe82 Gerrit-Change-Number: 46089 Gerrit-PatchSet: 1 Gerrit-Owner: Shiyu Sun <sshiyu(a)google.com> Gerrit-MessageType: newchange

3 34

Change in flashrom[master]: it87spi.c: Fix laying violation of default_spi_read
by Edward O'Callaghan (Code Review) 11 Oct '20

11 Oct '20

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/46233 ) Change subject: it87spi.c: Fix laying violation of default_spi_read ...................................................................... it87spi.c: Fix laying violation of default_spi_read default_spi_read() calls spi_read_chunked() with the correct max_read value of 3 set in the spi master struct. Change-Id: I199c81e1ba501e86dbfb7cf18e2d1556e30db62e Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M it87spi.c 1 file changed, 2 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/33/46233/1 diff --git a/it87spi.c b/it87spi.c index 8224db0..e34f5e8 100644 --- a/it87spi.c +++ b/it87spi.c @@ -108,7 +108,7 @@ unsigned int start, unsigned int len); static const struct spi_master spi_master_it87xx = { - .max_data_read = MAX_DATA_UNSPECIFIED, + .max_data_read = 3, .max_data_write = MAX_DATA_UNSPECIFIED, .command = it8716f_spi_send_command, .multicommand = default_spi_send_multicommand, @@ -380,7 +380,7 @@ * via a programmer parameter for the internal programmer. */ if ((flash->chip->total_size * 1024 > 512 * 1024)) { - spi_read_chunked(flash, buf, start, len, 3); + default_spi_read(flash, buf, start, len); } else { mmio_readn((void *)(flash->virtual_memory + start), buf, len); } -- To view, visit https://review.coreboot.org/c/flashrom/+/46233 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I199c81e1ba501e86dbfb7cf18e2d1556e30db62e Gerrit-Change-Number: 46233 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

3 6

Change in flashrom[master]: dediprog.c: Fix laying violation of default_spi_read
by Edward O'Callaghan (Code Review) 11 Oct '20

11 Oct '20

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/46232 ) Change subject: dediprog.c: Fix laying violation of default_spi_read ...................................................................... dediprog.c: Fix laying violation of default_spi_read default_spi_read() calls spi_read_chunked() with the correct max_read value of 16 set in the spi master struct. Change-Id: Ic0897f74056e3d723a33c063ed0bd8cb6e88ba45 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M dediprog.c 1 file changed, 3 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/32/46232/1 diff --git a/dediprog.c b/dediprog.c index 827d5e4..a9d0034 100644 --- a/dediprog.c +++ b/dediprog.c @@ -545,7 +545,7 @@ if (residue) { msg_pdbg("Slow read for partial block from 0x%x, length 0x%x\n", start, residue); - ret = spi_read_chunked(flash, buf, start, residue, 16); + ret = default_spi_read(flash, buf, start, residue); if (ret) goto err; } @@ -560,8 +560,8 @@ if (len != 0) { msg_pdbg("Slow read for partial block from 0x%x, length 0x%x\n", start, len); - ret = spi_read_chunked(flash, buf + residue + bulklen, - start + residue + bulklen, len, 16); + ret = default_spi_read(flash, buf + residue + bulklen, + start + residue + bulklen, len); if (ret) goto err; } -- To view, visit https://review.coreboot.org/c/flashrom/+/46232 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic0897f74056e3d723a33c063ed0bd8cb6e88ba45 Gerrit-Change-Number: 46232 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

3 6

Change in flashrom[master]: realtek_mst_i2c_spi.c: Fixup get_params() err ctrl flow
by Edward O'Callaghan (Code Review) 02 Oct '20

02 Oct '20

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/45944 ) Change subject: realtek_mst_i2c_spi.c: Fixup get_params() err ctrl flow ...................................................................... realtek_mst_i2c_spi.c: Fixup get_params() err ctrl flow Ensure that when bus number and reset params are specified at the same time are both correctly parsed by get_params(). Also renames the goto err cleanup path to make it clear. Change-Id: Icb45b1ab39181b0f1a2dec1cce549d30db984936 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M realtek_mst_i2c_spi.c 1 file changed, 4 insertions(+), 5 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/44/45944/1 diff --git a/realtek_mst_i2c_spi.c b/realtek_mst_i2c_spi.c index e975290..d1f7227 100644 --- a/realtek_mst_i2c_spi.c +++ b/realtek_mst_i2c_spi.c @@ -416,23 +416,22 @@ int bus = (int)strtol(bus_str, &bus_suffix, 10); if (errno != 0 || bus_str == bus_suffix) { msg_perr("%s: Could not convert 'bus'.\n", __func__); - goto get_params_done; + goto _get_params_failed; } if (bus < 0 || bus > 255) { msg_perr("%s: Value for 'bus' is out of range(0-255).\n", __func__); - goto get_params_done; + goto _get_params_failed; } if (strlen(bus_suffix) > 0) { msg_perr("%s: Garbage following 'bus' value.\n", __func__); - goto get_params_done; + goto _get_params_failed; } msg_pinfo("Using i2c bus %i.\n", bus); *i2c_bus = bus; ret = 0; - goto get_params_done; } else { msg_perr("%s: Bus number not specified.\n", __func__); } @@ -451,7 +450,7 @@ *reset = 0; /* Default behaviour is no MCU reset on tear-down. */ free(reset_str); -get_params_done: +_get_params_failed: if (bus_str) free(bus_str); -- To view, visit https://review.coreboot.org/c/flashrom/+/45944 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Icb45b1ab39181b0f1a2dec1cce549d30db984936 Gerrit-Change-Number: 45944 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

3 8

Change in flashrom[master]: realtek_mst_i2c_spi.c: Introduce MCU reset param
by Edward O'Callaghan (Code Review) 01 Oct '20

01 Oct '20

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/45674 ) Change subject: realtek_mst_i2c_spi.c: Introduce MCU reset param ...................................................................... realtek_mst_i2c_spi.c: Introduce MCU reset param Modify the spi master as to not automatically reset the MCU unless explicitly stated by a param. Change-Id: Ib70bf7399e7541f30b6905cdb950a6fb7b74ae18 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M realtek_mst_i2c_spi.c 1 file changed, 15 insertions(+), 5 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/74/45674/1 diff --git a/realtek_mst_i2c_spi.c b/realtek_mst_i2c_spi.c index 6158524..6ac57d5 100644 --- a/realtek_mst_i2c_spi.c +++ b/realtek_mst_i2c_spi.c @@ -49,6 +49,7 @@ struct realtek_mst_i2c_spi_data { int fd; + int reset; }; static int realtek_mst_i2c_spi_write_data(int fd, uint16_t addr, void *buf, uint16_t len) @@ -392,14 +393,15 @@ struct realtek_mst_i2c_spi_data *realtek_mst_data = (struct realtek_mst_i2c_spi_data *)data; int fd = realtek_mst_data->fd; - ret |= realtek_mst_i2c_spi_reset_mpu(fd); + if (data->reset) + ret |= realtek_mst_i2c_spi_reset_mpu(fd); i2c_close(fd); free(data); return ret; } -static int get_params(int *i2c_bus) +static int get_params(int *i2c_bus, int *reset) { char *bus_str = NULL; int ret = SPI_GENERIC_ERROR; @@ -431,6 +433,12 @@ } else { msg_perr("%s: Bus number not specified.\n", __func__); } + + reset_str = extract_programmer_param("reset-mcu"); + if (reset_str && reset_str[0] == '1') + reset = 1; + free(reset_str); + get_params_done: if (bus_str) free(bus_str); @@ -441,9 +449,9 @@ int realtek_mst_i2c_spi_init(void) { int ret = 0; - int i2c_bus = 0; + int i2c_bus = 0, reset = 0; - if (get_params(&i2c_bus)) + if (get_params(&i2c_bus, &reset)) return SPI_GENERIC_ERROR; int fd = i2c_open(i2c_bus, REGISTER_ADDRESS, 0); @@ -451,7 +459,8 @@ return fd; /* Ensure we are in a known state before entering ISP mode */ - ret |= realtek_mst_i2c_spi_reset_mpu(fd); + if (reset) + ret |= realtek_mst_i2c_spi_reset_mpu(fd); if (ret) return ret; @@ -466,6 +475,7 @@ } data->fd = fd; + data->reset = reset; ret |= register_shutdown(realtek_mst_i2c_spi_shutdown, data); spi_master_i2c_realtek_mst.data = data; -- To view, visit https://review.coreboot.org/c/flashrom/+/45674 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ib70bf7399e7541f30b6905cdb950a6fb7b74ae18 Gerrit-Change-Number: 45674 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

4 30

Change in flashrom[master]: realtek_mst_i2c_spi.c: Remove reset from init fn
by Edward O'Callaghan (Code Review) 01 Oct '20

01 Oct '20

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/45896 ) Change subject: realtek_mst_i2c_spi.c: Remove reset from init fn ...................................................................... realtek_mst_i2c_spi.c: Remove reset from init fn Remove MCU reset on init as this was only introduced when MCU fw requirements for correct flashing were unknown however it turns out no MCU fw is required to flash and so no MCU reset should occur upon initialization. Change-Id: Ia03f94effc4b720964638c032bbde5acfb13960d Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M realtek_mst_i2c_spi.c 1 file changed, 0 insertions(+), 5 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/96/45896/1 diff --git a/realtek_mst_i2c_spi.c b/realtek_mst_i2c_spi.c index 6158524..7867528 100644 --- a/realtek_mst_i2c_spi.c +++ b/realtek_mst_i2c_spi.c @@ -450,11 +450,6 @@ if (fd < 0) return fd; - /* Ensure we are in a known state before entering ISP mode */ - ret |= realtek_mst_i2c_spi_reset_mpu(fd); - if (ret) - return ret; - ret |= realtek_mst_i2c_spi_enter_isp_mode(fd); if (ret) return ret; -- To view, visit https://review.coreboot.org/c/flashrom/+/45896 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia03f94effc4b720964638c032bbde5acfb13960d Gerrit-Change-Number: 45896 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

3 3

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit October 2020