September 2022 - flashrom-gerrit

[M] Change in flashrom[master]: tree: Fix drivers to pass prog_param to pcidev_init()
by Felix Singer (Code Review) 03 Sep '22

03 Sep '22

Attention is currently required from: Edward O'Callaghan, Anastasia Klimchuk. Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/66671 ) Change subject: tree: Fix drivers to pass prog_param to pcidev_init() ...................................................................... Patch Set 7: (1 comment) File include/programmer.h: https://review.coreboot.org/c/flashrom/+/66671/comment/a1dae453_cabbe534 PS3, Line 126: const char * > well it is higher typed now is it ok? usually prototypes don't need identifiers in truth and they te […] It would be consistent with the other commits and the lines below. Please do that. -- To view, visit https://review.coreboot.org/c/flashrom/+/66671 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie0c9d1c0866d44f64d037c596f2e30547fcfd58f Gerrit-Change-Number: 66671 Gerrit-PatchSet: 7 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Sat, 03 Sep 2022 02:14:06 +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> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: it87spi.c: Allow passing prog_param directly
by Felix Singer (Code Review) 03 Sep '22

03 Sep '22

Attention is currently required from: Edward O'Callaghan. Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/66662 ) Change subject: it87spi.c: Allow passing prog_param directly ...................................................................... Patch Set 6: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/66662/comment/d8a96c31_af08c174 PS6, Line 14: init_superio_ite() probably should be renamed to superio_ite_init(). I would remove that. Doesn't have much to do with the rest of the commit. Though, good point. -- To view, visit https://review.coreboot.org/c/flashrom/+/66662 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I598b1811c9734f41eee205d5a2b51ad8ac79e3ab Gerrit-Change-Number: 66662 Gerrit-PatchSet: 6 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Sat, 03 Sep 2022 01:51:31 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: tree: Allow passing prog_param string directly to programmer
by Felix Singer (Code Review) 03 Sep '22

03 Sep '22

Attention is currently required from: Edward O'Callaghan, Nikolai Artemiev. Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/66655 ) Change subject: tree: Allow passing prog_param string directly to programmer ...................................................................... Patch Set 6: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/66655/comment/509f7e65_8925622a PS6, Line 7: prog_param string programmer cfg -- To view, visit https://review.coreboot.org/c/flashrom/+/66655 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I8bab51a635b9d3a43e1619a7a32b334f4ce2cdd2 Gerrit-Change-Number: 66655 Gerrit-PatchSet: 6 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Sat, 03 Sep 2022 01:49:22 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: tree: Allow passing prog_param string directly to programmer
by Felix Singer (Code Review) 03 Sep '22

03 Sep '22

Attention is currently required from: Edward O'Callaghan, Nikolai Artemiev. Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/66655 ) Change subject: tree: Allow passing prog_param string directly to programmer ...................................................................... Patch Set 6: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/66655/comment/5ef96142_5ea8f6a4 PS6, Line 14: prog_param > I would adjust this instead of adding the other two commands It's not that important for me. The result is the same. Done. -- To view, visit https://review.coreboot.org/c/flashrom/+/66655 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I8bab51a635b9d3a43e1619a7a32b334f4ce2cdd2 Gerrit-Change-Number: 66655 Gerrit-PatchSet: 6 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Sat, 03 Sep 2022 01:43:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: ichspi.c: Retype appropiate variables with bool
by Nico Huber (Code Review) 02 Sep '22

02 Sep '22

Attention is currently required from: Felix Singer. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/66892 ) Change subject: ichspi.c: Retype appropiate variables with bool ...................................................................... Patch Set 10: (1 comment) File ichspi.c: https://review.coreboot.org/c/flashrom/+/66892/comment/44588e14_7f14e98d PS7, Line 872: write_cmd = true; > > (Also not nice to change the style that the original author choose, IMO.) > > Why should the bool type be a problem for the author but not a change to the variable name? It might be not wanted by the author? Maybe the author doesn't like it? I try to balance between noise in the Git history, the state of the code, review resources, diversity, and how people might perceive what happens to their code. Changing the type alone is mostly noise to me. Changing the identifiers would make the code more readable, IMO, enough to alter the balance. It's all subjective of course. > > This is an open source project and not anyones personal project. It's a project made by people. Please don't ignore that they have feelings about it. > > > > Changing the same lines over and over again also unnecessarily drains review resources. > > How much time did it take to write these two comments? I guess you wasted more of your time with writing these comments than with actually reviewing this patch. Much less than it took me to review the whole series. -- To view, visit https://review.coreboot.org/c/flashrom/+/66892 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If7eeacc44921f52aa593ab1302f17a5c5190f830 Gerrit-Change-Number: 66892 Gerrit-PatchSet: 10 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Comment-Date: Fri, 02 Sep 2022 18:51:56 +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> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: ichspi.c: Retype appropiate variables with bool
by Felix Singer (Code Review) 02 Sep '22

02 Sep '22

