Attention is currently required from: Felix Singer, Thomas Heijligen.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/63826 )
Change subject: meson: use `platform/` as subdir()
......................................................................
Patch Set 4: Code-Review+1
(1 comment)
File meson.build:
https://review.coreboot.org/c/flashrom/+/63826/comment/76755d72_ec06ced6
PS2, Line 80: host_is_x86 = ['x86', 'x86_64'].contains(host_machine.cpu_family())
> Is the single sentence enough?
Yes, thanks!
--
To view, visit https://review.coreboot.org/c/flashrom/+/63826
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I88044a3f903f316138483dd872a6d95f8686ae69
Gerrit-Change-Number: 63826
Gerrit-PatchSet: 4
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: Peter Marheine <pmarheine(a)chromium.org>
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: Thomas Heijligen <src(a)posteo.de>
Gerrit-Comment-Date: Thu, 05 May 2022 05:34:37 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Thomas Heijligen <src(a)posteo.de>
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Thomas Heijligen.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/64026 )
Change subject: meson: relocate source file list
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/c/flashrom/+/64026
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I921be8a8a99b93d23c1dbd4ea54672ba9998558d
Gerrit-Change-Number: 64026
Gerrit-PatchSet: 1
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: 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: Thomas Heijligen <src(a)posteo.de>
Gerrit-Comment-Date: Thu, 05 May 2022 05:27:37 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Thomas Heijligen, Anastasia Klimchuk.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/64028 )
Change subject: meson: use build in options for install paths
......................................................................
Patch Set 1:
(2 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/64028/comment/38d83f77_f36f85c9
PS1, Line 10: automaticly
automatic*al*ly
Patchset:
PS1:
> In the documentation, it says: […]
Ack, thanks!
--
To view, visit https://review.coreboot.org/c/flashrom/+/64028
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I9cb9faf4bdbcfd66098478cc3a260eb3b664a2e6
Gerrit-Change-Number: 64028
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
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: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Wed, 04 May 2022 14:18:10 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Thomas Heijligen <src(a)posteo.de>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Nikolai Artemiev.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/63881 )
Change subject: flashrom: initialize restore func count in correct place
......................................................................
Patch Set 5:
(1 comment)
File flashrom.c:
https://review.coreboot.org/c/flashrom/+/63881/comment/575aeeff_5aaab134
PS5, Line 1868: flash->address_high_byte = -1;
: flash->in_4ba_mode = false;
> Why move this away from the other 4BA code?
+1
If unlocking doesn't need these values, I wouldn't move them.
--
To view, visit https://review.coreboot.org/c/flashrom/+/63881
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I4c7df424bd2ae2b5fb2a2ab6b47a3c9ff3233acf
Gerrit-Change-Number: 63881
Gerrit-PatchSet: 5
Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com>
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: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Comment-Date: Wed, 04 May 2022 13:56:16 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Gerrit-MessageType: comment
Attention is currently required from: Angel Pons, Nikolai Artemiev.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/63881 )
Change subject: flashrom: initialize restore func count in correct place
......................................................................
Patch Set 5:
(1 comment)
File flashrom.c:
https://review.coreboot.org/c/flashrom/+/63881/comment/4c81f1d7_9bf642a5
PS5, Line 1868: flash->address_high_byte = -1;
: flash->in_4ba_mode = false;
Why move this away from the other 4BA code?
--
To view, visit https://review.coreboot.org/c/flashrom/+/63881
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I4c7df424bd2ae2b5fb2a2ab6b47a3c9ff3233acf
Gerrit-Change-Number: 63881
Gerrit-PatchSet: 5
Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com>
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: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Comment-Date: Wed, 04 May 2022 10:59:14 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Thomas Heijligen, Edward O'Callaghan.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62951 )
Change subject: pcidev: Construct a API to handle phantom pci controllers
......................................................................
Patch Set 3:
(6 comments)
File include/programmer.h:
https://review.coreboot.org/c/flashrom/+/62951/comment/2c55d99b_a7be2e68
PS3, Line 131: pcidev_phantom_spi
Can we generalise the names, so that this can be used for any device? Something like `pcidev_phantom_dev` here would work for me, and functions would be renamed accordingly.
https://review.coreboot.org/c/flashrom/+/62951/comment/9a977def_690f922e
PS3, Line 132:
no space after unary *
https://review.coreboot.org/c/flashrom/+/62951/comment/bf73ba97_c301ef4f
PS3, Line 133:
no space after unary *
https://review.coreboot.org/c/flashrom/+/62951/comment/6541801f_32b26f67
PS3, Line 132: struct pcidev_phantom_spi * pcidev_phantom_new_spidev(struct pci_dev *const dev, const int slot, const int func);
: struct pci_dev * pcidev_phantom_spidev(struct pcidev_phantom_spi *const ph_spidev);
: void pcidev_phantom_release(struct pcidev_phantom_spi *const ph_spidev);
There's no need to declare parameters as const in the function declaration, the caller doesn't care.
Note: In something like `const char *const str`, only the second `const` is unnecessary. I mentally read `const char *str` as "`str` is a pointer to `const char`".
File pcidev.c:
https://review.coreboot.org/c/flashrom/+/62951/comment/ff8b8932_0e0d62e7
PS3, Line 54:
no space after unary *
https://review.coreboot.org/c/flashrom/+/62951/comment/84b3f3a5_462c6fa4
PS3, Line 85:
no space after unary *
--
To view, visit https://review.coreboot.org/c/flashrom/+/62951
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ic75573a14c9997a2d0347cd683ffc0c78d1b3a4f
Gerrit-Change-Number: 62951
Gerrit-PatchSet: 3
Gerrit-Owner: 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: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Subrata Banik <subratabanik(a)google.com>
Gerrit-CC: Thomas Heijligen <src(a)posteo.de>
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: Wed, 04 May 2022 09:37:01 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Angel Pons, Anastasia Klimchuk.
Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/64028 )
Change subject: meson: use build in options for install paths
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
> How does this work? I don't see where the prefix is used.
In the documentation, it says:
install_dir: override install directory for this file. If the value is a relative path, it will be considered relative the prefix option.
https://mesonbuild.com/Reference-manual_functions.html#library
--
To view, visit https://review.coreboot.org/c/flashrom/+/64028
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I9cb9faf4bdbcfd66098478cc3a260eb3b664a2e6
Gerrit-Change-Number: 64028
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
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: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Wed, 04 May 2022 09:29:08 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Thomas Heijligen, Anastasia Klimchuk.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/64031 )
Change subject: meson: use static library for tests
......................................................................
Patch Set 1: Code-Review+1
--
To view, visit https://review.coreboot.org/c/flashrom/+/64031
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: If10ed70e3bf629a6eb7e2c5bef3318d1e259dadd
Gerrit-Change-Number: 64031
Gerrit-PatchSet: 1
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: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Wed, 04 May 2022 09:17:59 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment