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

Change in flashrom[master]: programmer: Smoothen register_spi_master() API
by Angel Pons (Code Review) 12 May '21

12 May '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/+/54066 ) Change subject: programmer: Smoothen register_spi_master() API ...................................................................... Patch Set 1: (1 comment) File spi.c: https://review.coreboot.org/c/flashrom/+/54066/comment/590c4a3c_6926d00d PS1, Line 151: if (data) : rmst.spi.data = data; > Yes, I agree on follow-up for par/opaque, but this follow up should actually happen for this approac […] You can do it. -- To view, visit https://review.coreboot.org/c/flashrom/+/54066 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0c753b3db050fb87d4bbe2301a7ead854f28456f Gerrit-Change-Number: 54066 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: 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-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, 12 May 2021 11:03:15 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: stlinkv3_spi.c: Refactor singleton states into reentrant pattern
by Anastasia Klimchuk (Code Review) 12 May '21

12 May '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/+/54042 ) Change subject: stlinkv3_spi.c: Refactor singleton states into reentrant pattern ...................................................................... Patch Set 1: (1 comment) File stlinkv3_spi.c: https://review.coreboot.org/c/flashrom/+/54042/comment/2ce2fb60_57b29ba1 PS1, Line 533: stlinkv3_spi_open(sck_freq_kHz, stlinkv3_handle > Uh, good point. >>> Before this commit, the handle was closed unless register_shutdown() failed. I am confused, because register_shutdown is called after this line, and it was after this line before. So if stlinkv3_spi_open fails, at that point shutdown hasn't been registered yet and so handle wasn't closed. Maybe I am missing something obvious or misunderstand something? I don't want to do a regression, definitely don't want. I really want to make things better, not worse... sorry if my words sound as "blaming existing code". So, I don't want to leave this in a state which looks worse than it was. Maybe I can just fix all init error paths here? what do you think -- To view, visit https://review.coreboot.org/c/flashrom/+/54042 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id044661b864b506028720ea809bc524f0640469f Gerrit-Change-Number: 54042 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: Miklós Márton <martonmiklosqdev(a)gmail.com> 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: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Wed, 12 May 2021 10:27:05 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: programmer: Smoothen register_spi_master() API
by Anastasia Klimchuk (Code Review) 12 May '21

12 May '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/+/54066 ) Change subject: programmer: Smoothen register_spi_master() API ...................................................................... Patch Set 1: (1 comment) File spi.c: https://review.coreboot.org/c/flashrom/+/54066/comment/48382af8_816327c1 PS1, Line 151: if (data) : rmst.spi.data = data; > I agree that par_master and opaque_master could also benefit from this API change, but they can be a […] Yes, I agree on follow-up for par/opaque, but this follow up should actually happen for this approach to work. I can do it myself, or not myself, in any case I would really appreciate to have some kind of coordination before! Also CB:51761 is still needed, but will need rebasing of course. -- To view, visit https://review.coreboot.org/c/flashrom/+/54066 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0c753b3db050fb87d4bbe2301a7ead854f28456f Gerrit-Change-Number: 54066 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: 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-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: Wed, 12 May 2021 10:13:43 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: programmer: Smoothen register_spi_master() API
by Angel Pons (Code Review) 12 May '21

12 May '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/+/54066 ) Change subject: programmer: Smoothen register_spi_master() API ...................................................................... Patch Set 1: (1 comment) File spi.c: https://review.coreboot.org/c/flashrom/+/54066/comment/8e6dbc02_9407b9c7 PS1, Line 151: if (data) : rmst.spi.data = data; > This would need to move to register_master at some point, because the same situation exists now for […] I agree that par_master and opaque_master could also benefit from this API change, but they can be adapted in follow-ups. Would this approach work for you? -- To view, visit https://review.coreboot.org/c/flashrom/+/54066 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0c753b3db050fb87d4bbe2301a7ead854f28456f Gerrit-Change-Number: 54066 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: 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-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, 12 May 2021 09:12:00 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: usbblaster_spi.c: Refactor singleton states into reentrant pattern
by Angel Pons (Code Review) 12 May '21

12 May '21

Attention is currently required from: Nico Huber, Anastasia Klimchuk. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/54044 ) Change subject: usbblaster_spi.c: Refactor singleton states into reentrant pattern ...................................................................... Patch Set 1: (2 comments) File usbblaster_spi.c: https://review.coreboot.org/c/flashrom/+/54044/comment/23e52223_42e21f59 PS1, Line 55: struct ftdi_context ftdic; > Oh I had the same question in CB:52774 and I explained why I am doing this (in short, mostly so that […] Ack https://review.coreboot.org/c/flashrom/+/54044/comment/87f21f2b_1d35b965 PS1, Line 185: struct ftdi_context ftdic; > I usually try to allocate data as late as possible to simplify error paths (as soon as you allocate […] I believe this patch is fine, hence the +2. I don't have a strong opinion on this. -- To view, visit https://review.coreboot.org/c/flashrom/+/54044 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia81f9f40c7eab430a8b304d0b197ce7c75bf5ace Gerrit-Change-Number: 54044 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: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Wed, 12 May 2021 09:07:28 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

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

12 May '21

Attention is currently required from: Edward O'Callaghan, Anastasia Klimchuk. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52773 ) Change subject: pickit2_spi.c: Separate shutdown from failed init cleanup ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/52773 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I1b672b33169a7a1b6ceab190ad3f48c2f35c3a1f Gerrit-Change-Number: 52773 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: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Wed, 12 May 2021 09:04:31 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: pickit2_spi.c: Refactor singleton states into reentrant pattern
by Angel Pons (Code Review) 12 May '21

12 May '21

Attention is currently required from: Anastasia Klimchuk. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52774 ) Change subject: pickit2_spi.c: Refactor singleton states into reentrant pattern ...................................................................... Patch Set 2: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/52774 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ibacc4738bee02c371c41583d321e0337128ad18a Gerrit-Change-Number: 52774 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: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Wed, 12 May 2021 09:03:10 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: Allow opaque programmers to provide status register access
by Nikolai Artemiev (Code Review) 11 May '21

11 May '21

Nikolai Artemiev has abandoned this change. ( https://review.coreboot.org/c/flashrom/+/53948 ) Change subject: Allow opaque programmers to provide status register access ...................................................................... Abandoned We were able to drop most of this and use a more direct implementation in downstream flashrom - https://chromium-review.googlesource.com/c/chromiumos/third_party/flashrom/… -- To view, visit https://review.coreboot.org/c/flashrom/+/53948 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I11a03c84363ec68a8edcbb9b10c7ca115e222fe0 Gerrit-Change-Number: 53948 Gerrit-PatchSet: 2 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Damien Zammit Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: abandon

1 0

Change in flashrom[master]: programmer: Smoothen register_spi_master() API
by Anastasia Klimchuk (Code Review) 11 May '21

11 May '21

Attention is currently required from: Nico Huber, Edward O'Callaghan. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/54066 ) Change subject: programmer: Smoothen register_spi_master() API ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: > https://review.coreboot.org/c/flashrom/+/51676 […] Mine was CB:51761 which has everyone +2s and it was the end goal to submit after I do all refactorings. Yeah it has overlap with this one... but I would need to do more function signature changes, not only data, also data size and shutdown function. I still have lots of global state to remove, and if this one and next one in the chain both are submitted I would need one less line for every refactoring. -- To view, visit https://review.coreboot.org/c/flashrom/+/54066 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0c753b3db050fb87d4bbe2301a7ead854f28456f Gerrit-Change-Number: 54066 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: 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-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Wed, 12 May 2021 05:32:11 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: programmer: Smoothen register_spi_master() API
by Edward O'Callaghan (Code Review) 11 May '21

11 May '21

Attention is currently required from: Nico Huber. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/54066 ) Change subject: programmer: Smoothen register_spi_master() API ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: > I thought this was Anastasia work? https://review.coreboot.org/c/flashrom/+/51676 which patch stream should I monitor Anastasia? -- To view, visit https://review.coreboot.org/c/flashrom/+/54066 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0c753b3db050fb87d4bbe2301a7ead854f28456f Gerrit-Change-Number: 54066 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: 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-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Comment-Date: Wed, 12 May 2021 01:12:24 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit May 2021