[LinuxBIOS] r2670 - in trunk/LinuxBIOSv2/src/mainboard: asus/p2b bitworks/ims densitron/dpx114 via/epia via/epia-m

[Stefan Reinauer]

* Uwe Hermann <uwe at hermann-uwe.de> [070515 17:41]:
> On Tue, May 15, 2007 at 02:26:15PM +0200, Stefan Reinauer wrote:
> > >  static inline int spd_read_byte(unsigned device, unsigned address)
> > >  {
> > > -	uint8_t c;
> > > -	c = smbus_read_byte(device, address);
> > > -	return c;
> > > +	return smbus_read_byte(device, address);
> > >  }
> > 
> > Did you compare the assembler code produced by romcc, verifying that
> > this has no bad impact?
> 
> Nope, but the 'return smbus_read_byte(device, address)' variant is
> already used in many places in svn, so I'm somewhat confident it works as
> expected.
> 
> If it would _not_ work then there should have been a big fat warning in
> the code that this is done deliberately to workaround romcc issues.

Disagree. We can not expect all code to be 100% correctly commented
(yet, of course. this will all change ;-) 
Please check whether this does not have a bad impact. It might
not be a workaround to a bug but just a way to make romcc safe
registers, in which case you might hit the problem later 

It is natural that you can expect the code being like this and not
different because doing things differently would break the code.

This stuff has been changed in one or the other direction without any 
logic (from a pure C perspective). Like using "unsigned" instead of
"unsigned int" or "unsigned char" would create better code in romcc.

I doubt it ever did, but it was a lot of noise in the code, and I think
noise should be good for something (fix an issue)

The above code is not necessarily an equivalent conversion of the code.
Which is why I am reacting very careful here.

-- 
coresystems GmbH • Brahmsstr. 16 • D-79104 Freiburg i. Br.
      Tel.: +49 761 7668825 • Fax: +49 761 7664613
Email: info at coresystems.de  • http://www.coresystems.de/