Re: [PATCH v2 3/3] hwmon: ltc2990: support all measurement modes

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 






On Thu, 29 Jun 2017, Mike Looijmans wrote:

On 28-06-17 19:02, Tom Levens wrote:

 On Wed, 28 Jun 2017, Guenter Roeck wrote:

>  On Wed, Jun 28, 2017 at 05:29:38PM +0200, Tom Levens wrote:
> > > [ ... ] > > > > > > > > Whatever happened to this patch though? It didn't make it to > > > > mainline,
> > > >  otherwise I'd have found it sooner...
> > > > > > > I'll have to look it up, but I guess I didn't get an updated > > > version. > > > > As far as I remember I had a working V3 of this patch, but it is > > entirely > > possible that it was never submitted as I have been busy with other > > projects
> >  recently. I'll dig it out and check that it is complete.
> > > FWIW, I don't see it at
>  https://patchwork.kernel.org/project/linux-hwmon/list/?submitter=171225&state=*
> > Maybe you were waiting for a reply from Rob. Either case, it might make
>  sense to only provide valid modes, ie to abstract the mode bits from the
>  hardware, such as
> > 0 - internal temp only
>  1 - Tr1
>  2 - V1
>  3 - V1-V2
>  4 - Tr2
>  5 - V3
>  6 - V3-V4
>  7 to 14 - per bit 0..2
> > Guenter
>

 You are right, there was still an open question about how best to handle
 the mode selection in DT.

 In the latest version of my patch I have it implemented as an array for
 setting the two values, for example:

      lltc,meas-mode = <7 3>;

 This sets bits [2..0] = 7 and [4..3] = 3. Of course these could be split
 into two DT properties, but I was unsure what to name them as both fields
 are called "mode" in the datasheet and "mode-43"/"mode-20" (or similar) is
 ugly.

 With regards to your proposal, it is not clear to me whether the modes
 which have the same result are exactly equivalent. Does disabling a
 measurement with the mode[4..3] bits really leaves the part in a safe
 state for all possible HW connections? With this doubt in my head, I would
 prefer to keep the option available to the user to select any specific
 mode. But I am open to suggestions.

 Mike, if you would like to test it, the latest version of my code is here:

 https://github.com/levens/ltc2990/blob/dev/drivers/hwmon/ltc2990.c


I pulled your patches, added two lines to the devicetree and it works fine, I'm seeing plausible results:
root@topic-miami:/sys/class/hwmon# grep . */*
hwmon0/name:battery-gauge
hwmon0/temp1_input:291700
hwmon1/in0_input:3258
hwmon1/in1_input:1824
hwmon1/in2_input:1916
hwmon1/in3_input:1512
hwmon1/in4_input:2066
hwmon1/name:ltc2990
hwmon1/temp1_input:28062
hwmon1/uevent:OF_NAME=ltc2990
hwmon1/uevent:OF_FULLNAME=/gpios-i2c@50/ltc2990@4c
hwmon1/uevent:OF_COMPATIBLE_0=lltc,ltc2990
hwmon1/uevent:OF_COMPATIBLE_N=1
hwmon2/curr1_input:1825
hwmon2/curr2_input:349
hwmon2/in0_input:3244
hwmon2/name:ltc2990
hwmon2/temp1_input:31500
hwmon2/uevent:OF_NAME=ltc2990
hwmon2/uevent:OF_FULLNAME=/gpios-i2c@52/ltc2990@4C
hwmon2/uevent:OF_COMPATIBLE_0=ltc2990
hwmon2/uevent:OF_COMPATIBLE_N=1


As you can see, two ltc2990 on two busses, one measuring current and one measuring 4 independent voltages.

I also like your implementation of the mode setting better, it's much simpler than mine. And you've solved a bug in the temperature reading as well.

Excellent, I am glad it also works for you! In this case, I would propose that I submit the V3 of the patch for further discussion.

Thanks,


Kind regards,

Mike Looijmans
System Expert

TOPIC Products
Materiaalweg 4, NL-5681 RJ Best
Postbus 440, NL-5680 AK Best
Telefoon: +31 (0) 499 33 69 79
E-mail: mike.looijmans@xxxxxxxxxxxxxxxxx
Website: www.topicproducts.com

Please consider the environment before printing this e-mail




--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux