[PATCH] hwmon: (ads1015) Add devicetree documentation

[Grant Likely]

On Thu, Mar 3, 2011 at 3:02 PM, Jean Delvare <khali at linux-fr.org> wrote:
> On Thu, 3 Mar 2011 10:26:53 -0700, Grant Likely wrote:
>> On Thu, Mar 03, 2011 at 02:25:49PM +0100, Wolfram Sang wrote:
>> > OK. The thing is you can't map platform_data 1:1 to bindings, because
>> > most are very specific to the Linux-driver. Do you think something like
>> > "active-channels" would be sufficent for those other hwmon devices, too?
>> > (I still do not like "exported-channels", because there is no need to
>> > export the channels for the OS. The devicetree is primarily a hardware
>> > description language) Or maybe we go specific and say "ads1015,channel1
>> > = 1"? Maybe somebody knows of a similar chips as a reference?
>>
>> Yes, Wolfram's correct.  The focus should be on what the connections
>> actually are instead of what the OS should attempt to do with them.
>> ie. give the active channels actual names for what they do, or use a
>> phandle to reference them from another node.  The driver can then make
>> a decision based on whether or not a channel has a configuration
>> provided.
>>
>> From the little information I have, I'd recommend something like:
>>
>> sensor at 49 {
>>       compatible = "ti,ads1015"
>>       reg = <0x49>;
>>
>>       // Each child node (one node per channel) has an address with
>>       // no range
>>       #address-cells = <1>;
>>       #size-cells = <0>;
>>
>>       adc at 2 {
>>               ti,measurement = "cpu voltage";
>>               reg = <2>;
>>       };
>>
>>       adc at 4 {
>>               ti,measurement = "base voltage";
>>               reg = <4>;
>>       };
>> };
>>
>> However, after taking a little look at the data sheet, this binding
>> might end up being a little naive for what the part can do.  It might
>> make more sense to do something like:
>>
>> sensor at 49 {
>>       compatible = "ti,ads1015"
>>       reg = <0x49>;
>>
>>       // Each child node (one node per channel) has an address with
>>       // no range
>>       #address-cells = <1>;
>>       #size-cells = <0>;
>>
>>       measurement at 0 {
>>               reg = <0>;      // This reg value no longer reflects
>>                               // the chip, it's just a handle for
>>                               // logical measurement channels
>
> If there any hard requirement for this? From the driver's perspective,
> the multiplexer setting can be used as the channel ID, and it's easier
> this way than using arbitrary identifiers.

Nope, not a hard requirement.  Just a suggestion.  It would be
valuable if (and only if) there was ever reason to have more than one
configuration for a particular multiplexer setting.

>> It's more verbose than a single exported-channels property, but it is
>> flexible enough that it can be easily extended with extra
>> configuration data per channel.
>
> I will leave it up to Dirk to decide how much time he wants to spend on
> this. He has already been very patient with the driver review process
> and I do not want to abuse his patience.

BTW Dirk, I'm happy to help show how to write the parser code for the
sub-node approach if you need it.

g.