Nico Huber has submitted this change. ( https://review.coreboot.org/c/flashrom/+/61362 )
Change subject: ich_descriptors.c Invert the meaning of 'dual_output' bit
......................................................................
ich_descriptors.c Invert the meaning of 'dual_output' bit
In the Flash Component description register (FLCOMP) bit 30 reports the
capability of using dual output for fast read operation on the flash
component. According to various SPI Programming Guides (checked for
Panther Point, Lewisburg C620, Apollo Lake and Elkhart Lake) the dual
output is enabled when this bit is set and disabled if not. Currently the
logic displays it the other way around when parsing the descriptor.
This patch changes this so now if bit 30 in FLCOMP is not set, dual read
support for fast read operation is shown as disabled.
Change-Id: If6282ac8326ab0b92e9c70c09dba0299bf0deb6f
Signed-off-by: Werner Zeh <werner.zeh(a)siemens.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/61362
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Angel Pons <th3fanbus(a)gmail.com>
---
M ich_descriptors.c
1 file changed, 1 insertion(+), 1 deletion(-)
Approvals:
build bot (Jenkins): Verified
Angel Pons: Looks good to me, approved
diff --git a/ich_descriptors.c b/ich_descriptors.c
index d716d1d..0ce5720 100644
--- a/ich_descriptors.c
+++ b/ich_descriptors.c
@@ -380,7 +380,7 @@
pprint_freq(cs, desc->component.modes.freq_fastread));
if (cs > CHIPSET_6_SERIES_COUGAR_POINT)
msg_pdbg2("Dual Output Fast Read Support: %sabled\n",
- desc->component.modes.dual_output ? "dis" : "en");
+ desc->component.modes.dual_output ? "en" : "dis");
int has_forbidden_opcode = 0;
if (desc->component.FLILL != 0) {
2 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.
--
To view, visit https://review.coreboot.org/c/flashrom/+/61362
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: If6282ac8326ab0b92e9c70c09dba0299bf0deb6f
Gerrit-Change-Number: 61362
Gerrit-PatchSet: 4
Gerrit-Owner: Werner Zeh <werner.zeh(a)siemens.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-MessageType: merged
Attention is currently required from: Nico Huber, David Hendricks, Edward O'Callaghan, Angel Pons, Arthur Heymans.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/61943 )
Change subject: libflashrom/fmap: Don't use off_t for flash offsets
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
> Many thanks for spotting this! […]
Yeah I noticed layout.h but I wasn't sure if it was okay to pull that into libflashrom.h (and it wouldn't work as is for the signed case anyway).
Personally I think fixed width makes most sense here, you don't really gain anything from matching it to the machine width. The thing that determines what size this type needs to be (maximum size of available flash chips) doesn't change based on what processor you're running flashrom on. In practice, using long may be slightly less efficient (need REX prefixes on x86_64, takes up twice the size in memory for 64-bit). But I'm happy to implement whatever the majority here wants as long as it gets rid of off_t.
--
To view, visit https://review.coreboot.org/c/flashrom/+/61943
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I68a386973f79ea634f63dfcd7d95a63400e1fdee
Gerrit-Change-Number: 61943
Gerrit-PatchSet: 1
Gerrit-Owner: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: David Hendricks <david.hendricks(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-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Comment-Date: Wed, 16 Feb 2022 00:00:23 +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: Felix Singer, Nico Huber, Michał Żygowski, Paul Menzel.
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/55715 )
Change subject: ite_ecfw: Implement support for flashing ITE ECs found on TUXEDO laptops
......................................................................
Patch Set 28: Code-Review+2
--
To view, visit https://review.coreboot.org/c/flashrom/+/55715
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I0e42260155ffea38a6f60790871cd8da7b657031
Gerrit-Change-Number: 55715
Gerrit-PatchSet: 28
Gerrit-Owner: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de>
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-CC: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Comment-Date: Tue, 15 Feb 2022 19:46:41 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: David Hendricks, Edward O'Callaghan, Angel Pons, Julius Werner, Arthur Heymans.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/61943 )
Change subject: libflashrom/fmap: Don't use off_t for flash offsets
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
Many thanks for spotting this!
We already have some rarely used typedefs in `layout.h`, namely
typedef uint32_t chipoff_t; /* Able to store any addressable offset within a supported flash memory. */
typedef uint32_t chipsize_t; /* Able to store the number of bytes of any supported flash memory. */
I wonder if this is a good opportunity to move them into the libflashrom API. Generally,
I'm not a fan of using fixed-width unless we need exactly that width (e.g. to match an
ABI or hardware registers). So I propose these:
typedef long flashrom_off_t;
typedef unsigned long flashrom_size_t;
(we could still have shorter names internally)
--
To view, visit https://review.coreboot.org/c/flashrom/+/61943
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I68a386973f79ea634f63dfcd7d95a63400e1fdee
Gerrit-Change-Number: 61943
Gerrit-PatchSet: 1
Gerrit-Owner: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: David Hendricks <david.hendricks(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-Attention: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Comment-Date: Tue, 15 Feb 2022 14:15:11 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Thomas Heijligen, Angel Pons.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/61276 )
Change subject: hwaccess_x86_io: clean header concept
......................................................................
Patch Set 7: Code-Review+1
(3 comments)
Patchset:
PS7:
New code looks good, still need to compare it to the old code...
File hwaccess_x86_io.c:
https://review.coreboot.org/c/flashrom/+/61276/comment/e838271d_ae77207c
PS7, Line 36: * functions. `platform_get_io_perms()` is called to get
Please drop the trailing whitespace that Gerrit marks so keenly.
https://review.coreboot.org/c/flashrom/+/61276/comment/9a50d5ac_dc3ad27f
PS7, Line 352: a
ass*e*mbly
--
To view, visit https://review.coreboot.org/c/flashrom/+/61276
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I1400704e9ac5fed00c096796536108d5bfb875e3
Gerrit-Change-Number: 61276
Gerrit-PatchSet: 7
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: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Tue, 15 Feb 2022 13:47:54 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Thomas Heijligen, Angel Pons.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/61275 )
Change subject: hwaccess_x86_io.c: refactor rget_io_perms()
......................................................................
Patch Set 4: Code-Review+2
(5 comments)
File hwaccess_x86_io.c:
https://review.coreboot.org/c/flashrom/+/61275/comment/41bd4ffa_fb8c82bf
PS3, Line 92: return iopl(3);
> *BSD and Darwin use other includes. […]
Ack
https://review.coreboot.org/c/flashrom/+/61275/comment/e4488cdf_f205a07e
PS3, Line 115: NULL
> This can be done in a following ccommit.
Ack
File hwaccess_x86_io.c:
https://review.coreboot.org/c/flashrom/+/61275/comment/16d943f6_122c74eb
PS4, Line 124: msg_perr("Make sure you are root.\n");
If we change this already... There were some reports lately that the message
is not helpful on locked-down Linux. How about adding something that there may
be more to it, e.g.
Make sure you are root. If you are root, your kernel may still prevent access based on security policies.
https://review.coreboot.org/c/flashrom/+/61275/comment/cdef32d1_91e4a6f0
PS4, Line 126: "reboot, or reboot into single user mode.\n");
would be nice to keep the alignment, i.e. indent with another space
https://review.coreboot.org/c/flashrom/+/61275/comment/dc858681_c466f3e3
PS4, Line 128: "that your kernel configuration has the option INSECURE enabled.\n");
would be nice to keep the alignment, i.e. indent with another space
--
To view, visit https://review.coreboot.org/c/flashrom/+/61275
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: If4b2f8c2532f3732086ee1d479da6ae6693f9a42
Gerrit-Change-Number: 61275
Gerrit-PatchSet: 4
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-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Tue, 15 Feb 2022 13:11:07 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Thomas Heijligen <src(a)posteo.de>
Gerrit-MessageType: comment
Attention is currently required from: Thomas Heijligen.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/61966 )
Change subject: hwaccess_x86_io: merge error message in rget_io_perms()
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/c/flashrom/+/61966
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I7aae096deedd9b78f5fd38a73390cd8a33528545
Gerrit-Change-Number: 61966
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Comment-Date: Tue, 15 Feb 2022 13:00:16 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment