October 2022 - flashrom-gerrit

[M] Change in flashrom[master]: drivers/: Make 'fallback_{un}map' the default unless defined
by Felix Singer (Code Review) 06 Oct '22

06 Oct '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/+/67404 ) Change subject: drivers/: Make 'fallback_{un}map' the default unless defined ...................................................................... Patch Set 4: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/67404/comment/b4e567c1_9908f523 PS4, Line 7: drivers/ > Maybe "programmers" is better? I know there is a subtle difference, and I forgot what exactly it is, […] "driver" relates to the thing which hooks up the programmer and makes it usable, in this case the programmer_entry struct. So I think it's fine. -- To view, visit https://review.coreboot.org/c/flashrom/+/67404 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I5ea7bd68f7ae2cd4af9902ef07255ab6ce0bfdb3 Gerrit-Change-Number: 67404 Gerrit-PatchSet: 4 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: Jonathon Hall <jonathon.hall(a)puri.sm> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Thu, 06 Oct 2022 05:24:12 +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]: mediatek_i2c_spi: document programmer
by Anastasia Klimchuk (Code Review) 06 Oct '22

06 Oct '22

Attention is currently required from: Angel Pons, Peter Marheine. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67825 ) Change subject: mediatek_i2c_spi: document programmer ...................................................................... Patch Set 3: Code-Review+2 (1 comment) Patchset: PS3: > Looks good to me, but I would appreciate if someone else from Google could approve this. Sure, I just wanted to allow some time so that everyone can have a look! -- To view, visit https://review.coreboot.org/c/flashrom/+/67825 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia63df470170fbadcabadcdad8e5acc0cde3a274b Gerrit-Change-Number: 67825 Gerrit-PatchSet: 3 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> 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: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Thu, 06 Oct 2022 05:21:28 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: drivers/: Make 'fallback_{un}map' the default unless defined
by Anastasia Klimchuk (Code Review) 06 Oct '22

06 Oct '22

Attention is currently required from: Edward O'Callaghan. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67404 ) Change subject: drivers/: Make 'fallback_{un}map' the default unless defined ...................................................................... Patch Set 4: Code-Review+2 (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/67404/comment/8e83b7e2_de4f0b2c PS4, Line 7: drivers/ Maybe "programmers" is better? I know there is a subtle difference, and I forgot what exactly it is, but in this case I think the change is over programmers. -- To view, visit https://review.coreboot.org/c/flashrom/+/67404 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I5ea7bd68f7ae2cd4af9902ef07255ab6ce0bfdb3 Gerrit-Change-Number: 67404 Gerrit-PatchSet: 4 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: Jonathon Hall <jonathon.hall(a)puri.sm> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Thu, 06 Oct 2022 05:17:09 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: flashrom.c: Make programmer_{un}map_flash_region() static
by Anastasia Klimchuk (Code Review) 06 Oct '22

06 Oct '22

Attention is currently required from: Edward O'Callaghan. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/66677 ) Change subject: flashrom.c: Make programmer_{un}map_flash_region() static ...................................................................... Patch Set 7: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/66677 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic02092ce23c5b3233aad38343b888e3fa7e5bcf9 Gerrit-Change-Number: 66677 Gerrit-PatchSet: 7 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Jonathon Hall <jonathon.hall(a)puri.sm> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Thu, 06 Oct 2022 04:57:08 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: tree/: Move programmer_delay() out of programmer state machine
by Anastasia Klimchuk (Code Review) 06 Oct '22

06 Oct '22

Attention is currently required from: Felix Singer, Nico Huber, Edward O'Callaghan. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67393 ) Change subject: tree/: Move programmer_delay() out of programmer state machine ...................................................................... Patch Set 4: (2 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/67393/comment/5ab8260b_81e1d2c0 PS4, Line 13: I just read this conversation once again, I think I understand a bit more what Nico is saying. > I'm missing the reason for the change The change comes from an earlier discovery that two programmers (serprog and ch341a_spi) have custom delay functions and are using programmer data in those functions. As we have an ongoing effort to remove global state from all programmers, the question came up. Essentially, we need a way to access programmer data inside custom delay functions. As a result, a combination of 3 patches achieves the goal: CB:67391 (previous to this) this patch CB:66373 After these 3 patches, global state can be removed from serprog and ch341a_spi > Programmers that register multiple masters would need redundant code. I see what you mean. In practice, the change impacts only programmers with custom delay functions, out of them only one has multiple masters, serprog. So this results in two same lines in serprog. Maybe it's not too bad? I understand you have more longer-term ideas of modifying/improving libflashrom API. Do you think you can start a discussion about it? Can we maybe unblock removing global state from serprog and ch341a_spi and then start with the effort around libflashrom API (whatever would the an outcome of the discussion). Also a note about: > There are more complex programmer drivers that most likely have global state that isn't associated with a specific master. We actually should be able to answer the question exactly, like which drivers and what is that state (if any). As of today, most of programmers have global state removed and few left. The most complicated left, for sure, but not many by the count, so we can go through and answer the question. I can answer that the ones I haven't looked closely are ichspi and internal, the rest of what is left comes down to using data in programmer_delay or {un}map_flash functions. File serprog.c: https://review.coreboot.org/c/flashrom/+/67393/comment/8f9df498_cf2ffca4 PS4, Line 451: }; > That will be default delay, see first patch in the chain CB:67391 Actually I looked at this again, and yes this probably needs delay too. Same as in `par_master_serprog` as I understand. (somehow I got confused when I looked here earlier at the time of my previous comment) I also added some more thoughts to the other longer comment thread. -- To view, visit https://review.coreboot.org/c/flashrom/+/67393 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id059abb58b31a066a408009073912da2b224d40c Gerrit-Change-Number: 67393 Gerrit-PatchSet: 4 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Alexander Goncharov <chat(a)joursoir.net> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Thu, 06 Oct 2022 04:47:30 +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: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: mediatek_i2c_spi: document programmer
by Felix Singer (Code Review) 06 Oct '22

