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:

quoted

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@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@2 {
		ti,measurement = "cpu voltage";
		reg = <2>;
	};

	adc@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@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@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.

		ti,measurement = "cpu voltage";
		ti,multiplexer = <2>; // AIN0 vs. AIN3
		ti,gain = <5>; // +/-0.256V
		ti,inverse-polarity;
	};

	measurement@1 {
		reg = <1>;
		ti,measurement = "base voltage";
		ti,multiplexer = <4>; // AIN0 vs. GND
		ti,gain = <2>; // +/-2.048V
	};
};

...which splits it up by logical measurement configurations instead of
only the multiplexer setting.

I wouldn't disagree, as I did suggest making the gain a per-channel
setting while reviewing Dirk's driver. I had not thought of labelling,
but the hwmon sysfs API supports this too, so it would fit nicely
(although it can also be handled in user-space if needed.)

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.

-- 
Jean Delvare