On Thu, 16 Oct 2008 13:50:26 -0700, Ira Snyder wrote: > On Thu, Oct 16, 2008 at 10:38:10PM +0200, Hans de Goede wrote: > > snip > > > > +/* Return the voltage from the given register in millivolts */ > > > +static u32 ltc4245_get_voltage(struct device *dev, u8 reg) > > > +{ > > > > Why dont you make this an s32 (or just an int) and then .. > > > > snip > > > > + case LTC4245_VEEIN: > > > + case LTC4245_VEEOUT: > > > + voltage = regval * 55; > > > > Make this * -55, and ... > > > > Great idea. I didn't even think of that. :) I'll post up a final patch > after I hear back on the stuff below. > > Is it ok to have a negative power? The current value gets used to > calculate the (virtual) power in ltc4245_show_power(), but that's it. I'd use the absolute value for power. A negative power sounds weird. -- Jean Delvare