October 2022 - flashrom-gerrit

[M] Change in flashrom[master]: rayer_spi.c: Roll up programmer type search logic into func
by Edward O'Callaghan (Code Review) 09 Oct '22

09 Oct '22

Attention is currently required from: Felix Singer, Angel Pons. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68238 ) Change subject: rayer_spi.c: Roll up programmer type search logic into func ...................................................................... Patch Set 3: (3 comments) File rayer_spi.c: https://review.coreboot.org/c/flashrom/+/68238/comment/77e20375_a4514f32 PS2, Line 271: static const struct rayer_programmer *find_progtype(const char *prog_type) > If you want to keep CB:68239 as a separate commit, could you please move this function above `get_pa […] Done. Thank you for the future refinement of CB:68239 , looks much better after that. https://review.coreboot.org/c/flashrom/+/68238/comment/abb67604_17318915 PS2, Line 274: NULL > No, this changes behavior. […] Fixed, thanks for spotting this!. https://review.coreboot.org/c/flashrom/+/68238/comment/3986ef86_998e2427 PS2, Line 276: const struct rayer_programmer rayer_spi_types[] = { > Shouldn't this be marked `static`? Not sure if it matters for constants, but if this were to be plac […] Good question. I am not sure if this is actually required, I need to check the C spec again but I put `static` here for now just in case. Seems harmless if not strictly required. -- To view, visit https://review.coreboot.org/c/flashrom/+/68238 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id226ea61132ecc30fd8696e1d8ea50373e752cac Gerrit-Change-Number: 68238 Gerrit-PatchSet: 3 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Sun, 09 Oct 2022 02:10:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: rayer_spi.c: Roll up programmer type search logic into func
by Edward O'Callaghan (Code Review) 09 Oct '22

09 Oct '22

Attention is currently required from: Felix Singer, Edward O'Callaghan. Hello Felix Singer, build bot (Jenkins), Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/68238 to look at the new patch set (#4). Change subject: rayer_spi.c: Roll up programmer type search logic into func ...................................................................... rayer_spi.c: Roll up programmer type search logic into func Roll up the programmer type table search and match logic into it's own function and lexically scope the 'rayer_spi_types' table into the function while we are here. Change-Id: Id226ea61132ecc30fd8696e1d8ea50373e752cac Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M rayer_spi.c 1 file changed, 47 insertions(+), 24 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/38/68238/4 -- To view, visit https://review.coreboot.org/c/flashrom/+/68238 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id226ea61132ecc30fd8696e1d8ea50373e752cac Gerrit-Change-Number: 68238 Gerrit-PatchSet: 4 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[master]: rayer_spi.c: Move param parse logic into own func
by Edward O'Callaghan (Code Review) 09 Oct '22

09 Oct '22

Attention is currently required from: Felix Singer. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68230 ) Change subject: rayer_spi.c: Move param parse logic into own func ...................................................................... Patch Set 1: (1 comment) File rayer_spi.c: https://review.coreboot.org/c/flashrom/+/68230/comment/2fb232e1_c894e8f0 PS1, Line 292: if (prog_type) { : for (; prog->type != NULL; prog++) { : if (strcasecmp(prog_type, prog->type) == 0) { : break; : } : } : free(prog_type); : : if (prog->type == NULL) { : msg_perr("Error: Invalid device type specified.\n"); : return 1; : } : } > > No, actually I didn't push CB:68238 before which made it unclear what I was thinking. […] Excellent, Thank you! I'll take a look over the ft2232_spi changes too. -- To view, visit https://review.coreboot.org/c/flashrom/+/68230 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I287aa2e5d94e872553d08c0750f8dc6d60b9caff Gerrit-Change-Number: 68230 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Comment-Date: Sun, 09 Oct 2022 02:01:18 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: rayer_spi.c: Get rid of temporary `prog_type` string
by Edward O'Callaghan (Code Review) 09 Oct '22

09 Oct '22

Attention is currently required from: Angel Pons. Edward O'Callaghan has uploaded a new patch set (#2) to the change originally created by Angel Pons. ( https://review.coreboot.org/c/flashrom/+/68239 ) Change subject: rayer_spi.c: Get rid of temporary `prog_type` string ...................................................................... rayer_spi.c: Get rid of temporary `prog_type` string Make the `get_params()` function provide a pointer to `struct rayer_programmer` directly, instead of having a `prog_type` string passed around three functions. Change-Id: I83e34382ee9814f224025e21e5099fdab73cee8c Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> --- M rayer_spi.c 1 file changed, 22 insertions(+), 14 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/39/68239/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/68239 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I83e34382ee9814f224025e21e5099fdab73cee8c Gerrit-Change-Number: 68239 Gerrit-PatchSet: 2 Gerrit-Owner: 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: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[master]: rayer_spi.c: Roll up programmer type search logic into func
by Edward O'Callaghan (Code Review) 09 Oct '22

09 Oct '22

Attention is currently required from: Felix Singer, Edward O'Callaghan. Hello Felix Singer, build bot (Jenkins), Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/68238 to look at the new patch set (#3). Change subject: rayer_spi.c: Roll up programmer type search logic into func ...................................................................... rayer_spi.c: Roll up programmer type search logic into func Roll up the programmer type table search and match logic into it's own function and lexically scope the 'rayer_spi_types' table into the function while we are here. Change-Id: Id226ea61132ecc30fd8696e1d8ea50373e752cac Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M rayer_spi.c 1 file changed, 47 insertions(+), 24 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/38/68238/3 -- To view, visit https://review.coreboot.org/c/flashrom/+/68238 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id226ea61132ecc30fd8696e1d8ea50373e752cac Gerrit-Change-Number: 68238 Gerrit-PatchSet: 3 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

[S] Change in flashrom[master]: rayer_spi.c: Drop lpt_outbyte intermediate var
by Edward O'Callaghan (Code Review) 09 Oct '22

09 Oct '22

Attention is currently required from: Edward O'Callaghan. Hello Felix Singer, build bot (Jenkins), Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/68232 to look at the new patch set (#3). Change subject: rayer_spi.c: Drop lpt_outbyte intermediate var ...................................................................... rayer_spi.c: Drop lpt_outbyte intermediate var The intermediate variable in this case serves no extra assistance in readability or additional control flow branching. Just assign the result directly into the driver state tracker. Change-Id: Idedabb7b1c401d666b3b7e621e75704c7e765fd1 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M rayer_spi.c 1 file changed, 17 insertions(+), 5 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/32/68232/3 -- To view, visit https://review.coreboot.org/c/flashrom/+/68232 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Idedabb7b1c401d666b3b7e621e75704c7e765fd1 Gerrit-Change-Number: 68232 Gerrit-PatchSet: 3 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

[S] Change in flashrom[master]: rayer_spi.c: Drop lpt_outbyte intermediate var
by Felix Singer (Code Review) 09 Oct '22

09 Oct '22

Attention is currently required from: Edward O'Callaghan. Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68232 ) Change subject: rayer_spi.c: Drop lpt_outbyte intermediate var ...................................................................... Patch Set 2: Code-Review+2 (1 comment) File rayer_spi.c: https://review.coreboot.org/c/flashrom/+/68232/comment/9804df69_ac3ef10b PS1, Line 321: /* Get the initial value before writing to any line. */ > I didn't yet touch it as I didn't change the meaning of the code here. […] Probably. I'm fine with keeping it for now since the intention of this commit is just cleaning up some code here, and also the comment existed before. Marking as resolved. -- To view, visit https://review.coreboot.org/c/flashrom/+/68232 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Idedabb7b1c401d666b3b7e621e75704c7e765fd1 Gerrit-Change-Number: 68232 Gerrit-PatchSet: 2 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Sun, 09 Oct 2022 01:56:55 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net> Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: rayer_spi.c: Move rget_io_perms() check to pre-amble
by Edward O'Callaghan (Code Review) 09 Oct '22

09 Oct '22

Edward O'Callaghan has abandoned this change. ( https://review.coreboot.org/c/flashrom/+/68231 ) Change subject: rayer_spi.c: Move rget_io_perms() check to pre-amble ...................................................................... Abandoned long term the inverse of this idea is required - check programmer params before rget_io_perms() call. -- To view, visit https://review.coreboot.org/c/flashrom/+/68231 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ieb0a494bd49f6986e9ac230b7b123329493a82bc Gerrit-Change-Number: 68231 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: abandon

1 0

[S] Change in flashrom[master]: rayer_spi.c: Move rget_io_perms() check to pre-amble
by Edward O'Callaghan (Code Review) 09 Oct '22

09 Oct '22

Attention is currently required from: Nico Huber, Thomas Heijligen, Angel Pons. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68231 ) Change subject: rayer_spi.c: Move rget_io_perms() check to pre-amble ...................................................................... Patch Set 1: (1 comment) File rayer_spi.c: https://review.coreboot.org/c/flashrom/+/68231/comment/123eec6e_167e811a PS1, Line 292: if (get_params(cfg, &lpt_iobase, &prog_type) < 0) > The three steps seem reasonable, but i) and ii) seem to be in inverse order. […] You are correct, great insight Angel. Can we spawn a bug so this doesn't get lost? I will abandon this commit. -- To view, visit https://review.coreboot.org/c/flashrom/+/68231 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ieb0a494bd49f6986e9ac230b7b123329493a82bc Gerrit-Change-Number: 68231 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Sun, 09 Oct 2022 01:32:58 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: rayer_spi.c: Get rid of temporary `prog_type` string
by Edward O'Callaghan (Code Review) 09 Oct '22

09 Oct '22

Attention is currently required from: Angel Pons. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68239 ) Change subject: rayer_spi.c: Get rid of temporary `prog_type` string ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/68239 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I83e34382ee9814f224025e21e5099fdab73cee8c Gerrit-Change-Number: 68239 Gerrit-PatchSet: 1 Gerrit-Owner: 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: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Sun, 09 Oct 2022 01:29:23 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit October 2022