On Tue, 16 Aug 2011 13:38:05 +0200 Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net wrote:
Am 15.08.2011 23:42 schrieb Stefan Tauner:
On Mon, 15 Aug 2011 22:40:58 +0200 Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net wrote:
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.
oh my. now that i see the code, i think renaming the board enables to board_match is really a bad idea. mainly because we use "match" as verb in other places, but here as substantive. e.g. static const struct board_match *board_match_cbname a function that matches boards according to their cbname should return a board, not a "board match". it is just a question of taste, but i find it awful :)
It's a trap! The struct is there to match a board, so struct board_match is exactly the right name for the _type_.
if that would be its only purpose, it would contain identification attributes only. last time i looked it had general info about boards (vendor and board name) and board enable specific fields (phase, max_rom etc). its previous name also suggests it is not just for matching. ;)
The name of the function is another thing, you could probably call it match_board_match_cbname, but that would be silly, so we can either call it match_board_cbname or board_match_cbname. And the name of the variable which stores the result of the function call is even another thing, and there the name board might be appropriate.
the wiki table is named board_info hmhm maybe board_detail, but that's long.. :/ what about just "board"? another way to mitigate "my" problem would be to no use match as a verb for the method names, but using "get" or "find" instead.
"get" has the wrong semantic implication for the cbname matching, and "find" as in board_find_cbname suffers from the same problem because we're NOT interested in finding the cbname of the board, but rather in looking for a board which matches the supplied cbname.
i think the reason for our difference of opinion is how we view the table. for me it is a kind of database or container class. and we want to query it for objects with certain properties.
in sane languages (supporting polymorphy) you could write that as <container variable>.find_board(cbname). of course "match" would work too, but as explained above i would like to use different terms just to have a (mental) distinction.
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.
why do we have to? we find pci devs, not pci dev subsystems ;) hm maybe it should be find_pci_dev?
pci_dev_find() looks for a device matching the supplied vendor+dev IDs. pci_card_find() looks for a device matching the supplied vendor+dev+subvendor+subdev IDs.
The name "card" was picked to allow differentiating between PCI devices (cards) which share vendor+dev ID, but have different subvendor+subdev IDs. That's pretty common for network cards where dozens of vendors use the same network chip (and thus fixed vendor+dev ID), but completely different PCBs.
hm, ok
well... it is certainly an improvement over the current state and this discussion wont lead to something more sane i feel, so think of it as Acked-by: Stefan Tauner stefan.tauner@student.tuwien.ac.at