diff mbox

[v2] hwmon: Add LTC2471/LTC2473 driver

Message ID 1498738434-18252-1-git-send-email-mike.looijmans@topic.nl (mailing list archive)
State Rejected
Headers show

Commit Message

Mike Looijmans June 29, 2017, 12:13 p.m. UTC
The LTC2741 and LTC2473 are single voltage monitoring chips. The LTC2473
is similar to the LTC2471 but outputs a signed differential value.

Datasheet:
  http://cds.linear.com/docs/en/datasheet/24713fb.pdf

Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl>
---
v2: Use sizeof(*data) instead of struct name
    Use octal permissions

 drivers/hwmon/Kconfig   |  10 ++++
 drivers/hwmon/Makefile  |   1 +
 drivers/hwmon/ltc2471.c | 127 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 138 insertions(+)
 create mode 100644 drivers/hwmon/ltc2471.c

Comments

Lars-Peter Clausen June 29, 2017, 12:30 p.m. UTC | #1
On 06/29/2017 02:13 PM, Mike Looijmans wrote:
> The LTC2741 and LTC2473 are single voltage monitoring chips. The LTC2473
> is similar to the LTC2471 but outputs a signed differential value.
> 
> Datasheet:
>   http://cds.linear.com/docs/en/datasheet/24713fb.pdf

This looks more like a general purpose ADC rather than a specialized voltage
monitoring chip. In my opinion this driver would be better suited for the
IIO framework. But before you start working on that lets try to hear what
others think about this.

- Lars
--
To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mike Looijmans June 29, 2017, 1:22 p.m. UTC | #2
On 29-06-17 14:30, Lars-Peter Clausen wrote:
> On 06/29/2017 02:13 PM, Mike Looijmans wrote:
>> The LTC2741 and LTC2473 are single voltage monitoring chips. The LTC2473
>> is similar to the LTC2471 but outputs a signed differential value.
>>
>> Datasheet:
>>    http://cds.linear.com/docs/en/datasheet/24713fb.pdf
> 
> This looks more like a general purpose ADC rather than a specialized voltage
> monitoring chip. In my opinion this driver would be better suited for the
> IIO framework. But before you start working on that lets try to hear what
> others think about this.

I'm neutral on it, it's being used as a hardware monitor on the board I have, 
but it's indeed a generic I2C ADC.

Do you have a suggestion for a similar device (simple I2C ADC) that I could 
base the IIO driver on? I've come across the AD799x which seems a good 
starting point.



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@topicproducts.com
Website: www.topicproducts.com

Please consider the environment before printing this e-mail



--
To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Guenter Roeck June 29, 2017, 1:40 p.m. UTC | #3
On 06/29/2017 05:30 AM, Lars-Peter Clausen wrote:
> On 06/29/2017 02:13 PM, Mike Looijmans wrote:
>> The LTC2741 and LTC2473 are single voltage monitoring chips. The LTC2473
>> is similar to the LTC2471 but outputs a signed differential value.
>>
>> Datasheet:
>>    http://cds.linear.com/docs/en/datasheet/24713fb.pdf
> 
> This looks more like a general purpose ADC rather than a specialized voltage
> monitoring chip. In my opinion this driver would be better suited for the
> IIO framework. But before you start working on that lets try to hear what
> others think about this.
> 

Agreed (I just replied to that on another thread).

Guenter


--
To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Guenter Roeck June 29, 2017, 1:41 p.m. UTC | #4
On 06/29/2017 06:22 AM, Mike Looijmans wrote:
> On 29-06-17 14:30, Lars-Peter Clausen wrote:
>> On 06/29/2017 02:13 PM, Mike Looijmans wrote:
>>> The LTC2741 and LTC2473 are single voltage monitoring chips. The LTC2473
>>> is similar to the LTC2471 but outputs a signed differential value.
>>>
>>> Datasheet:
>>>    http://cds.linear.com/docs/en/datasheet/24713fb.pdf
>>
>> This looks more like a general purpose ADC rather than a specialized voltage
>> monitoring chip. In my opinion this driver would be better suited for the
>> IIO framework. But before you start working on that lets try to hear what
>> others think about this.
> 
> I'm neutral on it, it's being used as a hardware monitor on the board I have, but it's indeed a generic I2C ADC.
> 
> Do you have a suggestion for a similar device (simple I2C ADC) that I could base the IIO driver on? I've come across the AD799x which seems a good starting point.
> 

ltc2485, ltc2497 ? No idea how close they are.

