Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(1)

Issue 6611015: Support -i partition:file feature for both read and write. (Closed)

Created:
8 years, 9 months ago by Louis
Modified:
8 years, 4 months ago
CC:
chromium-os-reviews_chromium.org
Visibility:
Public.

Description

Support -i partition:file feature for both read and write. This is actually 2 steps: 1. support partial read, which is like partial write we already supported. 2. support <file> optional parameter for -i <image>. This is super convenient for flashrom clients when they only need to read/write partial flash. They no longer need to prepare the whole flash content. Just prepare fragmented files, and specify partition names for each. Stefan and Hungte, this is FYI. But still welcome your comments. Change-Id: I00465bd2b1cf8fec10b0b18958efc94872e1a44a BUG=chromium-os:12355 TEST=run the following commands on Mario: cat > l 0:0xFF LOG 0x220000:0x2201FF RO 0x240000:0x2401FF RO2 0x3FFFF0:0x3FFFFF BV Ctrl+D time flashrom -r /tmp/bios time flashrom -l l -i LOG:log -i RO -i RO2:ro2 -i BV:bv -r /tmp/bios2 # Partial read should be much faster than whole read (around 0.7 seconds). # File log should contain 256 bytes, and content are like "$LOG" FF FF ... # File ro2 may contain Mario's VPD 1.0 data. # File bv should contain boot vector of BIOS, which is 0F 09 E9 6B ... FC FF. # File bios2 should only contain LOG, RO, RO2, and BV. Rest regions should be 0xFF. time flashrom -l l -i RO:log -i RO2:bv -w /tmp/bios2 # Would take around 10 seconds (for 2 whole reads and 1 partial write). time flashrom -l l -i RO:ro_ -i RO2:ro2_ -r /tmp/bios3 # Should be very quick. # File ro_ should contain log (if RO originally is blank). # First 16-byte of file ro2_ should contain bv. Rest are original ro2. # File bios3 should only contain ro_ and ro2_. Rest are all 0xFF. Committed: http://chrome-svn/viewvc/chromeos?view=rev&revision=9c7525f

Patch Set 1 #

Total comments: 12

Patch Set 2 : fixed according to code review and security check. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+143 lines, -6 lines) Patch
M cli_mfg.c View 2 chunks +6 lines, -4 lines 0 comments Download
M flash.h View 1 chunk +7 lines, -0 lines 0 comments Download
M flashrom.c View 1 2 chunks +17 lines, -1 line 0 comments Download
M layout.c View 1 7 chunks +113 lines, -1 line 0 comments Download

Messages

Total messages: 9 (0 generated)
Louis
8 years, 9 months ago (2011-03-03 09:56:46 UTC) #1
Hung-Te
LGTM for the new syntax and behavior change. For the source and coding style, I'll ...
8 years, 9 months ago (2011-03-03 11:03:27 UTC) #2
stefanreinauer
LGTM, too. Let's get this in ASAP
8 years, 9 months ago (2011-03-03 19:26:00 UTC) #3
dhendrix
http://codereview.chromium.org/6611015/diff/1/layout.c File layout.c (right): http://codereview.chromium.org/6611015/diff/1/layout.c#newcode210 layout.c:210: name, file ? file : "<not specified>"); On 2011/03/03 ...
8 years, 9 months ago (2011-03-04 00:43:09 UTC) #4
Hung-Te
http://codereview.chromium.org/6611015/diff/1/layout.c File layout.c (right): http://codereview.chromium.org/6611015/diff/1/layout.c#newcode259 layout.c:259: if (file[0]) { On 2011/03/04 00:43:09, dhendrix wrote: > ...
8 years, 9 months ago (2011-03-04 01:09:42 UTC) #5
Louis
Thanks your guys' review. I've uploaded the refinements and my feedbacks. Please kindly take a ...
8 years, 9 months ago (2011-03-04 03:46:30 UTC) #6
Hung-Te
LGTM to me. Thanks!
8 years, 9 months ago (2011-03-04 03:49:44 UTC) #7
dhendrix
On 2011/03/04 03:49:44, Hung-Te wrote: > LGTM to me. Thanks! LGTM as well.
8 years, 9 months ago (2011-03-04 04:11:45 UTC) #8
Stefan Reinauer
8 years, 9 months ago (2011-03-04 04:26:37 UTC) #9
LGTM too. Thanks! This and the fmap CL will make life much better for
firmware updates

Powered by Google App Engine
This is Rietveld 408576698