Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/19341
to look at the new patch set (#3).
Change subject: Hack up test_v2.sh to work better with upstream
......................................................................
Hack up test_v2.sh to work better with upstream
Hacks applied on top of chromiumos version of test_v2.sh to get the
script running.
Change-Id: I9ace99fdb8d779804fe1887fe49f6409f8aff02e
Signed-off-by: David Hendricks <dhendricks(a)fb.com>
---
M tests/tests_v2/test_v2.sh
1 file changed, 39 insertions(+), 29 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/41/19341/3
--
To view, visit https://review.coreboot.org/19341
To unsubscribe, visit https://review.coreboot.org/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9ace99fdb8d779804fe1887fe49f6409f8aff02e
Gerrit-PatchSet: 3
Gerrit-Project: flashrom
Gerrit-Branch: staging
Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: build bot (Jenkins)
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/19340
to look at the new patch set (#3).
Change subject: Copy test_v2 directory from chromiumos branch
......................................................................
Copy test_v2 directory from chromiumos branch
This is a straight copy and should not be merged. The full patch
series will eventually be imported, but for now this is just a hack
to get stuff working.
Change-Id: I41bef98d32df61af400ecac46ea9571e722b2ac6
Signed-off-by: David Hendricks <dhendricks(a)fb.com>
---
A tests/tests_v2/cmd.sh
A tests/tests_v2/servo_hooks.sh
A tests/tests_v2/test_v2.sh
3 files changed, 1,371 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/40/19340/3
--
To view, visit https://review.coreboot.org/19340
To unsubscribe, visit https://review.coreboot.org/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I41bef98d32df61af400ecac46ea9571e722b2ac6
Gerrit-PatchSet: 3
Gerrit-Project: flashrom
Gerrit-Branch: staging
Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: build bot (Jenkins)
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/19339
to look at the new patch set (#3).
Change subject: Guard x86-specific IFD stuff
......................................................................
Guard x86-specific IFD stuff
This will get squashed into a preceeding patch.
Change-Id: If5abe84e47112fef0e3576045510b34ce94e6a4d
Signed-off-by: David Hendricks <dhendricks(a)fb.com>
---
M libflashrom.c
1 file changed, 6 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/39/19339/3
--
To view, visit https://review.coreboot.org/19339
To unsubscribe, visit https://review.coreboot.org/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If5abe84e47112fef0e3576045510b34ce94e6a4d
Gerrit-PatchSet: 3
Gerrit-Project: flashrom
Gerrit-Branch: staging
Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: build bot (Jenkins)
David Hendricks has posted comments on this change. ( https://review.coreboot.org/17944 )
Change subject: Give layouts their own type
......................................................................
Patch Set 4: Code-Review+2
--
To view, visit https://review.coreboot.org/17944
To unsubscribe, visit https://review.coreboot.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Icea1a58c283131cc9c5fde6f16d783538dc1a4c7
Gerrit-PatchSet: 4
Gerrit-Project: flashrom
Gerrit-Branch: staging
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: build bot (Jenkins)
Gerrit-HasComments: No
David Hendricks has posted comments on this change. ( https://review.coreboot.org/18740 )
Change subject: Enable writes with active ME
......................................................................
Patch Set 6:
er-- "a message at the end warning the user that the image is incomplete (skipped the ME region)" would apply only if we take the chromiumos approach and allow the user to read/write/verify a ROM-sized image (without specifying -i options) that just ignores the ME region.
--
To view, visit https://review.coreboot.org/18740
To unsubscribe, visit https://review.coreboot.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I94a5e7074b845c227e43d76d04dd1a71082a1cef
Gerrit-PatchSet: 6
Gerrit-Project: flashrom
Gerrit-Branch: staging
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: build bot (Jenkins)
Gerrit-HasComments: No
David Hendricks has posted comments on this change. ( https://review.coreboot.org/18740 )
Change subject: Enable writes with active ME
......................................................................
Patch Set 6:
Yeah, I think the huge message should be replaced with:
- If --noverify-all is not enabled automatically, an error telling the user they must enable that option.
or
- A warning that --noverify-all is being enabled on the user's behalf, and a message at the end warning the user that the image is incomplete (skipped the ME region).
Unless the user specifies --force in which case it should just try and fail as normal.
--
To view, visit https://review.coreboot.org/18740
To unsubscribe, visit https://review.coreboot.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I94a5e7074b845c227e43d76d04dd1a71082a1cef
Gerrit-PatchSet: 6
Gerrit-Project: flashrom
Gerrit-Branch: staging
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: build bot (Jenkins)
Gerrit-HasComments: No
David Hendricks has posted comments on this change. ( https://review.coreboot.org/18738 )
Change subject: Fix linking with libpayload
......................................................................
Patch Set 6: Code-Review-1
(2 comments)
The patch is OK, I'm just curious why we don't move flashbuses_to_text() into flashrom.c so the print can work consistently.
https://review.coreboot.org/#/c/18738/6//COMMIT_MSG
Commit Message:
PS6, Line 9: o flashbuses_to_text() is a cli function and should actually be moved
: or not be called from flashrom.c.
Why not move flashbuses_to_text() to flashrom.c?
https://review.coreboot.org/#/c/18738/6/helpers.c
File helpers.c:
Line 95: #if defined(__DJGPP__)
Hmm, I wonder if it's still useful to have a check for glibc compatibility while we're at it. According to the man page:
Since glibc 2.10:
_POSIX_C_SOURCE >= 200809L
Before glibc 2.10:
_GNU_SOURCE
--
To view, visit https://review.coreboot.org/18738
To unsubscribe, visit https://review.coreboot.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I561135209b819361d125eeaeef9ff886d6bae987
Gerrit-PatchSet: 6
Gerrit-Project: flashrom
Gerrit-Branch: staging
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: build bot (Jenkins)
Gerrit-HasComments: Yes
David Hendricks has posted comments on this change. ( https://review.coreboot.org/17953 )
Change subject: Add option to read ROM layout from IFD
......................................................................
Patch Set 7: Code-Review-1
(6 comments)
Good stuff overall. My only qualm is with using '-d' for a vendor-specific option since we don't have many of those to go around. Is that really needed?
https://review.coreboot.org/#/c/17953/5/cli_classic.c
File cli_classic.c:
PS5, Line 60: " -d | --ifd read layout from an Intel Firmware Descriptor\n"
We only have so many useful single characters to choose from, so I'd prefer to reserve shortopts for generic options. Since IFD is specific to Intel I think we should just use the longopt here.
https://review.coreboot.org/#/c/17953/7/cli_classic.c
File cli_classic.c:
PS7, Line 60: " -d | --ifd
Since we only have a few useful single letters available for short opts, I think we should reserve them for generic options. Since IFD is Intel-specific I'd like to just use --ifd. (Same goes for chromiumos-specific fmap stuff)
PS7, Line 574: if (ifd)
: fl_layout_release(layout);
Hm, it would be nice if this could be called unconditionally, or perhaps just gated by 'if (layout)'. That way we call it no matter where we get the layout from.
(comment added to https://review.coreboot.org/#/c/17946)
https://review.coreboot.org/#/c/17953/5/libflashrom.c
File libflashrom.c:
Line 34: #include "hwaccess.h"
Needs to be guarded with #if defined(__i386__) || defined(__x86_64__)
PS5, Line 339: ret = 2;
: goto _unmap_ret;
: }
: m
also needs to be guarded for x86-only
PS5, Line 345: ret = 3;
: goto _unmap_ret;
: }
:
same as above
--
To view, visit https://review.coreboot.org/17953
To unsubscribe, visit https://review.coreboot.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifafff2bf6d5c5e62283416b3269723f81fdc0fa3
Gerrit-PatchSet: 7
Gerrit-Project: flashrom
Gerrit-Branch: staging
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: build bot (Jenkins)
Gerrit-HasComments: Yes
David Hendricks has posted comments on this change. ( https://review.coreboot.org/17946 )
Change subject: Add a convenient libflashrom interface
......................................................................
Patch Set 6:
(1 comment)
Came up with an idea while reviewing another patch up the stack.
https://review.coreboot.org/#/c/17946/6/libflashrom.c
File libflashrom.c:
Line 301: {
How about adding:
if (layout != get_global_layout())
return;
That way we can do fl_layout_release() unconditionally at the end of cli_classic.c in 17953 ("
Add option to read ROM layout from IFD")
--
To view, visit https://review.coreboot.org/17946
To unsubscribe, visit https://review.coreboot.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I00f169990830aa17b7dfae5eb74010d40c476181
Gerrit-PatchSet: 6
Gerrit-Project: flashrom
Gerrit-Branch: staging
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: build bot (Jenkins)
Gerrit-HasComments: Yes