Attention is currently required from: Nico Huber, Michał Żygowski, Jonathan Zhang, David Hendricks.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/57793 )
Change subject: ich_descriptors: Add explicit checks for all chipsets
......................................................................
Patch Set 4:
(1 comment)
File ich_descriptors.c:
https://review.coreboot.org/c/flashrom/+/57793/comment/7418daad_5d531977
PS1, Line 968: if (content->ICCRIBA != 0x34)
: msg_pwarn("Unknown flash descriptor, assuming 300 series compatibility.\n");
> "Unknown flash descriptor, assuming 300 series compatibility" and "Peculiar flash descriptor, assumi […]
AIUI, the message in this path says "Unknown" because it's the path where none of the evaluated conditions is true. In the "Peculiar" paths, we know more about the flash descriptor because at least one check is true.
If one were to remove all instances of `else` in this function (while preserving behavior, of course), the "Unknown" message would be at the end of the function, whereas the "Peculiar" messages would all be inside an if-block. Hmmm, I like the refactoring idea.
--
To view, visit https://review.coreboot.org/c/flashrom/+/57793
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ica49477492876810a6fa212768b1ab9e8c12001f
Gerrit-Change-Number: 57793
Gerrit-PatchSet: 4
Gerrit-Owner: David Hendricks
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Jonathan Zhang <jonzhang(a)fb.com>
Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com>
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: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Attention: Jonathan Zhang <jonzhang(a)fb.com>
Gerrit-Attention: David Hendricks
Gerrit-Comment-Date: Mon, 27 Sep 2021 13:07:50 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: David Hendricks
Gerrit-MessageType: comment
Attention is currently required from: Felix Singer, Nico Huber, Angel Pons.
Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/57942 )
Change subject: internal.c: unify the macro for x86 only code
......................................................................
Patch Set 2:
(4 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/57942/comment/1c6b9576_0f89b1b2
PS1, Line 9: \#if defined(__i386__) || defined(__x86_64__)
> nit: To avoid having to escape the `#`, how about: […]
Done
https://review.coreboot.org/c/flashrom/+/57942/comment/aae6dbc1_4416dfc9
PS1, Line 11: platfrom
> typo: `platform`
Done
https://review.coreboot.org/c/flashrom/+/57942/comment/c482cfb8_dcc1393c
PS1, Line 11: uses
> plural: `use`
Done
Patchset:
PS1:
> I'd personally use the IS_X86 macro everywhere for consistency, but I don't have a strong preference […]
I'd looked for the most widespread variant for x86 only code.
Beside that it's now easier to make changes in platform.h and hwaccess which I want to do
--
To view, visit https://review.coreboot.org/c/flashrom/+/57942
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I94a599431f58666189c8cd601286e9b30c8bf62b
Gerrit-Change-Number: 57942
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
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-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Mon, 27 Sep 2021 10:47:43 +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: Felix Singer, Nico Huber, Thomas Heijligen.
Hello Felix Singer, build bot (Jenkins), Nico Huber, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/57942
to look at the new patch set (#2).
Change subject: internal.c: unify the macro for x86 only code
......................................................................
internal.c: unify the macro for x86 only code
The #if defined(__i386__) || defined(__x86_64__) guard is commonly used
for x86 only code across flashrom.
Only platform.h and hwaccess.* use the IS_X86 macro.
Change-Id: I94a599431f58666189c8cd601286e9b30c8bf62b
Signed-off-by: Thomas Heijligen <thomas.heijligen(a)secunet.de>
---
M internal.c
1 file changed, 7 insertions(+), 7 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/42/57942/2
--
To view, visit https://review.coreboot.org/c/flashrom/+/57942
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I94a599431f58666189c8cd601286e9b30c8bf62b
Gerrit-Change-Number: 57942
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
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-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-MessageType: newpatchset
Attention is currently required from: Felix Singer, Nico Huber, Angel Pons.
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/57810 )
Change subject: ft2232_spi: reintroduce generic GPIOL control
......................................................................
Patch Set 18:
(4 comments)
File ft2232_spi.c:
https://review.coreboot.org/c/flashrom/+/57810/comment/cb41a7a9_86eab32b
PS17, Line 493: unsigned int pin = temp + 4;
> Is this change intentional?
yes
https://review.coreboot.org/c/flashrom/+/57810/comment/a230bae3_cea0224a
PS17, Line 503: pindir |= 1 << pin;
> Is this change intentional?
yes
https://review.coreboot.org/c/flashrom/+/57810/comment/2adaae92_fafe3191
PS17, Line 524: int
> Why not `uint8_t`?
Ack
https://review.coreboot.org/c/flashrom/+/57810/comment/45c7a3a7_68cd7386
PS17, Line 526: %i
> Out of curiosity, why use `%i` here and `%d` in `snprintf()`?
Ah, that snprintf code was from Nico - aligned
--
To view, visit https://review.coreboot.org/c/flashrom/+/57810
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I3989f0f9596c090de52dca67183b1363dae59d3a
Gerrit-Change-Number: 57810
Gerrit-PatchSet: 18
Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Alan Green <avg(a)google.com>
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: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Mon, 27 Sep 2021 10:29:52 +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: Felix Singer, Nico Huber, Angel Pons.
Hello Felix Singer, build bot (Jenkins), Alan Green, Nico Huber, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/57810
to look at the new patch set (#18).
Change subject: ft2232_spi: reintroduce generic GPIOL control
......................................................................
ft2232_spi: reintroduce generic GPIOL control
This reintroduces a reworked version of the GPIOL pin control first
introduced in commit 3207844 (CB:49637), which was reverted in commit
6518cf3 (CB:55692) due to breakage.
This change introduces a new argument `gpiolX` to allow use of the four
GPIOL pins either as generic gpios or as additional CS# signal(s). `X`
specifies the GPIOL pin (0-3) to be set to one of [HLC] with the
following meaning:
* H - set the pin as output high
* L - set the pin as output low
* C - use the pin as additional CS# signal
The third value, `C`, aims to replace the parameter `csgpiol`, that is
now marked as deprecated and can be removed at some point in the future.
`gpiol` and `csgpiol` are mutually exclusive and use of both results in
an error.
Multiple pins may be set by specifying the parameter multiple times.
Documentation was updated/added accordingly.
Test: All pin levels/modes have been verified to behave correctly with a
logic analyzer.
Change-Id: I3989f0f9596c090de52dca67183b1363dae59d3a
Signed-off-by: Alan Green <avg(a)google.com>
Signed-off-by: Michael Niewöhner <foss(a)mniewoehner.de>
---
M flashrom.8.tmpl
M ft2232_spi.c
2 files changed, 109 insertions(+), 9 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/10/57810/18
--
To view, visit https://review.coreboot.org/c/flashrom/+/57810
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I3989f0f9596c090de52dca67183b1363dae59d3a
Gerrit-Change-Number: 57810
Gerrit-PatchSet: 18
Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Alan Green <avg(a)google.com>
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: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Felix Singer, Nico Huber, Michael Niewöhner.
Hello Felix Singer, build bot (Jenkins), Nico Huber, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/57893
to look at the new patch set (#4).
Change subject: flashrom.8: carve out `csgpiol` into its own section
......................................................................
flashrom.8: carve out `csgpiol` into its own section
Documentation for `csgpiol` was put into the generic programmer options
section. Move it to its own section.
Change-Id: Ic7379331d36b3068eacde5a983b4ccb3afc56c51
Signed-off-by: Michael Niewöhner <foss(a)mniewoehner.de>
---
M flashrom.8.tmpl
1 file changed, 11 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/93/57893/4
--
To view, visit https://review.coreboot.org/c/flashrom/+/57893
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ic7379331d36b3068eacde5a983b4ccb3afc56c51
Gerrit-Change-Number: 57893
Gerrit-PatchSet: 4
Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
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: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-MessageType: newpatchset
Attention is currently required from: Felix Singer, Nico Huber, Angel Pons.
Hello Felix Singer, build bot (Jenkins), Nico Huber, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/57809
to look at the new patch set (#8).
Change subject: ft2232_spi: prevent use of reserved pins on some programmers
......................................................................
ft2232_spi: prevent use of reserved pins on some programmers
On some programmers an output buffer needs to be enabled by pulling a
gpio high/low. This gpio can not be used for `csgpiol`. Prevent this by
printing an error.
Change-Id: Ied450fa5ef358153adefec3beabc63a62c9f60cd
Signed-off-by: Michael Niewöhner <foss(a)mniewoehner.de>
---
M ft2232_spi.c
1 file changed, 12 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/09/57809/8
--
To view, visit https://review.coreboot.org/c/flashrom/+/57809
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ied450fa5ef358153adefec3beabc63a62c9f60cd
Gerrit-Change-Number: 57809
Gerrit-PatchSet: 8
Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
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: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Nico Huber, Thomas Heijligen.
Hello Felix Singer, build bot (Jenkins), Nico Huber, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/57935
to look at the new patch set (#2).
Change subject: Makefile: move functions and macros to own file
......................................................................
Makefile: move functions and macros to own file
Move all define statements in its own file to tidy up the main
Makefile.
Change-Id: I451f2eeab2773982e02b2f2fdc9e8abe1cc87630
Signed-off-by: Thomas Heijligen <thomas.heijligen(a)secunet.de>
---
M Makefile
A Makefile.include
2 files changed, 215 insertions(+), 197 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/35/57935/2
--
To view, visit https://review.coreboot.org/c/flashrom/+/57935
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I451f2eeab2773982e02b2f2fdc9e8abe1cc87630
Gerrit-Change-Number: 57935
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
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: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-MessageType: newpatchset