Re: [lm-sensors] [PATCH V1] fix adc to voltage calculation in da9052 power driver

[Jean Delvare]

Hi Anthony,

On Thu, 19 Dec 2013 14:13:09 +0000, Opensource [Anthony Olech] wrote:
> > -----Original Message-----
> > From: Guenter Roeck [mailto:groeck7@xxxxxxxxx] On Behalf Of Guenter
> > Roeck
> > Sent: 18 December 2013 17:24
> > To: Opensource [Anthony Olech]
> > Cc: Jean Delvare; open list:HARDWARE MONITORING; Anton Vorontsov;
> > David Woodhouse; open list; David Dajun Chen
> > Subject: Re: [lm-sensors] [PATCH V1] fix adc to voltage calculation in da9052
> > power driver
> > On Wed, Dec 18, 2013 at 03:45:32PM +0000, Opensource [Anthony Olech]
> > wrote:
> > > > -----Original Message-----
> > > > From: Jean Delvare [mailto:khali@xxxxxxxxxxxx]
> > > > Sent: 18 December 2013 15:33
> > > > To: Opensource [Anthony Olech]
> > > > Cc: Anton Vorontsov; David Woodhouse; David Dajun Chen; open list;
> > > > open list:HARDWARE MONITORING
> > > > Subject: Re: [lm-sensors] [PATCH V1] fix adc to voltage calculation
> > > > in da9052 power driver On Wed, 18 Dec 2013 15:21:13 +0000, Anthony
> > > > Olech wrote:
> > > > > The ADC resolution of the PMIC is 10-bits, this means that the
> > > > > maximum possible value is 1023 and not the 1024 as in the code.
> > > > The conversion from register value to mV depends on the ADC's LSB,
> > > > not its range. So the maximum value which can be represented is
> > irrelevant.
> > > thanks for the speedy response, but the converted value returned by
> > > the function does depend on the scaling.
> > > For example take the two cases where value in 0x1F0, then the
> > > erroneous previous calculation yields 3469 and the new corrected
> > > calculation yields 3470
> > > So please apply the patch as it truly fixes an error.
> > AFAICS the previous calculation should return 3468. Replacing the division by
> > 512 with DIV_ROUND_CLOSEST would return 3469, and the new calculation
> > would return 3470.
> > Still, an ADC would typically have an LSB. Question here is what that LSB is (in
> > mV), or in other words what the maximim voltage represented by 0x3ff is.
> > Current assumption is that it is 1.9531 mV or 2000/1024, which admittedly is
> > odd. It might be 2000/1023 = 1.9550 mV, or it might be 2 mV.
> 
> the maximum voltage represented by 0x3ff is 2000 mV, so the existing code is
> incorrect and the patch is to correct the error.

What makes you think so? The datasheet is not public so unfortunately I
cannot check it myself. Please quote the parts of the datasheet which
you think are relevant to convince us.

I and Guenter are only speaking by experience. Product briefs often
mention the ADC range as an informative value and this is not something
you can rely on. For example 8-bit ADC are often referred to as having
a 2 V, 3 V or 4 V range, while when you look at the datasheet, they
have a 8 mV, 12 mV or 16 mV LSB, which means their actual range is
2.048 V, 3.072 V and 4.096 V respectively.

Additionally, due to the limited resolution, each ADC value represents
a range of input values, rather than an input value. 0x00 doesn't mean
"0 mV", is typically means "less than whatever 1 LSB represents". Some
datasheets are very clear on that, see for example the ADT7490
datasheet, page 16, table 7, "10âBit ADC Output Code vs. V IN". This
means that what the drivers report (register value * LSB) is off by 0.5
LSB on average. All drivers do that and this is considered OK.

This also means that the top value of the range is never reported.
Consider an 8-bit ADC with a LSB of 16 mV, it's range is 4.096 V but
the top value (0xff) really means "4.080 V <= input value". Which
the driver will report as 4.080 V by convention.

BTW, you (and we) probably shouldn't waste too much energy on this.
ADCs are only accurate to some degree anyway. If you take the
components connected to the input into consideration, the accuracy gets
even worse. For example, if the input voltage must be scaled down to
fit in the ADC's range, you need at least one resistor, which in best
cases will have a 1% accurate value (known as tolerance.) That's ten
times the LSB of your 10-bit ADC, at which point / 1023 or / 1024
really makes no practical difference. Sub-percent tolerant resistors are
expensive and rare in consumer electronics in my experience.

-- 
Jean Delvare
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/