December 2021 - flashrom-gerrit

Change in flashrom[master]: pcidev.c: Move pci_card_find() from internal to canonical place
by Nico Huber (Code Review) 15 Dec '21

15 Dec '21

Attention is currently required from: Edward O'Callaghan. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/59279 ) Change subject: pcidev.c: Move pci_card_find() from internal to canonical place ...................................................................... Patch Set 1: (2 comments) File pcidev.c: https://review.coreboot.org/c/flashrom/+/59279/comment/b39d0a4c_1b37ac33 PS1, Line 188: uint16_t card_vendor, uint16_t card_device) Please fix indentation. File programmer.h: https://review.coreboot.org/c/flashrom/+/59279/comment/a1eee550_feb4dfb7 PS1, Line 131: uint16_t card_vendor, uint16_t card_device); IIRC, the rule was no line breaks in header files. To ease greppability. -- To view, visit https://review.coreboot.org/c/flashrom/+/59279 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I026bfbecba114411728d4ad1ed8969b469fa7d2d Gerrit-Change-Number: 59279 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Wed, 15 Dec 2021 11:42:16 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: programmer.h: Make pacc global state static local
by Nico Huber (Code Review) 15 Dec '21

15 Dec '21

Attention is currently required from: Edward O'Callaghan, Anastasia Klimchuk, Peter Marheine. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/59278 ) Change subject: programmer.h: Make pacc global state static local ...................................................................... Patch Set 3: (2 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/59278/comment/31534fe5_25c0e780 PS3, Line 17: flow now. IMO it becomes harder to read. Is there anything in the queue ahead that follows up on this? File pcidev.c: https://review.coreboot.org/c/flashrom/+/59278/comment/c655bd76_b997c0b4 PS3, Line 37: saved_pacc This is not what one would expect. Usually the result of assignments and setters is the assigned value. -- To view, visit https://review.coreboot.org/c/flashrom/+/59278 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3bea1b93a7ca81dd42a64eec1e49a10f1fbda850 Gerrit-Change-Number: 59278 Gerrit-PatchSet: 3 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> 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-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Wed, 15 Dec 2021 11:36:37 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: pcidev: Move pci_get_dev() logic into canonical place
by Nico Huber (Code Review) 15 Dec '21

15 Dec '21

Attention is currently required from: Edward O'Callaghan. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/59276 ) Change subject: pcidev: Move pci_get_dev() logic into canonical place ...................................................................... Patch Set 2: (2 comments) File pcidev.c: https://review.coreboot.org/c/flashrom/+/59276/comment/0471b29c_1a33d9a4 PS2, Line 162: OLD_PCI_GET_DEV Thomas already looked into this and the old version practically pre-dates libpci (as a library separate from pciutils). I asked to keep it until the next release. But given that this won't happen anytime soon, I wouldn't mind if we drop it right away. File programmer.h: https://review.coreboot.org/c/flashrom/+/59276/comment/0487862c_75d55356 PS2, Line 128: struct pci_dev *pcidev_getdev(struct pci_dev *dev); The signature makes it very obvious that we already have a dev. How about a more meaningful name, e.g. pcidev_clonedev()? -- To view, visit https://review.coreboot.org/c/flashrom/+/59276 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id9ce055d5e5d347520ec5002b8c6548e60eaa0a7 Gerrit-Change-Number: 59276 Gerrit-PatchSet: 2 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: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Wed, 15 Dec 2021 11:31:20 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: pcidev: Avoid internal programmer relying on pacc global
by Edward O'Callaghan (Code Review) 15 Dec '21

15 Dec '21

Attention is currently required from: Nico Huber, Angel Pons, Anastasia Klimchuk, Peter Marheine. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/59275 ) Change subject: pcidev: Avoid internal programmer relying on pacc global ...................................................................... Patch Set 2: (5 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/59275/comment/798a687f_1e8ee16b PS2, Line 7: Avoid internal relying on pacc global > I guess the "internal" programmer was meant. Yes, "internal" was meant to mean "internal programmer". I added that to the commit message but feel free to adjust if my grammar isn't ideal. File internal.c: https://review.coreboot.org/c/flashrom/+/59275/comment/1f3e461b_e7f7570c PS2, Line 38: NULL > I guess this just happened to work. We might need `pacc` here too eventually. ACK, will look into that as a follow up in the pcidev series here. Thanks for pointing it out Nico. https://review.coreboot.org/c/flashrom/+/59275/comment/3c994fe4_098e28ff PS2, Line 41: temp = pcidev_scandev(&filter); : if (temp) { : /* Read PCI class */ : tmp2 = pci_read_word(temp, 0x0a); : if (tmp2 == devclass) : return temp; : } > The original code would continue walking the linked list of devices if `tmp2 == devclass` is false. […] Done. Thank you! https://review.coreboot.org/c/flashrom/+/59275/comment/be03f653_0fcce7e4 PS2, Line 73: temp = pcidev_scandev(&filter); : if (temp) { : if ((card_vendor == pci_read_word(temp, PCI_SUBSYSTEM_VENDOR_ID)) : && (card_device == pci_read_word(temp, PCI_SUBSYSTEM_ID))) : return temp; : } > Same situation as in `pci_dev_find_vendorclass()`, see comment above. Done. File pcidev.c: https://review.coreboot.org/c/flashrom/+/59275/comment/8f30f24d_eb7e29d6 PS2, Line 151: struct pci_dev *pcidev_scandev(struct pci_filter *filter) : { : struct pci_dev *temp; : for (temp = pacc->devices; temp; temp = temp->next) : if (pci_filter_match(filter, temp)) : return temp; : return NULL; : } > To allow resuming searches (see `internal.c` comments), I'd modify this function as follows: […] Done -- To view, visit https://review.coreboot.org/c/flashrom/+/59275 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id83bfd41f785f907e52a65a6689e8c7016fc1b77 Gerrit-Change-Number: 59275 Gerrit-PatchSet: 2 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: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.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: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Wed, 15 Dec 2021 11:12:26 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: pcidev: Avoid internal programmer relying on pacc global
by Edward O'Callaghan (Code Review) 15 Dec '21

15 Dec '21

Attention is currently required from: Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, Peter Marheine. Hello build bot (Jenkins), Nico Huber, Angel Pons, Anastasia Klimchuk, Peter Marheine, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/59275 to look at the new patch set (#3). Change subject: pcidev: Avoid internal programmer relying on pacc global ...................................................................... pcidev: Avoid internal programmer relying on pacc global Make progress towards the goal of removing pacc from global state as noted in the FIXME of programmer.h BUG=none TEST=builds Change-Id: Id83bfd41f785f907e52a65a6689e8c7016fc1b77 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M internal.c M pcidev.c M programmer.h 3 files changed, 20 insertions(+), 12 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/75/59275/3 -- To view, visit https://review.coreboot.org/c/flashrom/+/59275 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id83bfd41f785f907e52a65a6689e8c7016fc1b77 Gerrit-Change-Number: 59275 Gerrit-PatchSet: 3 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: Nico Huber <nico.h(a)gmx.de> 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: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: flashchips: add writeprotect bit layout map to chips
by Angel Pons (Code Review) 15 Dec '21

15 Dec '21

Attention is currently required from: Nico Huber, Edward O'Callaghan, Nikolai Artemiev, Sergii Dmytruk. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58477 ) Change subject: flashchips: add writeprotect bit layout map to chips ...................................................................... Patch Set 21: (1 comment) File flashchips.c: https://review.coreboot.org/c/flashrom/+/58477/comment/6d033f9d_389bec11 PS20, Line 6353: .tb = {STATUS1, 5, RW}, /* Called BP3 in datasheet, acts like TB */ : .sec = {STATUS1, 6, RW}, /* Called BP4 in datasheet, acts like SEC */ > Yeah, it's confusing especially since the complement bit is still called CMP. […] Hmmm, this can indeed be a problem. I haven't had time to check datasheets yet. -- To view, visit https://review.coreboot.org/c/flashrom/+/58477 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id08d77e6d4ca5109c0d698271146d026dbc21284 Gerrit-Change-Number: 58477 Gerrit-PatchSet: 21 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> 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: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> 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: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Wed, 15 Dec 2021 09:57:34 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: [RFC][WPTST] flashchips: enable write-protection for W25Q{64,128}.V
by Anastasia Klimchuk (Code Review) 15 Dec '21

15 Dec '21

Attention is currently required from: Nico Huber, Angel Pons, Sergii Dmytruk. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/59071 ) Change subject: [RFC][WPTST] flashchips: enable write-protection for W25Q{64,128}.V ...................................................................... Patch Set 13: Code-Review+1 (1 comment) Patchset: PS13: I think you can remove [RFC][WPTST] at this stage :) I am talking about 5 patches, dummy+tests (keep it for OTP for now). First prefix [RFC] is not needed anymore, this is already well into review. Speaking about [WPTST], if it has some meaning and you want to keep the meaning, you can express that as topic for patches. You can later search by topic (for example https://review.coreboot.org/q/topic:unit_tests). Topic is entirely optional, up to you. I usually set topic for my patches, but otherwise not many people use it. How to set the topic: p.5 in here https://www.flashrom.org/Development_Guidelines#Push_your_patch -- To view, visit https://review.coreboot.org/c/flashrom/+/59071 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iccb69a8d3a0dd2192e2c938caddaf07b1889ed35 Gerrit-Change-Number: 59071 Gerrit-PatchSet: 13 Gerrit-Owner: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> 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: Maciej Pijanowski <maciej.pijanowski(a)3mdeb.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Wed, 15 Dec 2021 02:03:07 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: [RFC][WPTST] tests: test write protection
by Anastasia Klimchuk (Code Review) 15 Dec '21

15 Dec '21

Attention is currently required from: Nico Huber, Nikolai Artemiev, Sergii Dmytruk. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/59075 ) Change subject: [RFC][WPTST] tests: test write protection ...................................................................... Patch Set 15: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/59075 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I49af7f6d173eb4c56c22d80b01a473b8c499c0f8 Gerrit-Change-Number: 59075 Gerrit-PatchSet: 15 Gerrit-Owner: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Maciej Pijanowski <maciej.pijanowski(a)3mdeb.com> Gerrit-CC: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Wed, 15 Dec 2021 01:53:02 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: [RFC][WPTST] dummyflasher: enforce write protection for W25Q128FV
by Anastasia Klimchuk (Code Review) 15 Dec '21

