April 2017 - flashrom-gerrit - mail.coreboot.org

Change in flashrom[staging]: Hack up test_v2.sh to work better with upstream
by David Hendricks (Code Review) 21 Apr '17

21 Apr '17

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)

1 0

Change in flashrom[staging]: Copy test_v2 directory from chromiumos branch
by David Hendricks (Code Review) 21 Apr '17

21 Apr '17

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)

1 0

Change in flashrom[staging]: Guard x86-specific IFD stuff
by David Hendricks (Code Review) 21 Apr '17

21 Apr '17

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)

1 0

Change in flashrom[staging]: Give layouts their own type
by David Hendricks (Code Review) 20 Apr '17

20 Apr '17

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

1 0

Change in flashrom[staging]: Enable writes with active ME
by David Hendricks (Code Review) 20 Apr '17

20 Apr '17

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

1 0

Change in flashrom[staging]: Enable writes with active ME
by David Hendricks (Code Review) 20 Apr '17

20 Apr '17

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

1 0

Change in flashrom[staging]: Fix linking with libpayload
by David Hendricks (Code Review) 20 Apr '17

20 Apr '17

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

1 0

Change in flashrom[staging]: Add option to read ROM layout from IFD
by David Hendricks (Code Review) 20 Apr '17

20 Apr '17

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

1 0

Change in flashrom[staging]: Add a convenient libflashrom interface
by David Hendricks (Code Review) 20 Apr '17

20 Apr '17

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

1 0

Change in flashrom[staging]: cli_classic: Remove layout-for-write-only check
by David Hendricks (Code Review) 20 Apr '17

20 Apr '17

David Hendricks has posted comments on this change. ( https://review.coreboot.org/17951 ) Change subject: cli_classic: Remove layout-for-write-only check ...................................................................... Patch Set 7: Code-Review+2 -- To view, visit https://review.coreboot.org/17951 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0f5e9623ca75bc4503aeb45ae346d7573c0fef2c 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: No

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit April 2017