January 2021 - flashrom-gerrit

Change in flashrom[master]: ft2232_spi.c: Generalize GPIOL pin control
by Edward O'Callaghan (Code Review) 21 Jan '21

21 Jan '21

Attention is currently required from: Alan Green, Paul Menzel, Angel Pons. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/49637 ) Change subject: ft2232_spi.c: Generalize GPIOL pin control ...................................................................... Patch Set 9: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/49637 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I1f2b3b968577e62e3c5b11bcdf4afe2de6eb84ab Gerrit-Change-Number: 49637 Gerrit-PatchSet: 9 Gerrit-Owner: Alan Green <avg(a)google.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Xiang Wang <merle(a)hardenedlinux.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Alan Green <avg(a)google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Thu, 21 Jan 2021 09:04:58 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: realtek_mst_i2c_spi.c: Skip return value check for reset function
by Edward O'Callaghan (Code Review) 21 Jan '21

21 Jan '21

Attention is currently required from: Shiyu Sun, Angel Pons. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/49786 ) Change subject: realtek_mst_i2c_spi.c: Skip return value check for reset function ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/49786 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia6200f7150db4368c26d8dfe779a9e85184b1b06 Gerrit-Change-Number: 49786 Gerrit-PatchSet: 1 Gerrit-Owner: Shiyu Sun <sshiyu(a)google.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Shiyu Sun <sshiyu(a)google.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Thu, 21 Jan 2021 08:41:46 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: realtek_mst_i2c_spi.c: Fix indentation
by Edward O'Callaghan (Code Review) 21 Jan '21

21 Jan '21

Attention is currently required from: Shiyu Sun. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/49784 ) Change subject: realtek_mst_i2c_spi.c: Fix indentation ...................................................................... Patch Set 1: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/49784/comment/6a970921_98ac4f59 PS1, Line 14: trim new here -- To view, visit https://review.coreboot.org/c/flashrom/+/49784 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I730882c97926dfbe8b68b286c3805d6470993da8 Gerrit-Change-Number: 49784 Gerrit-PatchSet: 1 Gerrit-Owner: Shiyu Sun <sshiyu(a)google.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Shiyu Sun <sshiyu(a)google.com> Gerrit-Comment-Date: Thu, 21 Jan 2021 08:39:53 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: realtek_mst_i2c_spi.c: Fix indentation
by Edward O'Callaghan (Code Review) 21 Jan '21

21 Jan '21

Attention is currently required from: Shiyu Sun. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/49784 ) Change subject: realtek_mst_i2c_spi.c: Fix indentation ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/49784 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I730882c97926dfbe8b68b286c3805d6470993da8 Gerrit-Change-Number: 49784 Gerrit-PatchSet: 1 Gerrit-Owner: Shiyu Sun <sshiyu(a)google.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Shiyu Sun <sshiyu(a)google.com> Gerrit-Comment-Date: Thu, 21 Jan 2021 08:39:12 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: realtek_mst_i2c_spi.c: Skip return value check for reset function
by Shiyu Sun (Code Review) 21 Jan '21

21 Jan '21

Shiyu Sun has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/49786 ) Change subject: realtek_mst_i2c_spi.c: Skip return value check for reset function ...................................................................... realtek_mst_i2c_spi.c: Skip return value check for reset function The return value for reset function can not be guaranteed when reset success. There is no way to check if reset success or not. BUG=b:147402710,b:152558985 BRANCH=none TEST=builds Signed-off-by: Shiyu Sun <sshiyu(a)chromium.org> Change-Id: Ia6200f7150db4368c26d8dfe779a9e85184b1b06 --- M realtek_mst_i2c_spi.c 1 file changed, 5 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/86/49786/1 diff --git a/realtek_mst_i2c_spi.c b/realtek_mst_i2c_spi.c index e9d6a58..c2bf2b0 100644 --- a/realtek_mst_i2c_spi.c +++ b/realtek_mst_i2c_spi.c @@ -428,9 +428,11 @@ int fd = realtek_mst_data->fd; ret |= realtek_mst_i2c_spi_toggle_gpio_88_strap(fd, false); if (realtek_mst_data->reset) { - ret |= realtek_mst_i2c_spi_reset_mpu(fd); - if (ret != 0) - msg_perr("%s: MCU failed to reset on tear-down.\n", __func__); + /* Return value for reset mpu is not checked since + * the return value is not guaranteed to be 0 on a + * success reset. Currently there is no way to fix that. + * For more details see b:147402710. */ + realtek_mst_i2c_spi_reset_mpu(fd); } i2c_close(fd); free(data); -- To view, visit https://review.coreboot.org/c/flashrom/+/49786 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia6200f7150db4368c26d8dfe779a9e85184b1b06 Gerrit-Change-Number: 49786 Gerrit-PatchSet: 1 Gerrit-Owner: Shiyu Sun <sshiyu(a)google.com> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: realtek_mst_i2c_spi.c: Move gpio 88 toggle outside write function
by Shiyu Sun (Code Review) 21 Jan '21

