From: Mike Looijmans <mike.looijmans@topic.nl>
To: Guenter Roeck <linux@roeck-us.net>, lm-sensors@lm-sensors.org
Cc: jdelvare@suse.com, linux-kernel@vger.kernel.org
Subject: Re: [lm-sensors] [PATCH] hwmon: Add LTC2990 sensor driver
Date: Wed, 13 Jan 2016 11:22:58 +0000 [thread overview]
Message-ID: <56963392.9080101@topic.nl> (raw)
In-Reply-To: <568FD131.4080308@roeck-us.net>
77u/T24gMDgtMDEtMTYgMTY6MDksIEd1ZW50ZXIgUm9lY2sgd3JvdGU6Cj4gT24gMDEvMDcvMjAx
NiAxMDo1OSBBTSwgTWlrZSBMb29pam1hbnMgd3JvdGU6Cj4+IFRoYW5rIHlvdSB2ZXJ5IG11Y2gg
Zm9yIHlvdXIgcmV2aWV3IGNvbW1lbnRzLCBJJ2xsIHVwZGF0ZSB0aGUgZHJpdmVyIGFuZAo+PiBw
b3N0IGEgdjIgcGF0Y2guCj4+Cj4+IElubGluZWQgc29tZSByZXBsaWVzIGJlbG93LiBBc3N1bWUg
dGhhdCBJICJ3aWxsIGRvIiBmb3IgYWxsIGNvbW1lbnRzIEkKPj4gZGlkbid0IGNvbW1lbnQgb24g
aW5saW5lLi4uCj4+Cj4+IE9uIDA2LTAxLTE2IDE2OjIyLCBHdWVudGVyIFJvZWNrIHdyb3RlOgo+
Pj4gSGVsbG8gTWlrZSwKPj4+Cj4+PiBPbiAwMS8wNi8yMDE2IDEyOjA3IEFNLCBNaWtlIExvb2lq
bWFucyB3cm90ZToKPj4+PiBUaGlzIGFkZHMgc3VwcG9ydCBmb3IgdGhlIExpbmVhciBUZWNobm9s
b2d5IExUQzI5OTAgIEkyQyBTeXN0ZW0gTW9uaXRvci4KPj4+Cj4+PiBzLyAgLyAvCj4+Pgo+Pj4+
IFRoZSBMVEMyOTkwIHN1cHBvcnRzIGEgY29tYmluYXRpb24gb2Ygdm9sdGFnZSwgY3VycmVudCBh
bmQgdGVtcGVyYXR1cmUKPj4+PiBtb25pdG9yaW5nLCBidXQgdGhpcyBkcml2ZXIgY3VycmVudGx5
IG9ubHkgc3VwcG9ydHMgcmVhZGluZyB0d28gY3VycmVudHMKPj4+PiBieSBtZWFzdXJpbmcgdHdv
IGRpZmZlcmVudGlhbCB2b2x0YWdlcyBhY3Jvc3Mgc2VyaWVzIHJlc2lzdG9ycy4KPj4+Pgo+Pj4g
UGx1cyBWQ0MsIHBsdXMgdGhlIGludGVybmFsIHRlbXBlcmF0dXJlLgo+Pgo+PiBZZWFoLCBJIHNo
b3VsZCBnaXZlIG15c2VsZiBtb3JlIGNyZWRpdCA6KSBJJ2xsIGFkZCB0aGF0IGluIEtjb25maWcg
dG9vLgo+Pgo+Pj4+IFRoaXMgaXMgc3VmZmljaWVudCB0byBzdXBwb3J0IHRoZSBUb3BpYyBNaWFt
aSBTT00gd2hpY2ggdXNlcyB0aGlzIGNoaXAKPj4+PiB0byBtb25pdG9yIHRoZSBjdXJyZW50cyBm
bG93aW5nIGludG8gdGhlIEZQR0EgYW5kIHRoZSBDUFUgcGFydHMuCj4+Pj4KPj4+PiBTaWduZWQt
b2ZmLWJ5OiBNaWtlIExvb2lqbWFucyA8bWlrZS5sb29pam1hbnNAdG9waWMubmw+Cj4+Pj4gLS0t
Cj4+Pj4gICBkcml2ZXJzL2h3bW9uL0tjb25maWcgICB8ICAxNSArKysKPj4+PiAgIGRyaXZlcnMv
aHdtb24vTWFrZWZpbGUgIHwgICAxICsKPj4+PiAgIGRyaXZlcnMvaHdtb24vbHRjMjk5MC5jIHwg
MjczCj4+Pj4gKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysr
Cj4+Pgo+Cj4gWyAuLi4gXQo+Cj4+Pj4gK30KPj4+PiArCj4+Pj4gK3N0YXRpYyBzdHJ1Y3QgbHRj
Mjk5MF9kYXRhICpsdGMyOTkwX3VwZGF0ZV9kZXZpY2Uoc3RydWN0IGRldmljZSAqZGV2KQo+Pj4+
ICt7Cj4+Pj4gKyAgICBzdHJ1Y3QgaTJjX2NsaWVudCAqaTJjID0gdG9faTJjX2NsaWVudChkZXYp
Owo+Pj4+ICsgICAgc3RydWN0IGx0YzI5OTBfZGF0YSAqZGF0YSA9IGkyY19nZXRfY2xpZW50ZGF0
YShpMmMpOwo+Pj4+ICsgICAgc3RydWN0IGx0YzI5OTBfZGF0YSAqcmV0ID0gZGF0YTsKPj4+PiAr
ICAgIHVuc2lnbmVkIGludCB0aW1lb3V0Owo+Pj4+ICsKPj4+PiArICAgIG11dGV4X2xvY2soJmRh
dGEtPnVwZGF0ZV9sb2NrKTsKPj4+PiArCj4+Pj4gKyAgICAvKiBVcGRhdGUgYWJvdXQgNCB0aW1l
cyBwZXIgc2Vjb25kIG1heCAqLwo+Pj4+ICsgICAgaWYgKHRpbWVfYWZ0ZXIoamlmZmllcywgZGF0
YS0+bGFzdF91cGRhdGVkICsgSFogLyA0KSB8fAo+Pj4+ICFkYXRhLT52YWxpZCkgewo+Pj4+ICsg
ICAgICAgIGludCB2YWw7Cj4+Pj4gKyAgICAgICAgaW50IGk7Cj4+Pj4gKwo+Pj4KPj4+IFBsZWFz
ZSBjb25zaWRlciB1c2luZyBjb250aW51b3VzIGNvbnZlcnNpb24uIFRoaXMgd291bGQgc2ltcGxp
ZnkgdGhlCj4+PiBjb2RlIHNpZ25pZmljYW50bHkKPj4+IGFuZCByZWR1Y2UgcmVhZCBkZWxheXMu
Cj4+Cj4+IEl0IG1pZ2h0IGluY3JlYXNlIHBvd2VyIGNvbnN1bXB0aW9uIHRob3VnaCwgYXMgdHlw
aWNhbGx5IHNvbWUgdXNlciBwcm9ncmFtCj4+IHdvdWxkIHBvbGwgdGhpcyBldmVyeSAxMCBzZWNv
bmRzIG9yIHNvLiBJJ2xsIGNoZWNrIHRoZSBkYXRhIHNoZWV0Lgo+Pgo+Cj4gSSBzdXNwZWN0IHRo
YXQgdGhlIHBvd2VyIHNhdmluZ3Mgd2lsbCBiZSBsZXNzIHRoYW4gdGhlIGFkZGVkIHBvd2VyCj4g
Y29uc3VtZWQgYnkgdGhlIENQVSBkdWUgdG8gdGhlIG1vcmUgY29tcGxleCBjb2RlLgo+Cj4gUmVh
bGx5LCB1bmxlc3MgeW91IGhhdmUgYW4gYXBwbGljYXRpb24gd2hlcmUgYSBmZXcgbVcgcG93ZXIg
c2F2aW5ncwo+IGFyZSBlc3NlbnRpYWwgYW5kIHdhcnJhbnQgdGhlIGFkZGl0aW9uYWwgY29kZSBj
b21wbGV4aXR5LCB0aGlzIGlzCj4gdGhlIHdyb25nIGFwcHJvYWNoLgoKWWVhaCwgeW91J3JlIHJp
Z2h0LCBjaGVja2VkIHRoZSBkYXRhc2hlZXQgYW5kIGl0IHJlcG9ydHMgYSAxbUEgdHlwaWNhbCBw
b3dlciAKdXNhZ2Ugd2hlbiBjb252ZXJ0aW5nLiBOb3Qgd29ydGggdGhlIHRyb3VibGUuCgpDb250
aW51b3VzIGNvbnZlcnNpb24gbWFkZSB0aGUgZHJpdmVyIGEgTE9UIHNpbXBsZXIsIGNvdWxkIGV2
ZW4gY29tcGxldGVseSAKZHJvcCB0aGUgcHJpdmF0ZSBkYXRhIHN0cnVjdHVyZS4KCkkgc2VudCB2
MiBhbHJlYWR5LCBidXQganVzdCBub3RpY2VkIHRoYXQgc29tZSBvZiB0aGUgI2luY2x1ZGVzIHdl
cmUgbm8gbG9uZ2VyCm5lZWRlZCwgc28gSSdsbCBzZW5kIGEgdjMgdG8gZml4IHRoYXQuCgouLi4K
CgpLaW5kIHJlZ2FyZHMsCgpNaWtlIExvb2lqbWFucwpTeXN0ZW0gRXhwZXJ0CgpUT1BJQyBFbWJl
ZGRlZCBQcm9kdWN0cwpFaW5kaG92ZW5zZXdlZyAzMi1DLCBOTC01NjgzIEtIIEJlc3QKUG9zdGJ1
cyA0NDAsIE5MLTU2ODAgQUsgQmVzdApUZWxlZm9vbjogKzMxICgwKSA0OTkgMzMgNjkgNzkKRS1t
YWlsOiBtaWtlLmxvb2lqbWFuc0B0b3BpY3Byb2R1Y3RzLmNvbQpXZWJzaXRlOiB3d3cudG9waWNw
cm9kdWN0cy5jb20KClBsZWFzZSBjb25zaWRlciB0aGUgZW52aXJvbm1lbnQgYmVmb3JlIHByaW50
aW5nIHRoaXMgZS1tYWlsCgoKCgoKCl9fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f
X19fX19fX19fX19fCmxtLXNlbnNvcnMgbWFpbGluZyBsaXN0CmxtLXNlbnNvcnNAbG0tc2Vuc29y
cy5vcmcKaHR0cDovL2xpc3RzLmxtLXNlbnNvcnMub3JnL21haWxtYW4vbGlzdGluZm8vbG0tc2Vu
c29ycw=
WARNING: multiple messages have this Message-ID (diff)
From: Mike Looijmans <mike.looijmans@topic.nl>
To: Guenter Roeck <linux@roeck-us.net>, <lm-sensors@lm-sensors.org>
Cc: <jdelvare@suse.com>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] hwmon: Add LTC2990 sensor driver
Date: Wed, 13 Jan 2016 12:22:58 +0100 [thread overview]
Message-ID: <56963392.9080101@topic.nl> (raw)
In-Reply-To: <568FD131.4080308@roeck-us.net>
On 08-01-16 16:09, Guenter Roeck wrote:
> On 01/07/2016 10:59 AM, Mike Looijmans wrote:
>> Thank you very much for your review comments, I'll update the driver and
>> post a v2 patch.
>>
>> Inlined some replies below. Assume that I "will do" for all comments I
>> didn't comment on inline...
>>
>> On 06-01-16 16:22, Guenter Roeck wrote:
>>> Hello Mike,
>>>
>>> On 01/06/2016 12:07 AM, Mike Looijmans wrote:
>>>> This adds support for the Linear Technology LTC2990 I2C System Monitor.
>>>
>>> s/ / /
>>>
>>>> The LTC2990 supports a combination of voltage, current and temperature
>>>> monitoring, but this driver currently only supports reading two currents
>>>> by measuring two differential voltages across series resistors.
>>>>
>>> Plus VCC, plus the internal temperature.
>>
>> Yeah, I should give myself more credit :) I'll add that in Kconfig too.
>>
>>>> This is sufficient to support the Topic Miami SOM which uses this chip
>>>> to monitor the currents flowing into the FPGA and the CPU parts.
>>>>
>>>> Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl>
>>>> ---
>>>> drivers/hwmon/Kconfig | 15 +++
>>>> drivers/hwmon/Makefile | 1 +
>>>> drivers/hwmon/ltc2990.c | 273
>>>> ++++++++++++++++++++++++++++++++++++++++++++++++
>>>
>
> [ ... ]
>
>>>> +}
>>>> +
>>>> +static struct ltc2990_data *ltc2990_update_device(struct device *dev)
>>>> +{
>>>> + struct i2c_client *i2c = to_i2c_client(dev);
>>>> + struct ltc2990_data *data = i2c_get_clientdata(i2c);
>>>> + struct ltc2990_data *ret = data;
>>>> + unsigned int timeout;
>>>> +
>>>> + mutex_lock(&data->update_lock);
>>>> +
>>>> + /* Update about 4 times per second max */
>>>> + if (time_after(jiffies, data->last_updated + HZ / 4) ||
>>>> !data->valid) {
>>>> + int val;
>>>> + int i;
>>>> +
>>>
>>> Please consider using continuous conversion. This would simplify the
>>> code significantly
>>> and reduce read delays.
>>
>> It might increase power consumption though, as typically some user program
>> would poll this every 10 seconds or so. I'll check the data sheet.
>>
>
> I suspect that the power savings will be less than the added power
> consumed by the CPU due to the more complex code.
>
> Really, unless you have an application where a few mW power savings
> are essential and warrant the additional code complexity, this is
> the wrong approach.
Yeah, you're right, checked the datasheet and it reports a 1mA typical power
usage when converting. Not worth the trouble.
Continuous conversion made the driver a LOT simpler, could even completely
drop the private data structure.
I sent v2 already, but just noticed that some of the #includes were no longer
needed, so I'll send a v3 to fix that.
...
Kind regards,
Mike Looijmans
System Expert
TOPIC Embedded Products
Eindhovenseweg 32-C, NL-5683 KH Best
Postbus 440, NL-5680 AK Best
Telefoon: +31 (0) 499 33 69 79
E-mail: mike.looijmans@topicproducts.com
Website: www.topicproducts.com
Please consider the environment before printing this e-mail
next prev parent reply other threads:[~2016-01-13 11:22 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-01-06 8:07 [lm-sensors] [PATCH] hwmon: Add LTC2990 sensor driver Mike Looijmans
2016-01-06 8:07 ` Mike Looijmans
2016-01-06 15:22 ` Guenter Roeck
2016-01-07 18:59 ` [lm-sensors] " Mike Looijmans
2016-01-07 18:59 ` Mike Looijmans
2016-01-08 15:09 ` [lm-sensors] " Guenter Roeck
2016-01-08 15:09 ` Guenter Roeck
2016-01-13 11:05 ` [lm-sensors] [PATCH v2] " Mike Looijmans
2016-01-13 11:05 ` Mike Looijmans
2016-01-13 13:24 ` [lm-sensors] " Guenter Roeck
2016-01-13 13:24 ` Guenter Roeck
2016-01-13 13:51 ` [lm-sensors] " Mike Looijmans
2016-01-13 13:51 ` Mike Looijmans
2016-01-13 13:57 ` [lm-sensors] " Guenter Roeck
2016-01-13 13:57 ` Guenter Roeck
2016-01-13 14:03 ` [lm-sensors] " Mike Looijmans
2016-01-13 14:03 ` Mike Looijmans
2016-01-13 14:45 ` [lm-sensors] [PATCH v3] " Mike Looijmans
2016-01-13 14:45 ` Mike Looijmans
2016-01-14 19:14 ` [lm-sensors] " Guenter Roeck
2016-01-14 19:14 ` Guenter Roeck
2016-01-15 9:54 ` [lm-sensors] " Mike Looijmans
2016-01-15 9:54 ` Mike Looijmans
2016-01-15 9:54 ` [lm-sensors] [PATCH v4] " Mike Looijmans
2016-01-15 9:54 ` Mike Looijmans
2016-01-15 15:40 ` [lm-sensors] " Guenter Roeck
2016-01-15 15:40 ` Guenter Roeck
2016-01-13 11:22 ` Mike Looijmans [this message]
2016-01-13 11:22 ` [PATCH] " Mike Looijmans
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=56963392.9080101@topic.nl \
--to=mike.looijmans@topic.nl \
--cc=jdelvare@suse.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@roeck-us.net \
--cc=lm-sensors@lm-sensors.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.