[coreboot] Intel: How to share `tsc_freq.c` to select `UDELAY_TSC`

[ron minnich]

On Wed, May 8, 2013 at 1:43 PM, Peter Stuge <peter at stuge.se> wrote:
> ron minnich wrote:
>> Given that the platforms work, and most of them are obsolete, it's
>> hard to see the virtue in this change.
>
> I disagree.

Are you going to test every one of the 19 platforms that we're talking
about changing? Code sharing is wonderful, but if you change something
this crucial you really need a way to test it. I've seen two proposals
from paul today and both proposals had subtle errors. I've been in the
situation of fixing firmware source that was "improved" 2 years after
the fact and it's not that much fun.

So sure, it's great to reduce code size and unify things. But you'd
better have a way to test it. And if you can't test it, you need to
think hard about what you're doing.

> Code reuse where reuse makes sense is one of several things that
> separate coreboot from commercial firmware products, and a very
> worthy goal indeed.

Well, yes, and I had something to do with that, you may recall :-)

I'm not arguing against reuse. But there are ways and there are ways.

One way is to unify new platforms going forward, and not touch old
ones that can't be tested.

But I've had people break things before while 'cleaning it up' and
it's not fun. The hardware bugs you deal with in firmware are far, far
harder to work out than the stuff you run into at the kernel level,
and it's best to keep that in mind.

ron