[coreboot] [PATCH] Factor out common CAR asm snippets

[Uwe Hermann]

On Sat, Oct 02, 2010 at 12:42:51PM -0700, ron minnich wrote:
> On Sat, Oct 2, 2010 at 12:27 PM, Warren Turkal <wt at penguintechs.org> wrote:
> > I think that making arguments against this code on grounds that it
> > doesn't feel like other common code is a little shortsighted. When
> > bugs are fixed in one bit of code, we should be able to take advantage
> > of those in other parts of code. For instance, I just had to
> > copy/paste some CPP logic to fix the via/vt8454c CAR init due to a
> > tiny bootblock. That would have already been fixed had the via and amd
> > logic been unified since they are basically identical bits of code.
> 
> 
> well, you have to be careful. We've had more than one problem with
> "common" code that was not so common. CAR is so tricky, and one
> platform changes may break another platform in ways we don't expect.
> CAR, in fact, scares me.

Yes, it scares me too (well, sort of :) It's one of the most complex and
non-obvious pieces of code we have in coreboot. Which makes it even more
important to make this code as clean and easily readable and
understandable as we can. Every little bit helps here, e.g. not open-coding
various asm snippets (in 3 or more slightly different variants) all over
the place is one very good measure to make it less confusing for people
trying to read the code. Less lines in cache_as_ram.inc is good.
Readable macros such as "enable_l2_cache" instead of some open-coded,
harder to understand assembler instructions is good.

High-level "look, here we enable cache"-style code is much better than
"look, here we set bit 2 in CR0, and clear bits 5-6 in some MSR and
write magic value 456 to magic address 123, now please go find the right
datasheet and look up what it actually does".

Of course I'm exagerrating a bit here, but I'm very convinced that
readability is of utmost importance in coreboot code, especially so
in CAR code.


> about the idea of unifying via and amd code.

Yeah, that should be considered very carefully indeed, and only be done
after sufficient testing on hardware (if it's feasible and makes sense
to do it al all, i.e., if the two implementations are >= 90% similar).

But the general idea of unifying similar code (especially if it stems
from copy-pasted code from elsewhere with only minimal customizations)
is very good and very necessary indeed. Yeah, things may break, that's
life. Someone will test it, we'll fix it, problem solved. No reason
to _not_ unify code which can be reasonably unified.

I'm actually planning to unify model_6ex, model_6fx, and model_106cx
CAR implementations myself, but that can be done relatively easily,
as the diff between all three files is just 2-3 simple lines.


Uwe.
-- 
http://hermann-uwe.de     | http://sigrok.org
http://randomprojects.org | http://unmaintained-free-software.org