September 2022 - flashrom-gerrit

[S] Change in flashrom[1.2.x]: pcidev: Always fetch ident info
by Nico Huber (Code Review) 29 Sep '22

29 Sep '22

Attention is currently required from: Nico Huber, Thomas Heijligen, Daniel Verkamp. Hello Felix Singer, build bot (Jenkins), Daniel Verkamp, Thomas Heijligen, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/67877 to look at the new patch set (#2). Change subject: pcidev: Always fetch ident info ...................................................................... pcidev: Always fetch ident info As discovered earlier[1], the `vendor_id` and `device_id` fields are not always automatically set. However, we use these fields throughout flash- rom. To not lose track when we actually fetched them, let's always call pci_fill_info(PCI_FILL_IDENT) before returning a `pci_dev` handle. [1] Commit ca2e3bce0 (pcidev.c: populate IDs with pci_fill_info()) Backported to older versions where pcidev handling was much more scattered. Signed-off-by: Nico Huber <nico.h(a)gmx.de> Change-Id: Iae2511178bec44343cbe902722fdca9eda036059 Ticket: https://ticket.coreboot.org/issues/367 Reviewed-on: https://review.coreboot.org/c/flashrom/+/64573 --- M board_enable.c M internal.c 2 files changed, 34 insertions(+), 4 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/77/67877/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/67877 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: 1.2.x Gerrit-Change-Id: Iae2511178bec44343cbe902722fdca9eda036059 Gerrit-Change-Number: 67877 Gerrit-PatchSet: 2 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Daniel Verkamp <dverkamp(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-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Daniel Verkamp <dverkamp(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

[S] Change in flashrom[1.1.x]: pcidev: Always fetch ident info
by Nico Huber (Code Review) 29 Sep '22

29 Sep '22

Attention is currently required from: Nico Huber, Thomas Heijligen, Daniel Verkamp. Hello Felix Singer, build bot (Jenkins), Daniel Verkamp, Thomas Heijligen, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/67859 to look at the new patch set (#2). Change subject: pcidev: Always fetch ident info ...................................................................... pcidev: Always fetch ident info As discovered earlier[1], the `vendor_id` and `device_id` fields are not always automatically set. However, we use these fields throughout flash- rom. To not lose track when we actually fetched them, let's always call pci_fill_info(PCI_FILL_IDENT) before returning a `pci_dev` handle. [1] Commit ca2e3bce0 (pcidev.c: populate IDs with pci_fill_info()) Backported to older versions where pcidev handling was much more scattered. Signed-off-by: Nico Huber <nico.h(a)gmx.de> Change-Id: Iae2511178bec44343cbe902722fdca9eda036059 Ticket: https://ticket.coreboot.org/issues/367 Reviewed-on: https://review.coreboot.org/c/flashrom/+/64573 --- M board_enable.c M internal.c 2 files changed, 39 insertions(+), 6 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/59/67859/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/67859 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: 1.1.x Gerrit-Change-Id: Iae2511178bec44343cbe902722fdca9eda036059 Gerrit-Change-Number: 67859 Gerrit-PatchSet: 2 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Daniel Verkamp <dverkamp(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-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Daniel Verkamp <dverkamp(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

[S] Change in flashrom[1.0.x]: pcidev: Always fetch ident info
by Nico Huber (Code Review) 29 Sep '22

29 Sep '22

Attention is currently required from: Nico Huber, Thomas Heijligen, Daniel Verkamp. Hello Felix Singer, build bot (Jenkins), Daniel Verkamp, Thomas Heijligen, Edward O'Callaghan, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/67845 to look at the new patch set (#2). Change subject: pcidev: Always fetch ident info ...................................................................... pcidev: Always fetch ident info As discovered earlier[1], the `vendor_id` and `device_id` fields are not always automatically set. However, we use these fields throughout flash- rom. To not lose track when we actually fetched them, let's always call pci_fill_info(PCI_FILL_IDENT) before returning a `pci_dev` handle. [1] Commit ca2e3bce0 (pcidev.c: populate IDs with pci_fill_info()) Backported to older versions where pcidev handling was much more scattered. Signed-off-by: Nico Huber <nico.h(a)gmx.de> Change-Id: Iae2511178bec44343cbe902722fdca9eda036059 Ticket: https://ticket.coreboot.org/issues/367 Reviewed-on: https://review.coreboot.org/c/flashrom/+/64573 --- M board_enable.c M internal.c 2 files changed, 39 insertions(+), 6 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/45/67845/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/67845 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: 1.0.x Gerrit-Change-Id: Iae2511178bec44343cbe902722fdca9eda036059 Gerrit-Change-Number: 67845 Gerrit-PatchSet: 2 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Daniel Verkamp <dverkamp(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-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Daniel Verkamp <dverkamp(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

[S] Change in flashrom[1.1.x]: pickit2_spi: Fix "dead" assignment
by Nico Huber (Code Review) 29 Sep '22

29 Sep '22

Attention is currently required from: Nico Huber. Hello Felix Singer, build bot (Jenkins), Elyes Haouas, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/67846 to look at the new patch set (#2). Change subject: pickit2_spi: Fix "dead" assignment ...................................................................... pickit2_spi: Fix "dead" assignment We never read the first 'ret'. Let's check the first 'ret' and exit if it failed. Also, print the version only when the command succeeded. Backported to libusb-v0 version (checking for CMD_LENGTH instead of 0 return value). Found-by: scan-build 7.0.1-8 Change-Id: I4aac5e1f3bd0604b079e1fdd9b7f09f1f4fc2d7f Signed-off-by: Elyes HAOUAS <ehaouas(a)noos.fr> Reviewed-on: https://review.coreboot.org/c/flashrom/+/34403 --- M pickit2_spi.c 1 file changed, 27 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/46/67846/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/67846 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: 1.1.x Gerrit-Change-Id: I4aac5e1f3bd0604b079e1fdd9b7f09f1f4fc2d7f Gerrit-Change-Number: 67846 Gerrit-PatchSet: 2 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> 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[1.0.x]: pickit2_spi: Fix "dead" assignment
by Nico Huber (Code Review) 29 Sep '22

29 Sep '22

Attention is currently required from: Nico Huber. Hello Felix Singer, build bot (Jenkins), Edward O'Callaghan, Angel Pons, Elyes Haouas, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/67838 to look at the new patch set (#2). Change subject: pickit2_spi: Fix "dead" assignment ...................................................................... pickit2_spi: Fix "dead" assignment We never read the first 'ret'. Let's check the first 'ret' and exit if it failed. Also, print the version only when the command succeeded. Backported to libusb-v0 version (checking for CMD_LENGTH instead of 0 return value). Found-by: scan-build 7.0.1-8 Change-Id: I4aac5e1f3bd0604b079e1fdd9b7f09f1f4fc2d7f Signed-off-by: Elyes HAOUAS <ehaouas(a)noos.fr> Reviewed-on: https://review.coreboot.org/c/flashrom/+/34403 --- M pickit2_spi.c 1 file changed, 27 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/38/67838/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/67838 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: 1.0.x Gerrit-Change-Id: I4aac5e1f3bd0604b079e1fdd9b7f09f1f4fc2d7f Gerrit-Change-Number: 67838 Gerrit-PatchSet: 2 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: newpatchset

1 0

[L] Change in flashrom[master]: dirtyjtag: Add DirtyJTAG programmer
by Nico Huber (Code Review) 29 Sep '22

29 Sep '22

Attention is currently required from: Felix Singer, Thomas Heijligen, Jean THOMAS, Alexander Goncharov. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67878 ) Change subject: dirtyjtag: Add DirtyJTAG programmer ...................................................................... Patch Set 6: Code-Review+1 (5 comments) Patchset: PS6: > Hi Nico, thanks for your feedback. […] Depends on the reader, I guess. I was a little confused by the double memcpy() at first. Other flashrom developers might have avoided the big allocation. I guess it doesn't matter. On systems with USB we probably have resources to spare. Current implementation looks good. PS6: One more style nit I've noticed: we also place a space before the asterisk in pointer casts, also always spaces around operators with two or more operands. File dirtyjtag_spi.c: https://review.coreboot.org/c/flashrom/+/67878/comment/24a53a86_7683491a PS5, Line 121: size_t num_xfer = (len + max_xfer_size - 1 ) / max_xfer_size; // ceil(len/max_xfer_size) > > 1. Lift the `.max_data_*` limits so this function gets called for bigger > > chunks. There should be less overhead as we wouldn't have to send the > > TMS/STOP sequence as often. I assume this could speed up long reads. > > Some of the memcpy() also seems unnecessary, but the USB transfers > > probably limit speed so much that it doesn't matter. > I went with that option. > Did you test if it affects the read/write speed? https://review.coreboot.org/c/flashrom/+/67878/comment/ef6fcd78_9f74d05c PS5, Line 267: } > This could probably be written much simpler if we'd focus on the strcmp. […] Minor misunderstanding, I think. I meant this `if` block is all that is needed. The `if ((units > tmp) && (units < end))` is not needed (or actually points to another problem, see below). `units < end` checks if there is a unit given at all. I misinterpreted your original intention but you fixed that already :) using only strcasecmp() implicitly also checks that there is a string. `units > tmp` could actually only be false if there was no number given. In this case we shouldn't complain about the unit. Probably best to check for `freq != 0` explicitly. Or for the final value `1 <= freq && freq <= UINT16_MAX`? NB. The canonical name for this option in flashrom is `spispeed`. Your code will probably be the most generic version so it might make sense to turn it into an API. Effort to unify this setting was already discussed a few times, but people seem to always get distracted ;) https://review.coreboot.org/c/flashrom/+/67878/comment/f938f198_65742b2b PS5, Line 275: except TRST > Yes SRST and TMS are also high during init. […] Looks good, thank you :) -- To view, visit https://review.coreboot.org/c/flashrom/+/67878 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic43e9a014ed7d04e429e73b30c9dcfdde1a78913 Gerrit-Change-Number: 67878 Gerrit-PatchSet: 6 Gerrit-Owner: Jean THOMAS <virgule(a)jeanthomas.me> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> 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: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Jean THOMAS <virgule(a)jeanthomas.me> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Comment-Date: Thu, 29 Sep 2022 11:21:29 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Jean THOMAS <virgule(a)jeanthomas.me> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: jlink_spi.c: Set `use_serial_number` to false by default
by Angel Pons (Code Review) 29 Sep '22

29 Sep '22

Attention is currently required from: Felix Singer. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67881 ) Change subject: jlink_spi.c: Set `use_serial_number` to false by default ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/67881 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I83b5e651097d1a248baf5ddb6b09e5cff161317d Gerrit-Change-Number: 67881 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Comment-Date: Thu, 29 Sep 2022 10:56:24 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: jlink_spi.c: Move function argument into previous line
by Angel Pons (Code Review) 29 Sep '22

29 Sep '22

Attention is currently required from: Felix Singer. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67883 ) Change subject: jlink_spi.c: Move function argument into previous line ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/67883 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I13b7f37adb242abbbd3a2961e7cdc4cf7ec578d8 Gerrit-Change-Number: 67883 Gerrit-PatchSet: 2 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Comment-Date: Thu, 29 Sep 2022 10:55:52 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[1.1.x]: flashchips: Fix N25Q512 bulk erase
by Angel Pons (Code Review) 29 Sep '22

29 Sep '22

Attention is currently required from: Nico Huber, Jacob Creedon. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67847 ) Change subject: flashchips: Fix N25Q512 bulk erase ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/67847 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: 1.1.x Gerrit-Change-Id: Ib408fbe5633abd8b657e3907142b997e88b33f84 Gerrit-Change-Number: 67847 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Jacob Creedon <jcreedon(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Jacob Creedon <jcreedon(a)google.com> Gerrit-Comment-Date: Thu, 29 Sep 2022 10:55:16 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[L] Change in flashrom[master]: dirtyjtag: Add DirtyJTAG programmer
by Angel Pons (Code Review) 29 Sep '22

29 Sep '22

Attention is currently required from: Felix Singer, Nico Huber, Thomas Heijligen, Jean THOMAS, Alexander Goncharov. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67878 ) Change subject: dirtyjtag: Add DirtyJTAG programmer ...................................................................... Patch Set 6: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/67878 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic43e9a014ed7d04e429e73b30c9dcfdde1a78913 Gerrit-Change-Number: 67878 Gerrit-PatchSet: 6 Gerrit-Owner: Jean THOMAS <virgule(a)jeanthomas.me> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> 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: Alexander Goncharov <chat(a)joursoir.net> 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: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Jean THOMAS <virgule(a)jeanthomas.me> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Comment-Date: Thu, 29 Sep 2022 10:51:07 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit September 2022