Guenter
--
To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mike Looijmans June 29, 2017, 2:11 p.m. UTC | #5
On 29-06-17 15:40, Guenter Roeck wrote:
> On 06/29/2017 05:30 AM, Lars-Peter Clausen wrote:
>> On 06/29/2017 02:13 PM, Mike Looijmans wrote:
>>> The LTC2741 and LTC2473 are single voltage monitoring chips. The LTC2473
>>> is similar to the LTC2471 but outputs a signed differential value.
>>>
>>> Datasheet:
>>>    http://cds.linear.com/docs/en/datasheet/24713fb.pdf
>>
>> This looks more like a general purpose ADC rather than a specialized voltage
>> monitoring chip. In my opinion this driver would be better suited for the
>> IIO framework. But before you start working on that lets try to hear what
>> others think about this.
>>
> 
> Agreed (I just replied to that on another thread).

Took a peek at the IIO drivers, looks simple enough, so I suggest to drop this 
patch and I'll create an IIO driver instead.

The ltc2485 (as suggested by Guenter on that other thread) is pretty similar 
indeed, it might be feasible to join the two and have one driver handle them 
both. That would be preferred over a separat driver, right?


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@topicproducts.com
Website: www.topicproducts.com

Please consider the environment before printing this e-mail



--
To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Guenter Roeck June 29, 2017, 3:08 p.m. UTC | #6
On Thu, Jun 29, 2017 at 04:11:42PM +0200, Mike Looijmans wrote:
> On 29-06-17 15:40, Guenter Roeck wrote:
> >On 06/29/2017 05:30 AM, Lars-Peter Clausen wrote:
> >>On 06/29/2017 02:13 PM, Mike Looijmans wrote:
> >>>The LTC2741 and LTC2473 are single voltage monitoring chips. The LTC2473
> >>>is similar to the LTC2471 but outputs a signed differential value.
> >>>
> >>>Datasheet:
> >>>   http://cds.linear.com/docs/en/datasheet/24713fb.pdf
> >>
> >>This looks more like a general purpose ADC rather than a specialized voltage
> >>monitoring chip. In my opinion this driver would be better suited for the
> >>IIO framework. But before you start working on that lets try to hear what
> >>others think about this.
> >>
> >
> >Agreed (I just replied to that on another thread).
> 
> Took a peek at the IIO drivers, looks simple enough, so I suggest to drop
> this patch and I'll create an IIO driver instead.
> 
> The ltc2485 (as suggested by Guenter on that other thread) is pretty similar
> indeed, it might be feasible to join the two and have one driver handle them
> both. That would be preferred over a separat driver, right?
> 
In general yes, but it really depends on the similarities and differences. If
the single driver ends up twice as large, it is probably better to keep them
separate. If all you need to do is to add chip IDs, obviously a single driver
is better. Everything in between really depends on the amount of chip specific
code.

Thanks,
Guenter
--
To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Lars-Peter Clausen June 29, 2017, 3:30 p.m. UTC | #7
On 06/29/2017 04:11 PM, Mike Looijmans wrote:
> On 29-06-17 15:40, Guenter Roeck wrote:
>> On 06/29/2017 05:30 AM, Lars-Peter Clausen wrote:
>>> On 06/29/2017 02:13 PM, Mike Looijmans wrote:
>>>> The LTC2741 and LTC2473 are single voltage monitoring chips. The LTC2473
>>>> is similar to the LTC2471 but outputs a signed differential value.
>>>>
>>>> Datasheet:
>>>>    http://cds.linear.com/docs/en/datasheet/24713fb.pdf
>>>
>>> This looks more like a general purpose ADC rather than a specialized voltage
>>> monitoring chip. In my opinion this driver would be better suited for the
>>> IIO framework. But before you start working on that lets try to hear what
>>> others think about this.
>>>
>>
>> Agreed (I just replied to that on another thread).
> 
> Took a peek at the IIO drivers, looks simple enough, so I suggest to drop
> this patch and I'll create an IIO driver instead.
> 
> The ltc2485 (as suggested by Guenter on that other thread) is pretty similar
> indeed, it might be feasible to join the two and have one driver handle them
> both. That would be preferred over a separat driver, right?
> 

I think the ltc2485 is close enough. It's even listed in the datasheet as a
related part. The main difference seems to be the resolution of the ADC and
the extra temperature sensor channel.

Although one thing to consider is that their control/mode register is quite
different. But the current driver doesn't write it.

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

Patch

diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 5ef2814..5acaa2d 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -696,6 +696,16 @@  config SENSORS_LINEAGE
 	  This driver can also be built as a module.  If so, the module
 	  will be called lineage-pem.
 
+config SENSORS_LTC2471
+	tristate "Linear Technology LTC2471 and LTC2473"
+	depends on I2C
+	help
+	  If you say yes here you get support for Linear Technology LTC2471
+	  and LTC2473 voltage monitors.
+
+	  This driver can also be built as a module. If so, the module will
+	  be called ltc2471.
+
 config SENSORS_LTC2945
 	tristate "Linear Technology LTC2945"
 	depends on I2C
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index d4641a9..49badff 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -102,6 +102,7 @@  obj-$(CONFIG_SENSORS_LM93)	+= lm93.o
 obj-$(CONFIG_SENSORS_LM95234)	+= lm95234.o
 obj-$(CONFIG_SENSORS_LM95241)	+= lm95241.o
 obj-$(CONFIG_SENSORS_LM95245)	+= lm95245.o
+obj-$(CONFIG_SENSORS_LTC2471)	+= ltc2471.o
 obj-$(CONFIG_SENSORS_LTC2945)	+= ltc2945.o
 obj-$(CONFIG_SENSORS_LTC2990)	+= ltc2990.o
 obj-$(CONFIG_SENSORS_LTC4151)	+= ltc4151.o
diff --git a/drivers/hwmon/ltc2471.c b/drivers/hwmon/ltc2471.c
new file mode 100644
index 0000000..f77f7b5
--- /dev/null
+++ b/drivers/hwmon/ltc2471.c
@@ -0,0 +1,127 @@ 
+/*
+ * Driver for Linear Technology LTC2471 and LTC2473 voltage monitors
+ * The LTC2473 is identical to the 2471, but reports a differential signal.
+ *
+ * Copyright (C) 2017 Topic Embedded Products
+ * Author: Mike Looijmans <mike.looijmans@topic.nl>
+ *
+ * License: GPLv2
+ */
+
+#include <linux/err.h>
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
+#include <linux/i2c.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+
+enum chips {
+	ltc2471,
+	ltc2473
+};
+
+struct ltc2471_data {
+	struct i2c_client *i2c;
+	bool differential;
+};
+
+/* Reference voltage is 1.25V */
+#define LTC2471_VREF 1250
+
+/* Read two bytes from the I2C bus to obtain the ADC result */
+static int ltc2471_get_value(struct i2c_client *i2c)
+{
+	int ret;
+	__be16 buf;
+
+	ret = i2c_master_recv(i2c, (char *)&buf, 2);
+	if (ret < 0)
+		return ret;
+	if (ret != 2)
+		return -EIO;
+
+	/* MSB first */
+	return be16_to_cpu(buf);
+}
+
+static ssize_t ltc2471_show_value(struct device *dev,
+				  struct device_attribute *da, char *buf)
+{
+	struct ltc2471_data *data = dev_get_drvdata(dev);
+	int value;
+
+	value = ltc2471_get_value(data->i2c);
+	if (unlikely(value < 0))
+		return value;
+
+	if (data->differential)
+		/* Ranges from -VREF to +VREF with "0" at 0x8000 */
+		value = ((s32)LTC2471_VREF * (s32)(value - 0x8000)) >> 15;
+	else
+		/* Ranges from 0 to +VREF */
+		value = ((u32)LTC2471_VREF * (u32)value) >> 16;
+
+	return snprintf(buf, PAGE_SIZE, "%d\n", value);
+}
+
+static SENSOR_DEVICE_ATTR(in0_input, 0444, ltc2471_show_value, NULL, 0);
+
+static struct attribute *ltc2471_attrs[] = {
+	&sensor_dev_attr_in0_input.dev_attr.attr,
+	NULL
+};
+
+ATTRIBUTE_GROUPS(ltc2471);
+
+static int ltc2471_i2c_probe(struct i2c_client *i2c,
+			     const struct i2c_device_id *id)
+{
+	int ret;
+	struct device *hwmon_dev;
+	struct ltc2471_data *data;
+
+	if (!i2c_check_functionality(i2c->adapter, I2C_FUNC_I2C))
+		return -ENODEV;
+
+	data = devm_kzalloc(&i2c->dev, sizeof(*data), GFP_KERNEL);
+	if (unlikely(!data))
+		return -ENOMEM;
+
+	data->i2c = i2c;
+	data->differential = (id->driver_data == ltc2473);
+
+	/* Trigger once to start conversion and check if chip is there */
+	ret = ltc2471_get_value(i2c);
+	if (ret < 0) {
+		dev_err(&i2c->dev, "Cannot read from device.\n");
+		return ret;
+	}
+
+	hwmon_dev = devm_hwmon_device_register_with_groups(&i2c->dev,
+							   i2c->name,
+							   data,
+							   ltc2471_groups);
+
+	return PTR_ERR_OR_ZERO(hwmon_dev);
+}
+
+static const struct i2c_device_id ltc2471_i2c_id[] = {
+	{ "ltc2471", ltc2471 },
+	{ "ltc2473", ltc2473 },
+	{}
+};
+MODULE_DEVICE_TABLE(i2c, ltc2471_i2c_id);
+
+static struct i2c_driver ltc2471_i2c_driver = {
+	.driver = {
+		.name = "ltc2471",
+	},
+	.probe    = ltc2471_i2c_probe,
+	.id_table = ltc2471_i2c_id,
+};
+
+module_i2c_driver(ltc2471_i2c_driver);
+
+MODULE_DESCRIPTION("LTC2471/LTC2473 Sensor Driver");
+MODULE_AUTHOR("Topic Embedded Products");
+MODULE_LICENSE("GPL v2");