Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(293)

Issue 6532040: Add NV storage API to vboot reference (Closed)

Created:
9 years, 10 months ago by Randall Spangler
Modified:
9 years, 7 months ago
CC:
chromium-os-reviews_chromium.org, Randall Spangler, gauravsh, Bill Richardson, Che-Liang Chiou
Visibility:
Public.

Description

Add NV storage API to vboot reference BUG=chromium-os:12282 TEST=make && make runtests Committed: http://chrome-svn/viewvc/chromeos?view=rev&revision=b944534

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+463 lines, -1 line) Patch
M firmware/Makefile View 1 chunk +2 lines, -1 line 0 comments Download
A firmware/include/vboot_nvstorage.h View 1 chunk +61 lines, -0 lines 0 comments Download
A firmware/lib/vboot_nvstorage.c View 1 chunk +189 lines, -0 lines 0 comments Download
M tests/Makefile View 1 chunk +1 line, -0 lines 0 comments Download
A tests/vboot_nvstorage_test.c View 1 chunk +210 lines, -0 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
Randall Spangler
9 years, 10 months ago (2011-02-17 23:25:10 UTC) #1
Bill Richardson
LGTM, at least for now.
9 years, 10 months ago (2011-02-17 23:46:29 UTC) #2
Luigi Semenzato
9 years, 10 months ago (2011-02-18 01:44:31 UTC) #3
Thank you for inviting me to review this.

You might consider putting a little more explanation in the header file for the
complete ignoramus.  I am guessing that the enumerated type represents variables
in the NV storage, but the "context" parameter to various functions isn't
sufficiently explained for me.

The #if 0 block in the test file is a little odd.  I see it can be helpful but
it may be hard to keep synchronized and editors/window systems can show that
code better than copying it.

I'll be happy to review more if you add more comments and even if you don't.

Powered by Google App Engine
This is Rietveld 408576698