October 2022 - flashrom-gerrit

[M] Change in flashrom[master]: rayer_spi.c: Move param parse logic into own func
by Angel Pons (Code Review) 08 Oct '22

08 Oct '22

Attention is currently required from: Felix Singer, Edward O'Callaghan. Angel Pons 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/19009ea2_77c475f4 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. […] The idea is that the `prog_type` string isn't needed in the init code. So, instead of obtaining the `prog_type` string from `get_params()`, you could obtain `prog` directly. -- 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-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Sat, 08 Oct 2022 12:39:09 +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: Drop lpt_outbyte intermediate var
by Edward O'Callaghan (Code Review) 08 Oct '22

08 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/+/68232 ) Change subject: rayer_spi.c: Drop lpt_outbyte intermediate var ...................................................................... Patch Set 1: (2 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/68232/comment/7fd77780_5dc48638 PS1, Line 9: itermediate > typo: i*n*termediate Thank you, done. File rayer_spi.c: https://review.coreboot.org/c/flashrom/+/68232/comment/82b2cd7f_19815d4c PS1, Line 321: /* Get the initial value before writing to any line. */ > I would rewrite or remove the comment. I don't understand it. I didn't yet touch it as I didn't change the meaning of the code here. I think the comment is trying to say "get the initial iobase before writing out onto any hardware lines"? -- 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: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Sat, 08 Oct 2022 11:47:39 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net> 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) 08 Oct '22

08 Oct '22

Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/68238 to look at the new patch set (#2). 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/2 -- 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: 2 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.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) 08 Oct '22

08 Oct '22

Attention is currently required from: Edward O'Callaghan. Hello Felix Singer, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/68232 to look at the new patch set (#2). 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/2 -- 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: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

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

08 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/fe388c8c_694f8d2a PS1, Line 292: if (get_params(cfg, &lpt_iobase, &prog_type) < 0) > Hmmm, does it make sense to get I/O perms before parsing programmer params? That is a very good question, this maybe subjective patch though so I don't have strong feelings yet about it. However my internal reasoning was the following: conceptual rubrics: i) check permissions&&resource requirements are meet before doing anything, ii) check programmer configuration before init'ing, iii) do programmer init. the next step based off these rubrics, or flow if you will, would be to move (i)&&(ii) into core logic and only ever enter the programmer entry once (i)&&(ii) has satisfiability. Though I admit this patch on its own doesn't tell the above story so we could leave it for now. -- 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: Sat, 08 Oct 2022 11:45:08 +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: Move param parse logic into own func
by Edward O'Callaghan (Code Review) 08 Oct '22

08 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/+/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/1536baaf_27613d0f 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; : } : } > Isn't this part of getting the params? No, actually I didn't push CB:68238 before which made it unclear what I was thinking. However the idea here was to deconflate the parameter parse logic from the programmer type lookup logic into their own respective functions. This makes the init entry-point more linear in its control flow without all the resource management getting in the way. Hopefully the thinking is a little more clear with CB:68238 push please do let me know if not. -- 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-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Sat, 08 Oct 2022 11:39:13 +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) 08 Oct '22

08 Oct '22

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/68238 ) 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/1 diff --git a/rayer_spi.c b/rayer_spi.c index 14eb0ea..6984099 100644 --- a/rayer_spi.c +++ b/rayer_spi.c @@ -174,16 +174,6 @@ .miso_bit = 7, }; -static const struct rayer_programmer rayer_spi_types[] = { - {"rayer", NT, "RayeR SPIPGM", &rayer_spipgm}, - {"xilinx", NT, "Xilinx Parallel Cable III (DLC 5)", &xilinx_dlc5}, - {"byteblastermv", OK, "Altera ByteBlasterMV", &altera_byteblastermv}, - {"stk200", NT, "Atmel STK200/300 adapter", &atmel_stk200}, - {"wiggler", OK, "Wiggler LPT", &wiggler_lpt}, - {"spi_tt", NT, "SPI Tiny Tools (SPI_TT LPT)", &spi_tt}, - {0}, -}; - static void rayer_bitbang_set_cs(int val, void *spi_data) { struct rayer_spi_data *data = spi_data; @@ -278,9 +268,37 @@ return 0; } +static const struct rayer_programmer *find_progtype(const char *prog_type) +{ + if (!prog_type) + return NULL; + + const struct rayer_programmer rayer_spi_types[] = { + {"rayer", NT, "RayeR SPIPGM", &rayer_spipgm}, + {"xilinx", NT, "Xilinx Parallel Cable III (DLC 5)", &xilinx_dlc5}, + {"byteblastermv", OK, "Altera ByteBlasterMV", &altera_byteblastermv}, + {"stk200", NT, "Atmel STK200/300 adapter", &atmel_stk200}, + {"wiggler", OK, "Wiggler LPT", &wiggler_lpt}, + {"spi_tt", NT, "SPI Tiny Tools (SPI_TT LPT)", &spi_tt}, + {0}, + }; + const struct rayer_programmer *prog = rayer_spi_types; + for (; prog->type != NULL; prog++) { + if (strcasecmp(prog_type, prog->type) == 0) { + break; + } + } + + if (!prog->type) { + msg_perr("Error: Invalid device type specified.\n"); + return NULL; + } + + return prog; +} + static int rayer_spi_init(const struct programmer_cfg *cfg) { - const struct rayer_programmer *prog = rayer_spi_types; struct rayer_pinout *pinout = NULL; uint16_t lpt_iobase; char *prog_type; @@ -291,19 +309,10 @@ if (get_params(cfg, &lpt_iobase, &prog_type) < 0) return 1; - 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; - } - } + const struct rayer_programmer *prog = find_progtype(prog_type); + free(prog_type); + if (!prog) + return 1; msg_pdbg("Using address 0x%x as I/O base for parallel port access.\n", lpt_iobase); -- 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: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

