- flashrom-gerrit - mail.coreboot.org

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

21 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

Change in flashrom[staging]: Add functions to read/erase/write/verify by layout
by David Hendricks (Code Review) 20 Apr '17

20 Apr '17

David Hendricks has posted comments on this change. ( https://review.coreboot.org/17945 ) Change subject: Add functions to read/erase/write/verify by layout ...................................................................... Patch Set 6: Code-Review+2 -- To view, visit https://review.coreboot.org/17945 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic6194cea4c4c430e0cf9d586052508a865b09c86 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: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Stefan Tauner <stefan.tauner(a)gmx.at> Gerrit-Reviewer: build bot (Jenkins) Gerrit-HasComments: No

1 0

Change in flashrom[staging]: Kill doit()
by David Hendricks (Code Review) 20 Apr '17

20 Apr '17

David Hendricks has posted comments on this change. ( https://review.coreboot.org/17949 ) Change subject: Kill doit() ...................................................................... Patch Set 7: Code-Review+2 doit! -- To view, visit https://review.coreboot.org/17949 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id81177c50b4410e68dcf8ebab48386a94cd9b714 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

Change in flashrom[staging]: Add option (-A, --noverify-all) to not verify the whole chip...
by David Hendricks (Code Review) 20 Apr '17

20 Apr '17

David Hendricks has posted comments on this change. ( https://review.coreboot.org/17950 ) Change subject: Add option (-A, --noverify-all) to not verify the whole chip after write ...................................................................... Patch Set 7: Code-Review+1 (2 comments) Code looks good, I just have some cosmetic nits. https://review.coreboot.org/#/c/17950/7/cli_classic.c File cli_classic.c: PS7, Line 58: don't auto-verify whole chip Instead of saying what it doesn't do, maybe it would be clearer to say what it does, for example "verify included regions only\n" PS7, Line 58: A I guess I'd prefer 'V' for "verify included". Might be worth asking a few others. -- To view, visit https://review.coreboot.org/17950 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I40b5983f56d62821d17b827b88b73d1d41a30bd7 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]: Adapt CLI to use new libflashrom interface' print callback
by David Hendricks (Code Review) 20 Apr '17

20 Apr '17

David Hendricks has posted comments on this change. ( https://review.coreboot.org/17948 ) Change subject: Adapt CLI to use new libflashrom interface' print callback ...................................................................... Patch Set 6: Code-Review+2 -- To view, visit https://review.coreboot.org/17948 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idf19978eb8e340d258199193d2978f37409e9983 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]: Add Doxyfile for libflashrom documentation
by David Hendricks (Code Review) 20 Apr '17

20 Apr '17

David Hendricks has posted comments on this change. ( https://review.coreboot.org/17947 ) Change subject: Add Doxyfile for libflashrom documentation ...................................................................... Patch Set 6: Code-Review+2 -- To view, visit https://review.coreboot.org/17947 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I583bf9aa8c43049723aff498625d490c37832f13 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]: 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: Code-Review+1 (6 comments) The code looks good overall, I just have a few general comments. I'd be fine with committing this as-is. https://review.coreboot.org/#/c/17946/5/flashrom.c File flashrom.c: PS5, Line 2557: t too https://review.coreboot.org/#/c/17946/6/flashrom.c File flashrom.c: Line 2507: return 1; Would it make sense to add a wrapper function that calls unmap_flash() and maybe other functions (e.g. to re-lock the chip) to undo prepare_flash_access()? Maybe unprepare_flash_access() or restore_original_flash_access() or something? PS6, Line 2610: that : * lap "containing" might be a better word for this Line 2696: } else Style nit: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Doc… https://review.coreboot.org/#/c/17946/6/libflashrom.c File libflashrom.c: PS6, Line 42: fl_ I had originally thought fl_ was used when describing flash layout stuff that's not necessarily tied with the library (internal function args and variables). Can you clarify when/how this prefix is to be used so it doesn't get copy+pasted inappropriately? Maybe use "fl_" for flash layout and "lf_" for libflashrom? PS6, Line 217: << 10 Probably should be '* KiB', but that's another matter. -- 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]: udelay: Use clock_gettime() if available and precise
by build bot (Jenkins) (Code Review) 20 Apr '17

20 Apr '17

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/19391 ) Change subject: udelay: Use clock_gettime() if available and precise ...................................................................... Patch Set 3: Verified+1 Build Successful https://qa.coreboot.org/job/flashrom-customrules/167/ : SUCCESS -- To view, visit https://review.coreboot.org/19391 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I85ad359823875237ada9cd027af3017d62e9a235 Gerrit-PatchSet: 3 Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) Gerrit-HasComments: No

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit