August 2021 - flashrom-gerrit - mail.coreboot.org

Change in flashrom[master]: realtek_mst_i2c_spi: don't always fail init
by Anastasia Klimchuk (Code Review) 05 Aug '21

05 Aug '21

Attention is currently required from: Paul Menzel, Angel Pons, Peter Marheine. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/56799 ) Change subject: realtek_mst_i2c_spi: don't always fail init ...................................................................... Patch Set 2: Code-Review+2 (1 comment) Patchset: PS1: > No bug, since I discovered and immediately fixed it. […] I wanted to say, get_params looks much better now, and thanks to Peter it is also working. In earlier code, it was hard to guess that "_get_params_failed" label serves successful path as well! -- To view, visit https://review.coreboot.org/c/flashrom/+/56799 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0596014fc9b76b4d11de230f9f5c414c1321dff1 Gerrit-Change-Number: 56799 Gerrit-PatchSet: 2 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> 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-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Thu, 05 Aug 2021 02:21:35 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Comment-In-Reply-To: Peter Marheine <pmarheine(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: realtek_mst_i2c_spi: don't always fail init
by Peter Marheine (Code Review) 05 Aug '21

05 Aug '21

Attention is currently required from: Paul Menzel, Angel Pons, Anastasia Klimchuk. Peter Marheine has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/56799 ) Change subject: realtek_mst_i2c_spi: don't always fail init ...................................................................... Patch Set 2: (1 comment) Patchset: PS1: > If there is a bug please add that as well. […] No bug, since I discovered and immediately fixed it. Breaking change was localized to this driver only: https://review.coreboot.org/c/flashrom/+/52831 -- To view, visit https://review.coreboot.org/c/flashrom/+/56799 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0596014fc9b76b4d11de230f9f5c414c1321dff1 Gerrit-Change-Number: 56799 Gerrit-PatchSet: 2 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> 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-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Thu, 05 Aug 2021 02:00:54 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Comment-In-Reply-To: Peter Marheine <pmarheine(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: realtek_mst_i2c_spi: don't always fail init
by Edward O'Callaghan (Code Review) 05 Aug '21

05 Aug '21

Attention is currently required from: Paul Menzel, Angel Pons, Anastasia Klimchuk, Peter Marheine. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/56799 ) Change subject: realtek_mst_i2c_spi: don't always fail init ...................................................................... Patch Set 2: Code-Review+2 (1 comment) Patchset: PS1: > Added. If there is a bug please add that as well. More importantly, do we know the regressing commit from a bisect? It could have broken other drivers as well? -- To view, visit https://review.coreboot.org/c/flashrom/+/56799 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0596014fc9b76b4d11de230f9f5c414c1321dff1 Gerrit-Change-Number: 56799 Gerrit-PatchSet: 2 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> 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-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Thu, 05 Aug 2021 01:58:39 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Comment-In-Reply-To: Peter Marheine <pmarheine(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: realtek_mst_i2c_spi: don't always fail init
by Peter Marheine (Code Review) 05 Aug '21

05 Aug '21

Attention is currently required from: Paul Menzel, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk. Peter Marheine has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/56799 ) Change subject: realtek_mst_i2c_spi: don't always fail init ...................................................................... Patch Set 1: (3 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/56799/comment/3215ab2c_ac470169 PS1, Line 14: Change-Id: I0596014fc9b76b4d11de230f9f5c414c1321dff1 > +1 Done Patchset: PS1: > I got confused at first because even initial version of this file has `int ret = SPI_GENERIC_ERROR;` […] Added. PS1: > well it was being used until it got re-implemented in fwupd right. No, we've never used this in production (yet). -- To view, visit https://review.coreboot.org/c/flashrom/+/56799 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0596014fc9b76b4d11de230f9f5c414c1321dff1 Gerrit-Change-Number: 56799 Gerrit-PatchSet: 1 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> 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-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Thu, 05 Aug 2021 01:45:38 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org> Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Comment-In-Reply-To: Peter Marheine <pmarheine(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: realtek_mst_i2c_spi: don't always fail init
by Peter Marheine (Code Review) 05 Aug '21

05 Aug '21

Attention is currently required from: Paul Menzel, Angel Pons, Peter Marheine. Hello build bot (Jenkins), Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/56799 to look at the new patch set (#2). Change subject: realtek_mst_i2c_spi: don't always fail init ...................................................................... realtek_mst_i2c_spi: don't always fail init In some earlier refactoring the init function for this programmer was broken such that it never succeeds, since the return value was never set to a value other than a generic failure. Fix it to return success unless there is a problem. TEST=flashrom -p realtek_mst_i2c_spi:bus=n --flash-size now works when n is a bus that has a connected MST. Change-Id: I0596014fc9b76b4d11de230f9f5c414c1321dff1 Signed-off-by: Peter Marheine <pmarheine(a)chromium.org> --- M realtek_mst_i2c_spi.c 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/99/56799/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/56799 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0596014fc9b76b4d11de230f9f5c414c1321dff1 Gerrit-Change-Number: 56799 Gerrit-PatchSet: 2 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> 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-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: tests: Split flashrom tests into two executables
by Anastasia Klimchuk (Code Review) 05 Aug '21

05 Aug '21

Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/56753 ) Change subject: tests: Split flashrom tests into two executables ...................................................................... Patch Set 2: (1 comment) File tests/meson.build: https://review.coreboot.org/c/flashrom/+/56753/comment/bdf26861_f4605e17 PS1, Line 89: # '-DSTANDALONE', > dead comment? Oh thank you! I don't even know the purpose of that... but I think my eyes just stopped noticing it because "this one always was here" effect. Removed the comment from both executables. -- To view, visit https://review.coreboot.org/c/flashrom/+/56753 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I22945cce3d0f36adaa8032167a3ef4e54eccb611 Gerrit-Change-Number: 56753 Gerrit-PatchSet: 2 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Thu, 05 Aug 2021 00:46:22 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tests: Split flashrom tests into two executables
by Anastasia Klimchuk (Code Review) 05 Aug '21