15 Dec '21

Attention is currently required from: Nico Huber, Angel Pons, Nikolai Artemiev, Sergii Dmytruk. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/59074 ) Change subject: [RFC][WPTST] dummyflasher: enforce write protection for W25Q128FV ...................................................................... Patch Set 14: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/59074 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9fd1417f941186391bd213bd355530143c8f04a0 Gerrit-Change-Number: 59074 Gerrit-PatchSet: 14 Gerrit-Owner: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> 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: Maciej Pijanowski <maciej.pijanowski(a)3mdeb.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Wed, 15 Dec 2021 01:49:00 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: [RFC][WPTST] dummyflasher: emulate SR2 for W25Q128FV
by Anastasia Klimchuk (Code Review) 15 Dec '21

15 Dec '21

Attention is currently required from: Nico Huber, Angel Pons, Sergii Dmytruk. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/59073 ) Change subject: [RFC][WPTST] dummyflasher: emulate SR2 for W25Q128FV ...................................................................... Patch Set 14: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/59073 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I79f9b4a0b604663d3288ad70dcbe3ea4075dede5 Gerrit-Change-Number: 59073 Gerrit-PatchSet: 14 Gerrit-Owner: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> 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: Maciej Pijanowski <maciej.pijanowski(a)3mdeb.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Wed, 15 Dec 2021 01:47:15 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit December 2021