1 0

[S] Change in flashrom[master]: libflashrom: Allow getting the progress_state from the flashctx
by Anastasia Klimchuk (Code Review) 08 Oct '22

08 Oct '22

Attention is currently required from: Felix Singer, Nico Huber, Richard Hughes, Edward O'Callaghan, Angel Pons, Daniel Campello. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/64663 ) Change subject: libflashrom: Allow getting the progress_state from the flashctx ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: Hello Angel, Nice to have a message from you! ;) You didn’t address me, but I see all the quotes are mine, so assume that’s for me. Firstly to the point of this patch and CB:49643. The bug that was discovered filed as https://ticket.coreboot.org/issues/390 and I tried to reach out to owners of the patch a few times, unfortunately with no success. Technically at the moment there are two options: 1 fix the bug and finish this patch 2 revert CB:49643 I am at the moment trying to find resources to fix the bug, I may or may not find them but I want to try. Meanwhile we discussed the situation on dev meeting (~2 months ago). There is an ongoing project of optimising erase functions selection and it is modifying the same areas of code as the libflashrom progress implementation. The project is important, and at the moment there is a chain of patches under testing, not yet merged. Modifying the same areas of code, doesn’t matter whether for fix or for revert would clash with the project, and no one wants it. At the same time, the bug itself doesn’t cause harm, it only shows wrong %progress in the terminal. So the decision was: don’t do anything at the moment, wait until erase optimisation is merged, and then either fix or revert. I added a comment to 390 after that meeting. You are welcome to comment on 390 with your thoughts on the matter, also you can click the Watch button and subscribe to any updates. As for the feature, myself I am not a user of the feature but I can see it is useful. Users were asking about it on the mailing list https://mail.coreboot.org/hyperkitty/list/flashrom@flashrom.org/thread/EJE3… Moreover it is in the TODO list for libflashrom on the website https://www.flashrom.org/Libflashrom The page was last modified by Nico in March 2019 (that’s before I joined) so maybe you can check with him for more details. So as you can see there is interest in the feature, both from users and from developers. Now some of my comments on your long text. I am confused by those your words: > patches from Gerrit get rubber-stamped and submitted without a proper review > customer wants to have experimental features available for testing ASAP etc None of this seems to apply to the patch CB:49643 Andel, the patch was in review for 1.5 years. Yes for that long. Moreover you Angel were one of the reviewers. During that time you didn’t say a word, didn’t add any comments. 6 other reviewers have added comments, and all comments have been resolved. What are all those your words about? > things being done in ways that do not align with flashrom's code review process Could you please give a link to where said process described? The thing is that our code review process is not clearly documented anywhere, but it should be. Each flashrom developer has a picture in their head of what the process should be, but what we need is one shared picture which is discussed, approved by everyone and documented. I really think this is important, and raised the topic earlier this year on the meeting, and then in April I started a thread on the mailing list, “Code Reviews guidelines discussion” here is the link https://mail.coreboot.org/hyperkitty/list/flashrom@flashrom.org/message/5WX… You are very welcome to respond to the thread! A little bit of info about reviews is in our Development Guidelines on the website, but it’s very generic and not sufficient. Let’s admit that flashrom has no clearly documented code review process, and let’s work together and create and document one. Meanwhile, I don’t think it’s fair to accuse people of not aligning with the process which is not documented. Believe me, contributors, especially new ones, love following clearly defined processes for the project they have joined. I am also interested in what exactly in your opinion was wrong in the process of CB:49643 (let’s collect some tribal knowledge at least). If you choose to answer this question, please answer it together with the question, why you haven’t raised any concerns during 1.5 years while the patch was in review. Also speaking about Development guidelines which is our little bit of documentation. I have read them many times this year, and I noticed some clashes with what you are saying. https://www.flashrom.org/Development_Guidelines > In most open-source projects, getting one's code reviewed takes an unspecified amount of time This would not fit into our Development guidelines that say: > All contributions should receive at least a preliminary review within one week of submission by some flashrom developer (if that doesn't happen in time, please be patient) Also you said > flashrom isn't the kind of project where "regressions in the master branch are fine, people should only use the releases" However Development guidelines say very differently: > As usual there is no quality promise for the state of the code on the master branch. Even though we will try to keep the regression rate as low as possible, the main purpose of the branch is to merge new commits and make them available to a broader audience for testing. I see that the word “customer” has sent your thoughts in a completely wrong direction (you even said it’s “my customer”). “User” is a better word. Let’s say user from now, and I meant flashrom user. Nico got it right. The User here is fwupd project. Both projects are FOSS so of course there are no payments involved. In any case, collaboration between fwupd and flashrom started before I joined. So you know the story better than me, you’ve been working on flashrom already at that time. > the result is no longer flashrom Speaking about what flashrom is and what it is not. I think flashrom is what we collectively create, we as the current group of people who are giving their time, skills, effort and a piece of their heart into running the project. Specifically on your statement of “if such and such mistakes have been done, then it’s not flashrom anymore”. It is still flashrom, with a room for improvement. IMO it is the responsibility of the group of maintainers, or “core developers” to analyse the mistakes and think how to prevent them from happening again. Flashrom is what we create as a result of our actions and words. You mentioned “historically”, by the way flashrom is ~20years old. Neither you nor me are here for that long. We can ask the founders what were their vision and goals, and whether flashrom now is still flashrom? > TL;DR: Let's work on finding solutions together. Your tldr is amazing! Yes please! Yes let’s do it, especially given that “let’s work together” is what I am repeating all this year! I agree, of course. But I want to ask you to speak honestly, without attempts to mislead the readers. Alright? So: For code reviews process/guidelines here is the thread, please share your ideas: https://mail.coreboot.org/hyperkitty/list/flashrom@flashrom.org/message/5WX… For the bug in showing progress in libflashrom, here it is: https://ticket.coreboot.org/issues/390 Also you had a remark about code quality which has dropped below expected levels in your opinion. Code quality is an important subject too, if you could start a thread on the mailing list about it? You could explain how you measure it, what are metrics? What is expected level? (especially in light of Development guidelines saying “As usual there is no quality promise for the state of the code on the master branch”) Ideally if you give examples of quality drops so that we can fix those and prevent from happening again. Thank you Angel, let’s work together. -- To view, visit https://review.coreboot.org/c/flashrom/+/64663 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I322bf56ff92f7b4d0ffc92768e9f0cdf7cb82010 Gerrit-Change-Number: 64663 Gerrit-PatchSet: 2 Gerrit-Owner: Richard Hughes <richard(a)hughsie.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Daniel Campello <campello(a)chromium.org> Gerrit-Reviewer: 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: Sean Rhodes <sean(a)starlabs.systems> 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-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Richard Hughes <richard(a)hughsie.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Daniel Campello <campello(a)chromium.org> Gerrit-Comment-Date: Sat, 08 Oct 2022 08:50:59 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net> Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Daniel Campello <campello(a)chromium.org> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: rayer_spi.c: Drop lpt_outbyte intermediate var
by Angel Pons (Code Review) 08 Oct '22

08 Oct '22

Attention is currently required from: Edward O'Callaghan. Angel Pons 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 1: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/68232/comment/086d2ff3_fdd1c0e5 PS1, Line 9: itermediate typo: i*n*termediate -- 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: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Sat, 08 Oct 2022 08:21:05 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: rayer_spi.c: Move param parse logic into own func
by Angel Pons (Code Review) 08 Oct '22

08 Oct '22

Attention is currently required from: Felix Singer, Edward O'Callaghan. Angel Pons 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: Code-Review+1 (1 comment) File rayer_spi.c: https://review.coreboot.org/c/flashrom/+/68230/comment/2ffe256c_c7f338c8 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; : } : } Isn't this part of getting the params? -- 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-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Sat, 08 Oct 2022 08:20:14 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit October 2022