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

Change in flashrom[master]: [RFC] tests: Abstract port I/O and factor mec1308 specifics out
by Anastasia Klimchuk (Code Review) 29 Apr '21

29 Apr '21

Attention is currently required from: Nico Huber. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52604 ) Change subject: [RFC] tests: Abstract port I/O and factor mec1308 specifics out ...................................................................... Patch Set 1: (1 comment) File tests/init_shutdown.c: https://review.coreboot.org/c/flashrom/+/52604/comment/ef8c6b0f_3a4173bc PS1, Line 63: .outb = mec1308_outb, > I'm actually not familiar with our port-i/o using programmer drivers. Without […] I just wanted to tell you that you are wise person! I am now adding another test with io mocking (for ene_lpc) and yes I need a additional variable in test data struct, a flag which is set in outb and checked/unset in inb. New test is already working! But I need to make the code look nice and then upload to 51487. (and then I will split the whole thing into smaller patches, but that's another story). -- To view, visit https://review.coreboot.org/c/flashrom/+/52604 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia3a424478456a7c968b3c736ba371d755c494f30 Gerrit-Change-Number: 52604 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Comment-Date: Thu, 29 Apr 2021 05:26:45 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

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

28 Apr '21

Attention is currently required from: Nico Huber. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52685 ) Change subject: ene_lpc.c: Untangle successful vs failed init paths ...................................................................... Patch Set 3: (1 comment) File ene_lpc.c: https://review.coreboot.org/c/flashrom/+/52685/comment/99dbdc13_fd7eef24 PS3, Line 582: return 1; > It should fall through instead, right? No because ene_leave_flash_mode frees ctx_data and there's nothing more to do after it. The same flow is in mec1308. One thing ene and mec have in common is that ctx_data is allocated at the very beginning of init function, means all error paths need to free it. Ideally, I want data to be allocated at the very end of init function, just before register_shutdown, and use local variables in most of init function body. I did it in some places where it was trivial, but for ene and mec that would be more diffs and I think to have a dedicated patch for this change. (from my memory dummy has the same issue: data allocated in the beginning of init, and some error paths forget to free it). -- To view, visit https://review.coreboot.org/c/flashrom/+/52685 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iac295f1353785cd73d7cb2f19e4a8cbb69beb576 Gerrit-Change-Number: 52685 Gerrit-PatchSet: 3 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: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-CC: Victor Ding <victording(a)google.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Comment-Date: Wed, 28 Apr 2021 22:59:31 +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: Ensure programmer param variable is always initialised
by Nico Huber (Code Review) 28 Apr '21

28 Apr '21

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52719 ) Change subject: mec1308.c: Ensure programmer param variable is always initialised ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: Oops, sorry for the early submission, I missed that this was rather new. -- To view, visit https://review.coreboot.org/c/flashrom/+/52719 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2d2b3039da2ef185cb31509b3901d56b428688b7 Gerrit-Change-Number: 52719 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: Victor Ding <victording(a)google.com> Gerrit-Comment-Date: Wed, 28 Apr 2021 15:15:55 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: mec1308.c: Ensure programmer param variable is always initialised
by Nico Huber (Code Review) 28 Apr '21

28 Apr '21

Nico Huber has submitted this change. ( https://review.coreboot.org/c/flashrom/+/52719 ) Change subject: mec1308.c: Ensure programmer param variable is always initialised ...................................................................... mec1308.c: Ensure programmer param variable is always initialised Programmer param variable is now declared inline with the code and is const, so that it cannot be used uninitialised. BUG=b:185191942 TEST=builds and ninja test from 51487 Change-Id: I2d2b3039da2ef185cb31509b3901d56b428688b7 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/52719 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> --- M mec1308.c 1 file changed, 1 insertion(+), 3 deletions(-) Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved diff --git a/mec1308.c b/mec1308.c index 0ec9be9..1530d9b 100644 --- a/mec1308.c +++ b/mec1308.c @@ -409,9 +409,7 @@ static int check_params(void) { int ret = 0; - char *p = NULL; - - p = extract_programmer_param("type"); + char *const p = extract_programmer_param("type"); if (p && strcmp(p, "ec")) { msg_pdbg("mec1308 only supports \"ec\" type devices\n"); ret = 1; -- To view, visit https://review.coreboot.org/c/flashrom/+/52719 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2d2b3039da2ef185cb31509b3901d56b428688b7 Gerrit-Change-Number: 52719 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: Victor Ding <victording(a)google.com> Gerrit-MessageType: merged

1 0

Change in flashrom[master]: ene_lpc.c: Untangle successful vs failed init paths
by Nico Huber (Code Review) 28 Apr '21

28 Apr '21

Attention is currently required from: Anastasia Klimchuk. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52685 ) Change subject: ene_lpc.c: Untangle successful vs failed init paths ...................................................................... Patch Set 3: (2 comments) File ene_lpc.c: https://review.coreboot.org/c/flashrom/+/52685/comment/7598d76c_bf1c1ee3 PS1, Line 575: ene_leave_flash_mode > I introduced second label here (same as in mec1308) which does cleanup. […] Sorry, didn't mean you should do it right now. Doesn't hurt, though. File ene_lpc.c: https://review.coreboot.org/c/flashrom/+/52685/comment/e5bdb385_4e95cf0f PS3, Line 582: return 1; It should fall through instead, right? -- To view, visit https://review.coreboot.org/c/flashrom/+/52685 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iac295f1353785cd73d7cb2f19e4a8cbb69beb576 Gerrit-Change-Number: 52685 Gerrit-PatchSet: 3 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: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-CC: Victor Ding <victording(a)google.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Wed, 28 Apr 2021 15:14:28 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ene_lpc.c: Extract params check into a function
by Nico Huber (Code Review) 28 Apr '21

