Attention is currently required from: Daniel Kurtz, Sam McNally, Edward O'Callaghan, Peter Marheine.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/49915 )
Change subject: CHROMIUM: avl_tool: more gracefully handle termination by SIGINT
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/c/flashrom/+/49915
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: If43aea0580fcc7e698daad2ffe085a3c9da5bc41
Gerrit-Change-Number: 49915
Gerrit-PatchSet: 1
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Daniel Kurtz <djkurtz(a)chromium.org>
Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: Sam McNally <sammc(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Daniel Kurtz <djkurtz(a)chromium.org>
Gerrit-Attention: Sam McNally <sammc(a)google.com>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Comment-Date: Fri, 29 Jan 2021 10:55:01 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Daniel Kurtz, Sam McNally, Edward O'Callaghan, Peter Marheine.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/49915 )
Change subject: CHROMIUM: avl_tool: more gracefully handle termination by SIGINT
......................................................................
Patch Set 1: Code-Review+1
(1 comment)
Commit Message:
https://review.coreboot.org/c/flashrom/+/49915/comment/c676d400_c788a5df
PS1, Line 7: CHROMIUM:
> happy to drop it, I don't particularly have strong feelings either way. […]
Ah, alright. Thanks for the clarification
--
To view, visit https://review.coreboot.org/c/flashrom/+/49915
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: If43aea0580fcc7e698daad2ffe085a3c9da5bc41
Gerrit-Change-Number: 49915
Gerrit-PatchSet: 1
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Daniel Kurtz <djkurtz(a)chromium.org>
Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: Sam McNally <sammc(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Daniel Kurtz <djkurtz(a)chromium.org>
Gerrit-Attention: Sam McNally <sammc(a)google.com>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Comment-Date: Fri, 29 Jan 2021 10:54:53 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Sam McNally <sammc(a)google.com>
Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Daniel Kurtz, Sam McNally, Angel Pons, Peter Marheine.
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/49915 )
Change subject: CHROMIUM: avl_tool: more gracefully handle termination by SIGINT
......................................................................
Patch Set 1:
(1 comment)
Commit Message:
https://review.coreboot.org/c/flashrom/+/49915/comment/911a8892_0946e64a
PS1, Line 7: CHROMIUM:
> What does the `CHROMIUM` prefix mean?
happy to drop it, I don't particularly have strong feelings either way. It simply means its a direct commit from downstream and downstream we prefix upstream commits with `UPSTREAM:`
--
To view, visit https://review.coreboot.org/c/flashrom/+/49915
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: If43aea0580fcc7e698daad2ffe085a3c9da5bc41
Gerrit-Change-Number: 49915
Gerrit-PatchSet: 1
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Daniel Kurtz <djkurtz(a)chromium.org>
Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: Sam McNally <sammc(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Daniel Kurtz <djkurtz(a)chromium.org>
Gerrit-Attention: Sam McNally <sammc(a)google.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Comment-Date: Fri, 29 Jan 2021 02:16:01 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Sam McNally <sammc(a)google.com>
Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment
Edward O'Callaghan has submitted this change. ( 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
Reviewed-on: https://review.coreboot.org/c/flashrom/+/49786
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org>
---
M realtek_mst_i2c_spi.c
1 file changed, 7 insertions(+), 3 deletions(-)
Approvals:
build bot (Jenkins): Verified
Edward O'Callaghan: Looks good to me, approved
diff --git a/realtek_mst_i2c_spi.c b/realtek_mst_i2c_spi.c
index 3dd1b36..1bbb71f 100644
--- a/realtek_mst_i2c_spi.c
+++ b/realtek_mst_i2c_spi.c
@@ -433,9 +433,13 @@
(struct realtek_mst_i2c_spi_data *)data;
int fd = realtek_mst_data->fd;
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: 4
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-MessageType: merged
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 2:
(1 comment)
File realtek_mst_i2c_spi.c:
https://review.coreboot.org/c/flashrom/+/49786/comment/1627c45d_0f3bff4f
PS1, Line 431: /* 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. */
> Comment style should be: […]
Done
--
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: 2
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, 28 Jan 2021 12:50:21 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Sam McNally, Edward O'Callaghan, Peter Marheine.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/49915 )
Change subject: CHROMIUM: avl_tool: more gracefully handle termination by SIGINT
......................................................................
Patch Set 1:
(1 comment)
Commit Message:
https://review.coreboot.org/c/flashrom/+/49915/comment/67306649_50bcabc1
PS1, Line 7: CHROMIUM:
> Since it is a new thing for Chromium Flashrom and upstream Flashrom to be collaborating now there is […]
What does the `CHROMIUM` prefix mean?
--
To view, visit https://review.coreboot.org/c/flashrom/+/49915
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: If43aea0580fcc7e698daad2ffe085a3c9da5bc41
Gerrit-Change-Number: 49915
Gerrit-PatchSet: 1
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: Sam McNally <sammc(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Sam McNally <sammc(a)google.com>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Comment-Date: Wed, 27 Jan 2021 09:28:27 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Sam McNally <sammc(a)google.com>
Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Sam McNally, Angel Pons, Peter Marheine.
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/49915 )
Change subject: CHROMIUM: avl_tool: more gracefully handle termination by SIGINT
......................................................................
Patch Set 1:
(1 comment)
Commit Message:
https://review.coreboot.org/c/flashrom/+/49915/comment/d38e7eb2_c8c4daa2
PS1, Line 7: CHROMIUM:
> Is the CHROMIUM: prefix commonly used here?
Since it is a new thing for Chromium Flashrom and upstream Flashrom to be collaborating now there is no prior art to a convention specifically for Flashrom obviously. However, this is a convention for Coreboot which is the closest thing to draw upon and so sensible to use that here.
--
To view, visit https://review.coreboot.org/c/flashrom/+/49915
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: If43aea0580fcc7e698daad2ffe085a3c9da5bc41
Gerrit-Change-Number: 49915
Gerrit-PatchSet: 1
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: Sam McNally <sammc(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Sam McNally <sammc(a)google.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Comment-Date: Wed, 27 Jan 2021 05:51:55 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Sam McNally <sammc(a)google.com>
Gerrit-MessageType: comment
Attention is currently required from: Edward O'Callaghan, Peter Marheine.
Sam McNally has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/49915 )
Change subject: CHROMIUM: avl_tool: more gracefully handle termination by SIGINT
......................................................................
Patch Set 1:
(1 comment)
Commit Message:
https://review.coreboot.org/c/flashrom/+/49915/comment/6b3e5bd4_233dd758
PS1, Line 7: CHROMIUM:
Is the CHROMIUM: prefix commonly used here?
--
To view, visit https://review.coreboot.org/c/flashrom/+/49915
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: If43aea0580fcc7e698daad2ffe085a3c9da5bc41
Gerrit-Change-Number: 49915
Gerrit-PatchSet: 1
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: Sam McNally <sammc(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Comment-Date: Tue, 26 Jan 2021 23:43:58 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment