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

Change in flashrom[master]: mec1308.c: Separate shutdown from failed init cleanup
by Angel Pons (Code Review) 23 Apr '21

23 Apr '21

Attention is currently required from: Nico Huber, Edward O'Callaghan, Anastasia Klimchuk. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52597 ) Change subject: mec1308.c: Separate shutdown from failed init cleanup ...................................................................... Patch Set 2: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/52597 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6d62d43dd8b6ebc595f9fd747e0f4cd80f3c10da Gerrit-Change-Number: 52597 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Victor Ding <victording(a)google.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Fri, 23 Apr 2021 08:44:25 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: mec1308.c: Untangle successful vs failed init paths
by Angel Pons (Code Review) 23 Apr '21

23 Apr '21

Attention is currently required from: Anastasia Klimchuk. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52596 ) Change subject: mec1308.c: Untangle successful vs failed init paths ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/52596 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ibf35335501e59636c544af124ad7a04a186790b4 Gerrit-Change-Number: 52596 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Victor Ding <victording(a)google.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Fri, 23 Apr 2021 08:43:41 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: mec1308.c: Extract params check into a function
by Angel Pons (Code Review) 23 Apr '21

23 Apr '21

Attention is currently required from: Anastasia Klimchuk. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52595 ) Change subject: mec1308.c: Extract params check into a function ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/52595 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If5be7709e93233a2e7ea9133de50382d2524a55f Gerrit-Change-Number: 52595 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Victor Ding <victording(a)google.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Fri, 23 Apr 2021 08:43:22 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: linux_spi.c: Drop some unnecessary initialisations and checks
by Angel Pons (Code Review) 23 Apr '21

23 Apr '21

Attention is currently required from: Nico Huber, Edward O'Callaghan, Anastasia Klimchuk. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52492 ) Change subject: linux_spi.c: Drop some unnecessary initialisations and checks ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/52492 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I27a718b515fc474f63b3e61be58a6f9302527559 Gerrit-Change-Number: 52492 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: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Sam McNally <sammc(a)google.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Fri, 23 Apr 2021 08:32:43 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tests: Add unit test to run init/shutdown for linux_spi.c
by Anastasia Klimchuk (Code Review) 23 Apr '21

23 Apr '21

Attention is currently required from: Nico Huber, Angel Pons. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52498 ) Change subject: tests: Add unit test to run init/shutdown for linux_spi.c ...................................................................... Patch Set 4: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/52498/comment/b001d6b7_dd721b71 PS3, Line 22: > AFAICS, the current implementation tests a particular path of the […] I used your words for both commit message and a comment, I won't say it better anyway. -- To view, visit https://review.coreboot.org/c/flashrom/+/52498 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I4911fbb6f04371283f0e62d2196bdd691a227584 Gerrit-Change-Number: 52498 Gerrit-PatchSet: 4 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-Comment-Date: Fri, 23 Apr 2021 05:57:04 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tests: Add unit test to run init/shutdown for linux_spi.c
by Anastasia Klimchuk (Code Review) 23 Apr '21

23 Apr '21

Attention is currently required from: 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/+/52498 to look at the new patch set (#4). Change subject: tests: Add unit test to run init/shutdown for linux_spi.c ...................................................................... tests: Add unit test to run init/shutdown for linux_spi.c The current implementation tests a particular path of the init procedure. There are two ways for it to succeed: reading the buffer size from sysfs and the fallback to getpagesize(). This test does the latter (fallback to getpagesize). Extract from meson-logs/testlog.txt for new test: [ RUN ] linux_spi_init_and_shutdown_test_success Testing programmer_init for programmer=25 ... __wrap_open64 is called __wrap_ioctl is called __wrap_ioctl is called __wrap_ioctl is called __wrap_fopen64 is called ... programmer_init for programmer=25 successful Testing programmer_shutdown for programmer=25 ... ... programmer_shutdown for programmer=25 successful [ OK ] linux_spi_init_and_shutdown_test_success BUG=b:181803212 TEST=builds and ninja test Change-Id: I4911fbb6f04371283f0e62d2196bdd691a227584 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M tests/init_shutdown.c M tests/meson.build M tests/tests.c M tests/tests.h 4 files changed, 62 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/98/52498/4 -- To view, visit https://review.coreboot.org/c/flashrom/+/52498 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I4911fbb6f04371283f0e62d2196bdd691a227584 Gerrit-Change-Number: 52498 Gerrit-PatchSet: 4 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: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: mec1308.c: Separate shutdown from failed init cleanup
by Anastasia Klimchuk (Code Review) 23 Apr '21

23 Apr '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/+/52597 ) Change subject: mec1308.c: Separate shutdown from failed init cleanup ...................................................................... Patch Set 2: (1 comment) File mec1308.c: https://review.coreboot.org/c/flashrom/+/52597/comment/c726989a_0c445551 PS1, Line 520: /* Exit passthru mode before performing commands which do not affect : the SPI ROM */ : mec1308_exit_passthru_mode(ctx_data); : : /* Re-enable SMI and ACPI. : FIXME: is there an ordering dependency? */ : if (mbx_write(ctx_data, MEC1308_MBX_CMD, MEC1308_CMD_SMI_ENABLE)) : msg_pdbg("%s: unable to re-enable SMI\n", __func__); : if (mbx_write(ctx_data, MEC1308_MBX_CMD, MEC1308_CMD_ACPI_ENABLE)) : msg_pdbg("%s: unable to re-enable ACPI\n", __func__); > Could we just call mec1308_shutdown() here? Would have to skip the free() below, though: […] Done -- To view, visit https://review.coreboot.org/c/flashrom/+/52597 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6d62d43dd8b6ebc595f9fd747e0f4cd80f3c10da Gerrit-Change-Number: 52597 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Victor Ding <victording(a)google.com> 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: Fri, 23 Apr 2021 01:45:14 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: mec1308.c: Extract params check into a function
by Anastasia Klimchuk (Code Review) 23 Apr '21

23 Apr '21

Attention is currently required from: Angel Pons. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52595 ) Change subject: mec1308.c: Extract params check into a function ...................................................................... Patch Set 2: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/52595/comment/4081b008_f1637d80 PS1, Line 9: This allows char *p to become a local variable in > nit: commit message line length limit is 72 chars done (and also for other patches in this chain). -- To view, visit https://review.coreboot.org/c/flashrom/+/52595 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If5be7709e93233a2e7ea9133de50382d2524a55f Gerrit-Change-Number: 52595 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Victor Ding <victording(a)google.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Fri, 23 Apr 2021 01:43:09 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: mec1308.c: Untangle successful vs failed init paths
by Anastasia Klimchuk (Code Review) 23 Apr '21

23 Apr '21

Attention is currently required from: Anastasia Klimchuk. Hello build bot (Jenkins), Edward O'Callaghan, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/52596 to look at the new patch set (#2). Change subject: mec1308.c: Untangle successful vs failed init paths ...................................................................... mec1308.c: Untangle successful vs failed init paths Label mec1308_init_exit now serves as failed init path, it does cleanup and returns 1. Since all error paths return 1, and successful init is separated from failure, there is no need to have ret variable anymore. TEST=builds and ninja test from 51487 BUG=b:185191942 Change-Id: Ibf35335501e59636c544af124ad7a04a186790b4 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M mec1308.c 1 file changed, 7 insertions(+), 18 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/96/52596/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/52596 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ibf35335501e59636c544af124ad7a04a186790b4 Gerrit-Change-Number: 52596 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Victor Ding <victording(a)google.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: mec1308.c: Separate shutdown from failed init cleanup
by Anastasia Klimchuk (Code Review) 23 Apr '21

23 Apr '21

Attention is currently required from: Edward O'Callaghan, Angel Pons, Anastasia Klimchuk. Hello build bot (Jenkins), Edward O'Callaghan, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/52597 to look at the new patch set (#2). Change subject: mec1308.c: Separate shutdown from failed init cleanup ...................................................................... mec1308.c: Separate shutdown from failed init cleanup Shutdown function was covering two different jobs here: 1) the actual shutdown which is run at the end of the driver's lifecycle and 2) cleanup in cases when initialisation failed. Now, shutdown is only doing its main job (#1), and the driver itself is doing cleanup when init fails (#2). The good thing is that now resources are released/closed immediately in cases when init fails (vs shutdown function which was run at some point later), and the driver leaves clean space after itself if init fails. And very importantly this unlocks API change which plans to move register_shutdown inside register master API, see https://review.coreboot.org/c/flashrom/+/51761 TEST=builds and ninja test from 51487 BUG=b:185191942 Change-Id: I6d62d43dd8b6ebc595f9fd747e0f4cd80f3c10da Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M mec1308.c 1 file changed, 8 insertions(+), 4 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/97/52597/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/52597 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6d62d43dd8b6ebc595f9fd747e0f4cd80f3c10da Gerrit-Change-Number: 52597 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Victor Ding <victording(a)google.com> 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-MessageType: newpatchset

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit April 2021