Am 03.08.2011 13:40 schrieb Stefan Tauner:
On Wed, 03 Aug 2011 09:02:52 +0200 Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net wrote:
Suggestion: struct board_pciid_enable -> struct board_match board_match_coreboot_name() -> board_match_cbname()
What do you think?
looks fine imo. board_match_pci_card_ids could also be renamed to board_match_pci_ids and if you are at it please change the comment of it:
Done.
/*
- Match boards on PCI IDs and subsystem IDs.
- Second set of IDs can be main only or missing completely.
*/ const static struct board_pciid_enable *board_match_pci_card_ids(enum board_match_phase phase)
- Second set of IDs can be main only or missing completely.
- Second set of IDs can contain primary IDs only or be missing completely.
I tried to find an alternate wording.
aaaand if we change board_match_pci_card_ids we should also change pci_card_find to pci_dev_find or something like that... :)
Well, if we rename that one, we'd have to call it pci_dev_subsys_find, and that's a net loss from the 80 column perspective, but it may indeed clarify the code. Further input is appreciated.
Shorten some board enable related function names
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Index: flashrom-cosmetic_shorten_boardenable_function_names/print_wiki.c =================================================================== --- flashrom-cosmetic_shorten_boardenable_function_names/print_wiki.c (Revision 1413) +++ flashrom-cosmetic_shorten_boardenable_function_names/print_wiki.c (Arbeitskopie) @@ -125,7 +125,7 @@ int num_notes = 0; char *notes = calloc(1, 1); char tmp[900 + 1]; - const struct board_pciid_enable *b = board_pciid_enables; + const struct board_match *b = board_matches;
for (i = 0; boards[i].vendor != NULL; i++) { if (boards[i].working) Index: flashrom-cosmetic_shorten_boardenable_function_names/flashrom.c =================================================================== --- flashrom-cosmetic_shorten_boardenable_function_names/flashrom.c (Revision 1413) +++ flashrom-cosmetic_shorten_boardenable_function_names/flashrom.c (Arbeitskopie) @@ -1731,7 +1731,7 @@ msg_gerr("Chipset enables table does not exist!\n"); ret = 1; } - if (board_pciid_enables == NULL) { + if (board_matches == NULL) { msg_gerr("Board enables table does not exist!\n"); ret = 1; } Index: flashrom-cosmetic_shorten_boardenable_function_names/programmer.h =================================================================== --- flashrom-cosmetic_shorten_boardenable_function_names/programmer.h (Revision 1413) +++ flashrom-cosmetic_shorten_boardenable_function_names/programmer.h (Arbeitskopie) @@ -158,7 +158,7 @@ P3 };
-struct board_pciid_enable { +struct board_match { /* Any device, but make it sensible, like the ISA bridge. */ uint16_t first_vendor; uint16_t first_device; @@ -190,7 +190,7 @@ int (*enable) (void); /* May be NULL. */ };
-extern const struct board_pciid_enable board_pciid_enables[]; +extern const struct board_match board_matches[];
struct board_info { const char *vendor; Index: flashrom-cosmetic_shorten_boardenable_function_names/print.c =================================================================== --- flashrom-cosmetic_shorten_boardenable_function_names/print.c (Revision 1413) +++ flashrom-cosmetic_shorten_boardenable_function_names/print.c (Arbeitskopie) @@ -204,7 +204,7 @@ const char *devicetype) { int i, boardcount_good = 0, boardcount_bad = 0; - const struct board_pciid_enable *e = board_pciid_enables; + const struct board_match *e = board_matches; const struct board_info *b = boards; int maxvendorlen = strlen("Vendor") + 1; int maxboardlen = strlen("Board") + 1; @@ -242,7 +242,7 @@ msg_ginfo(" "); msg_ginfo((b->working) ? "OK " : "BAD ");
- for (e = board_pciid_enables; e->vendor_name != NULL; e++) { + for (e = board_matches; e->vendor_name != NULL; e++) { if (strcmp(e->vendor_name, b->vendor) || strcmp(e->board_name, b->name)) continue; Index: flashrom-cosmetic_shorten_boardenable_function_names/board_enable.c =================================================================== --- flashrom-cosmetic_shorten_boardenable_function_names/board_enable.c (Revision 1413) +++ flashrom-cosmetic_shorten_boardenable_function_names/board_enable.c (Arbeitskopie) @@ -1971,7 +1971,7 @@ */
/* Please keep this list alphabetically ordered by vendor/board name. */ -const struct board_pciid_enable board_pciid_enables[] = { +const struct board_match board_matches[] = {
/* first pci-id set [4], second pci-id set [4], dmi identifier, coreboot id [2], phase, vendor name, board name max_rom_... OK? flash enable */ #if defined(__i386__) || defined(__x86_64__) @@ -2097,11 +2097,11 @@ * Match boards on coreboot table gathered vendor and part name. * Require main PCI IDs to match too as extra safety. */ -static const struct board_pciid_enable *board_match_coreboot_name( - const char *vendor, const char *part) +static const struct board_match *board_match_cbname(const char *vendor, + const char *part) { - const struct board_pciid_enable *board = board_pciid_enables; - const struct board_pciid_enable *partmatch = NULL; + const struct board_match *board = board_matches; + const struct board_match *partmatch = NULL;
for (; board->vendor_name; board++) { if (vendor && (!board->lb_vendor @@ -2148,12 +2148,11 @@
/* * Match boards on PCI IDs and subsystem IDs. - * Second set of IDs can be main only or missing completely. + * Second set of IDs can be either main+subsystem IDs, main IDs or no IDs. */ -const static struct board_pciid_enable *board_match_pci_card_ids( - enum board_match_phase phase) +const static struct board_match *board_match_pci_ids(enum board_match_phase phase) { - const struct board_pciid_enable *board = board_pciid_enables; + const struct board_match *board = board_matches;
for (; board->vendor_name; board++) { if ((!board->first_card_vendor || !board->first_card_device) && @@ -2199,7 +2198,7 @@ return NULL; }
-static int unsafe_board_handler(const struct board_pciid_enable *board) +static int unsafe_board_handler(const struct board_match *board) { if (!board) return 1; @@ -2226,9 +2225,9 @@ /* FIXME: Should this be identical to board_flash_enable? */ static int board_handle_phase(enum board_match_phase phase) { - const struct board_pciid_enable *board = NULL; + const struct board_match *board = NULL;
- board = board_match_pci_card_ids(phase); + board = board_match_pci_ids(phase);
if (unsafe_board_handler(board)) board = NULL; @@ -2257,14 +2256,14 @@
int board_flash_enable(const char *vendor, const char *part) { - const struct board_pciid_enable *board = NULL; + const struct board_match *board = NULL; int ret = 0;
if (part) - board = board_match_coreboot_name(vendor, part); + board = board_match_cbname(vendor, part);
if (!board) - board = board_match_pci_card_ids(P3); + board = board_match_pci_ids(P3);
if (unsafe_board_handler(board)) board = NULL;