[coreboot] patch: fix USB ports on DBE62, and other cs5536-based platforms

Jordan Crouse jordan.crouse at amd.com
Wed Jun 4 21:31:27 CEST 2008

On 04/06/08 20:30 +0200, Stefan Reinauer wrote:
> ron minnich wrote:
> > On Wed, Jun 4, 2008 at 11:10 AM, Stefan Reinauer <stepan at coresystems.de> wrote:
> >   
> >> I'm not against using structs, in fact maybe we should just do it, but
> >> they're not the optimal solution either. The guy who invented struct was
> >> not a firmware developer
> >
> > you gotta trust me on this, but, back then, writing an OS was not that
> > much different. In fact Unix V6 and coreboot are just about the same
> > size ...
> >   
> So are you saying we should make this part of our coding guidelines?


Sometimes, there are good reasons for practices to fall out of 
use.  Using a struct for mmio is not in common usage today for a
variety of reasons (see Linux kernel, Xorg, various RTOSen both
closed and opened).

That said, we shouldn't pick our guidelines based on tradition or what
Linus happens to be doing today, we should pick them based on common sense
and what works for our project.

Common sense dictates to me that it is far easier to keep a datasheet
synced with a list of #defines then a structure.  Verifying that you
have the right offsets by counting member sizes in the structure is
a recipe for disaster.  Not all MMIO spaces are sequential - sometimes the
register offsets are bunched together on 0x100 or 0x1000 boundaries -
take the Geode LX display filter for example - the regular display
registers are from offsets 0x00 - 0x120, but the panel control registers
start at 0x400. There is no clean way to map that in a structure without
causing some WTF sirens to go off.

Let us allow everybody to use what they want and try not to legislate
ourselves too badly.  And for the love of crumbcake, please start using
void * (or failing that u8 *) for MMIO pointers!  :)


Jordan Crouse
Systems Software Development Engineer 
Advanced Micro Devices, Inc.

More information about the coreboot mailing list