[flashrom] DMI matching patch

[Luc Verhaegen]

On Mon, Jan 04, 2010 at 03:15:11PM +0100, Michael Karcher wrote:
> Hello Carl-Daniel,
> 
> thanks for your review.
> 
> There seems to be a fundamental opinion difference what DMI is used for
> between you on the one side and me and Luc on the other side. You seem
> to want to use DMI for a good board identification (which is fair
> enough, I also started with that intention), whereas now we decided to
> use DMI only as an emergency helper in case the PCI IDs seem obviously
> not good enough, which should be the exception instead of the regular
> case.
> 
> Using DMI as emergency rescue has the advantage that it still works on
> most systems even if DMI is unavailable (boards having DMI info can not
> be matched by PCI IDs with the matching policy of this patch).
> Collecting DMI info on boards with good PCI subsystem IDs (like the MSI
> boards that have the internal 4-digit product number BCD-coded in the
> subsystem IDs) is not useful, in my opinion.
> 
> The usage of DMI as breaker explains the loose matching policy of "any
> DMI string matches", as the IDs that should go into the table should be
> unique enough even with this weak policy (I for example can't imagine
> any HP system, as the manufacturer has already been established by
> subsystem manufacturer IDs, where "^VL420$" is going to match in the
> wrong string). If you want to use DMI as general board identification
> mechanism, independent of the PCI IDs, one would of course have to check
> more than one field.

Agreed.
 
> > I'm not so happy about the static allocation here. Can't we use malloc()
> > for the read buffer and strdup() for filling in dmistrings?
>
> Of course we can. On the other hand, I don't really see the point in
> using dynamic allocation here. If it's about wasted memory, we should
> approach "struct flashchips" first. The fixed-size arrays in it waste a
> lot more. Nevertheless, I change it to dynamic allocation. Too bad the
> nice getline function that reads one line and allocates dynamically is
> GNU-only and not standard C or at least POSIX.

This is a run-once tool, the results are not stored, and we do not go 
and run dmidecode several times. Everything filling up any memory should 
be ran once.
 
> Fair enough. I added it. Specifying one string to match is not
> increasing complexity much, although it will be a three-character hit on
> the board_enable table (if you specify like "bp:^A8V MX$" for baseboard
> product match for example). Luc, please NACK if you think the it is too
> much.

That's way too contrived for my taste. If you really believe it is 
necessary, then you should provide an enum entry right in front of the 
string.

But then the board enable table is taking another big hit, and this i 
would like to avoid at all cost. We already have pci ids and pci 
subsystem ids, this is just an extra bit of info that will be the last 
in precedence for any match.

> As written in the introduction text, this is something I would like to
> avoid. You have a point in "it's nice if it's always there", but this
> makes the "we have DMI because it's nice" and "we *need* to match the
> DMI info to be sure we have the right board" indistinguishable. I
> understand your point of tighter matches being better, but I don't think
> that's worth a hard dependency on dmidecode if we get along nicely
> without.

I agree, this is a lowlevel tool and the less we depend on externals the 
better. That being said, if we have any board that requires dmidecode to 
be able to make a succesfull match, we already depend fully on 
dmidecode.

Luc Verhaegen.