[flashrom] [PATCH 1/7] Introduce and use enum test_state

[Carl-Daniel Hailfinger]

Am 03.03.2012 21:11 schrieb Stefan Tauner:
> Previously boards in the wiki were tagged either as working or as known
> bad. But we added support to various boards via board enables that were
> then never tested because the owners have not reported back. This can
> now be tagged with NT and is shown appropriately.
>
> Also, the underlying data structure indicating state was converted from
> macros to an enum while preserving original integer values.
>
> Because all lines specifying supported boards and laptops were touched
> anyway, this patch also re-indents them.
>
> ---
> TODO: change other occurrences to use it.  wanted to get feedack first.
>
> Signed-off-by: Stefan Tauner <stefan.tauner at student.tuwien.ac.at>
>
> diff --git a/print_wiki.c b/print_wiki.c
> index 377154d..9a9cd83 100644
> --- a/print_wiki.c
> +++ b/print_wiki.c
> @@ -136,9 +136,9 @@ static void wiki_helper(const char *devicetype, int cols,
>  	const struct board_match *b = board_matches;
>  
>  	for (i = 0; boards[i].vendor != NULL; i++) {
> -		if (boards[i].working)
> +		if (boards[i].working == OK)
>  			boardcount_good++;
> -		else
> +		if (boards[i].working == BAD)
>  			boardcount_bad++;

You could replace that construct with a switch(), and it might make
sense to count untested boards as well.


>  	}
>  
> @@ -171,7 +171,8 @@ static void wiki_helper(const char *devicetype, int cols,
>  		       b[k].lb_vendor ? b[k].lb_vendor : "",
>  		       b[k].lb_vendor ? ":" : "",
>  		       b[k].lb_vendor ? b[k].lb_part : "",
> -		       (boards[i].working) ? "OK" : "No");
> +		       (boards[i].working == OK) ? "OK" :
> +		       (boards[i].working == NT) ? "?3" : "No");

The ?3 looks odd, but I assume you tested it.


>  
>  		if (boards[i].note) {
>  			printf("<sup>%d</sup>\n", num_notes + 1);
> diff --git a/print.c b/print.c
> index 1fdeac7..544a846 100644
> --- a/print.c
> +++ b/print.c
> @@ -545,428 +545,427 @@ void print_supported(void)
> [...]
> -	B("ZOTAC",	"Fusion-ITX WiFi (FUSION350-A-E)", 1, NULL, NULL),
> -	B("ZOTAC",	"GeForce 8200",		1, "http://pden.zotac.com/index.php?page=shop.product_details&product_id=129&category_id=92", NULL),
> -	B("ZOTAC",	"H67-ITX WiFi (H67ITX-C-E)", 0, NULL, "Probing works (Winbond W25Q32, 4096 kB, SPI), but parts of the flash are problematic: descriptor is r/o (conforming to ICH reqs), ME region is locked."),
> -	B("ZOTAC",	"ZBOX HD-ID11",		1, "http://pdde.zotac.com/index.php?page=shop.product_details&product_id=240&category_id=75", NULL),
> +	B("A-Trend",	"ATC-6220",				OK,	"http://www.motherboard.cz/mb/atrend/atc6220.htm", NULL),
> +	B("abit",	"A-S78H",				OK,	"http://www.abit.com.tw/page/en/motherboard/motherboard_detail.php?pMODEL_NAME=A-S78H&fMTYPE=Socket+AM2", NULL),
> +	B("abit",	"AN-M2",				OK,	"http://www.abit.com.tw/page/en/motherboard/motherboard_detail.php?DEFTITLE=Y&fMTYPE=Socket%20AM2&pMODEL_NAME=AN-M2", NULL),
> +	B("abit",	"AV8",					OK,	"http://www.abit.com.tw/page/en/motherboard/motherboard_detail.php?DEFTITLE=Y&fMTYPE=Socket%20939&pMODEL_NAME=AV8", NULL),

Is it possible that this patch made the board table one or two tabs
wider? The patch looks like that, and while I agree that some
files/sections should not have line length limits, adding another 16
columns of whitespace is something I'd like to avoid.

print_wiki related code is something I rarely touch (except for
programmer additions), so please don't expect in-depth reviews from me.
A cursory review suggests that the patch at least doesn't make the code
worse and I don't have any strong feelings about this code. If you feel
this patch is beneficial, I can send a weak Acked-by, more like Meh-by.

Regards,
Carl-Daniel

-- 
http://www.hailfinger.org/