21 Jan '21

Shiyu Sun has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/49785 ) Change subject: realtek_mst_i2c_spi.c: Move gpio 88 toggle outside write function ...................................................................... realtek_mst_i2c_spi.c: Move gpio 88 toggle outside write function Gpio 88 toggle is used as write protection disable/enable now and we need that to happen at the initialization of programmer. BUG=b:147402710,b:152558985 BRANCH=none TEST=flashrom -p realtek_mst_i2c_spi:bus=x,reset-mcu=1,enter-isp=1 -w image.bin Signed-off-by: Shiyu Sun <sshiyu(a)chromium.org> Change-Id: I237bf9f8aa0fcbb904e7f0c09c74fd179e8c70c1 --- M realtek_mst_i2c_spi.c 1 file changed, 7 insertions(+), 6 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/85/49785/1 diff --git a/realtek_mst_i2c_spi.c b/realtek_mst_i2c_spi.c index 3dd1b36..e9d6a58 100644 --- a/realtek_mst_i2c_spi.c +++ b/realtek_mst_i2c_spi.c @@ -375,10 +375,6 @@ if (fd < 0) return SPI_GENERIC_ERROR; - ret = realtek_mst_i2c_spi_toggle_gpio_88_strap(fd, true); - 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,8 +400,6 @@ break; } - ret |= realtek_mst_i2c_spi_toggle_gpio_88_strap(fd, false); - return ret; } @@ -432,6 +426,7 @@ 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_toggle_gpio_88_strap(fd, false); if (realtek_mst_data->reset) { ret |= realtek_mst_i2c_spi_reset_mpu(fd); if (ret != 0) @@ -528,6 +523,12 @@ return ret; } + ret |= realtek_mst_i2c_spi_toggle_gpio_88_strap(fd, true); + if (ret) { + msg_perr("Unable to toggle gpio 88 strap to True.\n"); + 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/+/49785 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I237bf9f8aa0fcbb904e7f0c09c74fd179e8c70c1 Gerrit-Change-Number: 49785 Gerrit-PatchSet: 1 Gerrit-Owner: Shiyu Sun <sshiyu(a)google.com> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: realtek_mst_i2c_spi.c: Fix indentation
by Shiyu Sun (Code Review) 21 Jan '21

21 Jan '21

Shiyu Sun has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/49784 ) Change subject: realtek_mst_i2c_spi.c: Fix indentation ...................................................................... realtek_mst_i2c_spi.c: Fix indentation BUG=none BRANCH=none TEST=builds Signed-off-by: Shiyu Sun <sshiyu(a)chromium.org> Change-Id: I730882c97926dfbe8b68b286c3805d6470993da8 --- M realtek_mst_i2c_spi.c 1 file changed, 3 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/84/49784/1 diff --git a/realtek_mst_i2c_spi.c b/realtek_mst_i2c_spi.c index a0cd7c6..3dd1b36 100644 --- a/realtek_mst_i2c_spi.c +++ b/realtek_mst_i2c_spi.c @@ -26,7 +26,7 @@ #include "i2c_helper.h" -#define MCU_I2C_SLAVE_ADDR 0x94 +#define MCU_I2C_SLAVE_ADDR 0x94 #define REGISTER_ADDRESS (0x94 >> 1) #define PAGE_SIZE 128 #define MAX_SPI_WAIT_RETRIES 1000 @@ -78,7 +78,7 @@ msg_perr("Unable to extract fd from flash context.\n"); return SPI_GENERIC_ERROR; } - const struct realtek_mst_i2c_spi_data *data = + const struct realtek_mst_i2c_spi_data *data = (const struct realtek_mst_i2c_spi_data *)flash->mst->spi.data; return data->fd; @@ -429,7 +429,7 @@ static int realtek_mst_i2c_spi_shutdown(void *data) { int ret = 0; - struct realtek_mst_i2c_spi_data *realtek_mst_data = + struct realtek_mst_i2c_spi_data *realtek_mst_data = (struct realtek_mst_i2c_spi_data *)data; int fd = realtek_mst_data->fd; if (realtek_mst_data->reset) { -- To view, visit https://review.coreboot.org/c/flashrom/+/49784 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I730882c97926dfbe8b68b286c3805d6470993da8 Gerrit-Change-Number: 49784 Gerrit-PatchSet: 1 Gerrit-Owner: Shiyu Sun <sshiyu(a)google.com> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: ft2232_spi.c: Generalize GPIOL pin control
by Xiang Wang (Code Review) 21 Jan '21