06 Oct '22

Attention is currently required from: Angel Pons, Peter Marheine. Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67825 ) Change subject: mediatek_i2c_spi: document programmer ...................................................................... Patch Set 3: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/67825/comment/e6c645e3_006b3cfc PS3, Line 7: mediatek_i2c_spi: document programmer Better this: flashrom.8.tmpl: Document the programmer "mediatek_i2c_spi" -- To view, visit https://review.coreboot.org/c/flashrom/+/67825 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia63df470170fbadcabadcdad8e5acc0cde3a274b Gerrit-Change-Number: 67825 Gerrit-PatchSet: 3 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> 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: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Thu, 06 Oct 2022 03:50:22 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: mediatek_i2c_spi: document programmer
by Felix Singer (Code Review) 06 Oct '22

06 Oct '22

Attention is currently required from: Angel Pons, Peter Marheine. Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67825 ) Change subject: mediatek_i2c_spi: document programmer ...................................................................... Patch Set 3: Code-Review+2 (1 comment) Patchset: PS3: Looks good to me, but I would appreciate if someone else from Google could approve this. -- To view, visit https://review.coreboot.org/c/flashrom/+/67825 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia63df470170fbadcabadcdad8e5acc0cde3a274b Gerrit-Change-Number: 67825 Gerrit-PatchSet: 3 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> 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: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Thu, 06 Oct 2022 03:46:44 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: mediatek_i2c_spi: document programmer
by Peter Marheine (Code Review) 06 Oct '22

06 Oct '22

Attention is currently required from: Felix Singer, Angel Pons. Peter Marheine has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67825 ) Change subject: mediatek_i2c_spi: document programmer ...................................................................... Patch Set 3: (2 comments) File flashrom.8.tmpl: https://review.coreboot.org/c/flashrom/+/67825/comment/8f891686_18dab689 PS2, Line 1585: a > an? Done https://review.coreboot.org/c/flashrom/+/67825/comment/e92a3475_7195af24 PS2, Line 1588: is > it's? Seems like an unnecessary word completely, so removed. -- To view, visit https://review.coreboot.org/c/flashrom/+/67825 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia63df470170fbadcabadcdad8e5acc0cde3a274b Gerrit-Change-Number: 67825 Gerrit-PatchSet: 3 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> 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: 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: Thu, 06 Oct 2022 03:33:42 +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]: mediatek_i2c_spi: document programmer
by Peter Marheine (Code Review) 06 Oct '22

06 Oct '22

Attention is currently required from: Angel Pons, Peter Marheine. Hello Felix Singer, build bot (Jenkins), Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/67825 to look at the new patch set (#3). Change subject: mediatek_i2c_spi: document programmer ...................................................................... mediatek_i2c_spi: document programmer This adds a manpage section describing the mediatek_i2c_spi programmer, including some discussion of devices that are supported as well as which systems use them. Signed-off-by: Peter Marheine <pmarheine(a)chromium.org> Change-Id: Ia63df470170fbadcabadcdad8e5acc0cde3a274b --- M flashrom.8.tmpl 1 file changed, 38 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/25/67825/3 -- To view, visit https://review.coreboot.org/c/flashrom/+/67825 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia63df470170fbadcabadcdad8e5acc0cde3a274b Gerrit-Change-Number: 67825 Gerrit-PatchSet: 3 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> 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: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[master]: spi25_statusreg: support reading/writing "configuration registers"
by Edward O'Callaghan (Code Review) 06 Oct '22

06 Oct '22

Attention is currently required from: Nico Huber, Angel Pons, Anastasia Klimchuk, Sergii Dmytruk. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/66211 ) Change subject: spi25_statusreg: support reading/writing "configuration registers" ...................................................................... Patch Set 5: (1 comment) Patchset: PS2: > Hmm, so far it isn't but it could be indeed. But if these functions are called […] I noticed this comment after I hit submit on my review where the alias idea came from. I don't think it is a good idea to alias things if it is beyond just a identifier change. The vendor here is not only calling it something different but it is also semantically different with its own opcode. While the aliasing is useful to keep things compact it incurs its own cost (a surjection) where we trade off making one branch more fragile with more branch complexity around the feature flag than maintaining the function having a 1:1 mapping of register<->opcode as before. The function being a injection is quite a nice property to maintain in this case as it is invertible for reverse lookups. tl;dr I believe the idea of using a alias is good just not in this specific case. -- To view, visit https://review.coreboot.org/c/flashrom/+/66211 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I45f9afcc31f1928ef6263a749596380082963de4 Gerrit-Change-Number: 66211 Gerrit-PatchSet: 5 Gerrit-Owner: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-CC: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Thu, 06 Oct 2022 01:37:25 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit October 2022