[coreboot] flashrom: Handling of mainboard selection

[Mart Raudsepp]

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 at artecdesign.ee>


Awaiting for comments and suggestions,
Mart Raudsepp,
Software Engineer,
Artec Design LLC
-------------- next part --------------
A non-text attachment was scrubbed...
Name: flashrom-dbe61+dbe62.patch
Type: text/x-patch
Size: 2883 bytes
Desc: not available
URL: <http://www.coreboot.org/pipermail/coreboot/attachments/20080208/dc329002/attachment.patch>