21 Jan '21

Attention is currently required from: Alan Green, Paul Menzel, Angel Pons. Xiang Wang has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/49637 ) Change subject: ft2232_spi.c: Generalize GPIOL pin control ...................................................................... Patch Set 9: Code-Review+2 (1 comment) Patchset: PS9: I feel good -- To view, visit https://review.coreboot.org/c/flashrom/+/49637 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I1f2b3b968577e62e3c5b11bcdf4afe2de6eb84ab Gerrit-Change-Number: 49637 Gerrit-PatchSet: 9 Gerrit-Owner: Alan Green <avg(a)google.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Xiang Wang <merle(a)hardenedlinux.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Alan Green <avg(a)google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Thu, 21 Jan 2021 04:35:36 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: Simplifying the initialisation flow for it85spi
by Anastasia Klimchuk (Code Review) 20 Jan '21

20 Jan '21

Anastasia Klimchuk has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/48196 ) Change subject: Simplifying the initialisation flow for it85spi ...................................................................... Simplifying the initialisation flow for it85spi 1) Inlining it85xx_spi_common_init since it's only used once in it85xx_spi_init, after inlining ret value is not needed. 2) Creating it86_init_error section to ensure that data is freed if error happened in initialisaton flow. BUG=b:172876667 TEST=builds Change-Id: Iac1b4ae9c6f34c5265e729ae0a80024800c3c272 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M it85spi.c 1 file changed, 32 insertions(+), 39 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/96/48196/1 diff --git a/it85spi.c b/it85spi.c index 6215515..52b09b1 100644 --- a/it85spi.c +++ b/it85spi.c @@ -289,11 +289,18 @@ .write_aai = default_spi_write_aai, }; -static int it85xx_spi_common_init(struct superio s) +int it85xx_spi_init(struct superio s) { chipaddr base; + struct it85spi_data *data; + msg_pdbg("%s():%d superio.vendor=0x%02x\n", __func__, __LINE__, s.vendor); - struct it85spi_data *data = calloc(1, sizeof(struct it85spi_data)); + if (!(internal_buses_supported & BUS_FWH)) { + msg_pdbg("%s():%d buses not support FWH\n", __func__, __LINE__); + return 1; + } + + data = calloc(1, sizeof(struct it85spi_data)); if (!data) { msg_perr("Unable to allocate space for extra SPI master data.\n"); return SPI_GENERIC_ERROR; @@ -301,12 +308,8 @@ spi_master_it85xx.data = data; - msg_pdbg("%s():%d superio.vendor=0x%02x\n", __func__, __LINE__, - s.vendor); - if (register_shutdown(it85xx_shutdown, data)) { - free(data); - return 1; + goto it85spi_init_error; } #ifdef LPC_IO @@ -334,9 +337,9 @@ * Major TODO here, and it will be a lot of work. */ base = (chipaddr)physmap("it85 communication", 0xFFFFF000, 0x1000); - if (base == (chipaddr)ERROR_PTR) - free(data); - return 1; + if (base == (chipaddr)ERROR_PTR) { + goto it85spi_init_error; + } msg_pdbg("%s():%d base=0x%08x\n", __func__, __LINE__, (unsigned int)base); @@ -344,37 +347,27 @@ data->ce_low = (unsigned char *)(base + 0xD00); /* 0xFFFFFD00 */ #endif + msg_pdbg("%s: internal_buses_supported=0x%x\n", __func__, + internal_buses_supported); + /* Check for FWH because IT85 listens to FWH cycles. + * FIXME: The big question is whether FWH cycles are necessary + * for communication even if LPC_IO is defined. + */ + if (internal_buses_supported & BUS_FWH) + msg_pdbg("Registering IT85 SPI.\n"); + /* FIXME: Really leave FWH enabled? We can't use this region + * anymore since accessing it would mess up IT85 communication. + * If we decide to disable FWH for this region, we should print + * a debug message about it. + */ + /* Set this as SPI controller. */ + register_spi_master(&spi_master_it85xx); + return 0; -} -int it85xx_spi_init(struct superio s) -{ - int ret; - - if (!(internal_buses_supported & BUS_FWH)) { - msg_pdbg("%s():%d buses not support FWH\n", __func__, __LINE__); - return 1; - } - ret = it85xx_spi_common_init(s); - msg_pdbg("FWH: %s():%d ret=%d\n", __func__, __LINE__, ret); - if (!ret) { - msg_pdbg("%s: internal_buses_supported=0x%x\n", __func__, - internal_buses_supported); - /* Check for FWH because IT85 listens to FWH cycles. - * FIXME: The big question is whether FWH cycles are necessary - * for communication even if LPC_IO is defined. - */ - if (internal_buses_supported & BUS_FWH) - msg_pdbg("Registering IT85 SPI.\n"); - /* FIXME: Really leave FWH enabled? We can't use this region - * anymore since accessing it would mess up IT85 communication. - * If we decide to disable FWH for this region, we should print - * a debug message about it. - */ - /* Set this as SPI controller. */ - register_spi_master(&spi_master_it85xx); - } - return ret; +it85spi_init_error: + free(data); + return 1; } #endif -- To view, visit https://review.coreboot.org/c/flashrom/+/48196 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iac1b4ae9c6f34c5265e729ae0a80024800c3c272 Gerrit-Change-Number: 48196 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newchange