05 Aug '21

Attention is currently required from: Nico Huber, Angel Pons, Anastasia Klimchuk. Hello build bot (Jenkins), Nico Huber, Edward O'Callaghan, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/56753 to look at the new patch set (#2). Change subject: tests: Split flashrom tests into two executables ...................................................................... tests: Split flashrom tests into two executables The reason for splitting: some tests need to wrap spi_send_command, some other tests (specifically the tests in the next patch in this chain) need a real spi_send_command. Cmocka offers a capability to call __real_spi_send_command to call a real function and bypass the wrap. However, the function in question, spi_send_command, is not called in tests explicitly. There is no clear way to say "for this one test use the wrap, for other tests use real". There is one test file spi25.c which needs spi_send_command to be wrapped, and this test file is extracted into a separate executable in this patch. The rest of existing tests don't care about spi_send_command. However in the next patch new tests are added, and they need spi_send_command real. The approach used here is: use real function unless it is required to mock, so all tests that don't care go into the executable with the real spi_send_command. BUG=b:181803212 TEST=builds and ninja test Change-Id: I22945cce3d0f36adaa8032167a3ef4e54eccb611 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M tests/meson.build M tests/tests.c M tests/tests.h A tests/wrap_send_tests.c A tests/wrap_send_tests.h 5 files changed, 127 insertions(+), 35 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/53/56753/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/56753 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I22945cce3d0f36adaa8032167a3ef4e54eccb611 Gerrit-Change-Number: 56753 Gerrit-PatchSet: 2 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: realtek_mst_i2c_spi: don't always fail init
by Anastasia Klimchuk (Code Review) 04 Aug '21

04 Aug '21

Attention is currently required from: Paul Menzel, Angel Pons, Peter Marheine. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/56799 ) Change subject: realtek_mst_i2c_spi: don't always fail init ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: I got confused at first because even initial version of this file has `int ret = SPI_GENERIC_ERROR;`, so this line has always been here. But then I looked closely, the structure of get_params has changed at some point. Previously ret was assigned `ret = 0` in successful case(s), so it was working. And then at some point `ret = 0` was lost. Peter thank you for fixing this! I assume you tested on the device? Could you please add TEST info to commit message? -- To view, visit https://review.coreboot.org/c/flashrom/+/56799 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0596014fc9b76b4d11de230f9f5c414c1321dff1 Gerrit-Change-Number: 56799 Gerrit-PatchSet: 1 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> 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-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Wed, 04 Aug 2021 21:43:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: realtek_mst_i2c_spi: don't always fail init
by Edward O'Callaghan (Code Review) 04 Aug '21

04 Aug '21

Attention is currently required from: Paul Menzel, Angel Pons, Anastasia Klimchuk, Peter Marheine. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/56799 ) Change subject: realtek_mst_i2c_spi: don't always fail init ...................................................................... Patch Set 1: (2 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/56799/comment/17301a9d_0a24d76f PS1, Line 14: Change-Id: I0596014fc9b76b4d11de230f9f5c414c1321dff1 > Missing Signed-off-by line? +1 Patchset: PS1: > Clearly nobody is actually using this (aside from me using it to debug things), otherwise we would h […] well it was being used until it got re-implemented in fwupd right. -- To view, visit https://review.coreboot.org/c/flashrom/+/56799 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0596014fc9b76b4d11de230f9f5c414c1321dff1 Gerrit-Change-Number: 56799 Gerrit-PatchSet: 1 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> 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-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Wed, 04 Aug 2021 14:00:44 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org> Comment-In-Reply-To: Peter Marheine <pmarheine(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: realtek_mst_i2c_spi: don't always fail init
by Paul Menzel (Code Review) 04 Aug '21

04 Aug '21

Attention is currently required from: Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, Peter Marheine. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/56799 ) Change subject: realtek_mst_i2c_spi: don't always fail init ...................................................................... Patch Set 1: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/56799/comment/17bddbca_c5fd174a PS1, Line 14: Change-Id: I0596014fc9b76b4d11de230f9f5c414c1321dff1 Missing Signed-off-by line? -- To view, visit https://review.coreboot.org/c/flashrom/+/56799 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0596014fc9b76b4d11de230f9f5c414c1321dff1 Gerrit-Change-Number: 56799 Gerrit-PatchSet: 1 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> 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-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Wed, 04 Aug 2021 10:29:59 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit August 2021