- flashrom-gerrit - mail.coreboot.org

Change in flashrom[master]: ichspi: Split very long init function into two
by Anastasia Klimchuk (Code Review) 23 Dec '21

23 Dec '21

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: (2 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/58735/comment/302c9c75_a5a4b0f7 PS3, Line 20: TEST=1) probe-read-verify-erase section-write-reboot : on Intel octopus board with GD25LQ128C/GD25LQ128D/GD25LQ128E : 2) probe and read on Panther Point (7 series PCH) Thanks a lot everyone for explanations! I decided to take Angel's suggestion about TEST, and also I added Tested-By (commit message has been updated with all that). Tell me if you see something wrong. I repeated the same section in commit message for other patches in the chain. > Without this part, it might be that both tests failed spectacularly, but the commit message wouldn't be lying: the change *was* tested, but without success! 😄 :))) Patchset: PS1: > Sorry to disappoint. There seems to be a misunderstanding. PCHs inherited the […] Oh alright I got it. All good, I appreciate your help anyway! I will ask, not sure whether this year of maybe after the break, but definitely need to do this. -- 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:05:47 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net> Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ichspi: Split very long init function into two
by Anastasia Klimchuk (Code Review) 23 Dec '21

23 Dec '21

Attention is currently required from: Felix Singer, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, Nikolai Artemiev. Hello build bot (Jenkins), Nico Huber, Edward O'Callaghan, Angel Pons, Nikolai Artemiev, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/58735 to look at the new patch set (#4). Change subject: ichspi: Split very long init function into two ...................................................................... ichspi: Split very long init function into two ich_init_spi is very long, but logically it can be split. Init function detects the chipset and then the rest of operations depends on the chipset. Init function is more readable now, it consists of only a switch. Initialisation of hwseq and swseq that used to happen in the beginning of init function now moved to init_ich_default, because hwseq and swseq are only used for chipsets served by init_ich_default. TEST ME ON ICH7 BUG=b:204488958 TEST=Check that the following scenarios still behave properly: 1) probe-read-verify-erase section-write-reboot on Intel octopus board with GD25LQ128C/GD25LQ128D/GD25LQ128E 2) probe and read on Panther Point (7 series PCH) Change-Id: I6789bc456a4878e6555831ae0b80ecbdbf62938b Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> Tested-by: Anastasia Klimchuk <aklm(a)chromium.org> Tested-by: Nico Huber <nico.h(a)gmx.de> --- M ichspi.c 1 file changed, 268 insertions(+), 255 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/35/58735/4 -- 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: 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: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: ichspi: Extract handling programmer param into a function
by Anastasia Klimchuk (Code Review) 23 Dec '21

23 Dec '21

Attention is currently required from: Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, Nikolai Artemiev. Hello build bot (Jenkins), Nico Huber, Edward O'Callaghan, Angel Pons, Nikolai Artemiev, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/58736 to look at the new patch set (#4). Change subject: ichspi: Extract handling programmer param into a function ...................................................................... ichspi: Extract handling programmer param into a function Extract processing of ich_spi_mode into a separate function which is called from init_ich_default. This makes init_ich_default more readable and avoids one local variable. TEST ME ON ICH7 BUG=b:204488958 TEST=Check that the following scenarios still behave properly: 1) probe-read-verify-erase section-write-reboot on Intel octopus board with GD25LQ128C/GD25LQ128D/GD25LQ128E 2) probe and read on Panther Point (7 series PCH) Change-Id: I20e2379a6fd58c9346f0a2d6daf2b8decf1f6976 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> Tested-by: Anastasia Klimchuk <aklm(a)chromium.org> Tested-by: Nico Huber <nico.h(a)gmx.de> --- M ichspi.c 1 file changed, 37 insertions(+), 27 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/36/58736/4 -- 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: 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: 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-MessageType: newpatchset

1 0

Change in flashrom[master]: ichspi: Extract initialisation of swseq and hwseq into a function
by Anastasia Klimchuk (Code Review) 23 Dec '21

23 Dec '21

Attention is currently required from: Felix Singer, Angel Pons, Anastasia Klimchuk, Nikolai Artemiev. Hello build bot (Jenkins), Nico Huber, Edward O'Callaghan, Angel Pons, Nikolai Artemiev, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/58737 to look at the new patch set (#4). Change subject: ichspi: Extract initialisation of swseq and hwseq into a function ...................................................................... ichspi: Extract initialisation of swseq and hwseq into a function Initialisation of swseq_data and hwseq_data gets its own function, which is called from init_ich_default. This makes init_ich_default more readable. This patch also gives a name to (previously anonymous) struct swseq_data. Its sibling struct hwseq_data already has a name. Structs need names to be able to declare function parameters. TEST ME ON ICH7 BUG=b:204488958 TEST=Check that the following scenarios still behave properly: 1) probe-read-verify-erase section-write-reboot on Intel octopus board with GD25LQ128C/GD25LQ128D/GD25LQ128E 2) probe and read on Panther Point (7 series PCH) Change-Id: I7d62b1b380e497b82dcae1284d752204cc541bd3 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> Tested-by: Anastasia Klimchuk <aklm(a)chromium.org> Tested-by: Nico Huber <nico.h(a)gmx.de> --- M ichspi.c 1 file changed, 57 insertions(+), 50 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/37/58737/4 -- To view, visit https://review.coreboot.org/c/flashrom/+/58737 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7d62b1b380e497b82dcae1284d752204cc541bd3 Gerrit-Change-Number: 58737 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: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: ichspi: Remove unneeded line breaks, add useful ones
by Anastasia Klimchuk (Code Review) 23 Dec '21

23 Dec '21

