March 2022 - flashrom-gerrit - mail.coreboot.org

Change in flashrom[master]: tests: Remove mock struct pci_dev, use real pci symbols in tests
by Anastasia Klimchuk (Code Review) 22 Mar '22

22 Mar '22

Attention is currently required from: Nico Huber, Thomas Heijligen, Edward O'Callaghan. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62948 ) Change subject: tests: Remove mock struct pci_dev, use real pci symbols in tests ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: > Nit: For the future: can we have standalone mock headers for usb, pci, hwaccess, ... […] Yes! It came up in the conversations earlier, this is definitely option. And it always remains an option that we can decide and implement at any time. Technically "at any time", practically "at any time when someone has time" :) Out of the headers you listed, hwaccess is already mocked for tests: tests/hwaccess_x86_io_unittest.h which is included in flashrom_test_dep (line number at the time of writing this) meson.build#L522 So we know it is definitely a working solution, to have mock header for tests. -- To view, visit https://review.coreboot.org/c/flashrom/+/62948 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I1206fbdca392f190066a364376ce0db28071e53c Gerrit-Change-Number: 62948 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> 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-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Tue, 22 Mar 2022 04:12:23 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Thomas Heijligen <src(a)posteo.de> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: hwaccess: replace macros by C code
by Martin Roth (Code Review) 22 Mar '22

22 Mar '22

Attention is currently required from: Nico Huber, Thomas Heijligen, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, Nikolai Artemiev. Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62898 ) Change subject: hwaccess: replace macros by C code ...................................................................... Patch Set 6: (1 comment) File hwaccess.h: https://review.coreboot.org/c/flashrom/+/62898/comment/a52e4282_d57d1419 PS6, Line 34: inline > Why to force inline? The GCC should do a proper job for optimizing and I would trust it to do the ri […] Okay, I thought you wanted it to always be inline to match what the macro was doing. If you don't care whether it's actually inlined, then I agree the attribute isn't needed. GCC will decide what to put inline based on the optimization level. Currently, the Makefile sets the optimization to -Os, which should probably put this inline even without the inline statement since it enables inline-small-functions. If we're trusting the compiler to do the "right" thing here, why even mark it inline at all? -- To view, visit https://review.coreboot.org/c/flashrom/+/62898 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I86d38d816b37c283279c485fac8027f8fb94364a Gerrit-Change-Number: 62898 Gerrit-PatchSet: 6 Gerrit-Owner: Thomas Heijligen <src(a)posteo.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: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> 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-CC: Martin Roth <martinroth(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> 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: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Tue, 22 Mar 2022 03:29:09 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Martin Roth <martinroth(a)google.com> Comment-In-Reply-To: Thomas Heijligen <src(a)posteo.de> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: pcidev: Move pci_dev_find() from internal to canonical place
by Thomas Heijligen (Code Review) 21 Mar '22

21 Mar '22

Attention is currently required from: Nico Huber, Edward O'Callaghan. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/59280 ) Change subject: pcidev: Move pci_dev_find() from internal to canonical place ...................................................................... Patch Set 7: Code-Review+2 (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/59280/comment/cf50e500_b2da7eae PS6, Line 9: Squash in, > Done Thanks -- To view, visit https://review.coreboot.org/c/flashrom/+/59280 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie21f87699481a84398ca4450b3f03548f0528191 Gerrit-Change-Number: 59280 Gerrit-PatchSet: 7 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: Thomas Heijligen <src(a)posteo.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-Comment-Date: Tue, 22 Mar 2022 01:25:25 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Thomas Heijligen <src(a)posteo.de> Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: hwaccess: replace macros by C code
by Thomas Heijligen (Code Review) 21 Mar '22

21 Mar '22

Attention is currently required from: Nico Huber, Martin Roth, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, Nikolai Artemiev. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62898 ) Change subject: hwaccess: replace macros by C code ...................................................................... Patch Set 6: (1 comment) File hwaccess.h: https://review.coreboot.org/c/flashrom/+/62898/comment/b133881f_ef80fdaf PS6, Line 34: inline > Do these need __attribute__((always_inline)) to actually force it to be inline? My understanding is […] Why to force inline? The GCC should do a proper job for optimizing and I would trust it to do the right thing. In this case I wouldn't dictate the exact behavior to GCC -- To view, visit https://review.coreboot.org/c/flashrom/+/62898 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I86d38d816b37c283279c485fac8027f8fb94364a Gerrit-Change-Number: 62898 Gerrit-PatchSet: 6 Gerrit-Owner: Thomas Heijligen <src(a)posteo.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: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> 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-CC: Martin Roth <martinroth(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Martin Roth <martinroth(a)google.com> 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: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Tue, 22 Mar 2022 01:19:20 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Martin Roth <martinroth(a)google.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: pcidev: Move pci_dev_find() from internal to canonical place
by Edward O'Callaghan (Code Review) 21 Mar '22

21 Mar '22

Attention is currently required from: Nico Huber. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/59280 ) Change subject: pcidev: Move pci_dev_find() from internal to canonical place ...................................................................... Patch Set 7: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/59280/comment/f89ff2cf_3e3116f6 PS6, Line 9: Squash in, > Nit, align text with previous commit in chain. They do the same thing, only for different functions. Done -- To view, visit https://review.coreboot.org/c/flashrom/+/59280 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie21f87699481a84398ca4450b3f03548f0528191 Gerrit-Change-Number: 59280 Gerrit-PatchSet: 7 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: Thomas Heijligen <src(a)posteo.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-Comment-Date: Tue, 22 Mar 2022 01:03:18 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Thomas Heijligen <src(a)posteo.de> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: pcidev: Move pci_dev_find() from internal to canonical place
by Edward O'Callaghan (Code Review) 21 Mar '22

