> @@ -42,10 +45,11 @@
> option is
> .B not
> recommended, you should only use it if you know what you are doing and you
> -feel that the time for verification takes too long.
> +feel that the time for verification takes too long. Please note that flashrom
> +will automatically verify all operations unless you specify \-\-noverify.
Reword the whole paragraph instead of adding the note, like
-n, --noverify
Skip the automatic verification after writing...
Especially, I don't like "all operations", as the verify is only implied
for write, but read and erase are also "operations".
> + /* FIXME: --mainboard should be a programmer parameter */
Right.
> + " -f | --force force something\n"
"force specific operations (see man page)" ?
> + " -n | --noverify don't auto-verify\n"
> + " -l | --layout <file> read ROM layout from "
> + "file\n"
Style: I would indent "file" up to "read ROM layout". Your choice.
> +#if 0
> + /* FIXME: This is broken. printf_debug is a no-op because verbose=0
> + * at this point.
> + */
> if (argc > 1) {
> /* Yes, print them. */
> printf_debug("The arguments are:\n");
> for (i = 1; i < argc; ++i)
> printf_debug("%s\n", argv[i]);
> }
> +#endif
Throw it out completely. Whoever just wants to see the parameters
flashrom is called with should write a wrapper script.
> - exit(1);
> + cli_classic_abort_usage(argv[0]);
Don't dump the usage on syntax errors. It clutters output too much so
that you don't find the error message. Just print a one-liner like 'run
"flashrom --help" for usage info'. I especially liked the debian tool
that outputs "You need --help.", but don't recommend this very phrase to
flashrom.
> if (programmer == PROGRAMMER_INVALID) {
> printf("Error: Unknown programmer %s.\n",
> optarg);
> - exit(1);
> + cli_classic_abort_usage(argv[0]);
> }
In this case, list the supported programmers (and just them).
> - msg_cinfo("VERIFIED. \n");
> + msg_cinfo("VERIFIED.\n");
The spaces probably were to clear address counter characters. Did you
verify that it is not needed with current chip drivers?
The remaining stuff looks good, you get a tentative
Acked-by: Michael Karcher <[email protected]>
Regards,
Michael Karcher
_______________________________________________
flashrom mailing list
[email protected]
http://www.flashrom.org/mailman/listinfo/flashrom
