[coreboot] PCI register read/mod/write code

Arne Georg Gleditsch arne.gleditsch at numascale.com
Thu Oct 7 09:55:54 CEST 2010


Corey Osgood <corey.osgood at gmail.com> writes:
> I'm fully in agreement with Ron here. Lets fix whatever's broken so we
> can let the compiler tell us when we make foolish mistakes, rather
> then writing some mess that's sure to cause some WTFs down the road.
> This is *one* instance in which someone used an 8-bit instead of a
> 16-bit function by mistake. Lets not go nuts trying to idiot-proof
> things. Instead of hacking away at all the pci functions, and almost
> definitely causing some unintended breakage, can't we just pay a
> little more attention during reviews?

Personally, I wouldn't implement this primarily to catch this particular
foolish mistake.  I think this makes sense on its own as a useful
abstraction of a very common pattern that contains more redundancy than
needed.  The intent of the code becomes clearer and the signal-to-noise
ratio increases.

(Also, this has already snuck in, see for instance set_nbcfg_enable_bits
in rs780_cmn.c:
http://lxr.linux.no/#coreboot+r5917/src/southbridge/amd/rs780/rs780_cmn.c#L127
-- which I think is confusingly named, but which provides exactly this
abstraction.)

-- 
							Arne.




More information about the coreboot mailing list