[LinuxBIOS] r2489 - trunk/LinuxBIOSv2/util/flashrom

Segher Boessenkool segher at kernel.crashing.org
Sun Nov 5 20:49:54 CET 2006

> +/* Datasheet: http://www.intel.com/design/intarch/datashts/ 
> 290562.htm */

Trivial in this case, but in general: can we please add
the name or document number (not just the URL) for datasheets
and other documentation?  Vendors just _love_ to rearrange
their websites (and some pull their old docs altogether);
if we have the name/number (or even just the filename of
the pdf) at least there's a chance to google for it again
(either on the vendor's site or on some archive).

> +                      the lower 64-Kbyte BIOS block (E00000-  
> EFFFF) at the top

A 0 too many.

> +                      of 1 Mbyte, or the aliases at the top of 4  
> Gbyte
> +                      (FFFE0000-FFFEFFF) result in the generation  
> of BIOSCS#.

But you balanced it by dropping an F :-)

> +           Set bit 2: BIOSCS# Write Protect Enable (1=enable,  
> 0=disable). */

The bit is misnamed in the datasheet: it's the "write enable"
bit, not the "write protect enable" (which would be "write disable").

> +		printf("tried to set 0x%x to 0x%x on %s failed (WARNING ONLY) 
> \n", xbcs, new, name);

Warning only?  It's pretty damn fatal :-)  Or does this mean
"trying to continue anyway" (which might not be such a great
plan for a utility that can brick your system)?

Would be nice if this message could be made a bit less cryptic,
for example, "WARNING: failed setting %s register %#x to %#x\n".
Yes I realise this is probably copied all over the place ;-)


More information about the coreboot mailing list