[PATCH] iio: adc: Add bindigs documentation for twl6030 GPADC

[PATCH] iio: adc: Add bindigs documentation for twl6030 GPADC

[Oleksandr Kozaruk]

Add required documentation for twl6030 GPADC device tree
bindings.

Signed-off-by: Oleksandr Kozaruk <oleksandr.kozaruk@ti.com>
---
 .../devicetree/bindings/iio/adc/twl6030-gpadc.txt  | 45 ++++++++++++++++++++++
 1 file changed, 45 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/twl6030-gpadc.txt

diff --git a/Documentation/devicetree/bindings/iio/adc/twl6030-gpadc.txt b/Documentation/devicetree/bindings/iio/adc/twl6030-gpadc.txt
new file mode 100644
index 0000000..6cd3ef3
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/twl6030-gpadc.txt
@@ -0,0 +1,45 @@
+Texas Instruments twl6030/twl6032 GPADC device driver
+
+Required properties:
+	- compatible: must be "ti,twl6030-gpadc" for TWL6030 or
+	  "ti,twl6032-gpadc" for TWL6032
+	- interrupts: interrupt number associated with it
+	- #io-channel-cells: must be <1> - multiple IIO outputs are present
+	  iio consumers can use following io-channels:
+		twl6030:
+			0 - battery type
+			1 - battery temperature resistor value
+			2 - audio accessory/general purpose
+			3 - general purpose
+			4 - temperature/general purpose
+			5 - general purpose
+			6 - general purpose
+			7 - main battery
+			8 - backup battery
+			9 - charger input
+			10 - VBUS
+			11 - VBUS charging current
+			14 - USB ID
+		twl6032:
+			0 - battery type
+			1 - battery temperature resistor value
+			2 - audio accessory/general purpose
+			3 - temperature with external diode/general purpose
+			4 - temperature/general purpose
+			5 - general purpose
+			6 - general purpose
+			7 - system supply
+			8 - backup battery
+			9 - charger input
+			10 - VBUS
+			11 - VBUS charging current
+			14 - USB ID
+			17 - battery charging current
+			18 - battery voltage
+
+Example:
+	adc {
+		compatible = "ti,twl6030-gpadc";
+		interrupts = <3>;
+		#io-channel-cells = <1>;
+	};
-- 
1.8.1.2

Re: [PATCH] iio: adc: Add bindigs documentation for twl6030 GPADC

[Mark Rutland]

Hi Oleksandr,

[Adding Jonathan Cameron and Guenter Roeck to Cc]

Apologies for the delay replying to this. In attempting to verify this
made sense I went and read the IIO bindings documentation, and I'm
somewhat confused by the model.

As far as I can see, the only consumer of IIO channels is the
"iio-hwmon" binding, which seems to be a binding for Linux-specific
infrastructure rather than any actual device. This runs counter to the
way DT is supposed to function (describing the hardware rather than how
it's used). As far as I can see, this linkage is described because only
a subset of the ADCs on the device are actually wired to something?

I also see a couple of IIO bindings ("adi,adf435x*, and "adi,ad7303")
which don't describe any iio channel cells at all, so I'm somewhat
confused by what the IIO channels actually represent, and why they must
be consumed elsewhere. As far as I can see, an IIO channel represents a
single ADC's registers in an IIO device, so I'm not sure why this must
be exported via the channel concept -- it's not physically wired.

Have I misunderstood something here?

Thanks,
Mark.

On Mon, Aug 19, 2013 at 12:29:25PM +0100, Oleksandr Kozaruk wrote:
> Add required documentation for twl6030 GPADC device tree
> bindings.
> 
> Signed-off-by: Oleksandr Kozaruk <oleksandr.kozaruk@ti.com>
> ---
>  .../devicetree/bindings/iio/adc/twl6030-gpadc.txt  | 45 ++++++++++++++++++++++
>  1 file changed, 45 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/twl6030-gpadc.txt
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/twl6030-gpadc.txt b/Documentation/devicetree/bindings/iio/adc/twl6030-gpadc.txt
> new file mode 100644
> index 0000000..6cd3ef3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/twl6030-gpadc.txt
> @@ -0,0 +1,45 @@
> +Texas Instruments twl6030/twl6032 GPADC device driver
> +
> +Required properties:
> +	- compatible: must be "ti,twl6030-gpadc" for TWL6030 or
> +	  "ti,twl6032-gpadc" for TWL6032
> +	- interrupts: interrupt number associated with it
> +	- #io-channel-cells: must be <1> - multiple IIO outputs are present
> +	  iio consumers can use following io-channels:
> +		twl6030:
> +			0 - battery type
> +			1 - battery temperature resistor value
> +			2 - audio accessory/general purpose
> +			3 - general purpose
> +			4 - temperature/general purpose
> +			5 - general purpose
> +			6 - general purpose
> +			7 - main battery
> +			8 - backup battery
> +			9 - charger input
> +			10 - VBUS
> +			11 - VBUS charging current
> +			14 - USB ID
> +		twl6032:
> +			0 - battery type
> +			1 - battery temperature resistor value
> +			2 - audio accessory/general purpose
> +			3 - temperature with external diode/general purpose
> +			4 - temperature/general purpose
> +			5 - general purpose
> +			6 - general purpose
> +			7 - system supply
> +			8 - backup battery
> +			9 - charger input
> +			10 - VBUS
> +			11 - VBUS charging current
> +			14 - USB ID
> +			17 - battery charging current
> +			18 - battery voltage
> +
> +Example:
> +	adc {
> +		compatible = "ti,twl6030-gpadc";
> +		interrupts = <3>;
> +		#io-channel-cells = <1>;
> +	};
> -- 
> 1.8.1.2
> 
> 

Re: [PATCH] iio: adc: Add bindigs documentation for twl6030 GPADC

[Oleksandr Kozaruk]

Hi Mark,

On Tue, Aug 20, 2013 at 12:12 PM, Mark Rutland <mark.rutland@arm.com> wrote:
> Hi Oleksandr,
>
> [Adding Jonathan Cameron and Guenter Roeck to Cc]
>
> Apologies for the delay replying to this. In attempting to verify this
> made sense I went and read the IIO bindings documentation, and I'm
> somewhat confused by the model.
>
> As far as I can see, the only consumer of IIO channels is the
> "iio-hwmon" binding, which seems to be a binding for Linux-specific
> infrastructure rather than any actual device. This runs counter to the
> way DT is supposed to function (describing the hardware rather than how
> it's used). As far as I can see, this linkage is described because only
> a subset of the ADCs on the device are actually wired to something?
>
> I also see a couple of IIO bindings ("adi,adf435x*, and "adi,ad7303")
> which don't describe any iio channel cells at all, so I'm somewhat
> confused by what the IIO channels actually represent, and why they must
> be consumed elsewhere. As far as I can see, an IIO channel represents a
> single ADC's registers in an IIO device, so I'm not sure why this must
> be exported via the channel concept -- it's not physically wired.

The GPADC was used by battery driver and thermal subsystem.
In our local pre-DT branch battery driver reads channels 1 (battery
temperature),
7 (battery voltage), and 8 (battery backup voltage)

I'm guessing a consumer would be in some_board.dts, and describe it as
    battery_consumer {
            ....
            io-channels = <&adc 1>, <&adc 7>, <&adc 8>;
            ...
    }

The purpose of the channels is not arbitrary but dedicated to certain functions
in twl6030.

Regards,
Sasha.

>
> Have I misunderstood something here?
>
> Thanks,
> Mark.
>
> On Mon, Aug 19, 2013 at 12:29:25PM +0100, Oleksandr Kozaruk wrote:
>> Add required documentation for twl6030 GPADC device tree
>> bindings.
>>
>> Signed-off-by: Oleksandr Kozaruk <oleksandr.kozaruk@ti.com>
>> ---
>>  .../devicetree/bindings/iio/adc/twl6030-gpadc.txt  | 45 ++++++++++++++++++++++
>>  1 file changed, 45 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/iio/adc/twl6030-gpadc.txt
>>
>> diff --git a/Documentation/devicetree/bindings/iio/adc/twl6030-gpadc.txt b/Documentation/devicetree/bindings/iio/adc/twl6030-gpadc.txt
>> new file mode 100644
>> index 0000000..6cd3ef3
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/iio/adc/twl6030-gpadc.txt
>> @@ -0,0 +1,45 @@
>> +Texas Instruments twl6030/twl6032 GPADC device driver
>> +
>> +Required properties:
>> +     - compatible: must be "ti,twl6030-gpadc" for TWL6030 or
>> +       "ti,twl6032-gpadc" for TWL6032
>> +     - interrupts: interrupt number associated with it
>> +     - #io-channel-cells: must be <1> - multiple IIO outputs are present
>> +       iio consumers can use following io-channels:
>> +             twl6030:
>> +                     0 - battery type
>> +                     1 - battery temperature resistor value
>> +                     2 - audio accessory/general purpose
>> +                     3 - general purpose
>> +                     4 - temperature/general purpose
>> +                     5 - general purpose
>> +                     6 - general purpose
>> +                     7 - main battery
>> +                     8 - backup battery
>> +                     9 - charger input
>> +                     10 - VBUS
>> +                     11 - VBUS charging current
>> +                     14 - USB ID
>> +             twl6032:
>> +                     0 - battery type
>> +                     1 - battery temperature resistor value
>> +                     2 - audio accessory/general purpose
>> +                     3 - temperature with external diode/general purpose
>> +                     4 - temperature/general purpose
>> +                     5 - general purpose
>> +                     6 - general purpose
>> +                     7 - system supply
>> +                     8 - backup battery
>> +                     9 - charger input
>> +                     10 - VBUS
>> +                     11 - VBUS charging current
>> +                     14 - USB ID
>> +                     17 - battery charging current
>> +                     18 - battery voltage
>> +
>> +Example:
>> +     adc {
>> +             compatible = "ti,twl6030-gpadc";
>> +             interrupts = <3>;
>> +             #io-channel-cells = <1>;
>> +     };
>> --
>> 1.8.1.2
>>
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/



-- 
Regards,
Oleksandr Kozaruk | embedded developer
GlobalLogic
M +38.067.944.7905
www.globallogic.com

http://www.globallogic.com/email_disclaimer.txt

Re: [PATCH] iio: adc: Add bindigs documentation for twl6030 GPADC

[Guenter Roeck]

On Tue, Aug 20, 2013 at 10:12:28AM +0100, Mark Rutland wrote:
> Hi Oleksandr,
> 
> [Adding Jonathan Cameron and Guenter Roeck to Cc]
> 
> Apologies for the delay replying to this. In attempting to verify this
> made sense I went and read the IIO bindings documentation, and I'm
> somewhat confused by the model.
> 
> As far as I can see, the only consumer of IIO channels is the
> "iio-hwmon" binding, which seems to be a binding for Linux-specific
> infrastructure rather than any actual device. This runs counter to the

In respect to "iio-hwmon", I think you may actually be correct; we should
have found a better means to describe the system.
The intend was to describe that a set of adc inputs is connected
to a set of voltages or temperature sensors.

Is there a better way ? I am sure there is, but I have no idea what
it might be, nor do I have the time to find out.

However, I think that the "io-channels" property is well defined.

"gpios" describes a group of gpio pins which have a common purpose.
"io-channels" describes a group of io channels (or, ultimately, pins)
which have a common purpose. So this is not really linux specific,
unless other operating systems don't see the need of describing a group
of io channels as single entity. But then the same could be claimed
about groups of gpio pins.

> way DT is supposed to function (describing the hardware rather than how
> it's used). As far as I can see, this linkage is described because only
> a subset of the ADCs on the device are actually wired to something?
> 
Is that a problem ? I would think that the same is true for many chips
with multiple inputs and/or outputs.

> I also see a couple of IIO bindings ("adi,adf435x*, and "adi,ad7303")
> which don't describe any iio channel cells at all, so I'm somewhat
> confused by what the IIO channels actually represent, and why they must
> be consumed elsewhere. As far as I can see, an IIO channel represents a
> single ADC's registers in an IIO device, so I'm not sure why this must
> be exported via the channel concept -- it's not physically wired.
> 
I am sure there would be some other means to describe the same,
so I would agree that it does not _have_ to be the way it is.

Question is if there is a better way. Again, "io-channels" describes
a group of io channels with a common purpose. Sure, that does not _have_
to be described as a single property, but then I could argue that the same
is true for "gpios" and probably many other properties.

Thanks,
Guenter

Re: [PATCH] iio: adc: Add bindigs documentation for twl6030 GPADC

[Mark Rutland]

On Tue, Aug 20, 2013 at 04:34:56PM +0100, Guenter Roeck wrote:
> On Tue, Aug 20, 2013 at 10:12:28AM +0100, Mark Rutland wrote:
> > Hi Oleksandr,
> > 
> > [Adding Jonathan Cameron and Guenter Roeck to Cc]
> > 
> > Apologies for the delay replying to this. In attempting to verify this
> > made sense I went and read the IIO bindings documentation, and I'm
> > somewhat confused by the model.
> > 
> > As far as I can see, the only consumer of IIO channels is the
> > "iio-hwmon" binding, which seems to be a binding for Linux-specific
> > infrastructure rather than any actual device. This runs counter to the
> 
> In respect to "iio-hwmon", I think you may actually be correct; we should
> have found a better means to describe the system.
> The intend was to describe that a set of adc inputs is connected
> to a set of voltages or temperature sensors.
> 
> Is there a better way ? I am sure there is, but I have no idea what
> it might be, nor do I have the time to find out.

I'd imagine that a better option would be to embed that information in
subnodes of the device:

someadc {
	compatible = "vendor,someadc";
	/*
	 * Someadc has 20 independent ADCs, which may be wired
	 * arbitrarily:
	 */
	adc@1 {
		/* name from datasheet */
		name = "temp0";
		vendor,additional-calibration-data = <0x0 0x44>;
	};

	adc@15 {
		name = "temp1";
	};
};

The driver for the device then knows which inputs are actually wired,
and can export the channels as necessary to hwmon (or whatever driver we
see fit later down the line).

> 
> However, I think that the "io-channels" property is well defined.
> 
> "gpios" describes a group of gpio pins which have a common purpose.
> "io-channels" describes a group of io channels (or, ultimately, pins)
> which have a common purpose. So this is not really linux specific,
> unless other operating systems don't see the need of describing a group
> of io channels as single entity. But then the same could be claimed
> about groups of gpio pins.

While admittedly there's some correspondence between a gpio being a pin
and a channel being a pin, I don't think that's a good comparison. When
we describe gpios viald $NAME-gpios propertiese, we do so because there
is a physical link between the gpio output and the device.

As far as I can tell with io-channels, we describe them to say that they
are wired to something, but what they are actually wired to is not
described (at least in the case of the iio-hwmon binding). Do we have
any devices which require information from external ADCs to be used?

> 
> > way DT is supposed to function (describing the hardware rather than how
> > it's used). As far as I can see, this linkage is described because only
> > a subset of the ADCs on the device are actually wired to something?
> > 
> Is that a problem ? I would think that the same is true for many chips
> with multiple inputs and/or outputs.

What I meant was that I don't think describing a hardware to software
wiring is the best way of describing this, when we can simply describe
the hardware (as in the example above), and don't have to describe an
abstract concept (iio-hwmon) which may not correspond to how we want to
use the device in future.

> 
> > I also see a couple of IIO bindings ("adi,adf435x*, and "adi,ad7303")
> > which don't describe any iio channel cells at all, so I'm somewhat
> > confused by what the IIO channels actually represent, and why they must
> > be consumed elsewhere. As far as I can see, an IIO channel represents a
> > single ADC's registers in an IIO device, so I'm not sure why this must
> > be exported via the channel concept -- it's not physically wired.
> > 
> I am sure there would be some other means to describe the same,
> so I would agree that it does not _have_ to be the way it is.
> 
> Question is if there is a better way. Again, "io-channels" describes
> a group of io channels with a common purpose. Sure, that does not _have_
> to be described as a single property, but then I could argue that the same
> is true for "gpios" and probably many other properties.

Ok, I just want to raise the issue to ensure that we've properly
considered this before its an ever-lasting ABI.

Thanks,
Mark.

Re: [PATCH] iio: adc: Add bindigs documentation for twl6030 GPADC

[Guenter Roeck]

On Wed, Aug 21, 2013 at 10:14:51AM +0100, Mark Rutland wrote:
> On Tue, Aug 20, 2013 at 04:34:56PM +0100, Guenter Roeck wrote:
> > On Tue, Aug 20, 2013 at 10:12:28AM +0100, Mark Rutland wrote:
> > > Hi Oleksandr,
> > > 
> > > [Adding Jonathan Cameron and Guenter Roeck to Cc]
> > > 
> > > Apologies for the delay replying to this. In attempting to verify this
> > > made sense I went and read the IIO bindings documentation, and I'm
> > > somewhat confused by the model.
> > > 
> > > As far as I can see, the only consumer of IIO channels is the
> > > "iio-hwmon" binding, which seems to be a binding for Linux-specific
> > > infrastructure rather than any actual device. This runs counter to the
> > 
> > In respect to "iio-hwmon", I think you may actually be correct; we should
> > have found a better means to describe the system.
> > The intend was to describe that a set of adc inputs is connected
> > to a set of voltages or temperature sensors.
> > 
> > Is there a better way ? I am sure there is, but I have no idea what
> > it might be, nor do I have the time to find out.
> 
> I'd imagine that a better option would be to embed that information in
> subnodes of the device:
> 
> someadc {
> 	compatible = "vendor,someadc";
> 	/*
> 	 * Someadc has 20 independent ADCs, which may be wired
> 	 * arbitrarily:
> 	 */
> 	adc@1 {
> 		/* name from datasheet */
> 		name = "temp0";
> 		vendor,additional-calibration-data = <0x0 0x44>;
> 	};
> 
> 	adc@15 {
> 		name = "temp1";
> 	};
> };
> 
> The driver for the device then knows which inputs are actually wired,
> and can export the channels as necessary to hwmon (or whatever driver we
> see fit later down the line).
> 
It doesn't tell what those channels are connected to, though. It would be
important to know, for example, that an ADC channels is connected to a
temperature sensor (which would also need bindings) or to a specific voltage
channel. Just like the vcc pin of a chip is connected to a regulator,
the adc input pins are connected to a regulator as well if the adc is used 
to monitor voltages. For vcc that is well understood; for example, I have

	max1139: voltage-sensor@35 { /* PMB */
        	compatible = "maxim,max1139";
                reg = <0x35>;
                vcc-supply = <&reg_3p3v>;
                vref-supply = <&reg_3p3v>;
                #io-channel-cells = <1>;
        };

to specify both VCC and VREF for a MAX1139. What would be needed are properties
to describe what the ADC input pins are connected to in a generic way
so that drivers like iio_hwmon have a chance to pick it up.

> > 
> > However, I think that the "io-channels" property is well defined.
> > 
> > "gpios" describes a group of gpio pins which have a common purpose.
> > "io-channels" describes a group of io channels (or, ultimately, pins)
> > which have a common purpose. So this is not really linux specific,
> > unless other operating systems don't see the need of describing a group
> > of io channels as single entity. But then the same could be claimed
> > about groups of gpio pins.
> 
> While admittedly there's some correspondence between a gpio being a pin
> and a channel being a pin, I don't think that's a good comparison. When
> we describe gpios viald $NAME-gpios propertiese, we do so because there
> is a physical link between the gpio output and the device.
> 
> As far as I can tell with io-channels, we describe them to say that they
> are wired to something, but what they are actually wired to is not
> described (at least in the case of the iio-hwmon binding). Do we have
> any devices which require information from external ADCs to be used?
> 
The problem with iio_hwmon, as I see it, can be boiled down to its compatible
string. It doesn't directly describe hardware, so something like
	compatible = "iio-hwmon";
looks like a bad choice, though I am not sure if the culprit is the name
or what it provides.

Question is how to express this better. For example, we have "gpio-leds" to
describe LEDs connected to GPIO pins. What kind of property could we have to
describe IO channels (or adc inputs, if you like) connected to voltage sensors,
or any other kind of sensors ?

Guenter

Re: [PATCH] iio: adc: Add bindigs documentation for twl6030 GPADC

[Mark Rutland]

On Wed, Aug 21, 2013 at 04:41:27PM +0100, Guenter Roeck wrote:
> On Wed, Aug 21, 2013 at 10:14:51AM +0100, Mark Rutland wrote:
> > On Tue, Aug 20, 2013 at 04:34:56PM +0100, Guenter Roeck wrote:
> > > On Tue, Aug 20, 2013 at 10:12:28AM +0100, Mark Rutland wrote:
> > > > Hi Oleksandr,
> > > > 
> > > > [Adding Jonathan Cameron and Guenter Roeck to Cc]
> > > > 
> > > > Apologies for the delay replying to this. In attempting to verify this
> > > > made sense I went and read the IIO bindings documentation, and I'm
> > > > somewhat confused by the model.
> > > > 
> > > > As far as I can see, the only consumer of IIO channels is the
> > > > "iio-hwmon" binding, which seems to be a binding for Linux-specific
> > > > infrastructure rather than any actual device. This runs counter to the
> > > 
> > > In respect to "iio-hwmon", I think you may actually be correct; we should
> > > have found a better means to describe the system.
> > > The intend was to describe that a set of adc inputs is connected
> > > to a set of voltages or temperature sensors.
> > > 
> > > Is there a better way ? I am sure there is, but I have no idea what
> > > it might be, nor do I have the time to find out.
> > 
> > I'd imagine that a better option would be to embed that information in
> > subnodes of the device:
> > 
> > someadc {
> > 	compatible = "vendor,someadc";
> > 	/*
> > 	 * Someadc has 20 independent ADCs, which may be wired
> > 	 * arbitrarily:
> > 	 */
> > 	adc@1 {
> > 		/* name from datasheet */
> > 		name = "temp0";
> > 		vendor,additional-calibration-data = <0x0 0x44>;
> > 	};
> > 
> > 	adc@15 {
> > 		name = "temp1";
> > 	};
> > };
> > 
> > The driver for the device then knows which inputs are actually wired,
> > and can export the channels as necessary to hwmon (or whatever driver we
> > see fit later down the line).
> > 
> It doesn't tell what those channels are connected to, though. It would be
> important to know, for example, that an ADC channels is connected to a
> temperature sensor (which would also need bindings) or to a specific voltage
> channel. Just like the vcc pin of a chip is connected to a regulator,
> the adc input pins are connected to a regulator as well if the adc is used 
> to monitor voltages. For vcc that is well understood; for example, I have
> 
> 	max1139: voltage-sensor@35 { /* PMB */
>         	compatible = "maxim,max1139";
>                 reg = <0x35>;
>                 vcc-supply = <&reg_3p3v>;
>                 vref-supply = <&reg_3p3v>;
>                 #io-channel-cells = <1>;
>         };
> 
> to specify both VCC and VREF for a MAX1139. What would be needed are properties
> to describe what the ADC input pins are connected to in a generic way
> so that drivers like iio_hwmon have a chance to pick it up.

That can easily go in properties of the subnodes, alongside other data
(e.g. the "vendor,addtional-calibrartion-data" property). As far as I
can see the current binding still doesn't tell you what the channels are
actually wired to.

In the example above there are multiple channels, what do they
correspond to, and do all of them relate to the vcc and vref?

> 
> > > 
> > > However, I think that the "io-channels" property is well defined.
> > > 
> > > "gpios" describes a group of gpio pins which have a common purpose.
> > > "io-channels" describes a group of io channels (or, ultimately, pins)
> > > which have a common purpose. So this is not really linux specific,
> > > unless other operating systems don't see the need of describing a group
> > > of io channels as single entity. But then the same could be claimed
> > > about groups of gpio pins.
> > 
> > While admittedly there's some correspondence between a gpio being a pin
> > and a channel being a pin, I don't think that's a good comparison. When
> > we describe gpios viald $NAME-gpios propertiese, we do so because there
> > is a physical link between the gpio output and the device.
> > 
> > As far as I can tell with io-channels, we describe them to say that they
> > are wired to something, but what they are actually wired to is not
> > described (at least in the case of the iio-hwmon binding). Do we have
> > any devices which require information from external ADCs to be used?
> > 
> The problem with iio_hwmon, as I see it, can be boiled down to its compatible
> string. It doesn't directly describe hardware, so something like
> 	compatible = "iio-hwmon";
> looks like a bad choice, though I am not sure if the culprit is the name
> or what it provides.

As far as I can see, iio-hwmon just gets passed a set of channels with
no other information. How does it know what's wired to the ADCs
providing those channels? I don't think enough information's recorded
for that to be useful...

> 
> Question is how to express this better. For example, we have "gpio-leds" to
> describe LEDs connected to GPIO pins. What kind of property could we have to
> describe IO channels (or adc inputs, if you like) connected to voltage sensors,
> or any other kind of sensors ?

I don't see that we encode this in the current bindings. I think this
linkage can be described per-channel realtively easily if each channel
is described as a subnode of the device providing the ADC channels. In
the example I porvided previously, the channel from "temp0" encodes
calibration information that might be required on a per-device basis to
map from a raw value to degrees celsius. It may be possible to encode
additional type information in a relatively standard way:

someadc {
	compatible = "vendor,someadc";

	adc@0
		reg = <0>;
		name = "temp0"; 
		type = "temperature";
		vendor,temp-calibration-data = <0x0004 0xfee3>;
	};

	adc@3 {
		reg = <3>;
		type = "voltage";
		vcc-supply = <&reg_3p3v>;
		vref-supply = <&reg_3p3v>;
		vendor,vref-offset = <0x300>;
	};
};

I believe that would better describe the device, and describe what the
IIO framework needs, without requiring any software level abstraction
(i.e. io channels) to be described in the DT.

Thanks,
Mark.

Re: [PATCH] iio: adc: Add bindigs documentation for twl6030 GPADC

[Guenter Roeck]

On Wed, Aug 21, 2013 at 05:22:01PM +0100, Mark Rutland wrote:
> On Wed, Aug 21, 2013 at 04:41:27PM +0100, Guenter Roeck wrote:
> > On Wed, Aug 21, 2013 at 10:14:51AM +0100, Mark Rutland wrote:
> > > On Tue, Aug 20, 2013 at 04:34:56PM +0100, Guenter Roeck wrote:
> > > > On Tue, Aug 20, 2013 at 10:12:28AM +0100, Mark Rutland wrote:
> > > > > Hi Oleksandr,
> > > > > 
> > > > > [Adding Jonathan Cameron and Guenter Roeck to Cc]
> > > > > 
> > > > > Apologies for the delay replying to this. In attempting to verify this
> > > > > made sense I went and read the IIO bindings documentation, and I'm
> > > > > somewhat confused by the model.
> > > > > 
> > > > > As far as I can see, the only consumer of IIO channels is the
> > > > > "iio-hwmon" binding, which seems to be a binding for Linux-specific
> > > > > infrastructure rather than any actual device. This runs counter to the
> > > > 
> > > > In respect to "iio-hwmon", I think you may actually be correct; we should
> > > > have found a better means to describe the system.
> > > > The intend was to describe that a set of adc inputs is connected
> > > > to a set of voltages or temperature sensors.
> > > > 
> > > > Is there a better way ? I am sure there is, but I have no idea what
> > > > it might be, nor do I have the time to find out.
> > > 
> > > I'd imagine that a better option would be to embed that information in
> > > subnodes of the device:
> > > 
> > > someadc {
> > > 	compatible = "vendor,someadc";
> > > 	/*
> > > 	 * Someadc has 20 independent ADCs, which may be wired
> > > 	 * arbitrarily:
> > > 	 */
> > > 	adc@1 {
> > > 		/* name from datasheet */
> > > 		name = "temp0";
> > > 		vendor,additional-calibration-data = <0x0 0x44>;
> > > 	};
> > > 
> > > 	adc@15 {
> > > 		name = "temp1";
> > > 	};
> > > };
> > > 
> > > The driver for the device then knows which inputs are actually wired,
> > > and can export the channels as necessary to hwmon (or whatever driver we
> > > see fit later down the line).
> > > 
> > It doesn't tell what those channels are connected to, though. It would be
> > important to know, for example, that an ADC channels is connected to a
> > temperature sensor (which would also need bindings) or to a specific voltage
> > channel. Just like the vcc pin of a chip is connected to a regulator,
> > the adc input pins are connected to a regulator as well if the adc is used 
> > to monitor voltages. For vcc that is well understood; for example, I have
> > 
> > 	max1139: voltage-sensor@35 { /* PMB */
> >         	compatible = "maxim,max1139";
> >                 reg = <0x35>;
> >                 vcc-supply = <&reg_3p3v>;
> >                 vref-supply = <&reg_3p3v>;
> >                 #io-channel-cells = <1>;
> >         };
> > 
> > to specify both VCC and VREF for a MAX1139. What would be needed are properties
> > to describe what the ADC input pins are connected to in a generic way
> > so that drivers like iio_hwmon have a chance to pick it up.
> 
> That can easily go in properties of the subnodes, alongside other data
> (e.g. the "vendor,addtional-calibrartion-data" property). As far as I
> can see the current binding still doesn't tell you what the channels are
> actually wired to.
> 
> In the example above there are multiple channels, what do they
> correspond to, and do all of them relate to the vcc and vref?
> 
The example refers to the current bindings. vcc and vref specify chip supply
and reference voltages, not voltages connected to the adc inputs. That would
require a new set of properties.

> > 
> > > > 
> > > > However, I think that the "io-channels" property is well defined.
> > > > 
> > > > "gpios" describes a group of gpio pins which have a common purpose.
> > > > "io-channels" describes a group of io channels (or, ultimately, pins)
> > > > which have a common purpose. So this is not really linux specific,
> > > > unless other operating systems don't see the need of describing a group
> > > > of io channels as single entity. But then the same could be claimed
> > > > about groups of gpio pins.
> > > 
> > > While admittedly there's some correspondence between a gpio being a pin
> > > and a channel being a pin, I don't think that's a good comparison. When
> > > we describe gpios viald $NAME-gpios propertiese, we do so because there
> > > is a physical link between the gpio output and the device.
> > > 
> > > As far as I can tell with io-channels, we describe them to say that they
> > > are wired to something, but what they are actually wired to is not
> > > described (at least in the case of the iio-hwmon binding). Do we have
> > > any devices which require information from external ADCs to be used?
> > > 
> > The problem with iio_hwmon, as I see it, can be boiled down to its compatible
> > string. It doesn't directly describe hardware, so something like
> > 	compatible = "iio-hwmon";
> > looks like a bad choice, though I am not sure if the culprit is the name
> > or what it provides.
> 
> As far as I can see, iio-hwmon just gets passed a set of channels with
> no other information. How does it know what's wired to the ADCs
> providing those channels? I don't think enough information's recorded
> for that to be useful...
> 
That information is currently provided by the iio subsystem, which AFAIK
gets it from the chip driver (Jonathan, any comments ?).

> > 
> > Question is how to express this better. For example, we have "gpio-leds" to
> > describe LEDs connected to GPIO pins. What kind of property could we have to
> > describe IO channels (or adc inputs, if you like) connected to voltage sensors,
> > or any other kind of sensors ?
> 
> I don't see that we encode this in the current bindings. I think this
> linkage can be described per-channel realtively easily if each channel
> is described as a subnode of the device providing the ADC channels. In
> the example I porvided previously, the channel from "temp0" encodes
> calibration information that might be required on a per-device basis to
> map from a raw value to degrees celsius. It may be possible to encode
> additional type information in a relatively standard way:
> 
> someadc {
> 	compatible = "vendor,someadc";
> 
> 	adc@0
> 		reg = <0>;
> 		name = "temp0"; 
> 		type = "temperature";
> 		vendor,temp-calibration-data = <0x0004 0xfee3>;
> 	};
> 
> 	adc@3 {
> 		reg = <3>;
> 		type = "voltage";
> 		vcc-supply = <&reg_3p3v>;
> 		vref-supply = <&reg_3p3v>;
> 		vendor,vref-offset = <0x300>;
> 	};
> };
> 
> I believe that would better describe the device, and describe what the
> IIO framework needs, without requiring any software level abstraction
> (i.e. io channels) to be described in the DT.
> 
Except that vcc-supply and vref-supply are chip properties, not adc channel
properties. I like the general idea, but the property would need a more generic
name (it is not necessarily vcc or vref but could be any voltage).

Guenter

Re: [PATCH] iio: adc: Add bindigs documentation for twl6030 GPADC

[Jonathan Cameron]

On 08/21/13 18:02, Guenter Roeck wrote:
> On Wed, Aug 21, 2013 at 05:22:01PM +0100, Mark Rutland wrote:
>> On Wed, Aug 21, 2013 at 04:41:27PM +0100, Guenter Roeck wrote:
>>> On Wed, Aug 21, 2013 at 10:14:51AM +0100, Mark Rutland wrote:
>>>> On Tue, Aug 20, 2013 at 04:34:56PM +0100, Guenter Roeck wrote:
>>>>> On Tue, Aug 20, 2013 at 10:12:28AM +0100, Mark Rutland wrote:
>>>>>> Hi Oleksandr,
>>>>>>
>>>>>> [Adding Jonathan Cameron and Guenter Roeck to Cc]
>>>>>>
>>>>>> Apologies for the delay replying to this. In attempting to verify this
>>>>>> made sense I went and read the IIO bindings documentation, and I'm
>>>>>> somewhat confused by the model.
>>>>>>
>>>>>> As far as I can see, the only consumer of IIO channels is the
>>>>>> "iio-hwmon" binding, which seems to be a binding for Linux-specific
>>>>>> infrastructure rather than any actual device. This runs counter to the
>>>>>
>>>>> In respect to "iio-hwmon", I think you may actually be correct; we should
>>>>> have found a better means to describe the system.
>>>>> The intend was to describe that a set of adc inputs is connected
>>>>> to a set of voltages or temperature sensors.
>>>>>
>>>>> Is there a better way ? I am sure there is, but I have no idea what
>>>>> it might be, nor do I have the time to find out.
>>>>
>>>> I'd imagine that a better option would be to embed that information in
>>>> subnodes of the device:
>>>>
>>>> someadc {
>>>> 	compatible = "vendor,someadc";
>>>> 	/*
>>>> 	 * Someadc has 20 independent ADCs, which may be wired
>>>> 	 * arbitrarily:
>>>> 	 */
>>>> 	adc@1 {
>>>> 		/* name from datasheet */
>>>> 		name = "temp0";
>>>> 		vendor,additional-calibration-data = <0x0 0x44>;
>>>> 	};
>>>>
>>>> 	adc@15 {
>>>> 		name = "temp1";
>>>> 	};
>>>> };
>>>>
>>>> The driver for the device then knows which inputs are actually wired,
>>>> and can export the channels as necessary to hwmon (or whatever driver we
>>>> see fit later down the line).
>>>>
>>> It doesn't tell what those channels are connected to, though. It would be
>>> important to know, for example, that an ADC channels is connected to a
>>> temperature sensor (which would also need bindings) or to a specific voltage
>>> channel. Just like the vcc pin of a chip is connected to a regulator,
>>> the adc input pins are connected to a regulator as well if the adc is used 
>>> to monitor voltages. For vcc that is well understood; for example, I have
>>>
>>> 	max1139: voltage-sensor@35 { /* PMB */
>>>         	compatible = "maxim,max1139";
>>>                 reg = <0x35>;
>>>                 vcc-supply = <&reg_3p3v>;
>>>                 vref-supply = <&reg_3p3v>;
>>>                 #io-channel-cells = <1>;
>>>         };
>>>
>>> to specify both VCC and VREF for a MAX1139. What would be needed are properties
>>> to describe what the ADC input pins are connected to in a generic way
>>> so that drivers like iio_hwmon have a chance to pick it up.
>>
>> That can easily go in properties of the subnodes, alongside other data
>> (e.g. the "vendor,addtional-calibrartion-data" property). As far as I
>> can see the current binding still doesn't tell you what the channels are
>> actually wired to.
>>
>> In the example above there are multiple channels, what do they
>> correspond to, and do all of them relate to the vcc and vref?
>>
> The example refers to the current bindings. vcc and vref specify chip supply
> and reference voltages, not voltages connected to the adc inputs. That would
> require a new set of properties.
> 
>>>
>>>>>
>>>>> However, I think that the "io-channels" property is well defined.
>>>>>
>>>>> "gpios" describes a group of gpio pins which have a common purpose.
>>>>> "io-channels" describes a group of io channels (or, ultimately, pins)
>>>>> which have a common purpose. So this is not really linux specific,
>>>>> unless other operating systems don't see the need of describing a group
>>>>> of io channels as single entity. But then the same could be claimed
>>>>> about groups of gpio pins.
>>>>
>>>> While admittedly there's some correspondence between a gpio being a pin
>>>> and a channel being a pin, I don't think that's a good comparison. When
>>>> we describe gpios viald $NAME-gpios propertiese, we do so because there
>>>> is a physical link between the gpio output and the device.
>>>>
>>>> As far as I can tell with io-channels, we describe them to say that they
>>>> are wired to something, but what they are actually wired to is not
>>>> described (at least in the case of the iio-hwmon binding). Do we have
>>>> any devices which require information from external ADCs to be used?
>>>>
>>> The problem with iio_hwmon, as I see it, can be boiled down to its compatible
>>> string. It doesn't directly describe hardware, so something like
>>> 	compatible = "iio-hwmon";
>>> looks like a bad choice, though I am not sure if the culprit is the name
>>> or what it provides.
>>
>> As far as I can see, iio-hwmon just gets passed a set of channels with
>> no other information. How does it know what's wired to the ADCs
>> providing those channels? I don't think enough information's recorded
>> for that to be useful...
>>
> That information is currently provided by the iio subsystem, which AFAIK
> gets it from the chip driver (Jonathan, any comments ?).

There is no real way of providing that information unless the wiring
is fixed - e.g. what is there is always the same for the individual part.
IIO itself tends to be low level enough that it doesn't care.  Whilst
we've talked about this in the past it was back in the pre DT days
and was 'left for another day'.

> 
>>>
>>> Question is how to express this better. For example, we have "gpio-leds" to
>>> describe LEDs connected to GPIO pins. What kind of property could we have to
>>> describe IO channels (or adc inputs, if you like) connected to voltage sensors,
>>> or any other kind of sensors ?
>>
>> I don't see that we encode this in the current bindings. I think this
>> linkage can be described per-channel realtively easily if each channel
>> is described as a subnode of the device providing the ADC channels. In
>> the example I porvided previously, the channel from "temp0" encodes
>> calibration information that might be required on a per-device basis to
>> map from a raw value to degrees celsius. It may be possible to encode
>> additional type information in a relatively standard way:
>>
>> someadc {
>> 	compatible = "vendor,someadc";
>>
>> 	adc@0
>> 		reg = <0>;
>> 		name = "temp0"; 
>> 		type = "temperature";
>> 		vendor,temp-calibration-data = <0x0004 0xfee3>;
>> 	};
>>
>> 	adc@3 {
>> 		reg = <3>;
>> 		type = "voltage";
>> 		vcc-supply = <&reg_3p3v>;
>> 		vref-supply = <&reg_3p3v>;
>> 		vendor,vref-offset = <0x300>;
>> 	};
>> };
>>
>> I believe that would better describe the device, and describe what the
>> IIO framework needs, without requiring any software level abstraction
>> (i.e. io channels) to be described in the DT.
>>
> Except that vcc-supply and vref-supply are chip properties, not adc channel
> properties. I like the general idea, but the property would need a more generic
> name (it is not necessarily vcc or vref but could be any voltage).
> 
Just to clarify on some points in my mind. I've kind of lost track of what
this discussion is focusing on.

One thing to keep in mind throughout this.  The general purpose ADC case
is, whilst interesting is only a corner of what we have to cover.

Firstly the core of IIO itself doesn't currently use any bindings whatsoever
beyond simple 'it is there' + the usual regulators etc.
Much like hwmon it provides all channels to userspace. The unconnected channel
case only really became relevant as the number of SoC ADC drivers increased.

If you are using a discrete ADC, typically you want all or very
nearly all the channels.  Also keep very much in mind that for a lot of
input / output drivers the concept of 'connectivity doesn't really apply
as the sensor is directly measuring, say, light or acceleration. Arguably the
connectivity is there but within the single package so we don't want to have
to describe in the the DT.  What we need is some way of mapping 'purpose'.
Is that accelerometer useful for input or not?  As that ADC measuring temperature
and if so is that relevant for hardware monitoring?   For voltages
some of them are useful to monitor that a level is correct, others are
useful to know about the battery voltage.  As far as I can see there is no
description of this in the above interface.  This is kind of why we ended
up with the explicity mappings to say 'this channel to this driver' as a
fairly nasty way of indicating what it was for.

Agreed what we have now is less than elegant.  Anyhow I'll be following
how this develops with interest.  My big question right now is how does
the kernel know what to use to expose a given channel to userspace?
It's far from obvious in some cases, but how does this get encoded if it
isn't in the device tree?  Take an example.  An accelerometer on an embedded
unit for measuring the vibration on a bridge vs the same chip in a mobile phone?
In the phone it is there to act primarily as a human input device though
it might have a secondary consumer using it as a free fall monitor.  In the
bridge sensor neigther of these cases make any sense (hopefully!).

If that information is not in the device tree, where should it be?  This
decision is about hardware design, just not about wiring.

Jonathan

Re: [PATCH] iio: adc: Add bindigs documentation for twl6030 GPADC

[Jonathan Cameron]

Just realised that we have dropped linux-iio (could have sworn it was 
there back at the beginning of this thread) from the cc list so put
it back. Might be worth adding lmsensors as well. Will leave that up
to Guenter.

A follow up below.
On 21/08/13 22:03, Jonathan Cameron wrote:
> On 08/21/13 18:02, Guenter Roeck wrote:
>> On Wed, Aug 21, 2013 at 05:22:01PM +0100, Mark Rutland wrote:
>>> On Wed, Aug 21, 2013 at 04:41:27PM +0100, Guenter Roeck wrote:
>>>> On Wed, Aug 21, 2013 at 10:14:51AM +0100, Mark Rutland wrote:
>>>>> On Tue, Aug 20, 2013 at 04:34:56PM +0100, Guenter Roeck wrote:
>>>>>> On Tue, Aug 20, 2013 at 10:12:28AM +0100, Mark Rutland wrote:
>>>>>>> Hi Oleksandr,
>>>>>>>
>>>>>>> [Adding Jonathan Cameron and Guenter Roeck to Cc]
>>>>>>>
>>>>>>> Apologies for the delay replying to this. In attempting to verify this
>>>>>>> made sense I went and read the IIO bindings documentation, and I'm
>>>>>>> somewhat confused by the model.
>>>>>>>
>>>>>>> As far as I can see, the only consumer of IIO channels is the
>>>>>>> "iio-hwmon" binding, which seems to be a binding for Linux-specific
>>>>>>> infrastructure rather than any actual device. This runs counter to the
>>>>>>
>>>>>> In respect to "iio-hwmon", I think you may actually be correct; we should
>>>>>> have found a better means to describe the system.
>>>>>> The intend was to describe that a set of adc inputs is connected
>>>>>> to a set of voltages or temperature sensors.
>>>>>>
>>>>>> Is there a better way ? I am sure there is, but I have no idea what
>>>>>> it might be, nor do I have the time to find out.
>>>>>
>>>>> I'd imagine that a better option would be to embed that information in
>>>>> subnodes of the device:
>>>>>
>>>>> someadc {
>>>>> 	compatible = "vendor,someadc";
>>>>> 	/*
>>>>> 	 * Someadc has 20 independent ADCs, which may be wired
>>>>> 	 * arbitrarily:
>>>>> 	 */
>>>>> 	adc@1 {
>>>>> 		/* name from datasheet */
>>>>> 		name = "temp0";
>>>>> 		vendor,additional-calibration-data = <0x0 0x44>;
>>>>> 	};
>>>>>
>>>>> 	adc@15 {
>>>>> 		name = "temp1";
>>>>> 	};
>>>>> };
>>>>>
>>>>> The driver for the device then knows which inputs are actually wired,
>>>>> and can export the channels as necessary to hwmon (or whatever driver we
>>>>> see fit later down the line).
>>>>>
>>>> It doesn't tell what those channels are connected to, though. It would be
>>>> important to know, for example, that an ADC channels is connected to a
>>>> temperature sensor (which would also need bindings) or to a specific voltage
>>>> channel. Just like the vcc pin of a chip is connected to a regulator,
>>>> the adc input pins are connected to a regulator as well if the adc is used
>>>> to monitor voltages. For vcc that is well understood; for example, I have
>>>>
>>>> 	max1139: voltage-sensor@35 { /* PMB */
>>>>          	compatible = "maxim,max1139";
>>>>                  reg = <0x35>;
>>>>                  vcc-supply = <&reg_3p3v>;
>>>>                  vref-supply = <&reg_3p3v>;
>>>>                  #io-channel-cells = <1>;
>>>>          };
>>>>
>>>> to specify both VCC and VREF for a MAX1139. What would be needed are properties
>>>> to describe what the ADC input pins are connected to in a generic way
>>>> so that drivers like iio_hwmon have a chance to pick it up.
>>>
>>> That can easily go in properties of the subnodes, alongside other data
>>> (e.g. the "vendor,addtional-calibrartion-data" property). As far as I
>>> can see the current binding still doesn't tell you what the channels are
>>> actually wired to.
>>>
>>> In the example above there are multiple channels, what do they
>>> correspond to, and do all of them relate to the vcc and vref?
>>>
>> The example refers to the current bindings. vcc and vref specify chip supply
>> and reference voltages, not voltages connected to the adc inputs. That would
>> require a new set of properties.
>>
>>>>
>>>>>>
>>>>>> However, I think that the "io-channels" property is well defined.
>>>>>>
>>>>>> "gpios" describes a group of gpio pins which have a common purpose.
>>>>>> "io-channels" describes a group of io channels (or, ultimately, pins)
>>>>>> which have a common purpose. So this is not really linux specific,
>>>>>> unless other operating systems don't see the need of describing a group
>>>>>> of io channels as single entity. But then the same could be claimed
>>>>>> about groups of gpio pins.
>>>>>
>>>>> While admittedly there's some correspondence between a gpio being a pin
>>>>> and a channel being a pin, I don't think that's a good comparison. When
>>>>> we describe gpios viald $NAME-gpios propertiese, we do so because there
>>>>> is a physical link between the gpio output and the device.
>>>>>
>>>>> As far as I can tell with io-channels, we describe them to say that they
>>>>> are wired to something, but what they are actually wired to is not
>>>>> described (at least in the case of the iio-hwmon binding). Do we have
>>>>> any devices which require information from external ADCs to be used?
>>>>>
>>>> The problem with iio_hwmon, as I see it, can be boiled down to its compatible
>>>> string. It doesn't directly describe hardware, so something like
>>>> 	compatible = "iio-hwmon";
>>>> looks like a bad choice, though I am not sure if the culprit is the name
>>>> or what it provides.
>>>
>>> As far as I can see, iio-hwmon just gets passed a set of channels with
>>> no other information. How does it know what's wired to the ADCs
>>> providing those channels? I don't think enough information's recorded
>>> for that to be useful...
>>>
>> That information is currently provided by the iio subsystem, which AFAIK
>> gets it from the chip driver (Jonathan, any comments ?).
>
> There is no real way of providing that information unless the wiring
> is fixed - e.g. what is there is always the same for the individual part.
> IIO itself tends to be low level enough that it doesn't care.  Whilst
> we've talked about this in the past it was back in the pre DT days
> and was 'left for another day'.
>
I realised overnight that this might have come across as rather negative 
which is definitely not the intent.  Having this information
available would be great.  It will be fiddly enough to deal with that
at least initially I'd leave handling it to the drivers rather than the
core code.  Some utility functions would of course make sense for the
easy cases. Note the analog front ends can be 'interesting'.
 From below I see that leaving it entirely to the ADC driver is the 
current suggestion?

Also note that the 'analog' front end may be far from passive and may
be controllable.  To that end we'll need to allow for callbacks 
notifiers and locks.  From the point of view of DT though we will
need to have some sort of 'This is upstream with this stuff in between'
element. In many ways this is kind of similar to the stuff already
done for audio devices - perhaps there are some ideas to be gained
from there?

>>
>>>>
>>>> Question is how to express this better. For example, we have "gpio-leds" to
>>>> describe LEDs connected to GPIO pins. What kind of property could we have to
>>>> describe IO channels (or adc inputs, if you like) connected to voltage sensors,
>>>> or any other kind of sensors ?
>>>
>>> I don't see that we encode this in the current bindings. I think this
>>> linkage can be described per-channel realtively easily if each channel
>>> is described as a subnode of the device providing the ADC channels. In
>>> the example I porvided previously, the channel from "temp0" encodes
>>> calibration information that might be required on a per-device basis to
>>> map from a raw value to degrees celsius. It may be possible to encode
>>> additional type information in a relatively standard way:
>>>
>>> someadc {
>>> 	compatible = "vendor,someadc";
>>>
>>> 	adc@0
>>> 		reg = <0>;
>>> 		name = "temp0";
>>> 		type = "temperature";
>>> 		vendor,temp-calibration-data = <0x0004 0xfee3>;
>>> 	};
>>>
>>> 	adc@3 {
>>> 		reg = <3>;
>>> 		type = "voltage";
>>> 		vcc-supply = <&reg_3p3v>;
>>> 		vref-supply = <&reg_3p3v>;
>>> 		vendor,vref-offset = <0x300>;
>>> 	};
>>> };
>>>
>>> I believe that would better describe the device, and describe what the
>>> IIO framework needs, without requiring any software level abstraction
>>> (i.e. io channels) to be described in the DT.
>>>
>> Except that vcc-supply and vref-supply are chip properties, not adc channel
>> properties. I like the general idea, but the property would need a more generic
>> name (it is not necessarily vcc or vref but could be any voltage).
>>
> Just to clarify on some points in my mind. I've kind of lost track of what
> this discussion is focusing on.
>
> One thing to keep in mind throughout this.  The general purpose ADC case
> is, whilst interesting is only a corner of what we have to cover.
>
> Firstly the core of IIO itself doesn't currently use any bindings whatsoever
> beyond simple 'it is there' + the usual regulators etc.
> Much like hwmon it provides all channels to userspace. The unconnected channel
> case only really became relevant as the number of SoC ADC drivers increased.
>
> If you are using a discrete ADC, typically you want all or very
> nearly all the channels.  Also keep very much in mind that for a lot of
> input / output drivers the concept of 'connectivity doesn't really apply
> as the sensor is directly measuring, say, light or acceleration. Arguably the
> connectivity is there but within the single package so we don't want to have
> to describe in the the DT.  What we need is some way of mapping 'purpose'.
> Is that accelerometer useful for input or not?  As that ADC measuring temperature
> and if so is that relevant for hardware monitoring?   For voltages
> some of them are useful to monitor that a level is correct, others are
> useful to know about the battery voltage.  As far as I can see there is no
> description of this in the above interface.  This is kind of why we ended
> up with the explicity mappings to say 'this channel to this driver' as a
> fairly nasty way of indicating what it was for.
>
> Agreed what we have now is less than elegant.  Anyhow I'll be following
> how this develops with interest.  My big question right now is how does
> the kernel know what to use to expose a given channel to userspace?
> It's far from obvious in some cases, but how does this get encoded if it
> isn't in the device tree?  Take an example.  An accelerometer on an embedded
> unit for measuring the vibration on a bridge vs the same chip in a mobile phone?
> In the phone it is there to act primarily as a human input device though
> it might have a secondary consumer using it as a free fall monitor.  In the
> bridge sensor neigther of these cases make any sense (hopefully!).
>
> If that information is not in the device tree, where should it be?  This
> decision is about hardware design, just not about wiring.
>
Note the above is not meant to be negative but rather looking at where
we are and what else needs to be taken into account to get to where we
want to be.

> Jonathan
>
>