[lm-sensors] [PATCH 2/2] hwmon: (ltc4245) expose all GPIO pins as analog voltages

Grant Likely grant.likely at secretlab.ca
Wed May 26 23:10:01 CEST 2010


On Wed, May 26, 2010 at 2:56 PM, Ira W. Snyder <iws at ovro.caltech.edu> wrote:
> On Wed, May 26, 2010 at 02:37:28PM -0600, Grant Likely wrote:
>> On Wed, May 26, 2010 at 1:39 PM, Ira W. Snyder <iws at ovro.caltech.edu> wrote:
>> > On Wed, May 26, 2010 at 08:42:22PM +0200, Jean Delvare wrote:
>> >> On Wed, 26 May 2010 10:22:44 -0700, Ira W. Snyder wrote:
>> >> > On Wed, May 26, 2010 at 06:36:59PM +0200, Jean Delvare wrote:
>> >> > > Sorry for not being clear. I wasn't objecting to the stub's existence,
>> >> > > but to the fact that it was doing something. I expected it to do
>> >> > > nothing at all, and ltc4245_show_voltage() would be used for in9.
>> >> >
>> >> > Oh, ok. The ltc4245_show_voltage() function reads the voltage register
>> >> > values directly from data->vregs[]. When the extra GPIO inputs are
>> >> > enabled, I have two choices:
>> >> >
>> >> > 1) store them in the new data->gpios[] array
>> >> > 2) store them in data->vregs[]
>> >> >
>> >> > For #1, ltc4245_show_voltage() doesn't work anymore, since it reads
>> >> > voltage register values from data->vregs[].
>> >> >
>> >> > For #2, I would have to re-expand data->vregs[] to include all of the
>> >> > GPIO inputs, like it was before. Also, I would need to make
>> >> > ltc4245_get_voltage() handle the -EAGAIN error code.
>> >> >
>> >> > I think the code is easier to understand if all the GPIOs are treated in
>> >> > the same way, and not completely special for GPIOADC1 vs GPIOADC[23].
>> >> > That's why I chose #1.
>> >> >
>> >> > If I do a hybrid approach (store GPIOADC1 in data->vregs[] and
>> >> > GPIOADC[23] in data->gpios[]), then I have to make ltc4245_get_voltage()
>> >> > robust against errors too. Is this what you're suggesting?
>> >>
>> >> No. But let's wait and see if you move to OF platform data, the whole
>> >> question will be moot. If you keep the config option approach, I'll
>> >> propose an iterative patch and we can discuss it then.
>> >>
>> >> > (...)
>> >> > Any thoughts about the kernel config option vs. platform data, and how
>> >> > it relates to the OF bindings?
>> >>
>> >> I would prefer platform-data-based. But I fear I don't know anything
>> >> about OF bindings. Better ask the embedded folks (e.g. Grant Likely),
>> >> they will know.
>> >>
>> >
>> > Grant, you're listed in MAINTAINERS as the OF bindings maintainer. Is it
>> > currently possible for the i2c OF binding to pass platform_data to hwmon
>> > drivers that are probed via the device tree? If so, can you point me to
>> > an example. If not, can you suggest what I should do?
>> >
>> > Here is a short example from my device tree (a lightly modified 834x_mds
>> > device tree):
>> >
>> > i2c at 3100 {
>> > á á á á#address-cells = <1>;
>> > á á á á#size-cells = <0>;
>> > á á á ácell-index = <1>;
>> > á á á ácompatible = "fsl-i2c";
>> > á á á áreg = <0x3100 0x100>;
>> > á á á áinterrupts = <15 0x8>;
>> > á á á áinterrupt-parent = <&ipic>;
>> > á á á áclock-frequency = <400000>;
>> >
>> > á á á áltc4245 at XX {
>> > á á á á á á á ácompatible = "LLTC,ltc4245";
>> > á á á á á á á áreg = <0>; á á á// from bootloader
>> > á á á á};
>> > };
>> >
>> > I'd like to be able to send platform data to the drivers/hwmon/ltc4245.c
>> > driver. On my platform, this is probed automatically using the device
>> > tree snippet above.
>>
>> What is in the platform data?
>
> A bool/int should be sufficient.
>
>> As of the .35 merge window, the device
>> node pointer lives in struct device, so it is available to all Linux
>> device drivers if CONFIG_OF is set.  If it is simply data that needs
>> to be passed, then the driver itself can extract it from the tree in
>> the absence of pdata.  If you need to pass function pointers or the
>> like, then probably the best thing to do is to register a bus notifier
>> in the platform code before registering devices.  That way the
>> platform code can intercept the device and register pdata before the
>> device is bound to a driver.
>
> Ok, so I should wrap the OF detection code in CONFIG_OF in my driver,
> right? I just quickly browsed through the code, and it seems like it
> should be straightforward to use pdata and then fall back on OF data.

Correct.  The goal is to allow the driver-specific OF adapter code to
live with the driver, while still having as little as possible impact
on driver design as a whole.

g.




More information about the lm-sensors mailing list