[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <56963392.9080101@topic.nl>
Date: Wed, 13 Jan 2016 12:22:58 +0100
From: Mike Looijmans <mike.looijmans@...ic.nl>
To: Guenter Roeck <linux@...ck-us.net>, <lm-sensors@...sensors.org>
CC: <jdelvare@...e.com>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] hwmon: Add LTC2990 sensor driver
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@...ic.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@...icproducts.com
Website: www.topicproducts.com
Please consider the environment before printing this e-mail
Powered by blists - more mailing lists