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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20130821154127.GA8242@roeck-us.net>
Date:	Wed, 21 Aug 2013 08:41:27 -0700
From:	Guenter Roeck <linux@...ck-us.net>
To:	Mark Rutland <mark.rutland@....com>
Cc:	Oleksandr Kozaruk <oleksandr.kozaruk@...com>,
	"grant.likely@...aro.org" <grant.likely@...aro.org>,
	"rob.herring@...xeda.com" <rob.herring@...xeda.com>,
	"rob@...dley.net" <rob@...dley.net>,
	"linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Jonathan Cameron <jic23@...nel.org>
Subject: Re: [PATCH] iio: adc: Add bindigs documentation for twl6030 GPADC

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/