Attention is currently required from: Felix Singer, Nico Huber, Edward O'Callaghan, Anastasia Klimchuk, Nikolai Artemiev. Hello build bot (Jenkins), Nico Huber, Edward O'Callaghan, Angel Pons, Nikolai Artemiev, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/60272 to look at the new patch set (#3). Change subject: ichspi: Remove unneeded line breaks, add useful ones ...................................................................... ichspi: Remove unneeded line breaks, add useful ones BUG=b:204488958 TEST=Check that the following scenarios still behave properly: 1) probe-read-verify-erase section-write-reboot on Intel octopus board with GD25LQ128C/GD25LQ128D/GD25LQ128E 2) probe and read on Panther Point (7 series PCH) Change-Id: I7ca2902b7caaa95418b828b068c661afafdcd171 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> Tested-by: Anastasia Klimchuk <aklm(a)chromium.org> Tested-by: Nico Huber <nico.h(a)gmx.de> --- M ichspi.c 1 file changed, 36 insertions(+), 58 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/72/60272/3 -- 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: 3 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: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: ichspi: Extract initialisation of swseq and hwseq into a function
by Nico Huber (Code Review) 23 Dec '21

23 Dec '21

Attention is currently required from: Felix Singer, Angel Pons, Anastasia Klimchuk, Nikolai Artemiev. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58737 ) Change subject: ichspi: Extract initialisation of swseq and hwseq into a function ...................................................................... Patch Set 3: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/58737 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7d62b1b380e497b82dcae1284d752204cc541bd3 Gerrit-Change-Number: 58737 Gerrit-PatchSet: 3 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: 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: Wed, 22 Dec 2021 23:58:08 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ichspi: Extract handling programmer param into a function
by Nico Huber (Code Review) 23 Dec '21

23 Dec '21

Attention is currently required from: Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, Nikolai Artemiev. Nico Huber 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 3: Code-Review+2 -- 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: 3 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: Wed, 22 Dec 2021 23:55:47 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ichspi: Split very long init function into two
by Nico Huber (Code Review) 23 Dec '21

23 Dec '21

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 3: (3 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/58735/comment/d678342d_572a1381 PS3, Line 20: TEST=1) probe-read-verify-erase section-write-reboot : on Intel octopus board with GD25LQ128C/GD25LQ128D/GD25LQ128E : 2) probe and read on Panther Point (7 series PCH) > If you still need a `TEST=` tag for some reason, you could do something like this: […] Generally it seems fine to me to add all the existing test info. About Tested-by: You are right to ask, IMHO, as it sets somebody else's name in stone, kind of (Git virtually never forgets). When somebody is CC'ed on Gerrit, I guess it doesn't matter much (people could at least object). But if not, better ask them personally. For active contributors I think we can imply their consent :) We don't have a rule for presence of Tested-by tags, AFAIK. Patchset: PS1: > To close the comment: I could not find devices with older chipsets, but Nico saved me and tested tho […] Sorry to disappoint. There seems to be a misunderstanding. PCHs inherited the generation numbers from MCHs, not ICHs. So ICH7 is something different, much older. We could ask in #flashrom and #coreboot channels for ICH7 testers. There are some famous machines, e.g. ThinkPad X60/T60, with it. Or let's try this: Angel, do you have any ICH7 board ready to boot? :) File ichspi.c: https://review.coreboot.org/c/flashrom/+/58735/comment/2b377824_4f881118 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)); > Done in CB:60272. […] Would have preferred it here, it seems wrong to change lines and then fix them up right away. No strong feelings, though. -- 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: 3 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: Wed, 22 Dec 2021 23:53:56 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net> Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: Makefile: clean up variables
by Nico Huber (Code Review) 23 Dec '21

23 Dec '21

Attention is currently required from: Thomas Heijligen, Paul Menzel, Angel Pons. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/59230 ) Change subject: Makefile: clean up variables ...................................................................... Patch Set 6: Code-Review+2 (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/59230/comment/40fa9c4a_48a42337 PS6, Line 12: unusd unus*e*d -- To view, visit https://review.coreboot.org/c/flashrom/+/59230 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I1c9e869377677d624469af1ee9ece9a28fc3b559 Gerrit-Change-Number: 59230 Gerrit-PatchSet: 6 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: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Wed, 22 Dec 2021 23:24:16 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: Makefile: Make pkg-config mandatory to find libpci
by Nico Huber (Code Review) 23 Dec '21

23 Dec '21

Attention is currently required from: Thomas Heijligen. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/59050 ) Change subject: Makefile: Make pkg-config mandatory to find libpci ...................................................................... Patch Set 15: Code-Review+1 (2 comments) File Makefile: https://review.coreboot.org/c/flashrom/+/59050/comment/740bae06_48f34f9e PS15, Line 866: USE_X86_MSR := $(if $(call filter_deps,$(DEPENDS_ON_X86_MSR)),yes,no) Is this split done intentionally in this patch? or did it sneak into the rebase? Risking to state the obvious: I'd prefer a separate patch ;) If it needs to be in this patch, the commit message should cover it. There is also an important message: we don't imply anymore that libpci requires raw access (AFAICS). https://review.coreboot.org/c/flashrom/+/59050/comment/d6b5c72e_baebba42 PS15, Line 882: $(filter yes, $(USE_X86_MSR) $(USE_X86_PORT_IO) $(USE_RAW_MEM_ACCESS))) `$(filter )` picks words from the second argument, kind of the haystack. So this could result in multiple `yes` and then the `ifeq` wouldn't match anymore. You can turn the arguments around: $(filter $(USE_X86_MSR) $(USE_X86_PORT_IO) $(USE_RAW_MEM_ACCESS), yes) This can only return one `yes` because there is only one in the haystack to find. -- 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: 15 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-Comment-Date: Wed, 22 Dec 2021 23:22:36 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit