Hello,
I am working on adding board_enable code for Artec Group DBE61 and DBE62 boards to utils/flashrom to set up everything fully for flash writing to work in all circumstances, but I've hit a problem in the infrastructure code for mainboard selection handling that I'd like to discuss on how to approach.
These boards are based on the AMD Geode line, so the PCI devices that exist for the outside are whatever the VSA exposes, which is typically the very same set of PCI devices that a big number of other Geode based products get exposed (save maybe network adapter and a few other things that can easily overlap with other boards). Therefore, it is undesirable to have the board PCI ID based auto-detection consider a board with Geode PCI devices a DBE61 without the user having said so specifically via -m artecgroup:dbe61 in scenarios when the ROM flash is empty or contains something else than a coreboot image with an appropriate name that would get auto-detected first.
However, the current infrastructure in board_enable.c doesn't allow this scenario, due to the combination of the following in the current code:
a) for a user specified board to be accepted, at least one set of IDs has to match to something that's on board (reasonable safeguard by itself); b) auto-detection is ran whenever a mainboard is not specified - the first board that has all non-NULL IDs existent is deemed the correct board even if the check would succeed for other boards in the list as well
So, as soon as I add a board entry to board_pciid_enables[] that describes one or two Geode VSA exposed PCI devices, I suspect all other Geode products will start getting auto-detected as the board I add, unless they happen to have a coreboot image with a name that is in the struct as well (lb_vendor/lb_name).
As I'm not sure how it's desired to handle this, I'm going to list the various approaches that I can think of, and will wait for comments and suggestions on the matter:
* Allow all NULL PCI IDs, making the board auto-detection skip such entries, and use it only when the coreboot name match succeeds or the user has specified the board with -m (--mainboard) argument. Pros: can leave such hardly identifiable (by PCI devices) out of the auto-detection logic Cons: No safeguarding at all
* Add a field to the struct (either treated as boolean, or a flag), that tells the logic if this entry should be considered in the auto-detection logic or skipped Pros: * can skip boards from PCI based auto-detection logic, requiring user specifying it; * has safeguards still in place (in this specific case will allow to claim it's a DBE61 only on Geode based products) Potential cons: extra struct field
* Make the PCI id based auto-detection logic run over all the struct entries, instead of going with the first that meets all checks. If multiple are found, report ambiguity and require a board name to be specified by the user. This can be considered independently of the above options. Pros: * Doesn't automatically pick the first suitable entry when there is an ambiguity, whereas the pick is made purely on internal struct entry order * Takes care of the possible wrong choice auto-detection in this specific case, as two boards (DBE61 and DBE62) would get added, triggering the ambiguity check. But doesn't help for future cases where two boards with the same IDs don't get introduced at once; also a user friendly ambiguity report would report only the listed entries with the matched IDs, while it could be a completely different board that just isn't listed yet
The patch I would like to see in SVN after this issue is taken care of is attached for reference. Without prior improvements to the general code in board_enable.c, applying this patch would make all Geode based products (that don't expose a separate PCI subsystem) be auto-detected as DBE61 (first in list) - if their ROM doesn't already have a coreboot image with a name that is listed in the struct. If this isn't a problem for the time being, then here's an addition to the commit log:
Signed-off-by: Mart Raudsepp mart.raudsepp@artecdesign.ee
Awaiting for comments and suggestions, Mart Raudsepp, Software Engineer, Artec Design LLC