Hello!
2010/5/26 Michael Karcher flashrom@mkarcher.dialup.fu-berlin.de:
I fully appreciate the idea of merging the tables from print.c and print_wiki.c . Do you know http://patchwork.coreboot.org/patch/1044/ which I submitted with basically the same idea?
Unfortunately I wasn't aware of your patch - sorry for stealing your idea. That was unintentional.
Is it *really* necessary to put this data table into a header file? It means it will be included *twice* if you want print_wiki support.
Moved all data to board_enable.c to avoid duplicating.
Another change to current code is that you code will unconditionally include all the URL strings into the executable, even if wiki printing is not enabled, without ever using them.
Fixed. I used the idea from your patch, mentioned above.
For the "working" field, please use OK/NT instead of 1/0 to be consistent with other parts of flashrom.
Done.
- if(boards[i].write_enable){
You can omit storing the "write enable" flag by just traversing the board_pciid_enable table for each board, and print " -" if the board is not listed in the table. This circumvents another source of possible inconsistencies.
Fixed it too.
- for (j = 0; b[j].vendor_name != NULL; j++) {
- if ((strcmp(b[j].vendor_name, boards[i].vendor) == 0) && (strcmp(b[j].board_name, boards[i].name) == 0)){
- if (b[j].lb_vendor != NULL)
- printf("-m %s:%s", b[j].lb_vendor, b[j].lb_part);
- else
- printf("(none, board is autodetected)");
Whether you *need* an option for the board does not depend on whether a coreboot ID for that board is present, but on the set of PCI IDs in the board enable table, read the comment in board_enable.c, please
I don't fully understand - the idea of this particular chunk of code is just to know whether board_enable is required by using its name, and not by its pciids (we're just traversing board's names list). After finding that the mobo does require board_enable (by looking at the vendor_name and board_name fiends in board_pciid_enables), I'm trying to find whether autodetection is possible (empty coreboot ids) and if it's not possible, then I construct appropriate command-line switch for that board (from coreboot ids). Perhaps I'm missing something here, but I was sure that I don't need to look at the other fields of board_pciid_enables except mentioned above.
Anyway, here is a new patch attached (I completely get rid of board.h introduced in previous version of patch).