Re: [PATCH v3 2/5] ARM: dts: add reference voltage property for MXS LRADC

[Alexandre Belloni <alexandre.belloni@free-electrons.com>]

Hi Pawel,

On 14/08/2013 16:44, Pawel Moll wrote:
> On Tue, 2013-08-13 at 22:23 +0100, Jonathan Cameron wrote:
>> On 07/22/13 15:04, Hector Palacios wrote:
>>> Some LRADC channels have fixed pre-dividers so they can measure
>>> different voltages at full scale. The reference voltage allows to
>>> expose a scaling attribute through the IIO sysfs so that a user can
>>> compute the real voltage out of a measured sample value.
>>>
>>> diff --git a/Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt b/Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt
>>> index 4688205..6ec485c 100644
>>> --- a/Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt
>>> +++ b/Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt
>>> @@ -1,9 +1,12 @@
>>>  * Freescale i.MX28 LRADC device driver
>>>  
>>>  Required properties:
>>> -- compatible: Should be "fsl,imx28-lradc"
>>> +- compatible: "fsl,imx28-lradc", "fsl,imx23-lradc"
>>>  - reg: Address and length of the register set for the device
>>>  - interrupts: Should contain the LRADC interrupts
>>> +- fsl,vref: Reference voltage (in mV) for each LRADC channel. This is the
>>> +	    maximum voltage that can be measured at full scale in each channel
>>> +	    considering fixed pre-dividers.
> 
> So, let me try to rephrase what I read above.
> 
> There's an ADC with X channels. And there's a reference voltage source
> (one?). Now, each of the ADC channels have a (different?) voltage
> divider, taking the voltage from the reference source and feeding it to
> the ADC comparator. How much am I wrong?
> 

You are not so wrong. There is indeed actually only one reference
voltage (and that is 1.85V). But, before feeding the voltage to the ADC
channels, you sometimes have a divider. Then, after the channel muxing,
you can add a by 2 divider.

Mandatory ascii art:

            +-----+
            |     |
   +-ch1--->|     |
            |     |
            |     |
            |     |     +-----+
   +-ch2--->|     |     |     |
            | MUX |++-->| ADC +----------->
     ch3    |     | |   |     |
    +----+  |     | |   +-----+
    |    |  |     | |      |
  +-> :4 +->|     | |  +---+--+
    |    |  |     | |  |      |
    +----+  |     | +->|  :2  |
            +-----+    |      |
                       +------+


> If I'm not wrong at all, I'd say that the reference source could be
> described as a standard fixed regulator
> (Documentation/devicetree/bindings/regulator/fixed-regulator.txt) and
> the ADC node should have some king of "reference-supply" phandle to the
> regulator node. Now, if the dividers factors are *really* fixed, the
> driver could know about them and calculate the effective reference
> voltage on its own, couldn't it?
> 
> Let me repeat the "DT standard disclaimer": the tree, in general, should
> describe the way components are *wired up*, not much more.
> 

So, from my point of view, the divider that is before the mux (the by 4
divider on channel 3 on my drawing) is not part of the the ADC, it is
not fixed by that IP. And indeed, that changed between the i.mx23 and
i.mx28 while the IP is the same.

So, the two solutions you suggest are:
1/ using a fixed-regulator phandle per channel
2/ hard-coding the dividers in the driver using the compatible string to
know which divider is on which channel.

I feel that solution 2 is less future proof but at the same time, I
don't believe we will see that IP in another chip in the future.

Are my explanations clear enough to take a decision ?

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com