See patch.
The code may have some potential for simplification, but it basically works. I already used the output of the new -z option in the http://coreboot.org/Flashrom page, please check if there are errors in the output (or code).
Thanks, Uwe.
On 09.05.2009 04:07, Uwe Hermann wrote:
See patch.
The code may have some potential for simplification, but it basically works. I already used the output of the new -z option in the http://coreboot.org/Flashrom page, please check if there are errors in the output (or code).
Add a --list-supported-wiki / -z option which outputs the currently supported flash chips (and their status), chipsets (plus status), and mainboards (plus status) to stdout.
This allows for very easy pasting into the http://coreboot.org/Flashrom page, so we can keep that page up-to-date without much hassle.
The list of boards is mostly new (known good ones which don't need write-enable code, and known-bad ones) and also lists URLs to the vendor's mainboard pages. These URLs are NOT printed in -L output per default, but they are if you use verbose mode, e.g. via -LV. They're also used in the --list-supported-wiki output for the wiki, of course.
Signed-off-by: Uwe Hermann uwe@hermann-uwe.de
Thanks for this patch.
A few comments if I may: Having the ability to get wikified output is definitely a huge improvement over what we have today. Please make sure the "... unknown SPI chip" variants are not listed. Status: "Broken" ends up as "?". IMHO the wiki output code uglifies quite a lot of files. Please stick it into one common file called wikifier.c or something like that. URLs in arrays feel weird. Having the URLs in wikifier.c as well would make sure they are out of the way.
Regards, Carl-Daniel
On Tue, May 12, 2009 at 06:31:15PM +0200, Carl-Daniel Hailfinger wrote:
Please make sure the "... unknown SPI chip" variants are not listed. Status: "Broken" ends up as "?".
True, but I'd like to fix these issues in a different patch, this one's quite huge and invasive already.
IMHO the wiki output code uglifies quite a lot of files. Please stick it into one common file called wikifier.c or something like that.
Yeah, good idea. Attached is the second iteration of the patch which has the wiki printing code in wikify.c. That requires to put some structs into flash.h though and make some stuff non-'static'.
Uwe.
On Tue, May 12, 2009 at 10:03:56PM +0200, Uwe Hermann wrote:
On Tue, May 12, 2009 at 06:31:15PM +0200, Carl-Daniel Hailfinger wrote:
Please make sure the "... unknown SPI chip" variants are not listed. Status: "Broken" ends up as "?".
True, but I'd like to fix these issues in a different patch, this one's quite huge and invasive already.
IMHO the wiki output code uglifies quite a lot of files. Please stick it into one common file called wikifier.c or something like that.
Yeah, good idea. Attached is the second iteration of the patch which has the wiki printing code in wikify.c. That requires to put some structs into flash.h though and make some stuff non-'static'.
Forgot the patch.
Uwe.
On 12.05.2009 22:06, Uwe Hermann wrote:
On Tue, May 12, 2009 at 10:03:56PM +0200, Uwe Hermann wrote:
On Tue, May 12, 2009 at 06:31:15PM +0200, Carl-Daniel Hailfinger wrote:
Please make sure the "... unknown SPI chip" variants are not listed. Status: "Broken" ends up as "?".
True, but I'd like to fix these issues in a different patch, this one's quite huge and invasive already.
Since that code seems to be new, I figured you could fix it in the first commit, but I'm not picky as long as you promise to send a followup patch.
IMHO the wiki output code uglifies quite a lot of files. Please stick it into one common file called wikifier.c or something like that.
Yeah, good idea. Attached is the second iteration of the patch which has the wiki printing code in wikify.c. That requires to put some structs into flash.h though and make some stuff non-'static'.
Add a --list-supported-wiki / -z option which outputs the currently supported flash chips (and their status), chipsets (plus status), and mainboards (plus status) to stdout.
This allows for very easy pasting into the http://coreboot.org/Flashrom page, so we can keep that page up-to-date without much hassle.
The list of boards is mostly new (known good ones which don't need write-enable code, and known-bad ones) and also lists URLs to the vendor's mainboard pages. These URLs are NOT printed in -L output per default, but they are if you use verbose mode, e.g. via -LV. They're also used in the --list-supported-wiki output for the wiki, of course.
Signed-off-by: Uwe Hermann uwe@hermann-uwe.de
Index: flashrom.8
--- flashrom.8 (Revision 498) +++ flashrom.8 (Arbeitskopie) @@ -2,7 +2,7 @@ .SH NAME flashrom - read, write, verify and erase BIOS/ROM/flash chips .SH SYNOPSIS -.B flashrom \fR[\fB-rwvEVfLhR\fR] [\fB-c\fR chipname] [\fB-s\fR exclude_start] [\fB-e\fR exclude_end] +.B flashrom \fR[\fB-rwvEVfLzhR\fR] [\fB-c\fR chipname] [\fB-s\fR exclude_start] [\fB-e\fR exclude_end] [\fB-m\fR [vendor:]part] [\fB-l\fR file.layout] [\fB-i\fR image_name] [file] .SH DESCRIPTION .B flashrom @@ -123,6 +123,12 @@ Please let us know if you can verify other boards to work or not work out of the box. .TP +.B "-z, --list-supported-wiki" +Same as +.BR --list-supported , +but outputs the supported hardware in MediaWiki syntax, so that it can be +easily pasted into the wiki page at http://coreboot.org/Flashrom. +.TP .B "-p, --programmer <name>" Specify the programmer device. Currently supported are: .sp Index: flash.h =================================================================== --- flash.h (Revision 498) +++ flash.h (Arbeitskopie) @@ -29,6 +29,7 @@ #include <unistd.h> #include <stdint.h> #include <stdio.h> +#include <pci/pci.h>
/* for iopl and outb under Solaris */ #if defined (__sun) && (defined(__i386) || defined(__amd64)) @@ -201,6 +202,75 @@
extern struct flashchip flashchips[];
+typedef struct penable {
- uint16_t vendor_id;
- uint16_t device_id;
- int status;
- const char *vendor_name;
- const char *device_name;
- int (*doit) (struct pci_dev *dev, const char *name);
+} FLASH_ENABLE;
Please kill that ugly typedef while you're at it.
+#define OK 0 +#define NT 1 /* Not tested */
Prefix please. CHIPSET_OK etc.
+extern const FLASH_ENABLE enables[];
And rename enables to chipset_enables if at all possible. I never saw a more ambiguous variable name in flashrom. Sure, it's not your fault, but since you're touching that code anyway... no good deed goes unpunished.
+/**
- We use 2 sets of IDs here, you're free to choose which is which. This
- is to provide a very high degree of certainty when matching a board on
- the basis of subsystem/card IDs. As not every vendor handles
- subsystem/card IDs in a sane manner.
- Keep the second set NULLed if it should be ignored. Keep the subsystem IDs
- NULLed if they don't identify the board fully. But please take care to
- provide an as complete set of pci ids as possible; autodetection is the
- preferred behaviour and we would like to make sure that matches are unique.
- The coreboot ids are used two fold. When running with a coreboot firmware,
- the ids uniquely matches the coreboot board identification string. When a
- legacy bios is installed and when autodetection is not possible, these ids
- can be used to identify the board through the -m command line argument.
- When a board is identified through its coreboot ids (in both cases), the
- main pci ids are still required to match, as a safeguard.
- */
+struct board_pciid_enable {
- /* Any device, but make it sensible, like the ISA bridge. */
- uint16_t first_vendor;
- uint16_t first_device;
- uint16_t first_card_vendor;
- uint16_t first_card_device;
- /* Any device, but make it sensible, like
* the host bridge. May be NULL.*/- uint16_t second_vendor;
- uint16_t second_device;
- uint16_t second_card_vendor;
- uint16_t second_card_device;
- /* The vendor / part name from the coreboot table. */
- const char *lb_vendor;
- const char *lb_part;
- const char *vendor_name;
- const char *board_name;
- int (*enable) (const char *name);
+};
+extern struct board_pciid_enable board_pciid_enables[];
+struct board_info {
- const char *vendor;
- const char *name;
- const char *url;
+};
+extern const struct board_info boards_ok[]; +extern const struct board_info boards_no[];
boards_bad maybe?
/*
- Please keep this list sorted alphabetically by manufacturer. The first
- entry of each section should be the manufacturer ID, followed by the
@@ -537,6 +607,11 @@ #define W_49V002A 0xB0 #define W_49V002FA 0x32
+/* wikify.c */ +void print_supported_chips_wiki(void); +void print_supported_boards_wiki(void); +void print_supported_chipsets_wiki(void);
/* udelay.c */ void myusec_delay(int time); void myusec_calibrate_delay(void); Index: Makefile =================================================================== --- Makefile (Revision 498) +++ Makefile (Arbeitskopie) @@ -35,7 +35,7 @@ sst49lfxxxc.o sst_fwhub.o layout.o cbtable.o flashchips.o physmap.o \ flashrom.o w39v080fa.o sharplhf00l04.o w29ee011.o spi.o it87spi.o \ ichspi.o w39v040c.o sb600spi.o wbsio_spi.o m29f002.o internal.o \
- dummyflasher.o
- dummyflasher.o wikify.o
all: pciutils dep $(PROGRAM)
Index: wikify.c
--- wikify.c (Revision 0) +++ wikify.c (Revision 0) @@ -0,0 +1,196 @@ +/*
- This file is part of the flashrom project.
- Copyright (C) 2009 Uwe Hermann uwe@hermann-uwe.de
- This program is free software; you can redistribute it and/or modify
- it under the terms of the GNU General Public License as published by
- the Free Software Foundation; either version 2 of the License, or
- (at your option) any later version.
- This program is distributed in the hope that it will be useful,
- but WITHOUT ANY WARRANTY; without even the implied warranty of
- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- GNU General Public License for more details.
- You should have received a copy of the GNU General Public License
- along with this program; if not, write to the Free Software
- Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
- */
+#include <string.h> +#include "flash.h"
+#define CHIPSET_TH "{| border="0" style="font-size: smaller"\n\ +|- bgcolor="#6699dd"\n! align="left" | Vendor\n\ +! align="left" | Southbridge\n! align="left" | Status\n\n"
+void print_supported_chipsets_wiki(void) +{
- int i, j, enablescount = 0, color = 1;
- const FLASH_ENABLE *e = enables;
- printf("\n== Supported chipsets ==\n\n{| border="0" "
"valign=\"top\"\n| valign=\"top\"|\n\n" CHIPSET_TH);- for (e = enables; e->vendor_name != NULL; e++)
enablescount++;- for (i = 0, j = 0; enables[i].vendor_name != NULL; i++, j++) {
/* Alternate colors if the vendor changes. */if (i > 0 && strcmp(enables[i].vendor_name,enables[i - 1].vendor_name))color = !color;printf("|- bgcolor=\"#%s\" valign=\"top\"\n| %s\n| %s""\n| %s\n", (color) ? "eeeeee" : "dddddd",enables[i].vendor_name, enables[i].device_name,(enables[i].status == OK) ? "{{OK}}" : "?");/* Split table in three columns. */if (j >= (enablescount / 3 + 1)) {printf("\n|}\n\n| valign=\"top\"|\n\n" CHIPSET_TH);j = 0;}- }
- printf("\n|}\n\n|}\n");
+}
+#define BOARD_TH "{| border="0" style="font-size: smaller" valign="top"\ +\n|- bgcolor="#6699dd"\n! align="left" | Vendor\n\ +! align="left" | Mainboard\n! align="left" | Status\n\n"
+#define BOARD_TH2 "{| border="0" style="font-size: smaller" valign="top"\ +\n|- bgcolor="#6699dd"\n! align="left" | Vendor\n\ +! align="left" | Mainboard\n! align="left" | Required option\n\ +! align="left" | Status\n\n"
+static void wiki_helper(const char *heading, const char *status,
int cols, const struct board_info boards[])+{
- int i, j, boardcount = 0, color = 1;
- const struct board_info *b;
- printf("\n'''%s'''\n\n{| border="0" "
"valign=\"top\"\n| valign=\"top\"|\n\n%s", heading, BOARD_TH);- for (b = boards; b->vendor != NULL; b++)
boardcount++;for (i = 0, j = 0, b = boards; b[i].vendor != NULL; i++, j++) {/* Alternate colors if the vendor changes. */if (i > 0 && strcmp(b[i].vendor, b[i - 1].vendor))color = !color;printf("|- bgcolor=\"#%s\" valign=\"top\"\n| %s\n|%s%s %s%s\n|"" {{%s}}\n", (color) ? "eeeeee" : "dddddd", b[i].vendor,(b[i].url) ? " [" : "", (b[i].url) ? b[i].url : "",b[i].name, (b[i].url) ? "]" : "", status);/* Split table in three columns. */if (j >= (boardcount / cols + 1)) {printf("\n|}\n\n| valign=\"top\"|\n\n" BOARD_TH);j = 0;}- }
- printf("\n|}\n\n|}\n");
+}
+static void wiki_helper2(const char *heading) +{
- int i, j, boardcount = 0, color = 1;
- struct board_pciid_enable *b;
- printf("\n'''%s'''\n\n{| border="0" "
"valign=\"top\"\n| valign=\"top\"|\n\n%s", heading, BOARD_TH2);- for (b = board_pciid_enables; b->vendor_name != NULL; b++)
boardcount++;- b = board_pciid_enables;
- for (i = 0, j = 0; b[i].vendor_name != NULL; i++, j++) {
/* Alternate colors if the vendor changes. */if (i > 0 && strcmp(b[i].vendor_name, b[i - 1].vendor_name))color = !color;printf("|- bgcolor=\"#%s\" valign=\"top\"\n| %s\n| %s\n| ""%s%s%s%s\n| {{OK}}\n", (color) ? "eeeeee" : "dddddd",b[i].vendor_name, b[i].board_name,(b[i].lb_vendor) ? "-m " : "—",(b[i].lb_vendor) ? b[i].lb_vendor : "",(b[i].lb_vendor) ? ":" : "",(b[i].lb_vendor) ? b[i].lb_part : "");/* Split table in two columns. */if (j >= (boardcount / 2 + 1)) {printf("\n|}\n\n| valign=\"top\"|\n\n" BOARD_TH2);
To be honest, I don't understand the criteria of how the string above was split into a string and a #defined string.
j = 0;}- }
- printf("\n|}\n\n|}\n");
+}
+#define BOARD_INTRO "\ +\n== Supported mainboards ==\n\n\ +In general, it is very likely that flashrom works out of the box even if your \ +mainboard is not listed below.\n\nThis is a list of mainboards where we have \ +verified that they either do or do not need any special initialization to \ +make flashrom work (given flashrom supports the respective chipset and flash \ +chip), or that they do not yet work at all. If they do not work, support may \ +or may not be added later.\n\n\ +Mainboards which don't appear in the list may or may not work (we don't \ +know, someone has to give it a try). Please report any further verified \ +mainboards on the [[Mailinglist|mailing list]].\n"
Maybe insert that statement into the printf instead? Same for all other multiline statements.
+void print_supported_boards_wiki(void) +{
- printf("%s", BOARD_INTRO);
- wiki_helper("Known good (worked out of the box)", "OK", 3, boards_ok);
- wiki_helper2("Known good (with write-enable code in flashrom)");
- wiki_helper("Not supported (yet)", "No", 1, boards_no);
+}
+#define CHIP_TH "{| border="0" style="font-size: smaller" valign="top"\n\ +|- bgcolor="#6699dd"\n! align="left" | Vendor\n\ +! align="left" | Flash part\n! align="left" colspan="4" | Status\n\n\ +|- bgcolor="#6699ff"\n| colspan="2" | \n\ +| Probe\n| Read\n| Write\n| Erase\n\n"
+void print_supported_chips_wiki(void) +{
- int i = 0, c = 1, chipcount = 0;
- struct flashchip *f, *old = NULL;
- printf("\n== Supported chips ==\n\n{| border="0" valign="top"\n"
"| valign=\"top\"|\n\n" CHIP_TH);- for (f = flashchips; f->name != NULL; f++)
chipcount++;- for (f = flashchips; f->name != NULL; f++, i++) {
/* Alternate colors if the vendor changes. */if (old != NULL && strcmp(old->vendor, f->vendor))c = !c;printf("|- bgcolor=\"#%s\" valign=\"top\"\n| %s""\n| %s\n| {{%s}}\n| {{%s}}\n| {{%s}}\n| {{%s}}\n",(c == 1) ? "eeeeee" : "dddddd", f->vendor, f->name,((f->tested & TEST_OK_PROBE) ? "OK" : (c) ? "?2" : "?"),((f->tested & TEST_OK_READ) ? "OK" : (c) ? "?2" : "?"),((f->tested & TEST_OK_ERASE) ? "OK" : (c) ? "?2" : "?"),((f->tested & TEST_OK_WRITE) ? "OK" : (c) ? "?2" : "?"));/* Split table into three columns. */if (i >= (chipcount / 3 + 1)) {printf("\n|}\n\n| valign=\"top\"|\n\n" CHIP_TH);i = 0;}old = f;- }
- printf("\n|}\n\n|}\n");
+} Index: chipset_enable.c =================================================================== --- chipset_enable.c (Revision 498) +++ chipset_enable.c (Arbeitskopie) @@ -33,6 +33,7 @@ #include <sys/mman.h> #include <fcntl.h> #include <unistd.h> +#include <string.h> #include "flash.h"
unsigned long flashbase = 0; @@ -903,20 +904,8 @@ return 0; }
-typedef struct penable {
- uint16_t vendor_id;
- uint16_t device_id;
- int status;
- const char *vendor_name;
- const char *device_name;
- int (*doit) (struct pci_dev *dev, const char *name);
-} FLASH_ENABLE;
-#define OK 0 -#define NT 1 /* Not tested */
/* Please keep this list alphabetically sorted by vendor/device. */ -static const FLASH_ENABLE enables[] = { +const FLASH_ENABLE enables[] = { {0x10B9, 0x1533, OK, "ALi", "M1533", enable_flash_ali_m1533}, {0x1022, 0x7440, OK, "AMD", "AMD-768", enable_flash_amd8111}, {0x1022, 0x7468, OK, "AMD", "AMD8111", enable_flash_amd8111}, @@ -993,6 +982,8 @@ {0x1106, 0x3372, OK, "VIA", "VT8237S", enable_flash_vt8237s_spi}, {0x1106, 0x0586, OK, "VIA", "VT82C586A/B", enable_flash_amd8111}, {0x1106, 0x0686, NT, "VIA", "VT82C686A/B", enable_flash_amd8111},
- {0x0000, 0x0000, 0, NULL, NULL, NULL},
};
void print_supported_chipsets(void) @@ -1001,11 +992,12 @@
printf("\nSupported chipsets:\n\n");
- for (i = 0; i < ARRAY_SIZE(enables); i++)
- for (i = 0; enables[i].vendor_name != NULL; i++) {
Good find!
printf("%s %s [%04x:%04x]%s\n", enables[i].vendor_name, enables[i].device_name, enables[i].vendor_id, enables[i].device_id, (enables[i].status == OK) ? "" : " (untested)");
- }
}
int chipset_flash_enable(void) @@ -1015,7 +1007,7 @@ int i;
/* Now let's try to find the chipset we have... */
- for (i = 0; i < ARRAY_SIZE(enables); i++) {
- for (i = 0; enables[i].vendor_name != NULL; i++) { dev = pci_dev_find(enables[i].vendor_id, enables[i].device_id); if (dev) break;
Index: flashrom.c
--- flashrom.c (Revision 498) +++ flashrom.c (Arbeitskopie) @@ -292,7 +292,7 @@
void usage(const char *name) {
- printf("usage: %s [-rwvEVfLhR] [-c chipname] [-s exclude_start]\n",
- printf("usage: %s [-rwvEVfLzhR] [-c chipname] [-s exclude_start]\n", name); printf(" [-e exclude_end] [-m [vendor:]part] [-l file.layout] [-i imagename] [file]\n"); printf("Please note that the command line interface for flashrom will "
@@ -313,6 +313,7 @@ " -l | --layout <file.layout>: read rom layout from file\n" " -i | --image <name>: only flash image name from flash layout\n" " -L | --list-supported: print supported devices\n"
" -z | --list-supported-wiki: print supported devices in wiki syntax\n" " -p | --programmer <name>: specify the programmer device\n" " -h | --help: print this help text\n" " -R | --version: print the version (release)\n"@@ -337,6 +338,7 @@ int option_index = 0; int force = 0; int read_it = 0, write_it = 0, erase_it = 0, verify_it = 0;
int list_supported = 0, list_supported_wiki = 0; int ret = 0, i;
static struct option long_options[] = {
@@ -353,6 +355,7 @@ {"layout", 1, 0, 'l'}, {"image", 1, 0, 'i'}, {"list-supported", 0, 0, 'L'},
{"list-supported-wiki", 0, 0, 'z'},@@ -375,7 +378,7 @@ }
setbuf(stdout, NULL);
- while ((opt = getopt_long(argc, argv, "rRwvVEfc:s:e:m:l:i:p:Lh",
- while ((opt = getopt_long(argc, argv, "rRwvVEfc:s:e:m:l:i:p:Lzh", long_options, &option_index)) != EOF) { switch (opt) { case 'r':
@@ -429,11 +432,11 @@ find_romentry(tempstr); break; case 'L':
print_supported_chips();print_supported_chipsets();print_supported_boards();exit(0);
list_supported = 1; break;case 'z':list_supported_wiki = 1;break;@@ -455,6 +458,21 @@ } }
- if (list_supported) {
print_supported_chips();print_supported_chipsets();print_supported_boards();exit(0);- }
- if (list_supported_wiki) {
printf("= Supported devices =\n");print_supported_chips_wiki();print_supported_chipsets_wiki();print_supported_boards_wiki();exit(0);- }
- if (read_it && write_it) { printf("Error: -r and -w are mutually exclusive.\n"); usage(argv[0]);
Index: board_enable.c
--- board_enable.c (Revision 498) +++ board_enable.c (Arbeitskopie) @@ -645,50 +645,6 @@ return 0; }
-/**
- We use 2 sets of IDs here, you're free to choose which is which. This
- is to provide a very high degree of certainty when matching a board on
- the basis of subsystem/card IDs. As not every vendor handles
- subsystem/card IDs in a sane manner.
- Keep the second set NULLed if it should be ignored. Keep the subsystem IDs
- NULLed if they don't identify the board fully. But please take care to
- provide an as complete set of pci ids as possible; autodetection is the
- preferred behaviour and we would like to make sure that matches are unique.
- The coreboot ids are used two fold. When running with a coreboot firmware,
- the ids uniquely matches the coreboot board identification string. When a
- legacy bios is installed and when autodetection is not possible, these ids
- can be used to identify the board through the -m command line argument.
- When a board is identified through its coreboot ids (in both cases), the
- main pci ids are still required to match, as a safeguard.
- */
-struct board_pciid_enable {
- /* Any device, but make it sensible, like the ISA bridge. */
- uint16_t first_vendor;
- uint16_t first_device;
- uint16_t first_card_vendor;
- uint16_t first_card_device;
- /* Any device, but make it sensible, like
* the host bridge. May be NULL.*/- uint16_t second_vendor;
- uint16_t second_device;
- uint16_t second_card_vendor;
- uint16_t second_card_device;
- /* The vendor / part name from the coreboot table. */
- const char *lb_vendor;
- const char *lb_part;
- const char *vendor_name;
- const char *board_name;
- int (*enable) (const char *name);
-};
/* Please keep this list alphabetically ordered by vendor/board name. */ struct board_pciid_enable board_pciid_enables[] = { /* first pci-id set [4], second pci-id set [4], coreboot id [2], vendor name board name flash enable */ @@ -728,26 +684,131 @@ { 0, 0, 0, 0, 0, 0, 0, 0, NULL, NULL, NULL, NULL, NULL}, /* end marker */ };
+/* Please keep this list alphabetically ordered by vendor/board. */ +const struct board_info boards_ok[] = {
- /* Verified working boards that don't need write-enables. */
- { "Abit", "AX8", "http://www.abit.com.tw/page/en/motherboard/motherboard_detail.php?DEFTITLE=Y..." },
- { "Advantech", "PCM-5820", "http://taiwan.advantech.com.tw/products/Model_Detail.asp?model_id=1-1TGZL8&a..." },
- { "ASI", "MB-5BLMP", "http://www.hojerteknik.com/winnet.htm" },
- { "ASUS", "A8N-E", "http://www.asus.com.tw/products.aspx?l1=3&l2=15&l3=171&l4=0&..." },
- { "ASUS", "A8NE-FM/S", "http://www.hardwareschotte.de/hardware/preise/proid_1266090/preis_ASUS+A8NE-..." },
- { "ASUS", "A8N-SLI Premium", "http://www.asus.com.tw/products.aspx?l1=3&l2=15&l3=148&l4=0&..." },
- { "ASUS", "A8V-E Deluxe", "http://www.asus.com.tw/products.aspx?l1=3&l2=15&l3=143&l4=0&..." },
- { "ASUS", "M2A-VM", "http://www.asus.com.tw/products.aspx?l1=3&l2=101&l3=496&l4=0&..." },
- { "ASUS", "M2N-E", "http://www.asus.com/products.aspx?l1=3&l2=101&l3=308&l4=0&mo..." },
- { "ASUS", "P2B", "http://www.motherboard.cz/mb/asus/P2B.htm" },
- { "ASUS", "P2B-F", "http://www.motherboard.cz/mb/asus/P2B-F.htm" },
- { "ASUS", "P2B-D", "ftp://ftp.asus.com.tw/pub/ASUS/mb/slot1/440bx/p2b-d/" },
- { "ASUS", "P2B-DS", "ftp://ftp.asus.com.tw/pub/ASUS/mb/slot1/440bx/p2b-ds/" },
- { "ASUS", "A7V400-MX", "http://www.asus.com.tw/products.aspx?l1=3&l2=13&l3=63&l4=0&m..." },
- { "ASUS", "A7V8X-MX", "http://www.asus.com.tw/products.aspx?l1=3&l2=13&l3=64&l4=0&m..." },
- { "ASUS", "P4B266", "http://www.ciao.co.uk/ASUS_Intel_845D_Chipset_P4B266__5409807#productdetail" },
- { "ASUS", "A8V-E SE", "http://www.asus.com.tw/products.aspx?l1=3&l2=15&l3=143&l4=0&..." },
- { "ASUS", "P2L97-S", "http://www.motherboard.cz/mb/asus/P2L97-S.htm" },
- { "ASUS", "M2A-MX", "http://www.asus.com/products.aspx?l1=3&l2=101&l3=583&l4=0&mo..." },
- { "A-Trend", "ATC-6220", "http://www.motherboard.cz/mb/atrend/atc6220.htm" },
- { "BCOM", "WinNET100", "http://www.coreboot.org/BCOM_WINNET100_Build_Tutorial" },
- { "GIGABYTE", "GA-6BXC", "http://www.gigabyte.com.tw/Products/Motherboard/Products_Spec.aspx?ClassValu..." },
- { "GIGABYTE", "GA-6BXDU", "http://www.gigabyte.com.tw/Products/Motherboard/Products_Spec.aspx?ProductID..." },
- { "MSI", "KT4V", NULL },
- { "MSI", "MS-7046", NULL },
- { "MSI", "MS-7065", NULL },
- { "MSI", "MS-7236 (945PL Neo3)", "http://global.msi.com.tw/index.php?func=prodmbspec&maincat_no=1&cat2..." },
- { "NEC", "PowerMate 2000", "http://support.necam.com/mobilesolutions/hardware/Desktops/pm2000/celeron/" },
- { "PC Engines", "Alix.1c", "http://pcengines.ch/alix1c.htm" },
- { "PC Engines", "Alix.2c2", "http://pcengines.ch/alix2c2.htm" },
- { "PC Engines", "Alix.2c3", "http://pcengines.ch/alix2c3.htm" },
- { "PC Engines", "Alix.3c3", "http://pcengines.ch/alix3c3.htm" },
- { "RCA", "RM4100", "http://www.settoplinux.org" },
- { "Sun", "Blade x6250", "http://www.sun.com/servers/blades/x6250/" },
- { "Thomson", "IP1000", "http://www.settoplinux.org" },
- { "T-Online", "S-100", "http://wiki.freifunk-hannover.de/T-Online_S_100" },
- { "Tyan", "S1846", "http://www.tyan.com/archive/products/html/tsunamiatx.html" },
- { "Tyan", "S2498 (Tomcat K7M)", "http://www.tyan.com/archive/products/html/tomcatk7m.html" },
- { "Tyan", "S2881", "http://www.tyan.com/product_board_detail.aspx?pid=115" },
- { "Tyan", "S2882", "http://www.tyan.com/product_board_detail.aspx?pid=121" },
- { "Tyan", "S2882-D", "http://www.tyan.com/product_board_detail.aspx?pid=127" },
- { "Tyan", "S3095", "http://www.tyan.com/product_board_detail.aspx?pid=181" },
- { "Tyan", "S5180", "http://www.tyan.com/product_board_detail.aspx?pid=456" },
- { "Tyan", "S5191", "http://www.tyan.com/product_board_detail.aspx?pid=343" },
- { "Tyan", "S5197", "http://www.tyan.com/product_board_detail.aspx?pid=349" },
- { "Tyan", "S5211", "http://www.tyan.com/product_board_detail.aspx?pid=591" },
- { "Tyan", "S5211-1U", "http://www.tyan.com/product_board_detail.aspx?pid=593" },
- { "Tyan", "S5220", "http://www.tyan.com/product_board_detail.aspx?pid=597" },
- { "Tyan", "S5375", "http://www.tyan.com/product_board_detail.aspx?pid=566" },
- { "Tyan", "iS5375-1U", "http://www.tyan.com/product_board_detail.aspx?pid=610" },
- { "Tyan", "S5376G2NR/S5376WAG2NR","http://www.tyan.com/product_board_detail.aspx?pid=605" },
- { "Tyan", "S5377", "http://www.tyan.com/product_board_detail.aspx?pid=601" },
- { "Tyan", "S5397", "http://www.tyan.com/product_board_detail.aspx?pid=560" },
- { "VIA", "EPIA-M", "http://www.via.com.tw/en/products/mainboards/motherboards.jsp?motherboard_id..." },
- { "VIA", "EPIA-MII", "http://www.via.com.tw/en/products/mainboards/motherboards.jsp?motherboard_id..." },
- { "VIA", "EPIA-CN", "http://www.via.com.tw/en/products/mainboards/motherboards.jsp?motherboard_id..." },
- { "VIA", "EPIA-LN", "http://www.via.com.tw/en/products/mainboards/motherboards.jsp?motherboard_id..." },
- { "VIA", "VB700X", "http://www.via.com.tw/en/products/mainboards/motherboards.jsp?motherboard_id..." },
- { "VIA", "NAB74X0", "http://www.via.com.tw/en/products/mainboards/motherboards.jsp?motherboard_id..." },
- { "VIA", "pc2500e", "http://www.via.com.tw/en/initiatives/empowered/pc2500_mainboard/index.jsp" },
- { NULL, NULL, 0 },
+};
+/* Please keep this list alphabetically ordered by vendor/board. */ +const struct board_info boards_no[] = {
- /* Verified non-working boards (for now). */
- { "ASUS", "A7N8X-E Deluxe", "http://www.asus.com/products.aspx?l1=3&l2=13&l3=56&l4=0&mode..." },
- { "ASUS", "MEW-AM", "ftp://ftp.asus.com.tw/pub/ASUS/mb/sock370/810/mew-am/" },
- { "ASUS", "MEW-VM", "http://www.elhvb.com/mboards/OEM/HP/manual/ASUS%20MEW-VM.htm" },
- { "ASUS", "P3B-F", "ftp://ftp.asus.com.tw/pub/ASUS/mb/slot1/440bx/p3b-f/" },
- { "Biostar", "M6TBA", "ftp://ftp.biostar-usa.com/manuals/M6TBA/" },
- { "FIC", "VA-502", "ftp://ftp.fic.com.tw/motherboard/manual/socket7/va-502/" },
- { "MSI", "MS-7260 (K9N Neo)", "http://global.msi.com.tw/index.php?func=proddesc&prod_no=255&maincat..." },
- { "PCCHIPS", "M537DMA33", "http://motherboards.mbarron.net/models/pcchips/m537dma.htm" },
- { "Soyo", "SY-5VD", "http://www.soyo.com/content/Downloads/163/&c=80&p=464&l=English" },
- { "Sun", "Fire x4540", "http://www.sun.com/servers/x64/x4540/" },
- { "Sun", "Fire x4150", "http://www.sun.com/servers/x64/x4150/" },
- { "Sun", "Fire x4200", "http://www.sun.com/servers/entry/x4200/" },
- { "Sun", "Fire x4600", "http://www.sun.com/servers/x64/x4600/" },
- { NULL, NULL, 0 },
+};
I still don't like the fact that files outside wikify.c are polluted with URLs.
+void print_supported_boards_helper(const struct board_info *b) +{
- int i, j;
- for (i = 0; b[i].vendor != NULL; i++) {
printf("%s", b[i].vendor);for (j = 0; j < 25 - strlen(b[i].vendor); j++)printf(" ");printf("%s", b[i].name);for (j = 0; j < 23 - strlen(b[i].name); j++)printf(" ");printf("%s\n", (verbose) ? (b[i].url) ? b[i].url : "" : "");- }
+}
void print_supported_boards(void) {
- int i;
- int i, j;
- struct board_pciid_enable *b = board_pciid_enables;
- printf("\nSupported mainboards (this list is not exhaustive!):\n\n");
- for (i = 0; board_pciid_enables[i].vendor_name != NULL; i++) {
if (board_pciid_enables[i].lb_vendor != NULL) {printf("%s %s (-m %s:%s)\n",board_pciid_enables[i].vendor_name,board_pciid_enables[i].board_name,board_pciid_enables[i].lb_vendor,board_pciid_enables[i].lb_part);} else {printf("%s %s (autodetected)\n",board_pciid_enables[i].vendor_name,board_pciid_enables[i].board_name);}
printf("\nSupported boards which need write-enable code:\n\n");
for (i = 0; b[i].vendor_name != NULL; i++) {
printf("%s", b[i].vendor_name);for (j = 0; j < 25 - strlen(b[i].vendor_name); j++)printf(" ");printf("%s", b[i].board_name);for (j = 0; j < 25 - strlen(b[i].board_name); j++)printf(" ");if (b[i].lb_vendor != NULL)printf("(-m %s:%s)\n", b[i].lb_vendor, b[i].lb_part);elseprintf("(autodetected)\n");}
printf("\nSupported boards which don't need write-enable code:\n\n");
print_supported_boards_helper(boards_ok);
printf("\nBoards which have been verified to NOT work (yet):\n\n");
print_supported_boards_helper(boards_no);
printf("\nSee also: http://coreboot.org/Flashrom%5Cn");
}
In general, it seems you used #defines for strings quite heavily and sometimes the criteria for splitting a string into multiple strings (some of them as #define) is not clear to me. I'd feel better if these strigns were either placed inside the printfs as literal strings or at least declared const strings, removing the #defines.
Moving the code to wikify.c helped me a lot when reviewing the patch. It also allows other coders to ignore the wiki stuff without it ruining readability of the code in files outside wikify.c (yes, I consider wiki table code to be horrible). Thank you for keeping the other files clean!
I think the code is committable after one more iteration, but I still don't know whether to cry when I look at the URLs in arrays and would appreciate it a lot if you could kill the URLs from arrays outside wikify.c.
Regards, Carl-Daniel
-
Carl-Daniel Hailfinger -
Uwe Hermann