3 22

Change in flashrom[master]: ft2232_spi.c: Generalize GPIOL pin control
by Alan Green (Code Review) 20 Jan '21

20 Jan '21

Attention is currently required from: Xiang Wang, Paul Menzel, Angel Pons. Alan Green has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/49637 ) Change subject: ft2232_spi.c: Generalize GPIOL pin control ...................................................................... Patch Set 9: (2 comments) File ft2232_spi.c: https://review.coreboot.org/c/flashrom/+/49637/comment/54e9a4a5_88f36635 PS8, Line 396: "Valid values are 4 character strings of H, L, Z " : "and X.\n" > Please don't break user-visible strings. They can be as long as necessary. Done https://review.coreboot.org/c/flashrom/+/49637/comment/aabcff97_43961e49 PS8, Line 402: "Example: gpiol=LZXH drives GPIOL 0 low, and GPIOL 3 " : "high, sets GPIOL 1\n" : "to an input and leaves GPIOL 2 in the default " : "state\n", arg); > Ditto Done -- To view, visit https://review.coreboot.org/c/flashrom/+/49637 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I1f2b3b968577e62e3c5b11bcdf4afe2de6eb84ab Gerrit-Change-Number: 49637 Gerrit-PatchSet: 9 Gerrit-Owner: Alan Green <avg(a)google.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Xiang Wang <merle(a)hardenedlinux.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Xiang Wang <merle(a)hardenedlinux.org> Gerrit-Attention: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Thu, 21 Jan 2021 01:49:01 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit January 2021