21 Mar '22

Attention is currently required from: Nico Huber, Edward O'Callaghan. Hello build bot (Jenkins), Nico Huber, Thomas Heijligen, Anastasia Klimchuk, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/59280 to look at the new patch set (#7). Change subject: pcidev: Move pci_dev_find() from internal to canonical place ...................................................................... pcidev: Move pci_dev_find() from internal to canonical place Also rename to `pcidev_find()` in fitting with pcidev.c helpers. BUG=b:220950271 TEST=```sudo ./flashrom -p internal -r /tmp/bios <snip> Found Programmer flash chip "Opaque flash chip" (16384 kB, Programmer-specific) mapped at physical address 0x0000000000000000. Reading flash... done. ``` Change-Id: Ie21f87699481a84398ca4450b3f03548f0528191 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M board_enable.c M chipset_enable.c M internal.c M pcidev.c M programmer.h M sb600spi.c 6 files changed, 32 insertions(+), 33 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/80/59280/7 -- To view, visit https://review.coreboot.org/c/flashrom/+/59280 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie21f87699481a84398ca4450b3f03548f0528191 Gerrit-Change-Number: 59280 Gerrit-PatchSet: 7 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: Thomas Heijligen <src(a)posteo.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-MessageType: newpatchset

1 0

Change in flashrom[master]: hwaccess: replace macros by C code
by Martin Roth (Code Review) 21 Mar '22

21 Mar '22

Attention is currently required from: Nico Huber, Thomas Heijligen, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, Nikolai Artemiev. Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62898 ) Change subject: hwaccess: replace macros by C code ...................................................................... Patch Set 6: (1 comment) File hwaccess.h: https://review.coreboot.org/c/flashrom/+/62898/comment/eec624ee_b5b0c1e7 PS6, Line 34: inline Do these need __attribute__((always_inline)) to actually force it to be inline? My understanding is that without the attribute, the compiler (GCC at least) can choose what to do based on optimization for size/speed. -- To view, visit https://review.coreboot.org/c/flashrom/+/62898 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I86d38d816b37c283279c485fac8027f8fb94364a Gerrit-Change-Number: 62898 Gerrit-PatchSet: 6 Gerrit-Owner: Thomas Heijligen <src(a)posteo.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: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> 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-CC: Martin Roth <martinroth(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> 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: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Tue, 22 Mar 2022 01:00:24 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: hwaccess: replace macros by C code
by Peter Marheine (Code Review) 21 Mar '22

21 Mar '22

Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, Nikolai Artemiev. Peter Marheine has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62898 ) Change subject: hwaccess: replace macros by C code ...................................................................... Patch Set 6: Code-Review+1 (1 comment) Patchset: PS6: Build changes LGTM -- To view, visit https://review.coreboot.org/c/flashrom/+/62898 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I86d38d816b37c283279c485fac8027f8fb94364a Gerrit-Change-Number: 62898 Gerrit-PatchSet: 6 Gerrit-Owner: Thomas Heijligen <src(a)posteo.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: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> 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-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-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Tue, 22 Mar 2022 00:56:52 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: hwaccess: replace macros by C code
by Thomas Heijligen (Code Review) 21 Mar '22

21 Mar '22

Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, Nikolai Artemiev. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62898 ) Change subject: hwaccess: replace macros by C code ...................................................................... Patch Set 6: (1 comment) File hwaccess.h: https://review.coreboot.org/c/flashrom/+/62898/comment/6ff50ac5_b5d52d50 PS6, Line 28: swap > It used to be "swab" and now it is "swap" (one char difference in the name). […] I thought to swap (with the meaning of exchange) is a better wording. I'm not fixed with it. -- To view, visit https://review.coreboot.org/c/flashrom/+/62898 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I86d38d816b37c283279c485fac8027f8fb94364a Gerrit-Change-Number: 62898 Gerrit-PatchSet: 6 Gerrit-Owner: Thomas Heijligen <src(a)posteo.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: Felix Singer <felixsinger(a)posteo.net> 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: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Peter Marheine <pmarheine(a)chromium.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-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Tue, 22 Mar 2022 00:56:46 +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]: ichspi: Introduce FCYCLE_MASK(n) macro
by Edward O'Callaghan (Code Review) 21 Mar '22

21 Mar '22

Attention is currently required from: Nico Huber, Subrata Banik, Angel Pons, Anastasia Klimchuk. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62891 ) Change subject: ichspi: Introduce FCYCLE_MASK(n) macro ...................................................................... Patch Set 5: (1 comment) File ichspi.c: https://review.coreboot.org/c/flashrom/+/62891/comment/5c45b893_d51dae7e PS5, Line 506: _p > unrelated pp change to this patch. Ack -- To view, visit https://review.coreboot.org/c/flashrom/+/62891 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id222304165610c7ae48e365d72ec8fdeea51c51d Gerrit-Change-Number: 62891 Gerrit-PatchSet: 5 Gerrit-Owner: Subrata Banik <subratabanik(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: 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: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Mon, 21 Mar 2022 23:52:44 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: comment

1 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit March 2022