Attention is currently required from: Nico Huber. Hello build bot (Jenkins), Nico Huber, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/66892 to look at the new patch set (#10). Change subject: ichspi.c: Retype appropiate variables with bool ...................................................................... ichspi.c: Retype appropiate variables with bool Use the bool type instead of an integer for appropiate variables, since this represents their purpose much better. Signed-off-by: Felix Singer <felixsinger(a)posteo.net> Change-Id: If7eeacc44921f52aa593ab1302f17a5c5190f830 --- M ichspi.c 1 file changed, 24 insertions(+), 10 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/92/66892/10 -- To view, visit https://review.coreboot.org/c/flashrom/+/66892 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If7eeacc44921f52aa593ab1302f17a5c5190f830 Gerrit-Change-Number: 66892 Gerrit-PatchSet: 10 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: newpatchset

1 0

[S] Change in flashrom[master]: ichspi.c: Retype appropiate variables with bool
by Felix Singer (Code Review) 02 Sep '22

02 Sep '22

Attention is currently required from: Nico Huber. Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/66892 ) Change subject: ichspi.c: Retype appropiate variables with bool ...................................................................... Patch Set 9: (1 comment) File ichspi.c: https://review.coreboot.org/c/flashrom/+/66892/comment/82943f84_70210237 PS7, Line 872: write_cmd = true; > (Also not nice to change the style that the original author choose, IMO.) Why should the bool type be a problem for the author but not a change to the variable name? It might be not wanted by the author? Maybe the author doesn't like it? This is an open source project and not anyones personal project. > Changing the same lines over and over again also unnecessarily drains review resources. How much time did it take to write these two comments? I guess you wasted more of your time with writing these comments than with actually reviewing this patch. I think this discussion is pointless and I feel like you are creating arguments for no reason. I'm not going to dive into this discussion anymore as long as there are no real arguments against this patch, because this drains my resources unnecessarily. -- To view, visit https://review.coreboot.org/c/flashrom/+/66892 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If7eeacc44921f52aa593ab1302f17a5c5190f830 Gerrit-Change-Number: 66892 Gerrit-PatchSet: 9 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Comment-Date: Fri, 02 Sep 2022 17:36:55 +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> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: flashrom.c: Remove programmer_param global state
by Edward O'Callaghan (Code Review) 02 Sep '22

02 Sep '22

Attention is currently required from: Felix Singer, Thomas Heijligen, Edward O'Callaghan. Hello Felix Singer, build bot (Jenkins), Thomas Heijligen, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/66659 to look at the new patch set (#10). Change subject: flashrom.c: Remove programmer_param global state ...................................................................... flashrom.c: Remove programmer_param global state Change-Id: I778609e370e44ad2b63b8baa4984ac03ff4124d8 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M flashrom.c 1 file changed, 37 insertions(+), 25 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/59/66659/10 -- To view, visit https://review.coreboot.org/c/flashrom/+/66659 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I778609e370e44ad2b63b8baa4984ac03ff4124d8 Gerrit-Change-Number: 66659 Gerrit-PatchSet: 10 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[master]: tree: Fix drivers to pass prog_param to pcidev_init()
by Edward O'Callaghan (Code Review) 02 Sep '22

02 Sep '22

Attention is currently required from: Anastasia Klimchuk. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/66671 ) Change subject: tree: Fix drivers to pass prog_param to pcidev_init() ...................................................................... Patch Set 7: (1 comment) File include/programmer.h: https://review.coreboot.org/c/flashrom/+/66671/comment/c005f7ad_1df858ac PS3, Line 126: const char * > Is there a reason why parameter name is omitted? Other parameters have names. […] well it is higher typed now is it ok? usually prototypes don't need identifiers in truth and they tend to go stale anyway. happy to put cfg there if there is a strong feeling -- To view, visit https://review.coreboot.org/c/flashrom/+/66671 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie0c9d1c0866d44f64d037c596f2e30547fcfd58f Gerrit-Change-Number: 66671 Gerrit-PatchSet: 7 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Fri, 02 Sep 2022 12:12:58 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: ichspi.c: Retype appropiate variables with bool
by Nico Huber (Code Review) 02 Sep '22

02 Sep '22

Attention is currently required from: Felix Singer. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/66892 ) Change subject: ichspi.c: Retype appropiate variables with bool ...................................................................... Patch Set 9: (1 comment) File ichspi.c: https://review.coreboot.org/c/flashrom/+/66892/comment/89f315de_50aac599 PS7, Line 872: write_cmd = true; > Agree, but that's something for another patch series. Marking as resolved. Why? It achieves the same goal, but better. IMO, this patch alone is not worth the noise in the commit history. Changing the same lines over and over again also unnecessarily drains review resources. (Also not nice to change the style that the original author choose, IMO.) -- To view, visit https://review.coreboot.org/c/flashrom/+/66892 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If7eeacc44921f52aa593ab1302f17a5c5190f830 Gerrit-Change-Number: 66892 Gerrit-PatchSet: 9 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Comment-Date: Fri, 02 Sep 2022 11:46:45 +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> Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit September 2022