Attention is currently required from: Thomas Heijligen.
Hello build bot (Jenkins), Nico Huber, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/59050
to look at the new patch set (#16).
Change subject: Makefile: Make pkg-config mandatory to find libpci
......................................................................
Makefile: Make pkg-config mandatory to find libpci
Use `make HAS_LIBPCI=yes/no` to override the pkg-config detection and
`CONFIG_LIBPCI_CFLAGS` and `CONFIG_LIBPCI_LDFLAGS` to set cflags and
ldflags manually. The optional dependency of libpci libz, is
automatically handled by pkg-config.
Change-Id: I21b4a261b34b7e688635fc6e20b7beebfa64c7ed
Signed-off-by: Thomas Heijligen <thomas.heijligen(a)secunet.com>
---
M Makefile
A Makefile.d/pci_old_get_dev_test.c
M Makefile.include
3 files changed, 54 insertions(+), 105 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/50/59050/16
--
To view, visit https://review.coreboot.org/c/flashrom/+/59050
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I21b4a261b34b7e688635fc6e20b7beebfa64c7ed
Gerrit-Change-Number: 59050
Gerrit-PatchSet: 16
Gerrit-Owner: Thomas Heijligen <src(a)posteo.de>
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: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-MessageType: newpatchset
Attention is currently required from: Edward O'Callaghan, Daniel Campello, Anastasia Klimchuk.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/59291 )
Change subject: flashrom: Convert do_read() into a libflashrom user
......................................................................
Patch Set 4:
(3 comments)
File cli_classic.c:
https://review.coreboot.org/c/flashrom/+/59291/comment/51f2db08_7b1be3f8
PS4, Line 635: if (map_flash(&flashes[0]) != 0) {
: free(flashes[0].chip);
: ret = 1;
: goto out_shutdown;
: }
: msg_cinfo("Please note that forced reads most likely contain garbage.\n");
: ret = do_read(&flashes[0], filename);
: unmap_flash(&flashes[0]);
Mapping/unmapping is redundant with the calls in prepare_/finalize_flash_access().
Results are probably undefined.
I guess this might have been done on purpose to avoid the checks in prepare_flash_access().
File flashrom.c:
https://review.coreboot.org/c/flashrom/+/59291/comment/8e65c595_64fc1380
PS4, Line 1935: }
Looks redundant given the checks in prepare_flash_access()?
https://review.coreboot.org/c/flashrom/+/59291/comment/76b6ea56_d4961c90
PS4, Line 2169: 1024
KiB?
--
To view, visit https://review.coreboot.org/c/flashrom/+/59291
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I4b690b688acf9d5deb46e8642a252a2132ea8c73
Gerrit-Change-Number: 59291
Gerrit-PatchSet: 4
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: Daniel Campello <campello(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Daniel Campello <campello(a)chromium.org>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Thu, 23 Dec 2021 14:22:55 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Felix Singer, Edward O'Callaghan, Anastasia Klimchuk, Nikolai Artemiev.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/60272 )
Change subject: ichspi: Remove unneeded line breaks, add useful ones
......................................................................
Patch Set 4: Code-Review+2
(2 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/60272/comment/eee6416f_28d713ca
PS4, Line 7: ichspi: Remove unneeded line breaks, add useful ones
There are also some other changes, e.g. added tabs.
https://review.coreboot.org/c/flashrom/+/60272/comment/51afc439_f241d26f
PS4, Line 13: 2) probe and read on Panther Point (7 series PCH)
Not this patch, actually. Also, a better test would be to check if the binary stays the same ;)
--
To view, visit https://review.coreboot.org/c/flashrom/+/60272
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I7ca2902b7caaa95418b828b068c661afafdcd171
Gerrit-Change-Number: 60272
Gerrit-PatchSet: 4
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: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Singer <felixsinger(a)posteo.net>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Comment-Date: Thu, 23 Dec 2021 12:31:45 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Felix Singer, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, Nikolai Artemiev.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58735 )
Change subject: ichspi: Split very long init function into two
......................................................................
Patch Set 5: Code-Review+2
(1 comment)
Patchset:
PS5:
Nothing more to be done but waiting for a test, I suppose :)
--
To view, visit https://review.coreboot.org/c/flashrom/+/58735
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I6789bc456a4878e6555831ae0b80ecbdbf62938b
Gerrit-Change-Number: 58735
Gerrit-PatchSet: 5
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: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Singer <felixsinger(a)posteo.net>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
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: Thu, 23 Dec 2021 12:25:16 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Felix Singer, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, Nikolai Artemiev.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58735 )
Change subject: ichspi: Split very long init function into two
......................................................................
Patch Set 5:
(1 comment)
File ichspi.c:
https://review.coreboot.org/c/flashrom/+/58735/comment/8729a9b6_1702a483
PS1, Line 1717: msg_pdbg("0x00: 0x%04x (SPIS)\n",
: mmio_readw(spibar + 0));
: msg_pdbg("0x02: 0x%04x (SPIC)\n",
: mmio_readw(spibar + 2));
: msg_pdbg("0x04: 0x%08x (SPIA)\n",
: mmio_readl(spibar + 4));
: ichspi_bbar = mmio_readl(spibar + 0x50);
: msg_pdbg("0x50: 0x%08x (BBAR)\n",
: ichspi_bbar);
: msg_pdbg("0x54: 0x%04x (PREOP)\n",
: mmio_readw(spibar + 0x54));
: msg_pdbg("0x56: 0x%04x (OPTYPE)\n",
: mmio_readw(spibar + 0x56));
: msg_pdbg("0x58: 0x%08x (OPMENU)\n",
: mmio_readl(spibar + 0x58));
: msg_pdbg("0x5c: 0x%08x (OPMENU+4)\n",
: mmio_readl(spibar + 0x5c));
> I was initially thinking of not doing two different things in one commit, so here I only moved the c […]
Well, to me it's all just whitespace, line-breaks, tabs, spaces. When changing
one, why not the other? Not in this hunk but in others IIRC, the spurious line
break is actually a consequence of removing a tab.
--
To view, visit https://review.coreboot.org/c/flashrom/+/58735
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I6789bc456a4878e6555831ae0b80ecbdbf62938b
Gerrit-Change-Number: 58735
Gerrit-PatchSet: 5
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: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Singer <felixsinger(a)posteo.net>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
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: Thu, 23 Dec 2021 12:22:33 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, Nikolai Artemiev.
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58736 )
Change subject: ichspi: Extract handling programmer param into a function
......................................................................
Patch Set 5:
(1 comment)
File ichspi.c:
https://review.coreboot.org/c/flashrom/+/58736/comment/80e6ef41_041512cc
PS2, Line 1764: arg
> I like this idea, makes sense, and I want to do this. […]
Sure!
--
To view, visit https://review.coreboot.org/c/flashrom/+/58736
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I20e2379a6fd58c9346f0a2d6daf2b8decf1f6976
Gerrit-Change-Number: 58736
Gerrit-PatchSet: 5
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: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Singer <felixsinger(a)posteo.net>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.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: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Comment-Date: Thu, 23 Dec 2021 06:10:27 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Felix Singer, Nico Huber, Edward O'Callaghan, Angel Pons, Nikolai Artemiev.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58735 )
Change subject: ichspi: Split very long init function into two
......................................................................
Patch Set 5:
(1 comment)
File ichspi.c:
https://review.coreboot.org/c/flashrom/+/58735/comment/560d6573_78f359d7
PS1, Line 1717: msg_pdbg("0x00: 0x%04x (SPIS)\n",
: mmio_readw(spibar + 0));
: msg_pdbg("0x02: 0x%04x (SPIC)\n",
: mmio_readw(spibar + 2));
: msg_pdbg("0x04: 0x%08x (SPIA)\n",
: mmio_readl(spibar + 4));
: ichspi_bbar = mmio_readl(spibar + 0x50);
: msg_pdbg("0x50: 0x%08x (BBAR)\n",
: ichspi_bbar);
: msg_pdbg("0x54: 0x%04x (PREOP)\n",
: mmio_readw(spibar + 0x54));
: msg_pdbg("0x56: 0x%04x (OPTYPE)\n",
: mmio_readw(spibar + 0x56));
: msg_pdbg("0x58: 0x%08x (OPMENU)\n",
: mmio_readl(spibar + 0x58));
: msg_pdbg("0x5c: 0x%08x (OPMENU+4)\n",
: mmio_readl(spibar + 0x5c));
> Would have preferred it here, it seems wrong to change lines and then fix […]
I was initially thinking of not doing two different things in one commit, so here I only moved the code without changes. And then line breaks in separate commit. But now it looks like separate commit for line breaks is really useful: we can fix a lot more line breaks, throughout the file!
It looks like the file was written with firm 80-chars-width rule in mind.
--
To view, visit https://review.coreboot.org/c/flashrom/+/58735
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I6789bc456a4878e6555831ae0b80ecbdbf62938b
Gerrit-Change-Number: 58735
Gerrit-PatchSet: 5
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: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Singer <felixsinger(a)posteo.net>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
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-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Comment-Date: Thu, 23 Dec 2021 04:22:58 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Felix Singer, Nico Huber, Edward O'Callaghan, Nikolai Artemiev.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/60272 )
Change subject: ichspi: Remove unneeded line breaks, add useful ones
......................................................................
Patch Set 4:
(6 comments)
Patchset:
PS4:
Given that we now have a dedicated commit for line breaks, I went through the file and removed few more of those.
File ichspi.c:
https://review.coreboot.org/c/flashrom/+/60272/comment/01511769_baae195e
PS1, Line 930: REGREAD16(ICH7_REG_SPIS));
> This too
Done
https://review.coreboot.org/c/flashrom/+/60272/comment/5463b2dc_c3ead33a
PS1, Line 999: SSFC_DBC);
> I would remove this line break.
Done
https://review.coreboot.org/c/flashrom/+/60272/comment/202b3c2a_a8b4272e
PS1, Line 1052: REGREAD32(swseq_data.reg_ssfsc));
> This too
Done
https://review.coreboot.org/c/flashrom/+/60272/comment/fe9857ba_9cd8eddd
PS1, Line 1127: cmd);
> This too
Done
https://review.coreboot.org/c/flashrom/+/60272/comment/f2806f5a_3e95053b
PS1, Line 1402: "N
> This too
Done
--
To view, visit https://review.coreboot.org/c/flashrom/+/60272
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I7ca2902b7caaa95418b828b068c661afafdcd171
Gerrit-Change-Number: 60272
Gerrit-PatchSet: 4
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: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Singer <felixsinger(a)posteo.net>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
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-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Comment-Date: Thu, 23 Dec 2021 04:16:27 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net>
Gerrit-MessageType: comment
Attention is currently required from: Felix Singer, Edward O'Callaghan, Angel Pons, Nikolai Artemiev.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58736 )
Change subject: ichspi: Extract handling programmer param into a function
......................................................................
Patch Set 5:
(1 comment)
File ichspi.c:
https://review.coreboot.org/c/flashrom/+/58736/comment/2f23b65b_54f53ccf
PS2, Line 1764: arg
> I think so. I would do that in a separate commit so that changes don't get mixed up.
I like this idea, makes sense, and I want to do this.
Is it fine to have it later (in January)? and in a separate chain? There is another idea in previous patch, I will gather all that in a chain.
--
To view, visit https://review.coreboot.org/c/flashrom/+/58736
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I20e2379a6fd58c9346f0a2d6daf2b8decf1f6976
Gerrit-Change-Number: 58736
Gerrit-PatchSet: 5
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: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Singer <felixsinger(a)posteo.net>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Comment-Date: Thu, 23 Dec 2021 04:13:02 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: comment