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 <[email protected]> > > 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 <[email protected]> > + * > + * 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'}, > {"programmer", 1, 0, 'p'}, > {"help", 0, 0, 'h'}, > {"version", 0, 0, 'R'}, > @@ -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; > case 'p': > if (strncmp(optarg, "internal", 8) == 0) { > programmer = PROGRAMMER_INTERNAL; > @@ -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&fMTYPE=Socket%20939&pMODEL_NAME=AX8"; > }, > + { "Advantech", "PCM-5820", > "http://taiwan.advantech.com.tw/products/Model_Detail.asp?model_id=1-1TGZL8&BU=ACG&PD="; > }, > + { "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&model=455&modelmenu=2"; > }, > + { "ASUS", "A8NE-FM/S", > "http://www.hardwareschotte.de/hardware/preise/proid_1266090/preis_ASUS+A8NE-FM"; > }, > + { "ASUS", "A8N-SLI Premium", > "http://www.asus.com.tw/products.aspx?l1=3&l2=15&l3=148&l4=0&model=539&modelmenu=1"; > }, > + { "ASUS", "A8V-E Deluxe", > "http://www.asus.com.tw/products.aspx?l1=3&l2=15&l3=143&l4=0&model=376&modelmenu=1"; > }, > + { "ASUS", "M2A-VM", > "http://www.asus.com.tw/products.aspx?l1=3&l2=101&l3=496&l4=0&model=1568&modelmenu=1"; > }, > + { "ASUS", "M2N-E", > "http://www.asus.com/products.aspx?l1=3&l2=101&l3=308&l4=0&model=1181&modelmenu=1"; > }, > + { "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&model=228&modelmenu=1"; > }, > + { "ASUS", "A7V8X-MX", > "http://www.asus.com.tw/products.aspx?l1=3&l2=13&l3=64&l4=0&model=229&modelmenu=1"; > }, > + { "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&model=576&modelmenu=1"; > }, > + { "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&model=1909&modelmenu=1"; > }, > + { "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?ClassValue=Motherboard&ProductID=1445&ProductName=GA-6BXC"; > }, > + { "GIGABYTE", "GA-6BXDU", > "http://www.gigabyte.com.tw/Products/Motherboard/Products_Spec.aspx?ProductID=1429"; > }, > + { "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_no=&cat3_no=&prod_no=1173#menu"; > }, > + { "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=81"; > }, > + { "VIA", "EPIA-MII", > "http://www.via.com.tw/en/products/mainboards/motherboards.jsp?motherboard_id=202"; > }, > + { "VIA", "EPIA-CN", > "http://www.via.com.tw/en/products/mainboards/motherboards.jsp?motherboard_id=400"; > }, > + { "VIA", "EPIA-LN", > "http://www.via.com.tw/en/products/mainboards/motherboards.jsp?motherboard_id=473"; > }, > + { "VIA", "VB700X", > "http://www.via.com.tw/en/products/mainboards/motherboards.jsp?motherboard_id=490"; > }, > + { "VIA", "NAB74X0", > "http://www.via.com.tw/en/products/mainboards/motherboards.jsp?motherboard_id=590"; > }, > + { "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&model=217&modelmenu=1"; > }, > + { "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_no=1"; }, > + { "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); > + else > + printf("(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\n";); > } > > > 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 -- http://www.hailfinger.org/ -- coreboot mailing list: [email protected] http://www.coreboot.org/mailman/listinfo/coreboot