28 Apr '21

Nico Huber has submitted this change. ( https://review.coreboot.org/c/flashrom/+/52684 ) Change subject: ene_lpc.c: Extract params check into a function ...................................................................... ene_lpc.c: Extract params check into a function This allows char *p to become a local variable in check_params, and it is allocated and freed within check_params function. Which means init function does not need char *p anymore, in particular does not need to free it - and this makes cleanup after failed init easier. As a good side effect, init function becomes easier to read. BUG=b:185191942 TEST=builds Change-Id: I7c3b6dea0edbc7547f0b307a0508c7d2b2a6d370 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/52684 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> Reviewed-by: Nico Huber <nico.h(a)gmx.de> --- M ene_lpc.c 1 file changed, 14 insertions(+), 5 deletions(-) Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved Angel Pons: Looks good to me, approved diff --git a/ene_lpc.c b/ene_lpc.c index 1d045b8..c05fef8 100644 --- a/ene_lpc.c +++ b/ene_lpc.c @@ -513,11 +513,23 @@ .write_256 = default_spi_write_256, }; +static int check_params(void) +{ + int ret = 0; + char *const p = extract_programmer_param("type"); + if (p && strcmp(p, "ec")) { + msg_pdbg("ene_lpc only supports \"ec\" type devices\n"); + ret = 1; + } + + free(p); + return ret; +} + int ene_lpc_init() { uint8_t hwver, ediid, i; int ret = 0; - char *p = NULL; ene_lpc_data_t *ctx_data = NULL; msg_pdbg("%s\n", __func__); @@ -529,9 +541,7 @@ } ctx_data->ec_state = EC_STATE_NORMAL; - p = extract_programmer_param("type"); - if (p && strcmp(p, "ec")) { - msg_pdbg("ene_lpc only supports \"ec\" type devices\n"); + if (check_params()) { ret = 1; goto ene_probe_spi_flash_exit; } @@ -572,7 +582,6 @@ msg_pdbg("%s: successfully initialized ene\n", __func__); ene_probe_spi_flash_exit: - free(p); if (ret) free(ctx_data); return ret; -- To view, visit https://review.coreboot.org/c/flashrom/+/52684 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7c3b6dea0edbc7547f0b307a0508c7d2b2a6d370 Gerrit-Change-Number: 52684 Gerrit-PatchSet: 3 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: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-CC: Victor Ding <victording(a)google.com> Gerrit-MessageType: merged

1 0

Change in flashrom[master]: ene_lpc.c: Extract params check into a function
by Nico Huber (Code Review) 28 Apr '21

28 Apr '21

Attention is currently required from: Anastasia Klimchuk. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52684 ) Change subject: ene_lpc.c: Extract params check into a function ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/52684 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7c3b6dea0edbc7547f0b307a0508c7d2b2a6d370 Gerrit-Change-Number: 52684 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: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-CC: Victor Ding <victording(a)google.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Wed, 28 Apr 2021 15:12:05 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

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

28 Apr '21

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

1 0

Change in flashrom[master]: mec1308.c: Ensure programmer param variable is always initialised
by Angel Pons (Code Review) 28 Apr '21

28 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/+/52719 ) Change subject: mec1308.c: Ensure programmer param variable is always initialised ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/52719 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2d2b3039da2ef185cb31509b3901d56b428688b7 Gerrit-Change-Number: 52719 Gerrit-PatchSet: 1 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: 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: Wed, 28 Apr 2021 07:58:48 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

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

28 Apr '21

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

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit April 2021