[lm-sensors] [PATCH] hwmon: Add support for Texas Instruments

[lm-sensors] [PATCH] hwmon: Add support for Texas Instruments

[Dirk Eibach]

Signed-off-by: Dirk Eibach <eibach@gdsys.de>
---
 Documentation/hwmon/ads1015 |   31 +++++
 drivers/hwmon/Kconfig       |   10 ++
 drivers/hwmon/Makefile      |    1 +
 drivers/hwmon/ads1015.c     |  269 +++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 311 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/hwmon/ads1015
 create mode 100644 drivers/hwmon/ads1015.c

diff --git a/Documentation/hwmon/ads1015 b/Documentation/hwmon/ads1015
new file mode 100644
index 0000000..3772816
--- /dev/null
+++ b/Documentation/hwmon/ads1015
@@ -0,0 +1,31 @@
+Kernel driver ads1015
+==========+
+Supported chips:
+  * Texas Instruments ADS1015
+    Prefix: 'ads1015'
+    Addresses scanned: I2C 0x48, 0x49, 0x4a, 0x4b
+    Datasheet: Publicly available at the Texas Instruments website :
+               http://focus.ti.com/lit/ds/symlink/ads1015.pdf
+
+Authors:
+        Dirk Eibach, Guntermann & Drunck GmbH <eibach@gdsys.de>
+
+Description
+-----------
+
+This driver implements support for the Texas Instruments ADS1015.
+
+This device is a 12-bit A-D converter with 4 inputs.
+
+The inputs can be used single ended or in certain differential combinations.
+
+The driver offers access to all available combinations by 7 "virtual" inputs:
+in0: Voltage over AIN0 and AIN1.
+in1: Voltage over AIN0 and AIN3.
+in2: Voltage over AIN1 and AIN3.
+in3: Voltage over AIN2 and AIN3.
+in4: Voltage over AIN0 and GND.
+in5: Voltage over AIN1 and GND.
+in6: Voltage over AIN2 and GND.
+in7: Voltage over AIN3 and GND.
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 773e484..7e247f7 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -871,6 +871,16 @@ config SENSORS_SMSC47B397
 	  This driver can also be built as a module.  If so, the module
 	  will be called smsc47b397.
 
+config SENSORS_ADS1015
+	tristate "Texas Instruments ADS1015"
+	depends on I2C
+	help
+	  If you say yes here you get support for Texas Instruments ADS1015
+	  12-bit 4-input ADC device.
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called ads1015.
+
 config SENSORS_ADS7828
 	tristate "Texas Instruments ADS7828"
 	depends on I2C
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index dde02d9..aae4036 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -29,6 +29,7 @@ obj-$(CONFIG_SENSORS_ADM1026)	+= adm1026.o
 obj-$(CONFIG_SENSORS_ADM1029)	+= adm1029.o
 obj-$(CONFIG_SENSORS_ADM1031)	+= adm1031.o
 obj-$(CONFIG_SENSORS_ADM9240)	+= adm9240.o
+obj-$(CONFIG_SENSORS_ADS1015)	+= ads1015.o
 obj-$(CONFIG_SENSORS_ADS7828)	+= ads7828.o
 obj-$(CONFIG_SENSORS_ADS7871)	+= ads7871.o
 obj-$(CONFIG_SENSORS_ADT7411)	+= adt7411.o
diff --git a/drivers/hwmon/ads1015.c b/drivers/hwmon/ads1015.c
new file mode 100644
index 0000000..675fdfe
--- /dev/null
+++ b/drivers/hwmon/ads1015.c
@@ -0,0 +1,269 @@
+/*
+ * ads1015.c - lm_sensors driver for ads1015 12-bit 4-input ADC
+ * (C) Copyright 2010
+ * Dirk Eibach, Guntermann & Drunck GmbH <eibach@gdsys.de>
+ *
+ * Based on the ads7828 driver by Steve Hardy.
+ *
+ * Datasheet available at: http://focus.ti.com/lit/ds/symlink/ads1015.pdf
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/jiffies.h>
+#include <linux/i2c.h>
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
+#include <linux/err.h>
+#include <linux/mutex.h>
+
+/* ADS1015 registers */
+enum {
+	ADS1015_CONVERSION = 0,
+	ADS1015_CONFIG = 1,
+	ADS1015_LO_THRESH = 2,
+	ADS1015_HI_THRESH = 3,
+};
+
+/* PGA fullscale voltages */
+static const unsigned int fullscale_table[] = {
+	6144, 4096, 2048, 1024, 512, 256, 256, 256 };
+
+/* Addresses to scan */
+static const unsigned short normal_i2c[] = { 0x48, 0x49, 0x4a, 0x4b,
+	I2C_CLIENT_END };
+
+/* Insmod parameters */
+I2C_CLIENT_INSMOD_1(ads1015);
+
+/* Each client has this additional data */
+struct ads1015_data {
+	struct device *hwmon_dev;
+	struct mutex update_lock; /* mutex protect updates */
+};
+
+/* Function declaration - necessary due to function dependencies */
+static int ads1015_detect(struct i2c_client *client, int kind,
+			  struct i2c_board_info *info);
+static int ads1015_probe(struct i2c_client *client,
+			 const struct i2c_device_id *id);
+
+
+static s32 ads1015_read_reg(struct i2c_client *client, unsigned int reg)
+{
+	return swab16(i2c_smbus_read_word_data(client, reg));
+}
+
+static s32 ads1015_write_reg(struct i2c_client *client, unsigned int reg,
+			     u16 val)
+{
+	return i2c_smbus_write_word_data(client, reg, swab16(val));
+}
+
+static int ads1015_read_value(struct i2c_client *client, unsigned int channel)
+{
+	u16 config;
+	s16 conversion;
+	unsigned int pga;
+	int fullscale;
+	unsigned int k;
+
+	/* get fullscale voltage */
+	config = ads1015_read_reg(client, ADS1015_CONFIG);
+	pga = (config >> 9) & 0x0007;
+	fullscale = fullscale_table[pga];
+
+	/* set channel and start single conversion */
+	config &= ~(0x0007 << 12);
+	config &= ~(0x0001 << 8);
+	config |= (1 << 15) | (1 << 8) | (channel & 0x0007) << 12;
+
+	/* wait until conversion finished */
+	ads1015_write_reg(client, ADS1015_CONFIG, config);
+	for (k = 0; k < 5; ++k) {
+		config = ads1015_read_reg(client, ADS1015_CONFIG);
+		if (config & (1 << 15))
+			break;
+		schedule_timeout(msecs_to_jiffies(1));
+	}
+
+	conversion = ads1015_read_reg(client, ADS1015_CONVERSION);
+
+	return conversion * fullscale / 0x7ff0;
+}
+
+/* sysfs callback function */
+static ssize_t show_in(struct device *dev, struct device_attribute *da,
+	char *buf)
+{
+	struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
+	struct i2c_client *client = to_i2c_client(dev);
+	struct ads1015_data *data = i2c_get_clientdata(client);
+	int in;
+
+	mutex_lock(&data->update_lock);
+	in = ads1015_read_value(client, attr->index);
+	mutex_unlock(&data->update_lock);
+
+	return sprintf(buf, "%d\n", in);
+}
+
+#define in_reg(offset)\
+static SENSOR_DEVICE_ATTR(in##offset##_input, S_IRUGO, show_in,\
+	NULL, offset)
+
+in_reg(0);
+in_reg(1);
+in_reg(2);
+in_reg(3);
+in_reg(4);
+in_reg(5);
+in_reg(6);
+in_reg(7);
+
+static struct attribute *ads1015_attributes[] = {
+	&sensor_dev_attr_in0_input.dev_attr.attr,
+	&sensor_dev_attr_in1_input.dev_attr.attr,
+	&sensor_dev_attr_in2_input.dev_attr.attr,
+	&sensor_dev_attr_in3_input.dev_attr.attr,
+	&sensor_dev_attr_in4_input.dev_attr.attr,
+	&sensor_dev_attr_in5_input.dev_attr.attr,
+	&sensor_dev_attr_in6_input.dev_attr.attr,
+	&sensor_dev_attr_in7_input.dev_attr.attr,
+	NULL
+};
+
+static const struct attribute_group ads1015_group = {
+	.attrs = ads1015_attributes,
+};
+
+static int ads1015_remove(struct i2c_client *client)
+{
+	struct ads1015_data *data = i2c_get_clientdata(client);
+	hwmon_device_unregister(data->hwmon_dev);
+	sysfs_remove_group(&client->dev.kobj, &ads1015_group);
+	kfree(i2c_get_clientdata(client));
+	return 0;
+}
+
+static const struct i2c_device_id ads1015_id[] = {
+	{ "ads1015", ads1015 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, ads1015_id);
+
+/* This is the driver that will be inserted */
+static struct i2c_driver ads1015_driver = {
+	.class = I2C_CLASS_HWMON,
+	.driver = {
+		.name = "ads1015",
+	},
+	.probe = ads1015_probe,
+	.remove = ads1015_remove,
+	.id_table = ads1015_id,
+	.detect = ads1015_detect,
+	.address_data = &addr_data,
+};
+
+/* Return 0 if detection is successful, -ENODEV otherwise */
+static int ads1015_detect(struct i2c_client *client, int kind,
+			  struct i2c_board_info *info)
+{
+	struct i2c_adapter *adapter = client->adapter;
+
+	/* Check we have a valid client */
+	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_READ_WORD_DATA))
+		return -ENODEV;
+
+	/* Now, we do the remaining detection. There is no identification
+	dedicated register so attempt to sanity check using knowledge of
+	the chip
+	- Read from the 8 channels
+	- Check the bits 0-3 of each result are not set (12 data bits)
+	*/
+	if (kind < 0) {
+		int ch;
+		for (ch = 0; ch < 8; ++ch) {
+			u16 in_data;
+			in_data = ads1015_read_value(client, ch);
+			if (in_data & 0x000F) {
+				printk(KERN_DEBUG
+				"%s : Doesn't look like an ads1015 device\n",
+				__func__);
+				return -ENODEV;
+			}
+		}
+	}
+
+	strlcpy(info->type, "ads1015", I2C_NAME_SIZE);
+
+	return 0;
+}
+
+static int ads1015_probe(struct i2c_client *client,
+			 const struct i2c_device_id *id)
+{
+	struct ads1015_data *data;
+	int err;
+
+	data = kzalloc(sizeof(struct ads1015_data), GFP_KERNEL);
+	if (!data) {
+		err = -ENOMEM;
+		goto exit;
+	}
+
+	i2c_set_clientdata(client, data);
+	mutex_init(&data->update_lock);
+
+	/* Register sysfs hooks */
+	err = sysfs_create_group(&client->dev.kobj, &ads1015_group);
+	if (err)
+		goto exit_free;
+
+	data->hwmon_dev = hwmon_device_register(&client->dev);
+	if (IS_ERR(data->hwmon_dev)) {
+		err = PTR_ERR(data->hwmon_dev);
+		goto exit_remove;
+	}
+
+	return 0;
+
+exit_remove:
+	sysfs_remove_group(&client->dev.kobj, &ads1015_group);
+exit_free:
+	kfree(data);
+exit:
+	return err;
+}
+
+static int __init sensors_ads1015_init(void)
+{
+	return i2c_add_driver(&ads1015_driver);
+}
+
+static void __exit sensors_ads1015_exit(void)
+{
+	i2c_del_driver(&ads1015_driver);
+}
+
+MODULE_AUTHOR("Dirk Eibach <eibach@gdsys.de>");
+MODULE_DESCRIPTION("ADS1015 driver");
+MODULE_LICENSE("GPL");
+
+module_init(sensors_ads1015_init);
+module_exit(sensors_ads1015_exit);
-- 
1.5.6.5


_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

[PATCH] hwmon: Add support for Texas Instruments ADS1015

[Dirk Eibach]

Signed-off-by: Dirk Eibach <eibach@gdsys.de>
---
 Documentation/hwmon/ads1015 |   31 +++++
 drivers/hwmon/Kconfig       |   10 ++
 drivers/hwmon/Makefile      |    1 +
 drivers/hwmon/ads1015.c     |  269 +++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 311 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/hwmon/ads1015
 create mode 100644 drivers/hwmon/ads1015.c

diff --git a/Documentation/hwmon/ads1015 b/Documentation/hwmon/ads1015
new file mode 100644
index 0000000..3772816
--- /dev/null
+++ b/Documentation/hwmon/ads1015
@@ -0,0 +1,31 @@
+Kernel driver ads1015
+=====================
+
+Supported chips:
+  * Texas Instruments ADS1015
+    Prefix: 'ads1015'
+    Addresses scanned: I2C 0x48, 0x49, 0x4a, 0x4b
+    Datasheet: Publicly available at the Texas Instruments website :
+               http://focus.ti.com/lit/ds/symlink/ads1015.pdf
+
+Authors:
+        Dirk Eibach, Guntermann & Drunck GmbH <eibach@gdsys.de>
+
+Description
+-----------
+
+This driver implements support for the Texas Instruments ADS1015.
+
+This device is a 12-bit A-D converter with 4 inputs.
+
+The inputs can be used single ended or in certain differential combinations.
+
+The driver offers access to all available combinations by 7 "virtual" inputs:
+in0: Voltage over AIN0 and AIN1.
+in1: Voltage over AIN0 and AIN3.
+in2: Voltage over AIN1 and AIN3.
+in3: Voltage over AIN2 and AIN3.
+in4: Voltage over AIN0 and GND.
+in5: Voltage over AIN1 and GND.
+in6: Voltage over AIN2 and GND.
+in7: Voltage over AIN3 and GND.
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 773e484..7e247f7 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -871,6 +871,16 @@ config SENSORS_SMSC47B397
 	  This driver can also be built as a module.  If so, the module
 	  will be called smsc47b397.
 
+config SENSORS_ADS1015
+	tristate "Texas Instruments ADS1015"
+	depends on I2C
+	help
+	  If you say yes here you get support for Texas Instruments ADS1015
+	  12-bit 4-input ADC device.
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called ads1015.
+
 config SENSORS_ADS7828
 	tristate "Texas Instruments ADS7828"
 	depends on I2C
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index dde02d9..aae4036 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -29,6 +29,7 @@ obj-$(CONFIG_SENSORS_ADM1026)	+= adm1026.o
 obj-$(CONFIG_SENSORS_ADM1029)	+= adm1029.o
 obj-$(CONFIG_SENSORS_ADM1031)	+= adm1031.o
 obj-$(CONFIG_SENSORS_ADM9240)	+= adm9240.o
+obj-$(CONFIG_SENSORS_ADS1015)	+= ads1015.o
 obj-$(CONFIG_SENSORS_ADS7828)	+= ads7828.o
 obj-$(CONFIG_SENSORS_ADS7871)	+= ads7871.o
 obj-$(CONFIG_SENSORS_ADT7411)	+= adt7411.o
diff --git a/drivers/hwmon/ads1015.c b/drivers/hwmon/ads1015.c
new file mode 100644
index 0000000..675fdfe
--- /dev/null
+++ b/drivers/hwmon/ads1015.c
@@ -0,0 +1,269 @@
+/*
+ * ads1015.c - lm_sensors driver for ads1015 12-bit 4-input ADC
+ * (C) Copyright 2010
+ * Dirk Eibach, Guntermann & Drunck GmbH <eibach@gdsys.de>
+ *
+ * Based on the ads7828 driver by Steve Hardy.
+ *
+ * Datasheet available at: http://focus.ti.com/lit/ds/symlink/ads1015.pdf
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/jiffies.h>
+#include <linux/i2c.h>
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
+#include <linux/err.h>
+#include <linux/mutex.h>
+
+/* ADS1015 registers */
+enum {
+	ADS1015_CONVERSION = 0,
+	ADS1015_CONFIG = 1,
+	ADS1015_LO_THRESH = 2,
+	ADS1015_HI_THRESH = 3,
+};
+
+/* PGA fullscale voltages */
+static const unsigned int fullscale_table[] = {
+	6144, 4096, 2048, 1024, 512, 256, 256, 256 };
+
+/* Addresses to scan */
+static const unsigned short normal_i2c[] = { 0x48, 0x49, 0x4a, 0x4b,
+	I2C_CLIENT_END };
+
+/* Insmod parameters */
+I2C_CLIENT_INSMOD_1(ads1015);
+
+/* Each client has this additional data */
+struct ads1015_data {
+	struct device *hwmon_dev;
+	struct mutex update_lock; /* mutex protect updates */
+};
+
+/* Function declaration - necessary due to function dependencies */
+static int ads1015_detect(struct i2c_client *client, int kind,
+			  struct i2c_board_info *info);
+static int ads1015_probe(struct i2c_client *client,
+			 const struct i2c_device_id *id);
+
+
+static s32 ads1015_read_reg(struct i2c_client *client, unsigned int reg)
+{
+	return swab16(i2c_smbus_read_word_data(client, reg));
+}
+
+static s32 ads1015_write_reg(struct i2c_client *client, unsigned int reg,
+			     u16 val)
+{
+	return i2c_smbus_write_word_data(client, reg, swab16(val));
+}
+
+static int ads1015_read_value(struct i2c_client *client, unsigned int channel)
+{
+	u16 config;
+	s16 conversion;
+	unsigned int pga;
+	int fullscale;
+	unsigned int k;
+
+	/* get fullscale voltage */
+	config = ads1015_read_reg(client, ADS1015_CONFIG);
+	pga = (config >> 9) & 0x0007;
+	fullscale = fullscale_table[pga];
+
+	/* set channel and start single conversion */
+	config &= ~(0x0007 << 12);
+	config &= ~(0x0001 << 8);
+	config |= (1 << 15) | (1 << 8) | (channel & 0x0007) << 12;
+
+	/* wait until conversion finished */
+	ads1015_write_reg(client, ADS1015_CONFIG, config);
+	for (k = 0; k < 5; ++k) {
+		config = ads1015_read_reg(client, ADS1015_CONFIG);
+		if (config & (1 << 15))
+			break;
+		schedule_timeout(msecs_to_jiffies(1));
+	}
+
+	conversion = ads1015_read_reg(client, ADS1015_CONVERSION);
+
+	return conversion * fullscale / 0x7ff0;
+}
+
+/* sysfs callback function */
+static ssize_t show_in(struct device *dev, struct device_attribute *da,
+	char *buf)
+{
+	struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
+	struct i2c_client *client = to_i2c_client(dev);
+	struct ads1015_data *data = i2c_get_clientdata(client);
+	int in;
+
+	mutex_lock(&data->update_lock);
+	in = ads1015_read_value(client, attr->index);
+	mutex_unlock(&data->update_lock);
+
+	return sprintf(buf, "%d\n", in);
+}
+
+#define in_reg(offset)\
+static SENSOR_DEVICE_ATTR(in##offset##_input, S_IRUGO, show_in,\
+	NULL, offset)
+
+in_reg(0);
+in_reg(1);
+in_reg(2);
+in_reg(3);
+in_reg(4);
+in_reg(5);
+in_reg(6);
+in_reg(7);
+
+static struct attribute *ads1015_attributes[] = {
+	&sensor_dev_attr_in0_input.dev_attr.attr,
+	&sensor_dev_attr_in1_input.dev_attr.attr,
+	&sensor_dev_attr_in2_input.dev_attr.attr,
+	&sensor_dev_attr_in3_input.dev_attr.attr,
+	&sensor_dev_attr_in4_input.dev_attr.attr,
+	&sensor_dev_attr_in5_input.dev_attr.attr,
+	&sensor_dev_attr_in6_input.dev_attr.attr,
+	&sensor_dev_attr_in7_input.dev_attr.attr,
+	NULL
+};
+
+static const struct attribute_group ads1015_group = {
+	.attrs = ads1015_attributes,
+};
+
+static int ads1015_remove(struct i2c_client *client)
+{
+	struct ads1015_data *data = i2c_get_clientdata(client);
+	hwmon_device_unregister(data->hwmon_dev);
+	sysfs_remove_group(&client->dev.kobj, &ads1015_group);
+	kfree(i2c_get_clientdata(client));
+	return 0;
+}
+
+static const struct i2c_device_id ads1015_id[] = {
+	{ "ads1015", ads1015 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, ads1015_id);
+
+/* This is the driver that will be inserted */
+static struct i2c_driver ads1015_driver = {
+	.class = I2C_CLASS_HWMON,
+	.driver = {
+		.name = "ads1015",
+	},
+	.probe = ads1015_probe,
+	.remove = ads1015_remove,
+	.id_table = ads1015_id,
+	.detect = ads1015_detect,
+	.address_data = &addr_data,
+};
+
+/* Return 0 if detection is successful, -ENODEV otherwise */
+static int ads1015_detect(struct i2c_client *client, int kind,
+			  struct i2c_board_info *info)
+{
+	struct i2c_adapter *adapter = client->adapter;
+
+	/* Check we have a valid client */
+	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_READ_WORD_DATA))
+		return -ENODEV;
+
+	/* Now, we do the remaining detection. There is no identification
+	dedicated register so attempt to sanity check using knowledge of
+	the chip
+	- Read from the 8 channels
+	- Check the bits 0-3 of each result are not set (12 data bits)
+	*/
+	if (kind < 0) {
+		int ch;
+		for (ch = 0; ch < 8; ++ch) {
+			u16 in_data;
+			in_data = ads1015_read_value(client, ch);
+			if (in_data & 0x000F) {
+				printk(KERN_DEBUG
+				"%s : Doesn't look like an ads1015 device\n",
+				__func__);
+				return -ENODEV;
+			}
+		}
+	}
+
+	strlcpy(info->type, "ads1015", I2C_NAME_SIZE);
+
+	return 0;
+}
+
+static int ads1015_probe(struct i2c_client *client,
+			 const struct i2c_device_id *id)
+{
+	struct ads1015_data *data;
+	int err;
+
+	data = kzalloc(sizeof(struct ads1015_data), GFP_KERNEL);
+	if (!data) {
+		err = -ENOMEM;
+		goto exit;
+	}
+
+	i2c_set_clientdata(client, data);
+	mutex_init(&data->update_lock);
+
+	/* Register sysfs hooks */
+	err = sysfs_create_group(&client->dev.kobj, &ads1015_group);
+	if (err)
+		goto exit_free;
+
+	data->hwmon_dev = hwmon_device_register(&client->dev);
+	if (IS_ERR(data->hwmon_dev)) {
+		err = PTR_ERR(data->hwmon_dev);
+		goto exit_remove;
+	}
+
+	return 0;
+
+exit_remove:
+	sysfs_remove_group(&client->dev.kobj, &ads1015_group);
+exit_free:
+	kfree(data);
+exit:
+	return err;
+}
+
+static int __init sensors_ads1015_init(void)
+{
+	return i2c_add_driver(&ads1015_driver);
+}
+
+static void __exit sensors_ads1015_exit(void)
+{
+	i2c_del_driver(&ads1015_driver);
+}
+
+MODULE_AUTHOR("Dirk Eibach <eibach@gdsys.de>");
+MODULE_DESCRIPTION("ADS1015 driver");
+MODULE_LICENSE("GPL");
+
+module_init(sensors_ads1015_init);
+module_exit(sensors_ads1015_exit);
-- 
1.5.6.5

Re: [lm-sensors] [PATCH] hwmon: Add support for Texas Instruments

[Jean Delvare]

Hi Dirk,

On Mon, 14 Feb 2011 10:26:21 +0100, Dirk Eibach wrote:
> Signed-off-by: Dirk Eibach <eibach@gdsys.de>
> ---
>  Documentation/hwmon/ads1015 |   31 +++++
>  drivers/hwmon/Kconfig       |   10 ++
>  drivers/hwmon/Makefile      |    1 +
>  drivers/hwmon/ads1015.c     |  269 +++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 311 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/hwmon/ads1015
>  create mode 100644 drivers/hwmon/ads1015.c
> 
> diff --git a/Documentation/hwmon/ads1015 b/Documentation/hwmon/ads1015
> new file mode 100644
> index 0000000..3772816
> --- /dev/null
> +++ b/Documentation/hwmon/ads1015
> @@ -0,0 +1,31 @@
> +Kernel driver ads1015
> +==========> +
> +Supported chips:
> +  * Texas Instruments ADS1015
> +    Prefix: 'ads1015'
> +    Addresses scanned: I2C 0x48, 0x49, 0x4a, 0x4b
> +    Datasheet: Publicly available at the Texas Instruments website :
> +               http://focus.ti.com/lit/ds/symlink/ads1015.pdf
> +
> +Authors:
> +        Dirk Eibach, Guntermann & Drunck GmbH <eibach@gdsys.de>
> +
> +Description
> +-----------
> +
> +This driver implements support for the Texas Instruments ADS1015.
> +
> +This device is a 12-bit A-D converter with 4 inputs.
> +
> +The inputs can be used single ended or in certain differential combinations.
> +
> +The driver offers access to all available combinations by 7 "virtual" inputs:

I count 8.

> +in0: Voltage over AIN0 and AIN1.
> +in1: Voltage over AIN0 and AIN3.
> +in2: Voltage over AIN1 and AIN3.
> +in3: Voltage over AIN2 and AIN3.
> +in4: Voltage over AIN0 and GND.
> +in5: Voltage over AIN1 and GND.
> +in6: Voltage over AIN2 and GND.
> +in7: Voltage over AIN3 and GND.

This seems wrong. All 8 attributes can't possibly report sane values
for a given hardware setup, right? I think it would be much better to
have the platform provide setup data to the driver, telling it how the
chip is used and which input configurations should be exposed to
user-space.

> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 773e484..7e247f7 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -871,6 +871,16 @@ config SENSORS_SMSC47B397
>  	  This driver can also be built as a module.  If so, the module
>  	  will be called smsc47b397.
>  
> +config SENSORS_ADS1015
> +	tristate "Texas Instruments ADS1015"
> +	depends on I2C
> +	help
> +	  If you say yes here you get support for Texas Instruments ADS1015
> +	  12-bit 4-input ADC device.
> +
> +	  This driver can also be built as a module.  If so, the module
> +	  will be called ads1015.
> +
>  config SENSORS_ADS7828
>  	tristate "Texas Instruments ADS7828"
>  	depends on I2C
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index dde02d9..aae4036 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -29,6 +29,7 @@ obj-$(CONFIG_SENSORS_ADM1026)	+= adm1026.o
>  obj-$(CONFIG_SENSORS_ADM1029)	+= adm1029.o
>  obj-$(CONFIG_SENSORS_ADM1031)	+= adm1031.o
>  obj-$(CONFIG_SENSORS_ADM9240)	+= adm9240.o
> +obj-$(CONFIG_SENSORS_ADS1015)	+= ads1015.o
>  obj-$(CONFIG_SENSORS_ADS7828)	+= ads7828.o
>  obj-$(CONFIG_SENSORS_ADS7871)	+= ads7871.o
>  obj-$(CONFIG_SENSORS_ADT7411)	+= adt7411.o
> diff --git a/drivers/hwmon/ads1015.c b/drivers/hwmon/ads1015.c
> new file mode 100644
> index 0000000..675fdfe
> --- /dev/null
> +++ b/drivers/hwmon/ads1015.c
> @@ -0,0 +1,269 @@
> +/*
> + * ads1015.c - lm_sensors driver for ads1015 12-bit 4-input ADC
> + * (C) Copyright 2010
> + * Dirk Eibach, Guntermann & Drunck GmbH <eibach@gdsys.de>
> + *
> + * Based on the ads7828 driver by Steve Hardy.
> + *
> + * Datasheet available at: http://focus.ti.com/lit/ds/symlink/ads1015.pdf
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/slab.h>
> +#include <linux/jiffies.h>
> +#include <linux/i2c.h>
> +#include <linux/hwmon.h>
> +#include <linux/hwmon-sysfs.h>
> +#include <linux/err.h>
> +#include <linux/mutex.h>
> +
> +/* ADS1015 registers */
> +enum {
> +	ADS1015_CONVERSION = 0,
> +	ADS1015_CONFIG = 1,
> +	ADS1015_LO_THRESH = 2,
> +	ADS1015_HI_THRESH = 3,
> +};

You don't use the last two values anywhere.

> +
> +/* PGA fullscale voltages */
> +static const unsigned int fullscale_table[] = {
> +	6144, 4096, 2048, 1024, 512, 256, 256, 256 };

You'd rather hard-code the table size, as the rest of the code makes
assumptions on it. Please also add a comment stating the unit in which
these constants are expressed. I hope these are mV.

> +
> +/* Addresses to scan */
> +static const unsigned short normal_i2c[] = { 0x48, 0x49, 0x4a, 0x4b,
> +	I2C_CLIENT_END };
> +
> +/* Insmod parameters */
> +I2C_CLIENT_INSMOD_1(ads1015);

This macro was removed in kernel 2.6.33, almost one year ago. Please
provide patches which apply and build on Linus' latest kernel, or the
latest stable kernel at least.

> +
> +/* Each client has this additional data */
> +struct ads1015_data {
> +	struct device *hwmon_dev;
> +	struct mutex update_lock; /* mutex protect updates */
> +};
> +
> +/* Function declaration - necessary due to function dependencies */

No, not necessary at all. Just put the functions and driver declaration
in the right order.

> +static int ads1015_detect(struct i2c_client *client, int kind,
> +			  struct i2c_board_info *info);
> +static int ads1015_probe(struct i2c_client *client,
> +			 const struct i2c_device_id *id);
> +
> +
> +static s32 ads1015_read_reg(struct i2c_client *client, unsigned int reg)
> +{
> +	return swab16(i2c_smbus_read_word_data(client, reg));

This is wrong. If i2c_smbus_read_word_data() returns an error, your
function will return crap.

> +}
> +
> +static s32 ads1015_write_reg(struct i2c_client *client, unsigned int reg,
> +			     u16 val)
> +{
> +	return i2c_smbus_write_word_data(client, reg, swab16(val));
> +}
> +
> +static int ads1015_read_value(struct i2c_client *client, unsigned int channel)

Please either document the locking requirements of this function, or
move locking into it.

> +{
> +	u16 config;
> +	s16 conversion;
> +	unsigned int pga;
> +	int fullscale;
> +	unsigned int k;
> +
> +	/* get fullscale voltage */
> +	config = ads1015_read_reg(client, ADS1015_CONFIG);
> +	pga = (config >> 9) & 0x0007;
> +	fullscale = fullscale_table[pga];
> +
> +	/* set channel and start single conversion */
> +	config &= ~(0x0007 << 12);
> +	config &= ~(0x0001 << 8);

What's the point of clearing a bit you'll set again immediately?

> +	config |= (1 << 15) | (1 << 8) | (channel & 0x0007) << 12;
> +
> +	/* wait until conversion finished */
> +	ads1015_write_reg(client, ADS1015_CONFIG, config);
> +	for (k = 0; k < 5; ++k) {
> +		config = ads1015_read_reg(client, ADS1015_CONFIG);

What is the expected conversion time? Does it really make sense to
attempt a register read right away?

> +		if (config & (1 << 15))
> +			break;
> +		schedule_timeout(msecs_to_jiffies(1));
> +	}

What if k = 5? The conversion did not complete, and you return crap?

> +
> +	conversion = ads1015_read_reg(client, ADS1015_CONVERSION);
> +
> +	return conversion * fullscale / 0x7ff0;

Maybe it would make sense to use DIV_ROUND_CLOSEST?

> +}
> +
> +/* sysfs callback function */
> +static ssize_t show_in(struct device *dev, struct device_attribute *da,
> +	char *buf)
> +{
> +	struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
> +	struct i2c_client *client = to_i2c_client(dev);
> +	struct ads1015_data *data = i2c_get_clientdata(client);
> +	int in;
> +
> +	mutex_lock(&data->update_lock);
> +	in = ads1015_read_value(client, attr->index);
> +	mutex_unlock(&data->update_lock);
> +
> +	return sprintf(buf, "%d\n", in);
> +}
> +
> +#define in_reg(offset)\
> +static SENSOR_DEVICE_ATTR(in##offset##_input, S_IRUGO, show_in,\
> +	NULL, offset)
> +
> +in_reg(0);
> +in_reg(1);
> +in_reg(2);
> +in_reg(3);
> +in_reg(4);
> +in_reg(5);
> +in_reg(6);
> +in_reg(7);
> +
> +static struct attribute *ads1015_attributes[] = {
> +	&sensor_dev_attr_in0_input.dev_attr.attr,
> +	&sensor_dev_attr_in1_input.dev_attr.attr,
> +	&sensor_dev_attr_in2_input.dev_attr.attr,
> +	&sensor_dev_attr_in3_input.dev_attr.attr,
> +	&sensor_dev_attr_in4_input.dev_attr.attr,
> +	&sensor_dev_attr_in5_input.dev_attr.attr,
> +	&sensor_dev_attr_in6_input.dev_attr.attr,
> +	&sensor_dev_attr_in7_input.dev_attr.attr,
> +	NULL
> +};
> +
> +static const struct attribute_group ads1015_group = {
> +	.attrs = ads1015_attributes,
> +};
> +
> +static int ads1015_remove(struct i2c_client *client)
> +{
> +	struct ads1015_data *data = i2c_get_clientdata(client);
> +	hwmon_device_unregister(data->hwmon_dev);
> +	sysfs_remove_group(&client->dev.kobj, &ads1015_group);
> +	kfree(i2c_get_clientdata(client));

Please just use "data".

> +	return 0;
> +}
> +
> +static const struct i2c_device_id ads1015_id[] = {
> +	{ "ads1015", ads1015 },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, ads1015_id);
> +
> +/* This is the driver that will be inserted */
> +static struct i2c_driver ads1015_driver = {
> +	.class = I2C_CLASS_HWMON,
> +	.driver = {
> +		.name = "ads1015",
> +	},
> +	.probe = ads1015_probe,
> +	.remove = ads1015_remove,
> +	.id_table = ads1015_id,
> +	.detect = ads1015_detect,
> +	.address_data = &addr_data,
> +};
> +
> +/* Return 0 if detection is successful, -ENODEV otherwise */
> +static int ads1015_detect(struct i2c_client *client, int kind,
> +			  struct i2c_board_info *info)
> +{
> +	struct i2c_adapter *adapter = client->adapter;
> +
> +	/* Check we have a valid client */
> +	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_READ_WORD_DATA))
> +		return -ENODEV;
> +
> +	/* Now, we do the remaining detection. There is no identification
> +	dedicated register so attempt to sanity check using knowledge of
> +	the chip
> +	- Read from the 8 channels
> +	- Check the bits 0-3 of each result are not set (12 data bits)
> +	*/
> +	if (kind < 0) {
> +		int ch;
> +		for (ch = 0; ch < 8; ++ch) {
> +			u16 in_data;
> +			in_data = ads1015_read_value(client, ch);
> +			if (in_data & 0x000F) {
> +				printk(KERN_DEBUG
> +				"%s : Doesn't look like an ads1015 device\n",
> +				__func__);
> +				return -ENODEV;
> +			}
> +		}
> +	}
> +
> +	strlcpy(info->type, "ads1015", I2C_NAME_SIZE);
> +
> +	return 0;
> +}

Your device is obviously not easily and reliably detectable, so please
just don't provide a detect function. It's prohibited to write to the
device in detection functions anyway, and you do exactly this.

> +
> +static int ads1015_probe(struct i2c_client *client,
> +			 const struct i2c_device_id *id)
> +{
> +	struct ads1015_data *data;
> +	int err;
> +
> +	data = kzalloc(sizeof(struct ads1015_data), GFP_KERNEL);
> +	if (!data) {
> +		err = -ENOMEM;
> +		goto exit;
> +	}
> +
> +	i2c_set_clientdata(client, data);
> +	mutex_init(&data->update_lock);
> +
> +	/* Register sysfs hooks */
> +	err = sysfs_create_group(&client->dev.kobj, &ads1015_group);
> +	if (err)
> +		goto exit_free;
> +
> +	data->hwmon_dev = hwmon_device_register(&client->dev);
> +	if (IS_ERR(data->hwmon_dev)) {
> +		err = PTR_ERR(data->hwmon_dev);
> +		goto exit_remove;
> +	}
> +
> +	return 0;
> +
> +exit_remove:
> +	sysfs_remove_group(&client->dev.kobj, &ads1015_group);
> +exit_free:
> +	kfree(data);
> +exit:
> +	return err;
> +}
> +
> +static int __init sensors_ads1015_init(void)
> +{
> +	return i2c_add_driver(&ads1015_driver);
> +}
> +
> +static void __exit sensors_ads1015_exit(void)
> +{
> +	i2c_del_driver(&ads1015_driver);
> +}
> +
> +MODULE_AUTHOR("Dirk Eibach <eibach@gdsys.de>");
> +MODULE_DESCRIPTION("ADS1015 driver");
> +MODULE_LICENSE("GPL");
> +
> +module_init(sensors_ads1015_init);
> +module_exit(sensors_ads1015_exit);


-- 
Jean Delvare

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [PATCH] hwmon: Add support for Texas Instruments ADS1015

[Jean Delvare]

Hi Dirk,

On Mon, 14 Feb 2011 10:26:21 +0100, Dirk Eibach wrote:
> Signed-off-by: Dirk Eibach <eibach@gdsys.de>
> ---
>  Documentation/hwmon/ads1015 |   31 +++++
>  drivers/hwmon/Kconfig       |   10 ++
>  drivers/hwmon/Makefile      |    1 +
>  drivers/hwmon/ads1015.c     |  269 +++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 311 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/hwmon/ads1015
>  create mode 100644 drivers/hwmon/ads1015.c
> 
> diff --git a/Documentation/hwmon/ads1015 b/Documentation/hwmon/ads1015
> new file mode 100644
> index 0000000..3772816
> --- /dev/null
> +++ b/Documentation/hwmon/ads1015
> @@ -0,0 +1,31 @@
> +Kernel driver ads1015
> +=====================
> +
> +Supported chips:
> +  * Texas Instruments ADS1015
> +    Prefix: 'ads1015'
> +    Addresses scanned: I2C 0x48, 0x49, 0x4a, 0x4b
> +    Datasheet: Publicly available at the Texas Instruments website :
> +               http://focus.ti.com/lit/ds/symlink/ads1015.pdf
> +
> +Authors:
> +        Dirk Eibach, Guntermann & Drunck GmbH <eibach@gdsys.de>
> +
> +Description
> +-----------
> +
> +This driver implements support for the Texas Instruments ADS1015.
> +
> +This device is a 12-bit A-D converter with 4 inputs.
> +
> +The inputs can be used single ended or in certain differential combinations.
> +
> +The driver offers access to all available combinations by 7 "virtual" inputs:

I count 8.

> +in0: Voltage over AIN0 and AIN1.
> +in1: Voltage over AIN0 and AIN3.
> +in2: Voltage over AIN1 and AIN3.
> +in3: Voltage over AIN2 and AIN3.
> +in4: Voltage over AIN0 and GND.
> +in5: Voltage over AIN1 and GND.
> +in6: Voltage over AIN2 and GND.
> +in7: Voltage over AIN3 and GND.

This seems wrong. All 8 attributes can't possibly report sane values
for a given hardware setup, right? I think it would be much better to
have the platform provide setup data to the driver, telling it how the
chip is used and which input configurations should be exposed to
user-space.

> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 773e484..7e247f7 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -871,6 +871,16 @@ config SENSORS_SMSC47B397
>  	  This driver can also be built as a module.  If so, the module
>  	  will be called smsc47b397.
>  
> +config SENSORS_ADS1015
> +	tristate "Texas Instruments ADS1015"
> +	depends on I2C
> +	help
> +	  If you say yes here you get support for Texas Instruments ADS1015
> +	  12-bit 4-input ADC device.
> +
> +	  This driver can also be built as a module.  If so, the module
> +	  will be called ads1015.
> +
>  config SENSORS_ADS7828
>  	tristate "Texas Instruments ADS7828"
>  	depends on I2C
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index dde02d9..aae4036 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -29,6 +29,7 @@ obj-$(CONFIG_SENSORS_ADM1026)	+= adm1026.o
>  obj-$(CONFIG_SENSORS_ADM1029)	+= adm1029.o
>  obj-$(CONFIG_SENSORS_ADM1031)	+= adm1031.o
>  obj-$(CONFIG_SENSORS_ADM9240)	+= adm9240.o
> +obj-$(CONFIG_SENSORS_ADS1015)	+= ads1015.o
>  obj-$(CONFIG_SENSORS_ADS7828)	+= ads7828.o
>  obj-$(CONFIG_SENSORS_ADS7871)	+= ads7871.o
>  obj-$(CONFIG_SENSORS_ADT7411)	+= adt7411.o
> diff --git a/drivers/hwmon/ads1015.c b/drivers/hwmon/ads1015.c
> new file mode 100644
> index 0000000..675fdfe
> --- /dev/null
> +++ b/drivers/hwmon/ads1015.c
> @@ -0,0 +1,269 @@
> +/*
> + * ads1015.c - lm_sensors driver for ads1015 12-bit 4-input ADC
> + * (C) Copyright 2010
> + * Dirk Eibach, Guntermann & Drunck GmbH <eibach@gdsys.de>
> + *
> + * Based on the ads7828 driver by Steve Hardy.
> + *
> + * Datasheet available at: http://focus.ti.com/lit/ds/symlink/ads1015.pdf
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/slab.h>
> +#include <linux/jiffies.h>
> +#include <linux/i2c.h>
> +#include <linux/hwmon.h>
> +#include <linux/hwmon-sysfs.h>
> +#include <linux/err.h>
> +#include <linux/mutex.h>
> +
> +/* ADS1015 registers */
> +enum {
> +	ADS1015_CONVERSION = 0,
> +	ADS1015_CONFIG = 1,
> +	ADS1015_LO_THRESH = 2,
> +	ADS1015_HI_THRESH = 3,
> +};

You don't use the last two values anywhere.

> +
> +/* PGA fullscale voltages */
> +static const unsigned int fullscale_table[] = {
> +	6144, 4096, 2048, 1024, 512, 256, 256, 256 };

You'd rather hard-code the table size, as the rest of the code makes
assumptions on it. Please also add a comment stating the unit in which
these constants are expressed. I hope these are mV.

> +
> +/* Addresses to scan */
> +static const unsigned short normal_i2c[] = { 0x48, 0x49, 0x4a, 0x4b,
> +	I2C_CLIENT_END };
> +
> +/* Insmod parameters */
> +I2C_CLIENT_INSMOD_1(ads1015);

This macro was removed in kernel 2.6.33, almost one year ago. Please
provide patches which apply and build on Linus' latest kernel, or the
latest stable kernel at least.

> +
> +/* Each client has this additional data */
> +struct ads1015_data {
> +	struct device *hwmon_dev;
> +	struct mutex update_lock; /* mutex protect updates */
> +};
> +
> +/* Function declaration - necessary due to function dependencies */

No, not necessary at all. Just put the functions and driver declaration
in the right order.

> +static int ads1015_detect(struct i2c_client *client, int kind,
> +			  struct i2c_board_info *info);
> +static int ads1015_probe(struct i2c_client *client,
> +			 const struct i2c_device_id *id);
> +
> +
> +static s32 ads1015_read_reg(struct i2c_client *client, unsigned int reg)
> +{
> +	return swab16(i2c_smbus_read_word_data(client, reg));

This is wrong. If i2c_smbus_read_word_data() returns an error, your
function will return crap.

> +}
> +
> +static s32 ads1015_write_reg(struct i2c_client *client, unsigned int reg,
> +			     u16 val)
> +{
> +	return i2c_smbus_write_word_data(client, reg, swab16(val));
> +}
> +
> +static int ads1015_read_value(struct i2c_client *client, unsigned int channel)

Please either document the locking requirements of this function, or
move locking into it.

> +{
> +	u16 config;
> +	s16 conversion;
> +	unsigned int pga;
> +	int fullscale;
> +	unsigned int k;
> +
> +	/* get fullscale voltage */
> +	config = ads1015_read_reg(client, ADS1015_CONFIG);
> +	pga = (config >> 9) & 0x0007;
> +	fullscale = fullscale_table[pga];
> +
> +	/* set channel and start single conversion */
> +	config &= ~(0x0007 << 12);
> +	config &= ~(0x0001 << 8);

What's the point of clearing a bit you'll set again immediately?

> +	config |= (1 << 15) | (1 << 8) | (channel & 0x0007) << 12;
> +
> +	/* wait until conversion finished */
> +	ads1015_write_reg(client, ADS1015_CONFIG, config);
> +	for (k = 0; k < 5; ++k) {
> +		config = ads1015_read_reg(client, ADS1015_CONFIG);

What is the expected conversion time? Does it really make sense to
attempt a register read right away?

> +		if (config & (1 << 15))
> +			break;
> +		schedule_timeout(msecs_to_jiffies(1));
> +	}

What if k == 5? The conversion did not complete, and you return crap?

> +
> +	conversion = ads1015_read_reg(client, ADS1015_CONVERSION);
> +
> +	return conversion * fullscale / 0x7ff0;

Maybe it would make sense to use DIV_ROUND_CLOSEST?

> +}
> +
> +/* sysfs callback function */
> +static ssize_t show_in(struct device *dev, struct device_attribute *da,
> +	char *buf)
> +{
> +	struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
> +	struct i2c_client *client = to_i2c_client(dev);
> +	struct ads1015_data *data = i2c_get_clientdata(client);
> +	int in;
> +
> +	mutex_lock(&data->update_lock);
> +	in = ads1015_read_value(client, attr->index);
> +	mutex_unlock(&data->update_lock);
> +
> +	return sprintf(buf, "%d\n", in);
> +}
> +
> +#define in_reg(offset)\
> +static SENSOR_DEVICE_ATTR(in##offset##_input, S_IRUGO, show_in,\
> +	NULL, offset)
> +
> +in_reg(0);
> +in_reg(1);
> +in_reg(2);
> +in_reg(3);
> +in_reg(4);
> +in_reg(5);
> +in_reg(6);
> +in_reg(7);
> +
> +static struct attribute *ads1015_attributes[] = {
> +	&sensor_dev_attr_in0_input.dev_attr.attr,
> +	&sensor_dev_attr_in1_input.dev_attr.attr,
> +	&sensor_dev_attr_in2_input.dev_attr.attr,
> +	&sensor_dev_attr_in3_input.dev_attr.attr,
> +	&sensor_dev_attr_in4_input.dev_attr.attr,
> +	&sensor_dev_attr_in5_input.dev_attr.attr,
> +	&sensor_dev_attr_in6_input.dev_attr.attr,
> +	&sensor_dev_attr_in7_input.dev_attr.attr,
> +	NULL
> +};
> +
> +static const struct attribute_group ads1015_group = {
> +	.attrs = ads1015_attributes,
> +};
> +
> +static int ads1015_remove(struct i2c_client *client)
> +{
> +	struct ads1015_data *data = i2c_get_clientdata(client);
> +	hwmon_device_unregister(data->hwmon_dev);
> +	sysfs_remove_group(&client->dev.kobj, &ads1015_group);
> +	kfree(i2c_get_clientdata(client));

Please just use "data".

> +	return 0;
> +}
> +
> +static const struct i2c_device_id ads1015_id[] = {
> +	{ "ads1015", ads1015 },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, ads1015_id);
> +
> +/* This is the driver that will be inserted */
> +static struct i2c_driver ads1015_driver = {
> +	.class = I2C_CLASS_HWMON,
> +	.driver = {
> +		.name = "ads1015",
> +	},
> +	.probe = ads1015_probe,
> +	.remove = ads1015_remove,
> +	.id_table = ads1015_id,
> +	.detect = ads1015_detect,
> +	.address_data = &addr_data,
> +};
> +
> +/* Return 0 if detection is successful, -ENODEV otherwise */
> +static int ads1015_detect(struct i2c_client *client, int kind,
> +			  struct i2c_board_info *info)
> +{
> +	struct i2c_adapter *adapter = client->adapter;
> +
> +	/* Check we have a valid client */
> +	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_READ_WORD_DATA))
> +		return -ENODEV;
> +
> +	/* Now, we do the remaining detection. There is no identification
> +	dedicated register so attempt to sanity check using knowledge of
> +	the chip
> +	- Read from the 8 channels
> +	- Check the bits 0-3 of each result are not set (12 data bits)
> +	*/
> +	if (kind < 0) {
> +		int ch;
> +		for (ch = 0; ch < 8; ++ch) {
> +			u16 in_data;
> +			in_data = ads1015_read_value(client, ch);
> +			if (in_data & 0x000F) {
> +				printk(KERN_DEBUG
> +				"%s : Doesn't look like an ads1015 device\n",
> +				__func__);
> +				return -ENODEV;
> +			}
> +		}
> +	}
> +
> +	strlcpy(info->type, "ads1015", I2C_NAME_SIZE);
> +
> +	return 0;
> +}

Your device is obviously not easily and reliably detectable, so please
just don't provide a detect function. It's prohibited to write to the
device in detection functions anyway, and you do exactly this.

> +
> +static int ads1015_probe(struct i2c_client *client,
> +			 const struct i2c_device_id *id)
> +{
> +	struct ads1015_data *data;
> +	int err;
> +
> +	data = kzalloc(sizeof(struct ads1015_data), GFP_KERNEL);
> +	if (!data) {
> +		err = -ENOMEM;
> +		goto exit;
> +	}
> +
> +	i2c_set_clientdata(client, data);
> +	mutex_init(&data->update_lock);
> +
> +	/* Register sysfs hooks */
> +	err = sysfs_create_group(&client->dev.kobj, &ads1015_group);
> +	if (err)
> +		goto exit_free;
> +
> +	data->hwmon_dev = hwmon_device_register(&client->dev);
> +	if (IS_ERR(data->hwmon_dev)) {
> +		err = PTR_ERR(data->hwmon_dev);
> +		goto exit_remove;
> +	}
> +
> +	return 0;
> +
> +exit_remove:
> +	sysfs_remove_group(&client->dev.kobj, &ads1015_group);
> +exit_free:
> +	kfree(data);
> +exit:
> +	return err;
> +}
> +
> +static int __init sensors_ads1015_init(void)
> +{
> +	return i2c_add_driver(&ads1015_driver);
> +}
> +
> +static void __exit sensors_ads1015_exit(void)
> +{
> +	i2c_del_driver(&ads1015_driver);
> +}
> +
> +MODULE_AUTHOR("Dirk Eibach <eibach@gdsys.de>");
> +MODULE_DESCRIPTION("ADS1015 driver");
> +MODULE_LICENSE("GPL");
> +
> +module_init(sensors_ads1015_init);
> +module_exit(sensors_ads1015_exit);


-- 
Jean Delvare

[lm-sensors] [PATCH v2] hwmon: Add support for Texas Instruments

[Dirk Eibach]

Signed-off-by: Dirk Eibach <eibach@gdsys.de>
---
Changes since v1:
- fixed/extended Documentation
- removed unused register definitions
- hardcoded PGA fullscale table size
- made sure patch applies against v2.6.38-rc4
- reordered functions to avoid forward declaration
- results from i2c_smbus_read_word_data() are handled correctly
- moved locking into ads1015_read_value()
- removed unnecessray clearing of bit
- proper error handling in ads1015_read_value()
- use DIV_ROUND_CLOSEST for scaling result
- removed detect()

 Documentation/hwmon/ads1015 |   33 ++++++
 drivers/hwmon/Kconfig       |   10 ++
 drivers/hwmon/Makefile      |    1 +
 drivers/hwmon/ads1015.c     |  246 +++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 290 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/hwmon/ads1015
 create mode 100644 drivers/hwmon/ads1015.c

diff --git a/Documentation/hwmon/ads1015 b/Documentation/hwmon/ads1015
new file mode 100644
index 0000000..e2dc689
--- /dev/null
+++ b/Documentation/hwmon/ads1015
@@ -0,0 +1,33 @@
+Kernel driver ads1015
+==========+
+Supported chips:
+  * Texas Instruments ADS1015
+    Prefix: 'ads1015'
+    Addresses scanned: I2C 0x48, 0x49, 0x4a, 0x4b
+    Datasheet: Publicly available at the Texas Instruments website :
+               http://focus.ti.com/lit/ds/symlink/ads1015.pdf
+
+Authors:
+        Dirk Eibach, Guntermann & Drunck GmbH <eibach@gdsys.de>
+
+Description
+-----------
+
+This driver implements support for the Texas Instruments ADS1015.
+
+This device is a 12-bit A-D converter with 4 inputs.
+
+The inputs can be used single ended or in certain differential combinations.
+
+On certain systems it makes sense to access absolute voltage values as well
+as voltage differences. So all available combinations are made available by
+8 "virtual" inputs:
+in0: Voltage over AIN0 and AIN1.
+in1: Voltage over AIN0 and AIN3.
+in2: Voltage over AIN1 and AIN3.
+in3: Voltage over AIN2 and AIN3.
+in4: Voltage over AIN0 and GND.
+in5: Voltage over AIN1 and GND.
+in6: Voltage over AIN2 and GND.
+in7: Voltage over AIN3 and GND.
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 773e484..7e247f7 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -871,6 +871,16 @@ config SENSORS_SMSC47B397
 	  This driver can also be built as a module.  If so, the module
 	  will be called smsc47b397.
 
+config SENSORS_ADS1015
+	tristate "Texas Instruments ADS1015"
+	depends on I2C
+	help
+	  If you say yes here you get support for Texas Instruments ADS1015
+	  12-bit 4-input ADC device.
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called ads1015.
+
 config SENSORS_ADS7828
 	tristate "Texas Instruments ADS7828"
 	depends on I2C
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index dde02d9..aae4036 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -29,6 +29,7 @@ obj-$(CONFIG_SENSORS_ADM1026)	+= adm1026.o
 obj-$(CONFIG_SENSORS_ADM1029)	+= adm1029.o
 obj-$(CONFIG_SENSORS_ADM1031)	+= adm1031.o
 obj-$(CONFIG_SENSORS_ADM9240)	+= adm9240.o
+obj-$(CONFIG_SENSORS_ADS1015)	+= ads1015.o
 obj-$(CONFIG_SENSORS_ADS7828)	+= ads7828.o
 obj-$(CONFIG_SENSORS_ADS7871)	+= ads7871.o
 obj-$(CONFIG_SENSORS_ADT7411)	+= adt7411.o
diff --git a/drivers/hwmon/ads1015.c b/drivers/hwmon/ads1015.c
new file mode 100644
index 0000000..7fb30ae
--- /dev/null
+++ b/drivers/hwmon/ads1015.c
@@ -0,0 +1,246 @@
+/*
+ * ads1015.c - lm_sensors driver for ads1015 12-bit 4-input ADC
+ * (C) Copyright 2010
+ * Dirk Eibach, Guntermann & Drunck GmbH <eibach@gdsys.de>
+ *
+ * Based on the ads7828 driver by Steve Hardy.
+ *
+ * Datasheet available at: http://focus.ti.com/lit/ds/symlink/ads1015.pdf
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/jiffies.h>
+#include <linux/i2c.h>
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
+#include <linux/err.h>
+#include <linux/mutex.h>
+
+/* ADS1015 registers */
+enum {
+	ADS1015_CONVERSION = 0,
+	ADS1015_CONFIG = 1,
+};
+
+/* PGA fullscale voltages in mV */
+static const unsigned int fullscale_table[8] = {
+	6144, 4096, 2048, 1024, 512, 256, 256, 256 };
+
+/* Addresses to scan */
+static const unsigned short normal_i2c[] = { 0x48, 0x49, 0x4a, 0x4b,
+	I2C_CLIENT_END };
+
+/* Each client has this additional data */
+struct ads1015_data {
+	struct device *hwmon_dev;
+	struct mutex update_lock; /* mutex protect updates */
+};
+
+static s32 ads1015_read_reg(struct i2c_client *client, unsigned int reg)
+{
+	s32 data = i2c_smbus_read_word_data(client, reg);
+
+	return (data < 0) ? data : swab16(data);
+}
+
+static s32 ads1015_write_reg(struct i2c_client *client, unsigned int reg,
+			     u16 val)
+{
+	return i2c_smbus_write_word_data(client, reg, swab16(val));
+}
+
+static int ads1015_read_value(struct i2c_client *client, unsigned int channel,
+			      int *value)
+{
+	u16 config;
+	s16 conversion;
+	unsigned int pga;
+	int fullscale;
+	unsigned int k;
+	struct ads1015_data *data = i2c_get_clientdata(client);
+	int res;
+
+	mutex_lock(&data->update_lock);
+
+	/* get fullscale voltage */
+	res = ads1015_read_reg(client, ADS1015_CONFIG);
+	if (res < 0)
+		goto err_unlock;
+	config = res;
+	pga = (config >> 9) & 0x0007;
+	fullscale = fullscale_table[pga];
+
+	/* set channel and start single conversion */
+	config &= ~(0x0007 << 12);
+	config |= (1 << 15) | (1 << 8) | (channel & 0x0007) << 12;
+
+	/* wait until conversion finished */
+	res = ads1015_write_reg(client, ADS1015_CONFIG, config);
+	if (res < 0)
+		goto err_unlock;
+	for (k = 0; k < 5; ++k) {
+		schedule_timeout(msecs_to_jiffies(1));
+		res = ads1015_read_reg(client, ADS1015_CONFIG);
+		if (res < 0)
+			goto err_unlock;
+		config = res;
+		if (config & (1 << 15))
+			break;
+	}
+	if (k = 5)
+		return -EIO;
+
+	res = ads1015_read_reg(client, ADS1015_CONVERSION);
+	if (res < 0)
+		goto err_unlock;
+	conversion = res;
+
+	mutex_unlock(&data->update_lock);
+
+	*value = DIV_ROUND_CLOSEST(conversion * fullscale, 0x7ff0);
+
+	return 0;
+
+err_unlock:
+	mutex_unlock(&data->update_lock);
+	return res;
+}
+
+/* sysfs callback function */
+static ssize_t show_in(struct device *dev, struct device_attribute *da,
+	char *buf)
+{
+	struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
+	struct i2c_client *client = to_i2c_client(dev);
+	int in;
+	int res;
+
+	res = ads1015_read_value(client, attr->index, &in);
+
+	return (res < 0) ? res : sprintf(buf, "%d\n", in);
+}
+
+#define in_reg(offset)\
+static SENSOR_DEVICE_ATTR(in##offset##_input, S_IRUGO, show_in,\
+	NULL, offset)
+
+in_reg(0);
+in_reg(1);
+in_reg(2);
+in_reg(3);
+in_reg(4);
+in_reg(5);
+in_reg(6);
+in_reg(7);
+
+static struct attribute *ads1015_attributes[] = {
+	&sensor_dev_attr_in0_input.dev_attr.attr,
+	&sensor_dev_attr_in1_input.dev_attr.attr,
+	&sensor_dev_attr_in2_input.dev_attr.attr,
+	&sensor_dev_attr_in3_input.dev_attr.attr,
+	&sensor_dev_attr_in4_input.dev_attr.attr,
+	&sensor_dev_attr_in5_input.dev_attr.attr,
+	&sensor_dev_attr_in6_input.dev_attr.attr,
+	&sensor_dev_attr_in7_input.dev_attr.attr,
+	NULL
+};
+
+static const struct attribute_group ads1015_group = {
+	.attrs = ads1015_attributes,
+};
+
+static int ads1015_remove(struct i2c_client *client)
+{
+	struct ads1015_data *data = i2c_get_clientdata(client);
+	hwmon_device_unregister(data->hwmon_dev);
+	sysfs_remove_group(&client->dev.kobj, &ads1015_group);
+	kfree(data);
+	return 0;
+}
+
+static int ads1015_probe(struct i2c_client *client,
+			 const struct i2c_device_id *id)
+{
+	struct ads1015_data *data;
+	int err;
+
+	data = kzalloc(sizeof(struct ads1015_data), GFP_KERNEL);
+	if (!data) {
+		err = -ENOMEM;
+		goto exit;
+	}
+
+	i2c_set_clientdata(client, data);
+	mutex_init(&data->update_lock);
+
+	/* Register sysfs hooks */
+	err = sysfs_create_group(&client->dev.kobj, &ads1015_group);
+	if (err)
+		goto exit_free;
+
+	data->hwmon_dev = hwmon_device_register(&client->dev);
+	if (IS_ERR(data->hwmon_dev)) {
+		err = PTR_ERR(data->hwmon_dev);
+		goto exit_remove;
+	}
+
+	return 0;
+
+exit_remove:
+	sysfs_remove_group(&client->dev.kobj, &ads1015_group);
+exit_free:
+	kfree(data);
+exit:
+	return err;
+}
+
+static const struct i2c_device_id ads1015_id[] = {
+	{ "ads1015", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, ads1015_id);
+
+/* This is the driver that will be inserted */
+static struct i2c_driver ads1015_driver = {
+	.class = I2C_CLASS_HWMON,
+	.driver = {
+		.name = "ads1015",
+	},
+	.probe = ads1015_probe,
+	.remove = ads1015_remove,
+	.id_table = ads1015_id,
+	.address_list = normal_i2c,
+};
+
+static int __init sensors_ads1015_init(void)
+{
+	return i2c_add_driver(&ads1015_driver);
+}
+
+static void __exit sensors_ads1015_exit(void)
+{
+	i2c_del_driver(&ads1015_driver);
+}
+
+MODULE_AUTHOR("Dirk Eibach <eibach@gdsys.de>");
+MODULE_DESCRIPTION("ADS1015 driver");
+MODULE_LICENSE("GPL");
+
+module_init(sensors_ads1015_init);
+module_exit(sensors_ads1015_exit);
-- 
1.5.6.5


_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

[PATCH v2] hwmon: Add support for Texas Instruments ADS1015

[Dirk Eibach]

Signed-off-by: Dirk Eibach <eibach@gdsys.de>
---
Changes since v1:
- fixed/extended Documentation
- removed unused register definitions
- hardcoded PGA fullscale table size
- made sure patch applies against v2.6.38-rc4
- reordered functions to avoid forward declaration
- results from i2c_smbus_read_word_data() are handled correctly
- moved locking into ads1015_read_value()
- removed unnecessray clearing of bit
- proper error handling in ads1015_read_value()
- use DIV_ROUND_CLOSEST for scaling result
- removed detect()

 Documentation/hwmon/ads1015 |   33 ++++++
 drivers/hwmon/Kconfig       |   10 ++
 drivers/hwmon/Makefile      |    1 +
 drivers/hwmon/ads1015.c     |  246 +++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 290 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/hwmon/ads1015
 create mode 100644 drivers/hwmon/ads1015.c

diff --git a/Documentation/hwmon/ads1015 b/Documentation/hwmon/ads1015
new file mode 100644
index 0000000..e2dc689
--- /dev/null
+++ b/Documentation/hwmon/ads1015
@@ -0,0 +1,33 @@
+Kernel driver ads1015
+=====================
+
+Supported chips:
+  * Texas Instruments ADS1015
+    Prefix: 'ads1015'
+    Addresses scanned: I2C 0x48, 0x49, 0x4a, 0x4b
+    Datasheet: Publicly available at the Texas Instruments website :
+               http://focus.ti.com/lit/ds/symlink/ads1015.pdf
+
+Authors:
+        Dirk Eibach, Guntermann & Drunck GmbH <eibach@gdsys.de>
+
+Description
+-----------
+
+This driver implements support for the Texas Instruments ADS1015.
+
+This device is a 12-bit A-D converter with 4 inputs.
+
+The inputs can be used single ended or in certain differential combinations.
+
+On certain systems it makes sense to access absolute voltage values as well
+as voltage differences. So all available combinations are made available by
+8 "virtual" inputs:
+in0: Voltage over AIN0 and AIN1.
+in1: Voltage over AIN0 and AIN3.
+in2: Voltage over AIN1 and AIN3.
+in3: Voltage over AIN2 and AIN3.
+in4: Voltage over AIN0 and GND.
+in5: Voltage over AIN1 and GND.
+in6: Voltage over AIN2 and GND.
+in7: Voltage over AIN3 and GND.
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 773e484..7e247f7 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -871,6 +871,16 @@ config SENSORS_SMSC47B397
 	  This driver can also be built as a module.  If so, the module
 	  will be called smsc47b397.
 
+config SENSORS_ADS1015
+	tristate "Texas Instruments ADS1015"
+	depends on I2C
+	help
+	  If you say yes here you get support for Texas Instruments ADS1015
+	  12-bit 4-input ADC device.
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called ads1015.
+
 config SENSORS_ADS7828
 	tristate "Texas Instruments ADS7828"
 	depends on I2C
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index dde02d9..aae4036 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -29,6 +29,7 @@ obj-$(CONFIG_SENSORS_ADM1026)	+= adm1026.o
 obj-$(CONFIG_SENSORS_ADM1029)	+= adm1029.o
 obj-$(CONFIG_SENSORS_ADM1031)	+= adm1031.o
 obj-$(CONFIG_SENSORS_ADM9240)	+= adm9240.o
+obj-$(CONFIG_SENSORS_ADS1015)	+= ads1015.o
 obj-$(CONFIG_SENSORS_ADS7828)	+= ads7828.o
 obj-$(CONFIG_SENSORS_ADS7871)	+= ads7871.o
 obj-$(CONFIG_SENSORS_ADT7411)	+= adt7411.o
diff --git a/drivers/hwmon/ads1015.c b/drivers/hwmon/ads1015.c
new file mode 100644
index 0000000..7fb30ae
--- /dev/null
+++ b/drivers/hwmon/ads1015.c
@@ -0,0 +1,246 @@
+/*
+ * ads1015.c - lm_sensors driver for ads1015 12-bit 4-input ADC
+ * (C) Copyright 2010
+ * Dirk Eibach, Guntermann & Drunck GmbH <eibach@gdsys.de>
+ *
+ * Based on the ads7828 driver by Steve Hardy.
+ *
+ * Datasheet available at: http://focus.ti.com/lit/ds/symlink/ads1015.pdf
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/jiffies.h>
+#include <linux/i2c.h>
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
+#include <linux/err.h>
+#include <linux/mutex.h>
+
+/* ADS1015 registers */
+enum {
+	ADS1015_CONVERSION = 0,
+	ADS1015_CONFIG = 1,
+};
+
+/* PGA fullscale voltages in mV */
+static const unsigned int fullscale_table[8] = {
+	6144, 4096, 2048, 1024, 512, 256, 256, 256 };
+
+/* Addresses to scan */
+static const unsigned short normal_i2c[] = { 0x48, 0x49, 0x4a, 0x4b,
+	I2C_CLIENT_END };
+
+/* Each client has this additional data */
+struct ads1015_data {
+	struct device *hwmon_dev;
+	struct mutex update_lock; /* mutex protect updates */
+};
+
+static s32 ads1015_read_reg(struct i2c_client *client, unsigned int reg)
+{
+	s32 data = i2c_smbus_read_word_data(client, reg);
+
+	return (data < 0) ? data : swab16(data);
+}
+
+static s32 ads1015_write_reg(struct i2c_client *client, unsigned int reg,
+			     u16 val)
+{
+	return i2c_smbus_write_word_data(client, reg, swab16(val));
+}
+
+static int ads1015_read_value(struct i2c_client *client, unsigned int channel,
+			      int *value)
+{
+	u16 config;
+	s16 conversion;
+	unsigned int pga;
+	int fullscale;
+	unsigned int k;
+	struct ads1015_data *data = i2c_get_clientdata(client);
+	int res;
+
+	mutex_lock(&data->update_lock);
+
+	/* get fullscale voltage */
+	res = ads1015_read_reg(client, ADS1015_CONFIG);
+	if (res < 0)
+		goto err_unlock;
+	config = res;
+	pga = (config >> 9) & 0x0007;
+	fullscale = fullscale_table[pga];
+
+	/* set channel and start single conversion */
+	config &= ~(0x0007 << 12);
+	config |= (1 << 15) | (1 << 8) | (channel & 0x0007) << 12;
+
+	/* wait until conversion finished */
+	res = ads1015_write_reg(client, ADS1015_CONFIG, config);
+	if (res < 0)
+		goto err_unlock;
+	for (k = 0; k < 5; ++k) {
+		schedule_timeout(msecs_to_jiffies(1));
+		res = ads1015_read_reg(client, ADS1015_CONFIG);
+		if (res < 0)
+			goto err_unlock;
+		config = res;
+		if (config & (1 << 15))
+			break;
+	}
+	if (k == 5)
+		return -EIO;
+
+	res = ads1015_read_reg(client, ADS1015_CONVERSION);
+	if (res < 0)
+		goto err_unlock;
+	conversion = res;
+
+	mutex_unlock(&data->update_lock);
+
+	*value = DIV_ROUND_CLOSEST(conversion * fullscale, 0x7ff0);
+
+	return 0;
+
+err_unlock:
+	mutex_unlock(&data->update_lock);
+	return res;
+}
+
+/* sysfs callback function */
+static ssize_t show_in(struct device *dev, struct device_attribute *da,
+	char *buf)
+{
+	struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
+	struct i2c_client *client = to_i2c_client(dev);
+	int in;
+	int res;
+
+	res = ads1015_read_value(client, attr->index, &in);
+
+	return (res < 0) ? res : sprintf(buf, "%d\n", in);
+}
+
+#define in_reg(offset)\
+static SENSOR_DEVICE_ATTR(in##offset##_input, S_IRUGO, show_in,\
+	NULL, offset)
+
+in_reg(0);
+in_reg(1);
+in_reg(2);
+in_reg(3);
+in_reg(4);
+in_reg(5);
+in_reg(6);
+in_reg(7);
+
+static struct attribute *ads1015_attributes[] = {
+	&sensor_dev_attr_in0_input.dev_attr.attr,
+	&sensor_dev_attr_in1_input.dev_attr.attr,
+	&sensor_dev_attr_in2_input.dev_attr.attr,
+	&sensor_dev_attr_in3_input.dev_attr.attr,
+	&sensor_dev_attr_in4_input.dev_attr.attr,
+	&sensor_dev_attr_in5_input.dev_attr.attr,
+	&sensor_dev_attr_in6_input.dev_attr.attr,
+	&sensor_dev_attr_in7_input.dev_attr.attr,
+	NULL
+};
+
+static const struct attribute_group ads1015_group = {
+	.attrs = ads1015_attributes,
+};
+
+static int ads1015_remove(struct i2c_client *client)
+{
+	struct ads1015_data *data = i2c_get_clientdata(client);
+	hwmon_device_unregister(data->hwmon_dev);
+	sysfs_remove_group(&client->dev.kobj, &ads1015_group);
+	kfree(data);
+	return 0;
+}
+
+static int ads1015_probe(struct i2c_client *client,
+			 const struct i2c_device_id *id)
+{
+	struct ads1015_data *data;
+	int err;
+
+	data = kzalloc(sizeof(struct ads1015_data), GFP_KERNEL);
+	if (!data) {
+		err = -ENOMEM;
+		goto exit;
+	}
+
+	i2c_set_clientdata(client, data);
+	mutex_init(&data->update_lock);
+
+	/* Register sysfs hooks */
+	err = sysfs_create_group(&client->dev.kobj, &ads1015_group);
+	if (err)
+		goto exit_free;
+
+	data->hwmon_dev = hwmon_device_register(&client->dev);
+	if (IS_ERR(data->hwmon_dev)) {
+		err = PTR_ERR(data->hwmon_dev);
+		goto exit_remove;
+	}
+
+	return 0;
+
+exit_remove:
+	sysfs_remove_group(&client->dev.kobj, &ads1015_group);
+exit_free:
+	kfree(data);
+exit:
+	return err;
+}
+
+static const struct i2c_device_id ads1015_id[] = {
+	{ "ads1015", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, ads1015_id);
+
+/* This is the driver that will be inserted */
+static struct i2c_driver ads1015_driver = {
+	.class = I2C_CLASS_HWMON,
+	.driver = {
+		.name = "ads1015",
+	},
+	.probe = ads1015_probe,
+	.remove = ads1015_remove,
+	.id_table = ads1015_id,
+	.address_list = normal_i2c,
+};
+
+static int __init sensors_ads1015_init(void)
+{
+	return i2c_add_driver(&ads1015_driver);
+}
+
+static void __exit sensors_ads1015_exit(void)
+{
+	i2c_del_driver(&ads1015_driver);
+}
+
+MODULE_AUTHOR("Dirk Eibach <eibach@gdsys.de>");
+MODULE_DESCRIPTION("ADS1015 driver");
+MODULE_LICENSE("GPL");
+
+module_init(sensors_ads1015_init);
+module_exit(sensors_ads1015_exit);
-- 
1.5.6.5

Re: [lm-sensors] [PATCH v2] hwmon: Add support for Texas

[Guenter Roeck]

On Mon, Feb 14, 2011 at 08:21:50AM -0500, Dirk Eibach wrote:
> Signed-off-by: Dirk Eibach <eibach@gdsys.de>
> ---
> Changes since v1:
> - fixed/extended Documentation
> - removed unused register definitions
> - hardcoded PGA fullscale table size
> - made sure patch applies against v2.6.38-rc4
> - reordered functions to avoid forward declaration
> - results from i2c_smbus_read_word_data() are handled correctly
> - moved locking into ads1015_read_value()
> - removed unnecessray clearing of bit
> - proper error handling in ads1015_read_value()
> - use DIV_ROUND_CLOSEST for scaling result
> - removed detect()
> 
Acked-by: Guenter Roeck <guenter.roeck@ericsson.com>

Jean,

any further comments ?

If not, do you want me to apply it to my tree, or do you want to take it into yours ?

Thanks,
Guenter

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [PATCH v2] hwmon: Add support for Texas Instruments ADS1015

[Guenter Roeck]

On Mon, Feb 14, 2011 at 08:21:50AM -0500, Dirk Eibach wrote:
> Signed-off-by: Dirk Eibach <eibach@gdsys.de>
> ---
> Changes since v1:
> - fixed/extended Documentation
> - removed unused register definitions
> - hardcoded PGA fullscale table size
> - made sure patch applies against v2.6.38-rc4
> - reordered functions to avoid forward declaration
> - results from i2c_smbus_read_word_data() are handled correctly
> - moved locking into ads1015_read_value()
> - removed unnecessray clearing of bit
> - proper error handling in ads1015_read_value()
> - use DIV_ROUND_CLOSEST for scaling result
> - removed detect()
> 
Acked-by: Guenter Roeck <guenter.roeck@ericsson.com>

Jean,

any further comments ?

If not, do you want me to apply it to my tree, or do you want to take it into yours ?

Thanks,
Guenter

Re: [lm-sensors] [PATCH v2] hwmon: Add support for Texas

[Jean Delvare]

Hi Guenter,

On Tue, 15 Feb 2011 20:50:35 -0800, Guenter Roeck wrote:
> On Mon, Feb 14, 2011 at 08:21:50AM -0500, Dirk Eibach wrote:
> > Signed-off-by: Dirk Eibach <eibach@gdsys.de>
> > ---
> > Changes since v1:
> > - fixed/extended Documentation
> > - removed unused register definitions
> > - hardcoded PGA fullscale table size
> > - made sure patch applies against v2.6.38-rc4
> > - reordered functions to avoid forward declaration
> > - results from i2c_smbus_read_word_data() are handled correctly
> > - moved locking into ads1015_read_value()
> > - removed unnecessray clearing of bit
> > - proper error handling in ads1015_read_value()
> > - use DIV_ROUND_CLOSEST for scaling result
> > - removed detect()
>
> Acked-by: Guenter Roeck <guenter.roeck@ericsson.com>
> 
> Jean,
> 
> any further comments ?

I have some more comments on the patch, yes. I'll post them in a moment
when I'm done with the review.

> If not, do you want me to apply it to my tree, or do you want to take it into yours ?

I'll pick it in my tree when I'm happy with it.

-- 
Jean Delvare

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [PATCH v2] hwmon: Add support for Texas Instruments ADS1015

[Jean Delvare]

Hi Guenter,

On Tue, 15 Feb 2011 20:50:35 -0800, Guenter Roeck wrote:
> On Mon, Feb 14, 2011 at 08:21:50AM -0500, Dirk Eibach wrote:
> > Signed-off-by: Dirk Eibach <eibach@gdsys.de>
> > ---
> > Changes since v1:
> > - fixed/extended Documentation
> > - removed unused register definitions
> > - hardcoded PGA fullscale table size
> > - made sure patch applies against v2.6.38-rc4
> > - reordered functions to avoid forward declaration
> > - results from i2c_smbus_read_word_data() are handled correctly
> > - moved locking into ads1015_read_value()
> > - removed unnecessray clearing of bit
> > - proper error handling in ads1015_read_value()
> > - use DIV_ROUND_CLOSEST for scaling result
> > - removed detect()
>
> Acked-by: Guenter Roeck <guenter.roeck@ericsson.com>
> 
> Jean,
> 
> any further comments ?

I have some more comments on the patch, yes. I'll post them in a moment
when I'm done with the review.

> If not, do you want me to apply it to my tree, or do you want to take it into yours ?

I'll pick it in my tree when I'm happy with it.

-- 
Jean Delvare

Re: [lm-sensors] [PATCH v2] hwmon: Add support for Texas

[Jean Delvare]

Hi Dirk,

On Mon, 14 Feb 2011 14:21:50 +0100, Dirk Eibach wrote:
> Signed-off-by: Dirk Eibach <eibach@gdsys.de>
> ---
> Changes since v1:
> - fixed/extended Documentation
> - removed unused register definitions
> - hardcoded PGA fullscale table size
> - made sure patch applies against v2.6.38-rc4
> - reordered functions to avoid forward declaration
> - results from i2c_smbus_read_word_data() are handled correctly
> - moved locking into ads1015_read_value()
> - removed unnecessray clearing of bit
> - proper error handling in ads1015_read_value()
> - use DIV_ROUND_CLOSEST for scaling result
> - removed detect()

Thanks for the quick update. Second review:

> (...)
> --- /dev/null
> +++ b/Documentation/hwmon/ads1015
> @@ -0,0 +1,33 @@
> +Kernel driver ads1015
> +==========> +
> +Supported chips:
> +  * Texas Instruments ADS1015
> +    Prefix: 'ads1015'
> +    Addresses scanned: I2C 0x48, 0x49, 0x4a, 0x4b

With the detect function being gone, this is no longer true.

> +    Datasheet: Publicly available at the Texas Instruments website :
> +               http://focus.ti.com/lit/ds/symlink/ads1015.pdf
> +
> +Authors:
> +        Dirk Eibach, Guntermann & Drunck GmbH <eibach@gdsys.de>
> +
> +Description
> +-----------
> +
> +This driver implements support for the Texas Instruments ADS1015.
> +
> +This device is a 12-bit A-D converter with 4 inputs.
> +
> +The inputs can be used single ended or in certain differential combinations.
> +
> +On certain systems it makes sense to access absolute voltage values as well
> +as voltage differences. So all available combinations are made available by
> +8 "virtual" inputs:
> +in0: Voltage over AIN0 and AIN1.
> +in1: Voltage over AIN0 and AIN3.
> +in2: Voltage over AIN1 and AIN3.
> +in3: Voltage over AIN2 and AIN3.
> +in4: Voltage over AIN0 and GND.
> +in5: Voltage over AIN1 and GND.
> +in6: Voltage over AIN2 and GND.
> +in7: Voltage over AIN3 and GND.

I see you've updated the comment, presumably this is how you addressed
my concern about exposing all 8 input settings. I am really curious how
it can make sense to expose both direct and differential values
involving the same pins. The pcf8591 driver, which has to handle a
smiliar case, only exposes channels which make physical sense together
(it does so using a module parameter for historical reason, nowadays we
would use platform data for this.)

So I am still convinced that this part should be reworked. That being
said, you obviously know more than I do with regards to how you intend
to use the driver, so I'll leave you the last work on this.

> (...)
> --- /dev/null
> +++ b/drivers/hwmon/ads1015.c
> (...)
> +static int ads1015_read_value(struct i2c_client *client, unsigned int channel,
> +			      int *value)
> +{
> +	u16 config;
> +	s16 conversion;
> +	unsigned int pga;
> +	int fullscale;
> +	unsigned int k;
> +	struct ads1015_data *data = i2c_get_clientdata(client);
> +	int res;
> +
> +	mutex_lock(&data->update_lock);
> +
> +	/* get fullscale voltage */
> +	res = ads1015_read_reg(client, ADS1015_CONFIG);
> +	if (res < 0)
> +		goto err_unlock;
> +	config = res;
> +	pga = (config >> 9) & 0x0007;
> +	fullscale = fullscale_table[pga];
> +
> +	/* set channel and start single conversion */
> +	config &= ~(0x0007 << 12);
> +	config |= (1 << 15) | (1 << 8) | (channel & 0x0007) << 12;
> +
> +	/* wait until conversion finished */
> +	res = ads1015_write_reg(client, ADS1015_CONFIG, config);
> +	if (res < 0)
> +		goto err_unlock;
> +	for (k = 0; k < 5; ++k) {
> +		schedule_timeout(msecs_to_jiffies(1));
> +		res = ads1015_read_reg(client, ADS1015_CONFIG);
> +		if (res < 0)
> +			goto err_unlock;
> +		config = res;
> +		if (config & (1 << 15))
> +			break;
> +	}
> +	if (k = 5)
> +		return -EIO;

You return with data->update_lock held.

> +
> +	res = ads1015_read_reg(client, ADS1015_CONVERSION);
> +	if (res < 0)
> +		goto err_unlock;
> +	conversion = res;
> +
> +	mutex_unlock(&data->update_lock);
> +
> +	*value = DIV_ROUND_CLOSEST(conversion * fullscale, 0x7ff0);
> +
> +	return 0;
> +
> +err_unlock:
> +	mutex_unlock(&data->update_lock);
> +	return res;
> +}

> (...)
> +/* This is the driver that will be inserted */
> +static struct i2c_driver ads1015_driver = {
> +	.class = I2C_CLASS_HWMON,
> +	.driver = {
> +		.name = "ads1015",
> +	},
> +	.probe = ads1015_probe,
> +	.remove = ads1015_remove,
> +	.id_table = ads1015_id,
> +	.address_list = normal_i2c,
> +};

The only purpose of the address list is for the detect function, which
you just dropped. So you can remove the address list too. Same goes for
the class.

-- 
Jean Delvare

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [PATCH v2] hwmon: Add support for Texas Instruments ADS1015

[Jean Delvare]

Hi Dirk,

On Mon, 14 Feb 2011 14:21:50 +0100, Dirk Eibach wrote:
> Signed-off-by: Dirk Eibach <eibach@gdsys.de>
> ---
> Changes since v1:
> - fixed/extended Documentation
> - removed unused register definitions
> - hardcoded PGA fullscale table size
> - made sure patch applies against v2.6.38-rc4
> - reordered functions to avoid forward declaration
> - results from i2c_smbus_read_word_data() are handled correctly
> - moved locking into ads1015_read_value()
> - removed unnecessray clearing of bit
> - proper error handling in ads1015_read_value()
> - use DIV_ROUND_CLOSEST for scaling result
> - removed detect()

Thanks for the quick update. Second review:

> (...)
> --- /dev/null
> +++ b/Documentation/hwmon/ads1015
> @@ -0,0 +1,33 @@
> +Kernel driver ads1015
> +=====================
> +
> +Supported chips:
> +  * Texas Instruments ADS1015
> +    Prefix: 'ads1015'
> +    Addresses scanned: I2C 0x48, 0x49, 0x4a, 0x4b

With the detect function being gone, this is no longer true.

> +    Datasheet: Publicly available at the Texas Instruments website :
> +               http://focus.ti.com/lit/ds/symlink/ads1015.pdf
> +
> +Authors:
> +        Dirk Eibach, Guntermann & Drunck GmbH <eibach@gdsys.de>
> +
> +Description
> +-----------
> +
> +This driver implements support for the Texas Instruments ADS1015.
> +
> +This device is a 12-bit A-D converter with 4 inputs.
> +
> +The inputs can be used single ended or in certain differential combinations.
> +
> +On certain systems it makes sense to access absolute voltage values as well
> +as voltage differences. So all available combinations are made available by
> +8 "virtual" inputs:
> +in0: Voltage over AIN0 and AIN1.
> +in1: Voltage over AIN0 and AIN3.
> +in2: Voltage over AIN1 and AIN3.
> +in3: Voltage over AIN2 and AIN3.
> +in4: Voltage over AIN0 and GND.
> +in5: Voltage over AIN1 and GND.
> +in6: Voltage over AIN2 and GND.
> +in7: Voltage over AIN3 and GND.

I see you've updated the comment, presumably this is how you addressed
my concern about exposing all 8 input settings. I am really curious how
it can make sense to expose both direct and differential values
involving the same pins. The pcf8591 driver, which has to handle a
smiliar case, only exposes channels which make physical sense together
(it does so using a module parameter for historical reason, nowadays we
would use platform data for this.)

So I am still convinced that this part should be reworked. That being
said, you obviously know more than I do with regards to how you intend
to use the driver, so I'll leave you the last work on this.

> (...)
> --- /dev/null
> +++ b/drivers/hwmon/ads1015.c
> (...)
> +static int ads1015_read_value(struct i2c_client *client, unsigned int channel,
> +			      int *value)
> +{
> +	u16 config;
> +	s16 conversion;
> +	unsigned int pga;
> +	int fullscale;
> +	unsigned int k;
> +	struct ads1015_data *data = i2c_get_clientdata(client);
> +	int res;
> +
> +	mutex_lock(&data->update_lock);
> +
> +	/* get fullscale voltage */
> +	res = ads1015_read_reg(client, ADS1015_CONFIG);
> +	if (res < 0)
> +		goto err_unlock;
> +	config = res;
> +	pga = (config >> 9) & 0x0007;
> +	fullscale = fullscale_table[pga];
> +
> +	/* set channel and start single conversion */
> +	config &= ~(0x0007 << 12);
> +	config |= (1 << 15) | (1 << 8) | (channel & 0x0007) << 12;
> +
> +	/* wait until conversion finished */
> +	res = ads1015_write_reg(client, ADS1015_CONFIG, config);
> +	if (res < 0)
> +		goto err_unlock;
> +	for (k = 0; k < 5; ++k) {
> +		schedule_timeout(msecs_to_jiffies(1));
> +		res = ads1015_read_reg(client, ADS1015_CONFIG);
> +		if (res < 0)
> +			goto err_unlock;
> +		config = res;
> +		if (config & (1 << 15))
> +			break;
> +	}
> +	if (k == 5)
> +		return -EIO;

You return with data->update_lock held.

> +
> +	res = ads1015_read_reg(client, ADS1015_CONVERSION);
> +	if (res < 0)
> +		goto err_unlock;
> +	conversion = res;
> +
> +	mutex_unlock(&data->update_lock);
> +
> +	*value = DIV_ROUND_CLOSEST(conversion * fullscale, 0x7ff0);
> +
> +	return 0;
> +
> +err_unlock:
> +	mutex_unlock(&data->update_lock);
> +	return res;
> +}

> (...)
> +/* This is the driver that will be inserted */
> +static struct i2c_driver ads1015_driver = {
> +	.class = I2C_CLASS_HWMON,
> +	.driver = {
> +		.name = "ads1015",
> +	},
> +	.probe = ads1015_probe,
> +	.remove = ads1015_remove,
> +	.id_table = ads1015_id,
> +	.address_list = normal_i2c,
> +};

The only purpose of the address list is for the detect function, which
you just dropped. So you can remove the address list too. Same goes for
the class.

-- 
Jean Delvare

[lm-sensors] [PATCH v3] hwmon: Add support for Texas Instruments

[Dirk Eibach]

Signed-off-by: Dirk Eibach <eibach@gdsys.de>
---
Changes since v1:
- fixed/extended Documentation
- removed unused register definitions
- hardcoded PGA fullscale table size
- made sure patch applies against v2.6.38-rc4
- reordered functions to avoid forward declaration
- results from i2c_smbus_read_word_data() are handled correctly
- moved locking into ads1015_read_value()
- removed unnecessray clearing of bit
- proper error handling in ads1015_read_value()
- use DIV_ROUND_CLOSEST for scaling result
- removed detect()

Changes since v2:
- removed *all* leftovers from detect()
- fixed return with mutex held
- made sysfs representation configurable
  (hope this will be the reference implementation for generations to come ;)

 Documentation/hwmon/ads1015 |   72 +++++++++++
 drivers/hwmon/Kconfig       |   10 ++
 drivers/hwmon/Makefile      |    1 +
 drivers/hwmon/ads1015.c     |  295 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/i2c/ads1015.h |   30 +++++
 5 files changed, 408 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/hwmon/ads1015
 create mode 100644 drivers/hwmon/ads1015.c
 create mode 100644 include/linux/i2c/ads1015.h

diff --git a/Documentation/hwmon/ads1015 b/Documentation/hwmon/ads1015
new file mode 100644
index 0000000..2494e99
--- /dev/null
+++ b/Documentation/hwmon/ads1015
@@ -0,0 +1,72 @@
+Kernel driver ads1015
+==========+
+Supported chips:
+  * Texas Instruments ADS1015
+    Prefix: 'ads1015'
+    Datasheet: Publicly available at the Texas Instruments website :
+               http://focus.ti.com/lit/ds/symlink/ads1015.pdf
+
+Authors:
+        Dirk Eibach, Guntermann & Drunck GmbH <eibach@gdsys.de>
+
+Description
+-----------
+
+This driver implements support for the Texas Instruments ADS1015.
+
+This device is a 12-bit A-D converter with 4 inputs.
+
+The inputs can be used single ended or in certain differential combinations.
+
+The inputs are mapped to 8 sysfs input files in0_input - in7_input.
+The mapping can be configured using platform data or devicetree.
+
+Data sources for configuration:
+0: Voltage over AIN0 and AIN1.
+1: Voltage over AIN0 and AIN3.
+2: Voltage over AIN1 and AIN3.
+3: Voltage over AIN2 and AIN3.
+4: Voltage over AIN0 and GND.
+5: Voltage over AIN1 and GND.
+6: Voltage over AIN2 and GND.
+7: Voltage over AIN3 and GND.
+Any other value: disable
+
+By default in0_input is mapped to source 0, in1_input to source 1 and so on.
+
+Platform Data
+-------------
+
+In linux/i2c/ads1015.h platform data is defined as:
+
+struct ads1015_platform_data {
+	int exported_channels[8];
+};
+
+exported_channels contains the data sources for the 8 sysfs input files.
+
+Example:
+struct ads1015_platform_data data = {
+	4, 2, -1, -1, -1, -1, -1, -1 };
+
+In this case only in0_input and in1_input would be created.
+in0_input would give the voltage over AIN0 and GND.
+in0_input would give the voltage over AIN1 and AIN3.
+
+Devicetree
+----------
+
+The ads1015 node may have an "exported-channels" property with 8 integer
+values. The 8 values are the data sources for the 8 sysfs input files.
+
+Example:
+ads1015@49 {
+	compatible = "ti,ads1015";
+	reg = <0x49>;
+	exported-channels = < 4 2 0xff 0xff 0xff 0xff 0xff 0xff >;
+};
+
+In this case only in0_input and in1_input would be created.
+in0_input would give the voltage over AIN0 and GND.
+in0_input would give the voltage over AIN1 and AIN3.
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 773e484..7e247f7 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -871,6 +871,16 @@ config SENSORS_SMSC47B397
 	  This driver can also be built as a module.  If so, the module
 	  will be called smsc47b397.
 
+config SENSORS_ADS1015
+	tristate "Texas Instruments ADS1015"
+	depends on I2C
+	help
+	  If you say yes here you get support for Texas Instruments ADS1015
+	  12-bit 4-input ADC device.
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called ads1015.
+
 config SENSORS_ADS7828
 	tristate "Texas Instruments ADS7828"
 	depends on I2C
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index dde02d9..aae4036 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -29,6 +29,7 @@ obj-$(CONFIG_SENSORS_ADM1026)	+= adm1026.o
 obj-$(CONFIG_SENSORS_ADM1029)	+= adm1029.o
 obj-$(CONFIG_SENSORS_ADM1031)	+= adm1031.o
 obj-$(CONFIG_SENSORS_ADM9240)	+= adm9240.o
+obj-$(CONFIG_SENSORS_ADS1015)	+= ads1015.o
 obj-$(CONFIG_SENSORS_ADS7828)	+= ads7828.o
 obj-$(CONFIG_SENSORS_ADS7871)	+= ads7871.o
 obj-$(CONFIG_SENSORS_ADT7411)	+= adt7411.o
diff --git a/drivers/hwmon/ads1015.c b/drivers/hwmon/ads1015.c
new file mode 100644
index 0000000..cf7aff4
--- /dev/null
+++ b/drivers/hwmon/ads1015.c
@@ -0,0 +1,295 @@
+/*
+ * ads1015.c - lm_sensors driver for ads1015 12-bit 4-input ADC
+ * (C) Copyright 2010
+ * Dirk Eibach, Guntermann & Drunck GmbH <eibach@gdsys.de>
+ *
+ * Based on the ads7828 driver by Steve Hardy.
+ *
+ * Datasheet available at: http://focus.ti.com/lit/ds/symlink/ads1015.pdf
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/jiffies.h>
+#include <linux/i2c.h>
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
+#include <linux/err.h>
+#include <linux/mutex.h>
+#include <linux/i2c/ads1015.h>
+
+/* ADS1015 registers */
+enum {
+	ADS1015_CONVERSION = 0,
+	ADS1015_CONFIG = 1,
+};
+
+/* PGA fullscale voltages in mV */
+static const unsigned int fullscale_table[8] = {
+	6144, 4096, 2048, 1024, 512, 256, 256, 256 };
+
+/* Default set of exported channels */
+#define ADS1015_CONFIG_CHANNELS 8
+static const int default_channels[ADS1015_CONFIG_CHANNELS] = {
+	0, 1, 2, 3, 4, 5, 6, 7 };
+
+/* strings for sysfs */
+static const char *input_names[8] = {
+	"in0_input",
+	"in1_input",
+	"in2_input",
+	"in3_input",
+	"in4_input",
+	"in5_input",
+	"in6_input",
+	"in7_input"
+};
+
+struct ads1015_data {
+	struct device *hwmon_dev;
+	struct mutex update_lock; /* mutex protect updates */
+	struct sensor_device_attribute attr[ADS1015_CONFIG_CHANNELS];
+	struct attribute *attr_table[ADS1015_CONFIG_CHANNELS + 1];
+	struct attribute_group attr_group;
+};
+
+static s32 ads1015_read_reg(struct i2c_client *client, unsigned int reg)
+{
+	s32 data = i2c_smbus_read_word_data(client, reg);
+
+	return (data < 0) ? data : swab16(data);
+}
+
+static s32 ads1015_write_reg(struct i2c_client *client, unsigned int reg,
+			     u16 val)
+{
+	return i2c_smbus_write_word_data(client, reg, swab16(val));
+}
+
+static int ads1015_read_value(struct i2c_client *client, unsigned int channel,
+			      int *value)
+{
+	u16 config;
+	s16 conversion;
+	unsigned int pga;
+	int fullscale;
+	unsigned int k;
+	struct ads1015_data *data = i2c_get_clientdata(client);
+	int res;
+
+	mutex_lock(&data->update_lock);
+
+	/* get fullscale voltage */
+	res = ads1015_read_reg(client, ADS1015_CONFIG);
+	if (res < 0)
+		goto err_unlock;
+	config = res;
+	pga = (config >> 9) & 0x0007;
+	fullscale = fullscale_table[pga];
+
+	/* set channel and start single conversion */
+	config &= ~(0x0007 << 12);
+	config |= (1 << 15) | (1 << 8) | (channel & 0x0007) << 12;
+
+	/* wait until conversion finished */
+	res = ads1015_write_reg(client, ADS1015_CONFIG, config);
+	if (res < 0)
+		goto err_unlock;
+	for (k = 0; k < 5; ++k) {
+		schedule_timeout(msecs_to_jiffies(1));
+		res = ads1015_read_reg(client, ADS1015_CONFIG);
+		if (res < 0)
+			goto err_unlock;
+		config = res;
+		if (config & (1 << 15))
+			break;
+	}
+	if (k = 5) {
+		res = -EIO;
+		goto err_unlock;
+	}
+
+	res = ads1015_read_reg(client, ADS1015_CONVERSION);
+	if (res < 0)
+		goto err_unlock;
+	conversion = res;
+
+	mutex_unlock(&data->update_lock);
+
+	*value = DIV_ROUND_CLOSEST(conversion * fullscale, 0x7ff0);
+
+	return 0;
+
+err_unlock:
+	mutex_unlock(&data->update_lock);
+	return res;
+}
+
+/* sysfs callback function */
+static ssize_t show_in(struct device *dev, struct device_attribute *da,
+	char *buf)
+{
+	struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
+	struct i2c_client *client = to_i2c_client(dev);
+	int in;
+	int res;
+
+	res = ads1015_read_value(client, attr->index, &in);
+
+	return (res < 0) ? res : sprintf(buf, "%d\n", in);
+}
+
+/*
+ * Driver interface
+ */
+
+static int ads1015_remove(struct i2c_client *client)
+{
+	struct ads1015_data *data = i2c_get_clientdata(client);
+	hwmon_device_unregister(data->hwmon_dev);
+	sysfs_remove_group(&client->dev.kobj, &data->attr_group);
+	kfree(data);
+	return 0;
+}
+
+static void ads1015_get_exported_channels(struct i2c_client *client,
+					  int *exported_channels)
+{
+	struct ads1015_platform_data *pdata = dev_get_platdata(&client->dev);
+#ifdef CONFIG_OF
+	struct device_node *np = client->dev.of_node;
+	const int *of_channels;
+	int of_channels_size;
+#endif
+
+	/* prefer platform data */
+	if (pdata) {
+		memcpy(exported_channels, pdata->exported_channels,
+		       sizeof(default_channels));
+		return;
+	}
+
+#ifdef CONFIG_OF
+	/* fallback on OF */
+	of_channels = of_get_property(np, "exported-channels",
+				      &of_channels_size);
+	if (of_channels && (of_channels_size = sizeof(default_channels))) {
+		memcpy(exported_channels, of_channels,
+		       sizeof(default_channels));
+		return;
+	}
+#endif
+
+	/* fallback on default configuration */
+	memcpy(exported_channels, default_channels, sizeof(default_channels));
+}
+
+/* create sysfs attribute according to channel setup */
+static struct attribute *ads1015_export_channel(struct i2c_client *client,
+						unsigned int input, int channel)
+{
+	struct ads1015_data *data = i2c_get_clientdata(client);
+	struct sensor_device_attribute attr +		SENSOR_ATTR(input, S_IRUGO, show_in, NULL, channel);
+
+	attr_name(attr.dev_attr) = input_names[input];
+
+	memcpy(&data->attr[input], &attr, sizeof(attr));
+
+	return &data->attr[input].dev_attr.attr;
+}
+
+static int ads1015_probe(struct i2c_client *client,
+			 const struct i2c_device_id *id)
+{
+	struct ads1015_data *data;
+	int err;
+	int exported_channels[ADS1015_CONFIG_CHANNELS];
+	unsigned int k;
+	unsigned int act_attr = 0;
+
+	data = kzalloc(sizeof(struct ads1015_data), GFP_KERNEL);
+	if (!data) {
+		err = -ENOMEM;
+		goto exit;
+	}
+
+	i2c_set_clientdata(client, data);
+	mutex_init(&data->update_lock);
+
+	/* Register sysfs hooks */
+	data->attr_group.attrs = data->attr_table;
+	ads1015_get_exported_channels(client, exported_channels);
+	for (k = 0; k < ADS1015_CONFIG_CHANNELS; ++k) {
+		int channel = exported_channels[k];
+		if ((channel < 0) || (channel > 7))
+			continue;
+		data->attr_table[act_attr++] +			ads1015_export_channel(client, k, channel);
+	}
+	err = sysfs_create_group(&client->dev.kobj, &data->attr_group);
+	if (err)
+		goto exit_free;
+
+	data->hwmon_dev = hwmon_device_register(&client->dev);
+	if (IS_ERR(data->hwmon_dev)) {
+		err = PTR_ERR(data->hwmon_dev);
+		goto exit_remove;
+	}
+
+	return 0;
+
+exit_remove:
+	sysfs_remove_group(&client->dev.kobj, &data->attr_group);
+exit_free:
+	kfree(data);
+exit:
+	return err;
+}
+
+static const struct i2c_device_id ads1015_id[] = {
+	{ "ads1015", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, ads1015_id);
+
+static struct i2c_driver ads1015_driver = {
+	.driver = {
+		.name = "ads1015",
+	},
+	.probe = ads1015_probe,
+	.remove = ads1015_remove,
+	.id_table = ads1015_id,
+};
+
+static int __init sensors_ads1015_init(void)
+{
+	return i2c_add_driver(&ads1015_driver);
+}
+
+static void __exit sensors_ads1015_exit(void)
+{
+	i2c_del_driver(&ads1015_driver);
+}
+
+MODULE_AUTHOR("Dirk Eibach <eibach@gdsys.de>");
+MODULE_DESCRIPTION("ADS1015 driver");
+MODULE_LICENSE("GPL");
+
+module_init(sensors_ads1015_init);
+module_exit(sensors_ads1015_exit);
diff --git a/include/linux/i2c/ads1015.h b/include/linux/i2c/ads1015.h
new file mode 100644
index 0000000..152bf5f
--- /dev/null
+++ b/include/linux/i2c/ads1015.h
@@ -0,0 +1,30 @@
+/*
+ * Platform Data for ADS1015 12-bit 4-input ADC
+ * (C) Copyright 2010
+ * Dirk Eibach, Guntermann & Drunck GmbH <eibach@gdsys.de>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+#ifndef LINUX_ADS1015_H
+#define LINUX_ADS1015_H
+
+#include <linux/types.h>
+
+struct ads1015_platform_data {
+	int exported_channels[8];
+};
+
+#endif /* LINUX_ADS1015_H */
-- 
1.5.6.5


_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

[PATCH v3] hwmon: Add support for Texas Instruments ADS1015

[Dirk Eibach]

Signed-off-by: Dirk Eibach <eibach@gdsys.de>
---
Changes since v1:
- fixed/extended Documentation
- removed unused register definitions
- hardcoded PGA fullscale table size
- made sure patch applies against v2.6.38-rc4
- reordered functions to avoid forward declaration
- results from i2c_smbus_read_word_data() are handled correctly
- moved locking into ads1015_read_value()
- removed unnecessray clearing of bit
- proper error handling in ads1015_read_value()
- use DIV_ROUND_CLOSEST for scaling result
- removed detect()

Changes since v2:
- removed *all* leftovers from detect()
- fixed return with mutex held
- made sysfs representation configurable
  (hope this will be the reference implementation for generations to come ;)

 Documentation/hwmon/ads1015 |   72 +++++++++++
 drivers/hwmon/Kconfig       |   10 ++
 drivers/hwmon/Makefile      |    1 +
 drivers/hwmon/ads1015.c     |  295 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/i2c/ads1015.h |   30 +++++
 5 files changed, 408 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/hwmon/ads1015
 create mode 100644 drivers/hwmon/ads1015.c
 create mode 100644 include/linux/i2c/ads1015.h

diff --git a/Documentation/hwmon/ads1015 b/Documentation/hwmon/ads1015
new file mode 100644
index 0000000..2494e99
--- /dev/null
+++ b/Documentation/hwmon/ads1015
@@ -0,0 +1,72 @@
+Kernel driver ads1015
+=====================
+
+Supported chips:
+  * Texas Instruments ADS1015
+    Prefix: 'ads1015'
+    Datasheet: Publicly available at the Texas Instruments website :
+               http://focus.ti.com/lit/ds/symlink/ads1015.pdf
+
+Authors:
+        Dirk Eibach, Guntermann & Drunck GmbH <eibach@gdsys.de>
+
+Description
+-----------
+
+This driver implements support for the Texas Instruments ADS1015.
+
+This device is a 12-bit A-D converter with 4 inputs.
+
+The inputs can be used single ended or in certain differential combinations.
+
+The inputs are mapped to 8 sysfs input files in0_input - in7_input.
+The mapping can be configured using platform data or devicetree.
+
+Data sources for configuration:
+0: Voltage over AIN0 and AIN1.
+1: Voltage over AIN0 and AIN3.
+2: Voltage over AIN1 and AIN3.
+3: Voltage over AIN2 and AIN3.
+4: Voltage over AIN0 and GND.
+5: Voltage over AIN1 and GND.
+6: Voltage over AIN2 and GND.
+7: Voltage over AIN3 and GND.
+Any other value: disable
+
+By default in0_input is mapped to source 0, in1_input to source 1 and so on.
+
+Platform Data
+-------------
+
+In linux/i2c/ads1015.h platform data is defined as:
+
+struct ads1015_platform_data {
+	int exported_channels[8];
+};
+
+exported_channels contains the data sources for the 8 sysfs input files.
+
+Example:
+struct ads1015_platform_data data = {
+	4, 2, -1, -1, -1, -1, -1, -1 };
+
+In this case only in0_input and in1_input would be created.
+in0_input would give the voltage over AIN0 and GND.
+in0_input would give the voltage over AIN1 and AIN3.
+
+Devicetree
+----------
+
+The ads1015 node may have an "exported-channels" property with 8 integer
+values. The 8 values are the data sources for the 8 sysfs input files.
+
+Example:
+ads1015@49 {
+	compatible = "ti,ads1015";
+	reg = <0x49>;
+	exported-channels = < 4 2 0xff 0xff 0xff 0xff 0xff 0xff >;
+};
+
+In this case only in0_input and in1_input would be created.
+in0_input would give the voltage over AIN0 and GND.
+in0_input would give the voltage over AIN1 and AIN3.
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 773e484..7e247f7 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -871,6 +871,16 @@ config SENSORS_SMSC47B397
 	  This driver can also be built as a module.  If so, the module
 	  will be called smsc47b397.
 
+config SENSORS_ADS1015
+	tristate "Texas Instruments ADS1015"
+	depends on I2C
+	help
+	  If you say yes here you get support for Texas Instruments ADS1015
+	  12-bit 4-input ADC device.
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called ads1015.
+
 config SENSORS_ADS7828
 	tristate "Texas Instruments ADS7828"
 	depends on I2C
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index dde02d9..aae4036 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -29,6 +29,7 @@ obj-$(CONFIG_SENSORS_ADM1026)	+= adm1026.o
 obj-$(CONFIG_SENSORS_ADM1029)	+= adm1029.o
 obj-$(CONFIG_SENSORS_ADM1031)	+= adm1031.o
 obj-$(CONFIG_SENSORS_ADM9240)	+= adm9240.o
+obj-$(CONFIG_SENSORS_ADS1015)	+= ads1015.o
 obj-$(CONFIG_SENSORS_ADS7828)	+= ads7828.o
 obj-$(CONFIG_SENSORS_ADS7871)	+= ads7871.o
 obj-$(CONFIG_SENSORS_ADT7411)	+= adt7411.o
diff --git a/drivers/hwmon/ads1015.c b/drivers/hwmon/ads1015.c
new file mode 100644
index 0000000..cf7aff4
--- /dev/null
+++ b/drivers/hwmon/ads1015.c
@@ -0,0 +1,295 @@
+/*
+ * ads1015.c - lm_sensors driver for ads1015 12-bit 4-input ADC
+ * (C) Copyright 2010
+ * Dirk Eibach, Guntermann & Drunck GmbH <eibach@gdsys.de>
+ *
+ * Based on the ads7828 driver by Steve Hardy.
+ *
+ * Datasheet available at: http://focus.ti.com/lit/ds/symlink/ads1015.pdf
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/jiffies.h>
+#include <linux/i2c.h>
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
+#include <linux/err.h>
+#include <linux/mutex.h>
+#include <linux/i2c/ads1015.h>
+
+/* ADS1015 registers */
+enum {
+	ADS1015_CONVERSION = 0,
+	ADS1015_CONFIG = 1,
+};
+
+/* PGA fullscale voltages in mV */
+static const unsigned int fullscale_table[8] = {
+	6144, 4096, 2048, 1024, 512, 256, 256, 256 };
+
+/* Default set of exported channels */
+#define ADS1015_CONFIG_CHANNELS 8
+static const int default_channels[ADS1015_CONFIG_CHANNELS] = {
+	0, 1, 2, 3, 4, 5, 6, 7 };
+
+/* strings for sysfs */
+static const char *input_names[8] = {
+	"in0_input",
+	"in1_input",
+	"in2_input",
+	"in3_input",
+	"in4_input",
+	"in5_input",
+	"in6_input",
+	"in7_input"
+};
+
+struct ads1015_data {
+	struct device *hwmon_dev;
+	struct mutex update_lock; /* mutex protect updates */
+	struct sensor_device_attribute attr[ADS1015_CONFIG_CHANNELS];
+	struct attribute *attr_table[ADS1015_CONFIG_CHANNELS + 1];
+	struct attribute_group attr_group;
+};
+
+static s32 ads1015_read_reg(struct i2c_client *client, unsigned int reg)
+{
+	s32 data = i2c_smbus_read_word_data(client, reg);
+
+	return (data < 0) ? data : swab16(data);
+}
+
+static s32 ads1015_write_reg(struct i2c_client *client, unsigned int reg,
+			     u16 val)
+{
+	return i2c_smbus_write_word_data(client, reg, swab16(val));
+}
+
+static int ads1015_read_value(struct i2c_client *client, unsigned int channel,
+			      int *value)
+{
+	u16 config;
+	s16 conversion;
+	unsigned int pga;
+	int fullscale;
+	unsigned int k;
+	struct ads1015_data *data = i2c_get_clientdata(client);
+	int res;
+
+	mutex_lock(&data->update_lock);
+
+	/* get fullscale voltage */
+	res = ads1015_read_reg(client, ADS1015_CONFIG);
+	if (res < 0)
+		goto err_unlock;
+	config = res;
+	pga = (config >> 9) & 0x0007;
+	fullscale = fullscale_table[pga];
+
+	/* set channel and start single conversion */
+	config &= ~(0x0007 << 12);
+	config |= (1 << 15) | (1 << 8) | (channel & 0x0007) << 12;
+
+	/* wait until conversion finished */
+	res = ads1015_write_reg(client, ADS1015_CONFIG, config);
+	if (res < 0)
+		goto err_unlock;
+	for (k = 0; k < 5; ++k) {
+		schedule_timeout(msecs_to_jiffies(1));
+		res = ads1015_read_reg(client, ADS1015_CONFIG);
+		if (res < 0)
+			goto err_unlock;
+		config = res;
+		if (config & (1 << 15))
+			break;
+	}
+	if (k == 5) {
+		res = -EIO;
+		goto err_unlock;
+	}
+
+	res = ads1015_read_reg(client, ADS1015_CONVERSION);
+	if (res < 0)
+		goto err_unlock;
+	conversion = res;
+
+	mutex_unlock(&data->update_lock);
+
+	*value = DIV_ROUND_CLOSEST(conversion * fullscale, 0x7ff0);
+
+	return 0;
+
+err_unlock:
+	mutex_unlock(&data->update_lock);
+	return res;
+}
+
+/* sysfs callback function */
+static ssize_t show_in(struct device *dev, struct device_attribute *da,
+	char *buf)
+{
+	struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
+	struct i2c_client *client = to_i2c_client(dev);
+	int in;
+	int res;
+
+	res = ads1015_read_value(client, attr->index, &in);
+
+	return (res < 0) ? res : sprintf(buf, "%d\n", in);
+}
+
+/*
+ * Driver interface
+ */
+
+static int ads1015_remove(struct i2c_client *client)
+{
+	struct ads1015_data *data = i2c_get_clientdata(client);
+	hwmon_device_unregister(data->hwmon_dev);
+	sysfs_remove_group(&client->dev.kobj, &data->attr_group);
+	kfree(data);
+	return 0;
+}
+
+static void ads1015_get_exported_channels(struct i2c_client *client,
+					  int *exported_channels)
+{
+	struct ads1015_platform_data *pdata = dev_get_platdata(&client->dev);
+#ifdef CONFIG_OF
+	struct device_node *np = client->dev.of_node;
+	const int *of_channels;
+	int of_channels_size;
+#endif
+
+	/* prefer platform data */
+	if (pdata) {
+		memcpy(exported_channels, pdata->exported_channels,
+		       sizeof(default_channels));
+		return;
+	}
+
+#ifdef CONFIG_OF
+	/* fallback on OF */
+	of_channels = of_get_property(np, "exported-channels",
+				      &of_channels_size);
+	if (of_channels && (of_channels_size == sizeof(default_channels))) {
+		memcpy(exported_channels, of_channels,
+		       sizeof(default_channels));
+		return;
+	}
+#endif
+
+	/* fallback on default configuration */
+	memcpy(exported_channels, default_channels, sizeof(default_channels));
+}
+
+/* create sysfs attribute according to channel setup */
+static struct attribute *ads1015_export_channel(struct i2c_client *client,
+						unsigned int input, int channel)
+{
+	struct ads1015_data *data = i2c_get_clientdata(client);
+	struct sensor_device_attribute attr =
+		SENSOR_ATTR(input, S_IRUGO, show_in, NULL, channel);
+
+	attr_name(attr.dev_attr) = input_names[input];
+
+	memcpy(&data->attr[input], &attr, sizeof(attr));
+
+	return &data->attr[input].dev_attr.attr;
+}
+
+static int ads1015_probe(struct i2c_client *client,
+			 const struct i2c_device_id *id)
+{
+	struct ads1015_data *data;
+	int err;
+	int exported_channels[ADS1015_CONFIG_CHANNELS];
+	unsigned int k;
+	unsigned int act_attr = 0;
+
+	data = kzalloc(sizeof(struct ads1015_data), GFP_KERNEL);
+	if (!data) {
+		err = -ENOMEM;
+		goto exit;
+	}
+
+	i2c_set_clientdata(client, data);
+	mutex_init(&data->update_lock);
+
+	/* Register sysfs hooks */
+	data->attr_group.attrs = data->attr_table;
+	ads1015_get_exported_channels(client, exported_channels);
+	for (k = 0; k < ADS1015_CONFIG_CHANNELS; ++k) {
+		int channel = exported_channels[k];
+		if ((channel < 0) || (channel > 7))
+			continue;
+		data->attr_table[act_attr++] =
+			ads1015_export_channel(client, k, channel);
+	}
+	err = sysfs_create_group(&client->dev.kobj, &data->attr_group);
+	if (err)
+		goto exit_free;
+
+	data->hwmon_dev = hwmon_device_register(&client->dev);
+	if (IS_ERR(data->hwmon_dev)) {
+		err = PTR_ERR(data->hwmon_dev);
+		goto exit_remove;
+	}
+
+	return 0;
+
+exit_remove:
+	sysfs_remove_group(&client->dev.kobj, &data->attr_group);
+exit_free:
+	kfree(data);
+exit:
+	return err;
+}
+
+static const struct i2c_device_id ads1015_id[] = {
+	{ "ads1015", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, ads1015_id);
+
+static struct i2c_driver ads1015_driver = {
+	.driver = {
+		.name = "ads1015",
+	},
+	.probe = ads1015_probe,
+	.remove = ads1015_remove,
+	.id_table = ads1015_id,
+};
+
+static int __init sensors_ads1015_init(void)
+{
+	return i2c_add_driver(&ads1015_driver);
+}
+
+static void __exit sensors_ads1015_exit(void)
+{
+	i2c_del_driver(&ads1015_driver);
+}
+
+MODULE_AUTHOR("Dirk Eibach <eibach@gdsys.de>");
+MODULE_DESCRIPTION("ADS1015 driver");
+MODULE_LICENSE("GPL");
+
+module_init(sensors_ads1015_init);
+module_exit(sensors_ads1015_exit);
diff --git a/include/linux/i2c/ads1015.h b/include/linux/i2c/ads1015.h
new file mode 100644
index 0000000..152bf5f
--- /dev/null
+++ b/include/linux/i2c/ads1015.h
@@ -0,0 +1,30 @@
+/*
+ * Platform Data for ADS1015 12-bit 4-input ADC
+ * (C) Copyright 2010
+ * Dirk Eibach, Guntermann & Drunck GmbH <eibach@gdsys.de>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+#ifndef LINUX_ADS1015_H
+#define LINUX_ADS1015_H
+
+#include <linux/types.h>
+
+struct ads1015_platform_data {
+	int exported_channels[8];
+};
+
+#endif /* LINUX_ADS1015_H */
-- 
1.5.6.5

Re: [lm-sensors] [PATCH v3] hwmon: Add support for Texas

[Jean Delvare]

Hi Dirk,

Sorry for the late reply.

On Fri, 18 Feb 2011 11:15:58 +0100, Dirk Eibach wrote:
> Signed-off-by: Dirk Eibach <eibach@gdsys.de>
> ---
> Changes since v1:
> - fixed/extended Documentation
> - removed unused register definitions
> - hardcoded PGA fullscale table size
> - made sure patch applies against v2.6.38-rc4
> - reordered functions to avoid forward declaration
> - results from i2c_smbus_read_word_data() are handled correctly
> - moved locking into ads1015_read_value()
> - removed unnecessray clearing of bit
> - proper error handling in ads1015_read_value()
> - use DIV_ROUND_CLOSEST for scaling result
> - removed detect()
> 
> Changes since v2:
> - removed *all* leftovers from detect()
> - fixed return with mutex held
> - made sysfs representation configurable
>   (hope this will be the reference implementation for generations to come ;)

Thanks for your continued work on this driver. The changes this time
are important enough to warrant a full review. Here we go:

>  Documentation/hwmon/ads1015 |   72 +++++++++++
>  drivers/hwmon/Kconfig       |   10 ++
>  drivers/hwmon/Makefile      |    1 +
>  drivers/hwmon/ads1015.c     |  295 +++++++++++++++++++++++++++++++++++++++++++
>  include/linux/i2c/ads1015.h |   30 +++++
>  5 files changed, 408 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/hwmon/ads1015
>  create mode 100644 drivers/hwmon/ads1015.c
>  create mode 100644 include/linux/i2c/ads1015.h
> 
> diff --git a/Documentation/hwmon/ads1015 b/Documentation/hwmon/ads1015
> new file mode 100644
> index 0000000..2494e99
> --- /dev/null
> +++ b/Documentation/hwmon/ads1015
> @@ -0,0 +1,72 @@
> +Kernel driver ads1015
> +==========> +
> +Supported chips:
> +  * Texas Instruments ADS1015
> +    Prefix: 'ads1015'
> +    Datasheet: Publicly available at the Texas Instruments website :
> +               http://focus.ti.com/lit/ds/symlink/ads1015.pdf
> +
> +Authors:
> +        Dirk Eibach, Guntermann & Drunck GmbH <eibach@gdsys.de>
> +
> +Description
> +-----------
> +
> +This driver implements support for the Texas Instruments ADS1015.
> +
> +This device is a 12-bit A-D converter with 4 inputs.
> +
> +The inputs can be used single ended or in certain differential combinations.
> +
> +The inputs are mapped to 8 sysfs input files in0_input - in7_input.
> +The mapping can be configured using platform data or devicetree.
> +
> +Data sources for configuration:
> +0: Voltage over AIN0 and AIN1.
> +1: Voltage over AIN0 and AIN3.
> +2: Voltage over AIN1 and AIN3.
> +3: Voltage over AIN2 and AIN3.
> +4: Voltage over AIN0 and GND.
> +5: Voltage over AIN1 and GND.
> +6: Voltage over AIN2 and GND.
> +7: Voltage over AIN3 and GND.
> +Any other value: disable
> +
> +By default in0_input is mapped to source 0, in1_input to source 1 and so on.

I see that you went for dynamic naming of sysfs files. I would have
used a different strategy which would make the code much more simple.
You can keep static sysfs file names, and instantiate them
conditionally. Maybe you were not aware of this, but it is perfectly
fine for an hwmon device to number its inputs non-linearly, and as a
matter of fact many hwmon driver do this.

For example, a setup where each input is used single-ended would result
in a hwmon device with attributes in4_input, in5_input, in6_input and
in7_input.

> +
> +Platform Data
> +-------------
> +
> +In linux/i2c/ads1015.h platform data is defined as:
> +
> +struct ads1015_platform_data {
> +	int exported_channels[8];
> +};
> +
> +exported_channels contains the data sources for the 8 sysfs input files.
> +
> +Example:
> +struct ads1015_platform_data data = {
> +	4, 2, -1, -1, -1, -1, -1, -1 };
> +
> +In this case only in0_input and in1_input would be created.
> +in0_input would give the voltage over AIN0 and GND.
> +in0_input would give the voltage over AIN1 and AIN3.

With my proposal, the platform data could be a single bitfield, where
each bit says enable or disable the corresponding sysfs attribute. For
example:

struct ads1015_platform_data {
	unsigned int channels;
};

Example:
struct ads1015_platform_data data = {
	.channels = (1 << 4) | (1 << 2)
}

The only drawback of my proposal is that you can't create the
attributes by group, you have to create them individually. Not a
problem in practice though. I suggest that you give it a try and see
what you prefer.

> +
> +Devicetree
> +----------
> +
> +The ads1015 node may have an "exported-channels" property with 8 integer
> +values. The 8 values are the data sources for the 8 sysfs input files.
> +
> +Example:
> +ads1015@49 {
> +	compatible = "ti,ads1015";
> +	reg = <0x49>;
> +	exported-channels = < 4 2 0xff 0xff 0xff 0xff 0xff 0xff >;
> +};
> +
> +In this case only in0_input and in1_input would be created.
> +in0_input would give the voltage over AIN0 and GND.
> +in0_input would give the voltage over AIN1 and AIN3.

You meant "in1_input" the second time.

Do you have an actual need for this? I think devicetree attributes have
to be discussed and documented appropriately? I admit I am not too
familiar with this.

> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 773e484..7e247f7 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -871,6 +871,16 @@ config SENSORS_SMSC47B397
>  	  This driver can also be built as a module.  If so, the module
>  	  will be called smsc47b397.
>  
> +config SENSORS_ADS1015
> +	tristate "Texas Instruments ADS1015"
> +	depends on I2C
> +	help
> +	  If you say yes here you get support for Texas Instruments ADS1015
> +	  12-bit 4-input ADC device.
> +
> +	  This driver can also be built as a module.  If so, the module
> +	  will be called ads1015.
> +
>  config SENSORS_ADS7828
>  	tristate "Texas Instruments ADS7828"
>  	depends on I2C
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index dde02d9..aae4036 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -29,6 +29,7 @@ obj-$(CONFIG_SENSORS_ADM1026)	+= adm1026.o
>  obj-$(CONFIG_SENSORS_ADM1029)	+= adm1029.o
>  obj-$(CONFIG_SENSORS_ADM1031)	+= adm1031.o
>  obj-$(CONFIG_SENSORS_ADM9240)	+= adm9240.o
> +obj-$(CONFIG_SENSORS_ADS1015)	+= ads1015.o
>  obj-$(CONFIG_SENSORS_ADS7828)	+= ads7828.o
>  obj-$(CONFIG_SENSORS_ADS7871)	+= ads7871.o
>  obj-$(CONFIG_SENSORS_ADT7411)	+= adt7411.o
> diff --git a/drivers/hwmon/ads1015.c b/drivers/hwmon/ads1015.c
> new file mode 100644
> index 0000000..cf7aff4
> --- /dev/null
> +++ b/drivers/hwmon/ads1015.c
> @@ -0,0 +1,295 @@
> +/*
> + * ads1015.c - lm_sensors driver for ads1015 12-bit 4-input ADC
> + * (C) Copyright 2010
> + * Dirk Eibach, Guntermann & Drunck GmbH <eibach@gdsys.de>
> + *
> + * Based on the ads7828 driver by Steve Hardy.
> + *
> + * Datasheet available at: http://focus.ti.com/lit/ds/symlink/ads1015.pdf
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/slab.h>
> +#include <linux/jiffies.h>
> +#include <linux/i2c.h>
> +#include <linux/hwmon.h>
> +#include <linux/hwmon-sysfs.h>
> +#include <linux/err.h>
> +#include <linux/mutex.h>

You have to include <linux/of.h> for device tree support.

> +#include <linux/i2c/ads1015.h>
> +
> +/* ADS1015 registers */
> +enum {
> +	ADS1015_CONVERSION = 0,
> +	ADS1015_CONFIG = 1,
> +};
> +
> +/* PGA fullscale voltages in mV */
> +static const unsigned int fullscale_table[8] = {
> +	6144, 4096, 2048, 1024, 512, 256, 256, 256 };
> +
> +/* Default set of exported channels */
> +#define ADS1015_CONFIG_CHANNELS 8
> +static const int default_channels[ADS1015_CONFIG_CHANNELS] = {
> +	0, 1, 2, 3, 4, 5, 6, 7 };
> +
> +/* strings for sysfs */
> +static const char *input_names[8] = {
> +	"in0_input",
> +	"in1_input",
> +	"in2_input",
> +	"in3_input",
> +	"in4_input",
> +	"in5_input",
> +	"in6_input",
> +	"in7_input"
> +};
> +
> +struct ads1015_data {
> +	struct device *hwmon_dev;
> +	struct mutex update_lock; /* mutex protect updates */
> +	struct sensor_device_attribute attr[ADS1015_CONFIG_CHANNELS];
> +	struct attribute *attr_table[ADS1015_CONFIG_CHANNELS + 1];
> +	struct attribute_group attr_group;
> +};
> +
> +static s32 ads1015_read_reg(struct i2c_client *client, unsigned int reg)
> +{
> +	s32 data = i2c_smbus_read_word_data(client, reg);
> +
> +	return (data < 0) ? data : swab16(data);
> +}
> +
> +static s32 ads1015_write_reg(struct i2c_client *client, unsigned int reg,
> +			     u16 val)
> +{
> +	return i2c_smbus_write_word_data(client, reg, swab16(val));
> +}
> +
> +static int ads1015_read_value(struct i2c_client *client, unsigned int channel,
> +			      int *value)
> +{
> +	u16 config;
> +	s16 conversion;
> +	unsigned int pga;
> +	int fullscale;
> +	unsigned int k;
> +	struct ads1015_data *data = i2c_get_clientdata(client);
> +	int res;
> +
> +	mutex_lock(&data->update_lock);
> +
> +	/* get fullscale voltage */
> +	res = ads1015_read_reg(client, ADS1015_CONFIG);
> +	if (res < 0)
> +		goto err_unlock;
> +	config = res;
> +	pga = (config >> 9) & 0x0007;
> +	fullscale = fullscale_table[pga];

You only read the fullscale value. Now that we have platform data
attached to the device, don't you think it would make sense to let the
user set it, possibly even define different values for each "virtual
channel"? I can imagine that different scaling factors make sense for
single-ended vs. differential measurements, or even for different
single-ended inputs.

This is just a question, BTW, this feature can be added later if needed.

> +
> +	/* set channel and start single conversion */
> +	config &= ~(0x0007 << 12);
> +	config |= (1 << 15) | (1 << 8) | (channel & 0x0007) << 12;
> +
> +	/* wait until conversion finished */
> +	res = ads1015_write_reg(client, ADS1015_CONFIG, config);
> +	if (res < 0)
> +		goto err_unlock;
> +	for (k = 0; k < 5; ++k) {
> +		schedule_timeout(msecs_to_jiffies(1));
> +		res = ads1015_read_reg(client, ADS1015_CONFIG);
> +		if (res < 0)
> +			goto err_unlock;
> +		config = res;
> +		if (config & (1 << 15))
> +			break;
> +	}
> +	if (k = 5) {
> +		res = -EIO;
> +		goto err_unlock;
> +	}
> +
> +	res = ads1015_read_reg(client, ADS1015_CONVERSION);
> +	if (res < 0)
> +		goto err_unlock;
> +	conversion = res;
> +
> +	mutex_unlock(&data->update_lock);
> +
> +	*value = DIV_ROUND_CLOSEST(conversion * fullscale, 0x7ff0);
> +
> +	return 0;
> +
> +err_unlock:
> +	mutex_unlock(&data->update_lock);
> +	return res;
> +}
> +
> +/* sysfs callback function */
> +static ssize_t show_in(struct device *dev, struct device_attribute *da,
> +	char *buf)
> +{
> +	struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
> +	struct i2c_client *client = to_i2c_client(dev);
> +	int in;
> +	int res;
> +
> +	res = ads1015_read_value(client, attr->index, &in);
> +
> +	return (res < 0) ? res : sprintf(buf, "%d\n", in);
> +}
> +
> +/*
> + * Driver interface
> + */
> +
> +static int ads1015_remove(struct i2c_client *client)
> +{
> +	struct ads1015_data *data = i2c_get_clientdata(client);
> +	hwmon_device_unregister(data->hwmon_dev);
> +	sysfs_remove_group(&client->dev.kobj, &data->attr_group);
> +	kfree(data);
> +	return 0;
> +}
> +
> +static void ads1015_get_exported_channels(struct i2c_client *client,
> +					  int *exported_channels)
> +{
> +	struct ads1015_platform_data *pdata = dev_get_platdata(&client->dev);
> +#ifdef CONFIG_OF
> +	struct device_node *np = client->dev.of_node;
> +	const int *of_channels;
> +	int of_channels_size;
> +#endif
> +
> +	/* prefer platform data */
> +	if (pdata) {
> +		memcpy(exported_channels, pdata->exported_channels,
> +		       sizeof(default_channels));
> +		return;
> +	}
> +
> +#ifdef CONFIG_OF
> +	/* fallback on OF */
> +	of_channels = of_get_property(np, "exported-channels",
> +				      &of_channels_size);
> +	if (of_channels && (of_channels_size = sizeof(default_channels))) {
> +		memcpy(exported_channels, of_channels,
> +		       sizeof(default_channels));
> +		return;
> +	}
> +#endif
> +
> +	/* fallback on default configuration */
> +	memcpy(exported_channels, default_channels, sizeof(default_channels));
> +}

Why don't you just return a pointer to the data? You only need the data
during probe and you make no changes to it, so I see no need to copy
the data.

> +
> +/* create sysfs attribute according to channel setup */
> +static struct attribute *ads1015_export_channel(struct i2c_client *client,
> +						unsigned int input, int channel)
> +{
> +	struct ads1015_data *data = i2c_get_clientdata(client);
> +	struct sensor_device_attribute attr > +		SENSOR_ATTR(input, S_IRUGO, show_in, NULL, channel);
> +
> +	attr_name(attr.dev_attr) = input_names[input];
> +
> +	memcpy(&data->attr[input], &attr, sizeof(attr));
> +
> +	return &data->attr[input].dev_attr.attr;
> +}
> +
> +static int ads1015_probe(struct i2c_client *client,
> +			 const struct i2c_device_id *id)
> +{
> +	struct ads1015_data *data;
> +	int err;
> +	int exported_channels[ADS1015_CONFIG_CHANNELS];
> +	unsigned int k;
> +	unsigned int act_attr = 0;

"act"?

> +
> +	data = kzalloc(sizeof(struct ads1015_data), GFP_KERNEL);
> +	if (!data) {
> +		err = -ENOMEM;
> +		goto exit;
> +	}
> +
> +	i2c_set_clientdata(client, data);
> +	mutex_init(&data->update_lock);
> +
> +	/* Register sysfs hooks */
> +	data->attr_group.attrs = data->attr_table;
> +	ads1015_get_exported_channels(client, exported_channels);
> +	for (k = 0; k < ADS1015_CONFIG_CHANNELS; ++k) {
> +		int channel = exported_channels[k];
> +		if ((channel < 0) || (channel > 7))

You don't need all these parentheses.

> +			continue;

Is there any benefit in continuing here, as opposed to breaking?
Breaking would let you use k below to index data->attr_table, instead
of tracking act_attr separately.

> +		data->attr_table[act_attr++] > +			ads1015_export_channel(client, k, channel);
> +	}
> +	err = sysfs_create_group(&client->dev.kobj, &data->attr_group);
> +	if (err)
> +		goto exit_free;
> +
> +	data->hwmon_dev = hwmon_device_register(&client->dev);
> +	if (IS_ERR(data->hwmon_dev)) {
> +		err = PTR_ERR(data->hwmon_dev);
> +		goto exit_remove;
> +	}
> +
> +	return 0;
> +
> +exit_remove:
> +	sysfs_remove_group(&client->dev.kobj, &data->attr_group);
> +exit_free:
> +	kfree(data);
> +exit:
> +	return err;
> +}
> +
> +static const struct i2c_device_id ads1015_id[] = {
> +	{ "ads1015", 0 },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, ads1015_id);
> +
> +static struct i2c_driver ads1015_driver = {
> +	.driver = {
> +		.name = "ads1015",
> +	},
> +	.probe = ads1015_probe,
> +	.remove = ads1015_remove,
> +	.id_table = ads1015_id,
> +};
> +
> +static int __init sensors_ads1015_init(void)
> +{
> +	return i2c_add_driver(&ads1015_driver);
> +}
> +
> +static void __exit sensors_ads1015_exit(void)
> +{
> +	i2c_del_driver(&ads1015_driver);
> +}
> +
> +MODULE_AUTHOR("Dirk Eibach <eibach@gdsys.de>");
> +MODULE_DESCRIPTION("ADS1015 driver");
> +MODULE_LICENSE("GPL");
> +
> +module_init(sensors_ads1015_init);
> +module_exit(sensors_ads1015_exit);
> diff --git a/include/linux/i2c/ads1015.h b/include/linux/i2c/ads1015.h
> new file mode 100644
> index 0000000..152bf5f
> --- /dev/null
> +++ b/include/linux/i2c/ads1015.h
> @@ -0,0 +1,30 @@
> +/*
> + * Platform Data for ADS1015 12-bit 4-input ADC
> + * (C) Copyright 2010
> + * Dirk Eibach, Guntermann & Drunck GmbH <eibach@gdsys.de>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
> + */
> +
> +#ifndef LINUX_ADS1015_H
> +#define LINUX_ADS1015_H
> +
> +#include <linux/types.h>

You don't use anything from that header file.

> +
> +struct ads1015_platform_data {
> +	int exported_channels[8];
> +};
> +
> +#endif /* LINUX_ADS1015_H */


-- 
Jean Delvare

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [PATCH v3] hwmon: Add support for Texas Instruments ADS1015

[Jean Delvare]

Hi Dirk,

Sorry for the late reply.

On Fri, 18 Feb 2011 11:15:58 +0100, Dirk Eibach wrote:
> Signed-off-by: Dirk Eibach <eibach@gdsys.de>
> ---
> Changes since v1:
> - fixed/extended Documentation
> - removed unused register definitions
> - hardcoded PGA fullscale table size
> - made sure patch applies against v2.6.38-rc4
> - reordered functions to avoid forward declaration
> - results from i2c_smbus_read_word_data() are handled correctly
> - moved locking into ads1015_read_value()
> - removed unnecessray clearing of bit
> - proper error handling in ads1015_read_value()
> - use DIV_ROUND_CLOSEST for scaling result
> - removed detect()
> 
> Changes since v2:
> - removed *all* leftovers from detect()
> - fixed return with mutex held
> - made sysfs representation configurable
>   (hope this will be the reference implementation for generations to come ;)

Thanks for your continued work on this driver. The changes this time
are important enough to warrant a full review. Here we go:

>  Documentation/hwmon/ads1015 |   72 +++++++++++
>  drivers/hwmon/Kconfig       |   10 ++
>  drivers/hwmon/Makefile      |    1 +
>  drivers/hwmon/ads1015.c     |  295 +++++++++++++++++++++++++++++++++++++++++++
>  include/linux/i2c/ads1015.h |   30 +++++
>  5 files changed, 408 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/hwmon/ads1015
>  create mode 100644 drivers/hwmon/ads1015.c
>  create mode 100644 include/linux/i2c/ads1015.h
> 
> diff --git a/Documentation/hwmon/ads1015 b/Documentation/hwmon/ads1015
> new file mode 100644
> index 0000000..2494e99
> --- /dev/null
> +++ b/Documentation/hwmon/ads1015
> @@ -0,0 +1,72 @@
> +Kernel driver ads1015
> +=====================
> +
> +Supported chips:
> +  * Texas Instruments ADS1015
> +    Prefix: 'ads1015'
> +    Datasheet: Publicly available at the Texas Instruments website :
> +               http://focus.ti.com/lit/ds/symlink/ads1015.pdf
> +
> +Authors:
> +        Dirk Eibach, Guntermann & Drunck GmbH <eibach@gdsys.de>
> +
> +Description
> +-----------
> +
> +This driver implements support for the Texas Instruments ADS1015.
> +
> +This device is a 12-bit A-D converter with 4 inputs.
> +
> +The inputs can be used single ended or in certain differential combinations.
> +
> +The inputs are mapped to 8 sysfs input files in0_input - in7_input.
> +The mapping can be configured using platform data or devicetree.
> +
> +Data sources for configuration:
> +0: Voltage over AIN0 and AIN1.
> +1: Voltage over AIN0 and AIN3.
> +2: Voltage over AIN1 and AIN3.
> +3: Voltage over AIN2 and AIN3.
> +4: Voltage over AIN0 and GND.
> +5: Voltage over AIN1 and GND.
> +6: Voltage over AIN2 and GND.
> +7: Voltage over AIN3 and GND.
> +Any other value: disable
> +
> +By default in0_input is mapped to source 0, in1_input to source 1 and so on.

I see that you went for dynamic naming of sysfs files. I would have
used a different strategy which would make the code much more simple.
You can keep static sysfs file names, and instantiate them
conditionally. Maybe you were not aware of this, but it is perfectly
fine for an hwmon device to number its inputs non-linearly, and as a
matter of fact many hwmon driver do this.

For example, a setup where each input is used single-ended would result
in a hwmon device with attributes in4_input, in5_input, in6_input and
in7_input.

> +
> +Platform Data
> +-------------
> +
> +In linux/i2c/ads1015.h platform data is defined as:
> +
> +struct ads1015_platform_data {
> +	int exported_channels[8];
> +};
> +
> +exported_channels contains the data sources for the 8 sysfs input files.
> +
> +Example:
> +struct ads1015_platform_data data = {
> +	4, 2, -1, -1, -1, -1, -1, -1 };
> +
> +In this case only in0_input and in1_input would be created.
> +in0_input would give the voltage over AIN0 and GND.
> +in0_input would give the voltage over AIN1 and AIN3.

With my proposal, the platform data could be a single bitfield, where
each bit says enable or disable the corresponding sysfs attribute. For
example:

struct ads1015_platform_data {
	unsigned int channels;
};

Example:
struct ads1015_platform_data data = {
	.channels = (1 << 4) | (1 << 2)
}

The only drawback of my proposal is that you can't create the
attributes by group, you have to create them individually. Not a
problem in practice though. I suggest that you give it a try and see
what you prefer.

> +
> +Devicetree
> +----------
> +
> +The ads1015 node may have an "exported-channels" property with 8 integer
> +values. The 8 values are the data sources for the 8 sysfs input files.
> +
> +Example:
> +ads1015@49 {
> +	compatible = "ti,ads1015";
> +	reg = <0x49>;
> +	exported-channels = < 4 2 0xff 0xff 0xff 0xff 0xff 0xff >;
> +};
> +
> +In this case only in0_input and in1_input would be created.
> +in0_input would give the voltage over AIN0 and GND.
> +in0_input would give the voltage over AIN1 and AIN3.

You meant "in1_input" the second time.

Do you have an actual need for this? I think devicetree attributes have
to be discussed and documented appropriately? I admit I am not too
familiar with this.

> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 773e484..7e247f7 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -871,6 +871,16 @@ config SENSORS_SMSC47B397
>  	  This driver can also be built as a module.  If so, the module
>  	  will be called smsc47b397.
>  
> +config SENSORS_ADS1015
> +	tristate "Texas Instruments ADS1015"
> +	depends on I2C
> +	help
> +	  If you say yes here you get support for Texas Instruments ADS1015
> +	  12-bit 4-input ADC device.
> +
> +	  This driver can also be built as a module.  If so, the module
> +	  will be called ads1015.
> +
>  config SENSORS_ADS7828
>  	tristate "Texas Instruments ADS7828"
>  	depends on I2C
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index dde02d9..aae4036 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -29,6 +29,7 @@ obj-$(CONFIG_SENSORS_ADM1026)	+= adm1026.o
>  obj-$(CONFIG_SENSORS_ADM1029)	+= adm1029.o
>  obj-$(CONFIG_SENSORS_ADM1031)	+= adm1031.o
>  obj-$(CONFIG_SENSORS_ADM9240)	+= adm9240.o
> +obj-$(CONFIG_SENSORS_ADS1015)	+= ads1015.o
>  obj-$(CONFIG_SENSORS_ADS7828)	+= ads7828.o
>  obj-$(CONFIG_SENSORS_ADS7871)	+= ads7871.o
>  obj-$(CONFIG_SENSORS_ADT7411)	+= adt7411.o
> diff --git a/drivers/hwmon/ads1015.c b/drivers/hwmon/ads1015.c
> new file mode 100644
> index 0000000..cf7aff4
> --- /dev/null
> +++ b/drivers/hwmon/ads1015.c
> @@ -0,0 +1,295 @@
> +/*
> + * ads1015.c - lm_sensors driver for ads1015 12-bit 4-input ADC
> + * (C) Copyright 2010
> + * Dirk Eibach, Guntermann & Drunck GmbH <eibach@gdsys.de>
> + *
> + * Based on the ads7828 driver by Steve Hardy.
> + *
> + * Datasheet available at: http://focus.ti.com/lit/ds/symlink/ads1015.pdf
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/slab.h>
> +#include <linux/jiffies.h>
> +#include <linux/i2c.h>
> +#include <linux/hwmon.h>
> +#include <linux/hwmon-sysfs.h>
> +#include <linux/err.h>
> +#include <linux/mutex.h>

You have to include <linux/of.h> for device tree support.

> +#include <linux/i2c/ads1015.h>
> +
> +/* ADS1015 registers */
> +enum {
> +	ADS1015_CONVERSION = 0,
> +	ADS1015_CONFIG = 1,
> +};
> +
> +/* PGA fullscale voltages in mV */
> +static const unsigned int fullscale_table[8] = {
> +	6144, 4096, 2048, 1024, 512, 256, 256, 256 };
> +
> +/* Default set of exported channels */
> +#define ADS1015_CONFIG_CHANNELS 8
> +static const int default_channels[ADS1015_CONFIG_CHANNELS] = {
> +	0, 1, 2, 3, 4, 5, 6, 7 };
> +
> +/* strings for sysfs */
> +static const char *input_names[8] = {
> +	"in0_input",
> +	"in1_input",
> +	"in2_input",
> +	"in3_input",
> +	"in4_input",
> +	"in5_input",
> +	"in6_input",
> +	"in7_input"
> +};
> +
> +struct ads1015_data {
> +	struct device *hwmon_dev;
> +	struct mutex update_lock; /* mutex protect updates */
> +	struct sensor_device_attribute attr[ADS1015_CONFIG_CHANNELS];
> +	struct attribute *attr_table[ADS1015_CONFIG_CHANNELS + 1];
> +	struct attribute_group attr_group;
> +};
> +
> +static s32 ads1015_read_reg(struct i2c_client *client, unsigned int reg)
> +{
> +	s32 data = i2c_smbus_read_word_data(client, reg);
> +
> +	return (data < 0) ? data : swab16(data);
> +}
> +
> +static s32 ads1015_write_reg(struct i2c_client *client, unsigned int reg,
> +			     u16 val)
> +{
> +	return i2c_smbus_write_word_data(client, reg, swab16(val));
> +}
> +
> +static int ads1015_read_value(struct i2c_client *client, unsigned int channel,
> +			      int *value)
> +{
> +	u16 config;
> +	s16 conversion;
> +	unsigned int pga;
> +	int fullscale;
> +	unsigned int k;
> +	struct ads1015_data *data = i2c_get_clientdata(client);
> +	int res;
> +
> +	mutex_lock(&data->update_lock);
> +
> +	/* get fullscale voltage */
> +	res = ads1015_read_reg(client, ADS1015_CONFIG);
> +	if (res < 0)
> +		goto err_unlock;
> +	config = res;
> +	pga = (config >> 9) & 0x0007;
> +	fullscale = fullscale_table[pga];

You only read the fullscale value. Now that we have platform data
attached to the device, don't you think it would make sense to let the
user set it, possibly even define different values for each "virtual
channel"? I can imagine that different scaling factors make sense for
single-ended vs. differential measurements, or even for different
single-ended inputs.

This is just a question, BTW, this feature can be added later if needed.

> +
> +	/* set channel and start single conversion */
> +	config &= ~(0x0007 << 12);
> +	config |= (1 << 15) | (1 << 8) | (channel & 0x0007) << 12;
> +
> +	/* wait until conversion finished */
> +	res = ads1015_write_reg(client, ADS1015_CONFIG, config);
> +	if (res < 0)
> +		goto err_unlock;
> +	for (k = 0; k < 5; ++k) {
> +		schedule_timeout(msecs_to_jiffies(1));
> +		res = ads1015_read_reg(client, ADS1015_CONFIG);
> +		if (res < 0)
> +			goto err_unlock;
> +		config = res;
> +		if (config & (1 << 15))
> +			break;
> +	}
> +	if (k == 5) {
> +		res = -EIO;
> +		goto err_unlock;
> +	}
> +
> +	res = ads1015_read_reg(client, ADS1015_CONVERSION);
> +	if (res < 0)
> +		goto err_unlock;
> +	conversion = res;
> +
> +	mutex_unlock(&data->update_lock);
> +
> +	*value = DIV_ROUND_CLOSEST(conversion * fullscale, 0x7ff0);
> +
> +	return 0;
> +
> +err_unlock:
> +	mutex_unlock(&data->update_lock);
> +	return res;
> +}
> +
> +/* sysfs callback function */
> +static ssize_t show_in(struct device *dev, struct device_attribute *da,
> +	char *buf)
> +{
> +	struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
> +	struct i2c_client *client = to_i2c_client(dev);
> +	int in;
> +	int res;
> +
> +	res = ads1015_read_value(client, attr->index, &in);
> +
> +	return (res < 0) ? res : sprintf(buf, "%d\n", in);
> +}
> +
> +/*
> + * Driver interface
> + */
> +
> +static int ads1015_remove(struct i2c_client *client)
> +{
> +	struct ads1015_data *data = i2c_get_clientdata(client);
> +	hwmon_device_unregister(data->hwmon_dev);
> +	sysfs_remove_group(&client->dev.kobj, &data->attr_group);
> +	kfree(data);
> +	return 0;
> +}
> +
> +static void ads1015_get_exported_channels(struct i2c_client *client,
> +					  int *exported_channels)
> +{
> +	struct ads1015_platform_data *pdata = dev_get_platdata(&client->dev);
> +#ifdef CONFIG_OF
> +	struct device_node *np = client->dev.of_node;
> +	const int *of_channels;
> +	int of_channels_size;
> +#endif
> +
> +	/* prefer platform data */
> +	if (pdata) {
> +		memcpy(exported_channels, pdata->exported_channels,
> +		       sizeof(default_channels));
> +		return;
> +	}
> +
> +#ifdef CONFIG_OF
> +	/* fallback on OF */
> +	of_channels = of_get_property(np, "exported-channels",
> +				      &of_channels_size);
> +	if (of_channels && (of_channels_size == sizeof(default_channels))) {
> +		memcpy(exported_channels, of_channels,
> +		       sizeof(default_channels));
> +		return;
> +	}
> +#endif
> +
> +	/* fallback on default configuration */
> +	memcpy(exported_channels, default_channels, sizeof(default_channels));
> +}

Why don't you just return a pointer to the data? You only need the data
during probe and you make no changes to it, so I see no need to copy
the data.

> +
> +/* create sysfs attribute according to channel setup */
> +static struct attribute *ads1015_export_channel(struct i2c_client *client,
> +						unsigned int input, int channel)
> +{
> +	struct ads1015_data *data = i2c_get_clientdata(client);
> +	struct sensor_device_attribute attr =
> +		SENSOR_ATTR(input, S_IRUGO, show_in, NULL, channel);
> +
> +	attr_name(attr.dev_attr) = input_names[input];
> +
> +	memcpy(&data->attr[input], &attr, sizeof(attr));
> +
> +	return &data->attr[input].dev_attr.attr;
> +}
> +
> +static int ads1015_probe(struct i2c_client *client,
> +			 const struct i2c_device_id *id)
> +{
> +	struct ads1015_data *data;
> +	int err;
> +	int exported_channels[ADS1015_CONFIG_CHANNELS];
> +	unsigned int k;
> +	unsigned int act_attr = 0;

"act"?

> +
> +	data = kzalloc(sizeof(struct ads1015_data), GFP_KERNEL);
> +	if (!data) {
> +		err = -ENOMEM;
> +		goto exit;
> +	}
> +
> +	i2c_set_clientdata(client, data);
> +	mutex_init(&data->update_lock);
> +
> +	/* Register sysfs hooks */
> +	data->attr_group.attrs = data->attr_table;
> +	ads1015_get_exported_channels(client, exported_channels);
> +	for (k = 0; k < ADS1015_CONFIG_CHANNELS; ++k) {
> +		int channel = exported_channels[k];
> +		if ((channel < 0) || (channel > 7))

You don't need all these parentheses.

> +			continue;

Is there any benefit in continuing here, as opposed to breaking?
Breaking would let you use k below to index data->attr_table, instead
of tracking act_attr separately.

> +		data->attr_table[act_attr++] =
> +			ads1015_export_channel(client, k, channel);
> +	}
> +	err = sysfs_create_group(&client->dev.kobj, &data->attr_group);
> +	if (err)
> +		goto exit_free;
> +
> +	data->hwmon_dev = hwmon_device_register(&client->dev);
> +	if (IS_ERR(data->hwmon_dev)) {
> +		err = PTR_ERR(data->hwmon_dev);
> +		goto exit_remove;
> +	}
> +
> +	return 0;
> +
> +exit_remove:
> +	sysfs_remove_group(&client->dev.kobj, &data->attr_group);
> +exit_free:
> +	kfree(data);
> +exit:
> +	return err;
> +}
> +
> +static const struct i2c_device_id ads1015_id[] = {
> +	{ "ads1015", 0 },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, ads1015_id);
> +
> +static struct i2c_driver ads1015_driver = {
> +	.driver = {
> +		.name = "ads1015",
> +	},
> +	.probe = ads1015_probe,
> +	.remove = ads1015_remove,
> +	.id_table = ads1015_id,
> +};
> +
> +static int __init sensors_ads1015_init(void)
> +{
> +	return i2c_add_driver(&ads1015_driver);
> +}
> +
> +static void __exit sensors_ads1015_exit(void)
> +{
> +	i2c_del_driver(&ads1015_driver);
> +}
> +
> +MODULE_AUTHOR("Dirk Eibach <eibach@gdsys.de>");
> +MODULE_DESCRIPTION("ADS1015 driver");
> +MODULE_LICENSE("GPL");
> +
> +module_init(sensors_ads1015_init);
> +module_exit(sensors_ads1015_exit);
> diff --git a/include/linux/i2c/ads1015.h b/include/linux/i2c/ads1015.h
> new file mode 100644
> index 0000000..152bf5f
> --- /dev/null
> +++ b/include/linux/i2c/ads1015.h
> @@ -0,0 +1,30 @@
> +/*
> + * Platform Data for ADS1015 12-bit 4-input ADC
> + * (C) Copyright 2010
> + * Dirk Eibach, Guntermann & Drunck GmbH <eibach@gdsys.de>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
> + */
> +
> +#ifndef LINUX_ADS1015_H
> +#define LINUX_ADS1015_H
> +
> +#include <linux/types.h>

You don't use anything from that header file.

> +
> +struct ads1015_platform_data {
> +	int exported_channels[8];
> +};
> +
> +#endif /* LINUX_ADS1015_H */


-- 
Jean Delvare

[lm-sensors] [PATCH v4] hwmon: Add support for Texas Instruments

[Dirk Eibach]

Signed-off-by: Dirk Eibach <eibach@gdsys.de>
---
Changes since v1:
- fixed/extended Documentation
- removed unused register definitions
- hardcoded PGA fullscale table size
- made sure patch applies against v2.6.38-rc4
- reordered functions to avoid forward declaration
- results from i2c_smbus_read_word_data() are handled correctly
- moved locking into ads1015_read_value()
- removed unnecessray clearing of bit
- proper error handling in ads1015_read_value()
- use DIV_ROUND_CLOSEST for scaling result
- removed detect()

Changes since v2:
- removed *all* leftovers from detect()
- fixed return with mutex held
- made sysfs representation configurable
  (hope this will be the reference implementation for generations to come ;)

Changes since v3:
- included linux/of.h
- remove linux/types.h from header file
- sysfs is now configured with a bitmask
- assume big-endian of-properties

Documentation/hwmon/ads1015 |   67 ++++++++++
 drivers/hwmon/Kconfig       |   10 ++
 drivers/hwmon/Makefile      |    1 +
 drivers/hwmon/ads1015.c     |  283 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/i2c/ads1015.h |   28 +++++
 5 files changed, 389 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/hwmon/ads1015
 create mode 100644 drivers/hwmon/ads1015.c
 create mode 100644 include/linux/i2c/ads1015.h

diff --git a/Documentation/hwmon/ads1015 b/Documentation/hwmon/ads1015
new file mode 100644
index 0000000..56ee797
--- /dev/null
+++ b/Documentation/hwmon/ads1015
@@ -0,0 +1,67 @@
+Kernel driver ads1015
+==========+
+Supported chips:
+  * Texas Instruments ADS1015
+    Prefix: 'ads1015'
+    Datasheet: Publicly available at the Texas Instruments website :
+               http://focus.ti.com/lit/ds/symlink/ads1015.pdf
+
+Authors:
+        Dirk Eibach, Guntermann & Drunck GmbH <eibach@gdsys.de>
+
+Description
+-----------
+
+This driver implements support for the Texas Instruments ADS1015.
+
+This device is a 12-bit A-D converter with 4 inputs.
+
+The inputs can be used single ended or in certain differential combinations.
+
+The inputs can be exported to 8 sysfs input files in0_input - in7_input:
+in0: Voltage over AIN0 and AIN1.
+in1: Voltage over AIN0 and AIN3.
+in2: Voltage over AIN1 and AIN3.
+in3: Voltage over AIN2 and AIN3.
+in4: Voltage over AIN0 and GND.
+in5: Voltage over AIN1 and GND.
+in6: Voltage over AIN2 and GND.
+in7: Voltage over AIN3 and GND.
+
+Which inputs are exported can be configured using platform data or devicetree.
+
+By default all inputs are exported.
+
+Platform Data
+-------------
+
+In linux/i2c/ads1015.h platform data is defined as:
+
+struct ads1015_platform_data {
+	unsigned int exported_channels;
+};
+
+exported_channels is a bitmask that specifies which inputs should be exported.
+
+Example:
+struct ads1015_platform_data data = {
+	.exported_channels = (1 << 2) | (1 << 4)
+};
+
+In this case only in2_input and in4_input would be created.
+
+Devicetree
+----------
+
+The ads1015 node may have an "exported-channels" property.
+exported_channels is a bitmask that specifies which inputs should be exported.
+
+Example:
+ads1015@49 {
+	compatible = "ti,ads1015";
+	reg = <0x49>;
+	exported-channels = < 0x14 >;
+};
+
+In this case only in2_input and in4_input would be created.
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 773e484..7e247f7 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -871,6 +871,16 @@ config SENSORS_SMSC47B397
 	  This driver can also be built as a module.  If so, the module
 	  will be called smsc47b397.
 
+config SENSORS_ADS1015
+	tristate "Texas Instruments ADS1015"
+	depends on I2C
+	help
+	  If you say yes here you get support for Texas Instruments ADS1015
+	  12-bit 4-input ADC device.
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called ads1015.
+
 config SENSORS_ADS7828
 	tristate "Texas Instruments ADS7828"
 	depends on I2C
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index dde02d9..aae4036 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -29,6 +29,7 @@ obj-$(CONFIG_SENSORS_ADM1026)	+= adm1026.o
 obj-$(CONFIG_SENSORS_ADM1029)	+= adm1029.o
 obj-$(CONFIG_SENSORS_ADM1031)	+= adm1031.o
 obj-$(CONFIG_SENSORS_ADM9240)	+= adm9240.o
+obj-$(CONFIG_SENSORS_ADS1015)	+= ads1015.o
 obj-$(CONFIG_SENSORS_ADS7828)	+= ads7828.o
 obj-$(CONFIG_SENSORS_ADS7871)	+= ads7871.o
 obj-$(CONFIG_SENSORS_ADT7411)	+= adt7411.o
diff --git a/drivers/hwmon/ads1015.c b/drivers/hwmon/ads1015.c
new file mode 100644
index 0000000..7d593bb
--- /dev/null
+++ b/drivers/hwmon/ads1015.c
@@ -0,0 +1,283 @@
+/*
+ * ads1015.c - lm_sensors driver for ads1015 12-bit 4-input ADC
+ * (C) Copyright 2010
+ * Dirk Eibach, Guntermann & Drunck GmbH <eibach@gdsys.de>
+ *
+ * Based on the ads7828 driver by Steve Hardy.
+ *
+ * Datasheet available at: http://focus.ti.com/lit/ds/symlink/ads1015.pdf
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/jiffies.h>
+#include <linux/i2c.h>
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
+#include <linux/err.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+
+#include <linux/i2c/ads1015.h>
+
+/* ADS1015 registers */
+enum {
+	ADS1015_CONVERSION = 0,
+	ADS1015_CONFIG = 1,
+};
+
+/* PGA fullscale voltages in mV */
+static const unsigned int fullscale_table[8] = {
+	6144, 4096, 2048, 1024, 512, 256, 256, 256 };
+
+#define ADS1015_CONFIG_CHANNELS 8
+#define ADS1015_DEFAULT_CHANNELS 0xff
+
+struct ads1015_data {
+	struct device *hwmon_dev;
+	struct mutex update_lock; /* mutex protect updates */
+	struct attribute *attr_table[ADS1015_CONFIG_CHANNELS + 1];
+	struct attribute_group attr_group;
+};
+
+static s32 ads1015_read_reg(struct i2c_client *client, unsigned int reg)
+{
+	s32 data = i2c_smbus_read_word_data(client, reg);
+
+	return (data < 0) ? data : swab16(data);
+}
+
+static s32 ads1015_write_reg(struct i2c_client *client, unsigned int reg,
+			     u16 val)
+{
+	return i2c_smbus_write_word_data(client, reg, swab16(val));
+}
+
+static int ads1015_read_value(struct i2c_client *client, unsigned int channel,
+			      int *value)
+{
+	u16 config;
+	s16 conversion;
+	unsigned int pga;
+	int fullscale;
+	unsigned int k;
+	struct ads1015_data *data = i2c_get_clientdata(client);
+	int res;
+
+	mutex_lock(&data->update_lock);
+
+	/* get fullscale voltage */
+	res = ads1015_read_reg(client, ADS1015_CONFIG);
+	if (res < 0)
+		goto err_unlock;
+	config = res;
+	pga = (config >> 9) & 0x0007;
+	fullscale = fullscale_table[pga];
+
+	/* set channel and start single conversion */
+	config &= ~(0x0007 << 12);
+	config |= (1 << 15) | (1 << 8) | (channel & 0x0007) << 12;
+
+	/* wait until conversion finished */
+	res = ads1015_write_reg(client, ADS1015_CONFIG, config);
+	if (res < 0)
+		goto err_unlock;
+	for (k = 0; k < 5; ++k) {
+		schedule_timeout(msecs_to_jiffies(1));
+		res = ads1015_read_reg(client, ADS1015_CONFIG);
+		if (res < 0)
+			goto err_unlock;
+		config = res;
+		if (config & (1 << 15))
+			break;
+	}
+	if (k = 5) {
+		res = -EIO;
+		goto err_unlock;
+	}
+
+	res = ads1015_read_reg(client, ADS1015_CONVERSION);
+	if (res < 0)
+		goto err_unlock;
+	conversion = res;
+
+	mutex_unlock(&data->update_lock);
+
+	*value = DIV_ROUND_CLOSEST(conversion * fullscale, 0x7ff0);
+
+	return 0;
+
+err_unlock:
+	mutex_unlock(&data->update_lock);
+	return res;
+}
+
+/* sysfs callback function */
+static ssize_t show_in(struct device *dev, struct device_attribute *da,
+	char *buf)
+{
+	struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
+	struct i2c_client *client = to_i2c_client(dev);
+	int in;
+	int res;
+
+	res = ads1015_read_value(client, attr->index, &in);
+
+	return (res < 0) ? res : sprintf(buf, "%d\n", in);
+}
+
+#define in_reg(offset)\
+static SENSOR_DEVICE_ATTR(in##offset##_input, S_IRUGO, show_in,\
+	NULL, offset)
+
+in_reg(0);
+in_reg(1);
+in_reg(2);
+in_reg(3);
+in_reg(4);
+in_reg(5);
+in_reg(6);
+in_reg(7);
+
+static struct attribute *all_attributes[] = {
+	&sensor_dev_attr_in0_input.dev_attr.attr,
+	&sensor_dev_attr_in1_input.dev_attr.attr,
+	&sensor_dev_attr_in2_input.dev_attr.attr,
+	&sensor_dev_attr_in3_input.dev_attr.attr,
+	&sensor_dev_attr_in4_input.dev_attr.attr,
+	&sensor_dev_attr_in5_input.dev_attr.attr,
+	&sensor_dev_attr_in6_input.dev_attr.attr,
+	&sensor_dev_attr_in7_input.dev_attr.attr,
+};
+
+/*
+ * Driver interface
+ */
+
+static int ads1015_remove(struct i2c_client *client)
+{
+	struct ads1015_data *data = i2c_get_clientdata(client);
+	hwmon_device_unregister(data->hwmon_dev);
+	sysfs_remove_group(&client->dev.kobj, &data->attr_group);
+	kfree(data);
+	return 0;
+}
+
+static unsigned int ads1015_get_exported_channels(struct i2c_client *client)
+{
+	struct ads1015_platform_data *pdata = dev_get_platdata(&client->dev);
+#ifdef CONFIG_OF
+	struct device_node *np = client->dev.of_node;
+	const __be32 *of_channels;
+	int of_channels_size;
+#endif
+
+	/* prefer platform data */
+	if (pdata)
+		return pdata->exported_channels;
+
+#ifdef CONFIG_OF
+	/* fallback on OF */
+	of_channels = of_get_property(np, "exported-channels",
+				      &of_channels_size);
+	if (of_channels && (of_channels_size = sizeof(*of_channels)))
+		return be32_to_cpup(of_channels);
+#endif
+
+	/* fallback on default configuration */
+	return ADS1015_DEFAULT_CHANNELS;
+}
+
+static int ads1015_probe(struct i2c_client *client,
+			 const struct i2c_device_id *id)
+{
+	struct ads1015_data *data;
+	int err;
+	unsigned int exported_channels;
+	unsigned int k;
+	unsigned int n = 0;
+
+	data = kzalloc(sizeof(struct ads1015_data), GFP_KERNEL);
+	if (!data) {
+		err = -ENOMEM;
+		goto exit;
+	}
+
+	i2c_set_clientdata(client, data);
+	mutex_init(&data->update_lock);
+
+	/* build sysfs attribute group */
+	data->attr_group.attrs = data->attr_table;
+	exported_channels = ads1015_get_exported_channels(client);
+	for (k = 0; k < ADS1015_CONFIG_CHANNELS; ++k) {
+		if (!(exported_channels & (1<<k)))
+			continue;
+		data->attr_table[n++] +			all_attributes[k];
+	}
+	err = sysfs_create_group(&client->dev.kobj, &data->attr_group);
+	if (err)
+		goto exit_free;
+
+	data->hwmon_dev = hwmon_device_register(&client->dev);
+	if (IS_ERR(data->hwmon_dev)) {
+		err = PTR_ERR(data->hwmon_dev);
+		goto exit_remove;
+	}
+
+	return 0;
+
+exit_remove:
+	sysfs_remove_group(&client->dev.kobj, &data->attr_group);
+exit_free:
+	kfree(data);
+exit:
+	return err;
+}
+
+static const struct i2c_device_id ads1015_id[] = {
+	{ "ads1015", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, ads1015_id);
+
+static struct i2c_driver ads1015_driver = {
+	.driver = {
+		.name = "ads1015",
+	},
+	.probe = ads1015_probe,
+	.remove = ads1015_remove,
+	.id_table = ads1015_id,
+};
+
+static int __init sensors_ads1015_init(void)
+{
+	return i2c_add_driver(&ads1015_driver);
+}
+
+static void __exit sensors_ads1015_exit(void)
+{
+	i2c_del_driver(&ads1015_driver);
+}
+
+MODULE_AUTHOR("Dirk Eibach <eibach@gdsys.de>");
+MODULE_DESCRIPTION("ADS1015 driver");
+MODULE_LICENSE("GPL");
+
+module_init(sensors_ads1015_init);
+module_exit(sensors_ads1015_exit);
diff --git a/include/linux/i2c/ads1015.h b/include/linux/i2c/ads1015.h
new file mode 100644
index 0000000..8541c6a
--- /dev/null
+++ b/include/linux/i2c/ads1015.h
@@ -0,0 +1,28 @@
+/*
+ * Platform Data for ADS1015 12-bit 4-input ADC
+ * (C) Copyright 2010
+ * Dirk Eibach, Guntermann & Drunck GmbH <eibach@gdsys.de>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+#ifndef LINUX_ADS1015_H
+#define LINUX_ADS1015_H
+
+struct ads1015_platform_data {
+	unsigned int exported_channels;
+};
+
+#endif /* LINUX_ADS1015_H */
-- 
1.5.6.5


_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

[PATCH v4] hwmon: Add support for Texas Instruments ADS1015

[Dirk Eibach]

Signed-off-by: Dirk Eibach <eibach@gdsys.de>
---
Changes since v1:
- fixed/extended Documentation
- removed unused register definitions
- hardcoded PGA fullscale table size
- made sure patch applies against v2.6.38-rc4
- reordered functions to avoid forward declaration
- results from i2c_smbus_read_word_data() are handled correctly
- moved locking into ads1015_read_value()
- removed unnecessray clearing of bit
- proper error handling in ads1015_read_value()
- use DIV_ROUND_CLOSEST for scaling result
- removed detect()

Changes since v2:
- removed *all* leftovers from detect()
- fixed return with mutex held
- made sysfs representation configurable
  (hope this will be the reference implementation for generations to come ;)

Changes since v3:
- included linux/of.h
- remove linux/types.h from header file
- sysfs is now configured with a bitmask
- assume big-endian of-properties

Documentation/hwmon/ads1015 |   67 ++++++++++
 drivers/hwmon/Kconfig       |   10 ++
 drivers/hwmon/Makefile      |    1 +
 drivers/hwmon/ads1015.c     |  283 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/i2c/ads1015.h |   28 +++++
 5 files changed, 389 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/hwmon/ads1015
 create mode 100644 drivers/hwmon/ads1015.c
 create mode 100644 include/linux/i2c/ads1015.h

diff --git a/Documentation/hwmon/ads1015 b/Documentation/hwmon/ads1015
new file mode 100644
index 0000000..56ee797
--- /dev/null
+++ b/Documentation/hwmon/ads1015
@@ -0,0 +1,67 @@
+Kernel driver ads1015
+=====================
+
+Supported chips:
+  * Texas Instruments ADS1015
+    Prefix: 'ads1015'
+    Datasheet: Publicly available at the Texas Instruments website :
+               http://focus.ti.com/lit/ds/symlink/ads1015.pdf
+
+Authors:
+        Dirk Eibach, Guntermann & Drunck GmbH <eibach@gdsys.de>
+
+Description
+-----------
+
+This driver implements support for the Texas Instruments ADS1015.
+
+This device is a 12-bit A-D converter with 4 inputs.
+
+The inputs can be used single ended or in certain differential combinations.
+
+The inputs can be exported to 8 sysfs input files in0_input - in7_input:
+in0: Voltage over AIN0 and AIN1.
+in1: Voltage over AIN0 and AIN3.
+in2: Voltage over AIN1 and AIN3.
+in3: Voltage over AIN2 and AIN3.
+in4: Voltage over AIN0 and GND.
+in5: Voltage over AIN1 and GND.
+in6: Voltage over AIN2 and GND.
+in7: Voltage over AIN3 and GND.
+
+Which inputs are exported can be configured using platform data or devicetree.
+
+By default all inputs are exported.
+
+Platform Data
+-------------
+
+In linux/i2c/ads1015.h platform data is defined as:
+
+struct ads1015_platform_data {
+	unsigned int exported_channels;
+};
+
+exported_channels is a bitmask that specifies which inputs should be exported.
+
+Example:
+struct ads1015_platform_data data = {
+	.exported_channels = (1 << 2) | (1 << 4)
+};
+
+In this case only in2_input and in4_input would be created.
+
+Devicetree
+----------
+
+The ads1015 node may have an "exported-channels" property.
+exported_channels is a bitmask that specifies which inputs should be exported.
+
+Example:
+ads1015@49 {
+	compatible = "ti,ads1015";
+	reg = <0x49>;
+	exported-channels = < 0x14 >;
+};
+
+In this case only in2_input and in4_input would be created.
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 773e484..7e247f7 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -871,6 +871,16 @@ config SENSORS_SMSC47B397
 	  This driver can also be built as a module.  If so, the module
 	  will be called smsc47b397.
 
+config SENSORS_ADS1015
+	tristate "Texas Instruments ADS1015"
+	depends on I2C
+	help
+	  If you say yes here you get support for Texas Instruments ADS1015
+	  12-bit 4-input ADC device.
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called ads1015.
+
 config SENSORS_ADS7828
 	tristate "Texas Instruments ADS7828"
 	depends on I2C
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index dde02d9..aae4036 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -29,6 +29,7 @@ obj-$(CONFIG_SENSORS_ADM1026)	+= adm1026.o
 obj-$(CONFIG_SENSORS_ADM1029)	+= adm1029.o
 obj-$(CONFIG_SENSORS_ADM1031)	+= adm1031.o
 obj-$(CONFIG_SENSORS_ADM9240)	+= adm9240.o
+obj-$(CONFIG_SENSORS_ADS1015)	+= ads1015.o
 obj-$(CONFIG_SENSORS_ADS7828)	+= ads7828.o
 obj-$(CONFIG_SENSORS_ADS7871)	+= ads7871.o
 obj-$(CONFIG_SENSORS_ADT7411)	+= adt7411.o
diff --git a/drivers/hwmon/ads1015.c b/drivers/hwmon/ads1015.c
new file mode 100644
index 0000000..7d593bb
--- /dev/null
+++ b/drivers/hwmon/ads1015.c
@@ -0,0 +1,283 @@
+/*
+ * ads1015.c - lm_sensors driver for ads1015 12-bit 4-input ADC
+ * (C) Copyright 2010
+ * Dirk Eibach, Guntermann & Drunck GmbH <eibach@gdsys.de>
+ *
+ * Based on the ads7828 driver by Steve Hardy.
+ *
+ * Datasheet available at: http://focus.ti.com/lit/ds/symlink/ads1015.pdf
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/jiffies.h>
+#include <linux/i2c.h>
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
+#include <linux/err.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+
+#include <linux/i2c/ads1015.h>
+
+/* ADS1015 registers */
+enum {
+	ADS1015_CONVERSION = 0,
+	ADS1015_CONFIG = 1,
+};
+
+/* PGA fullscale voltages in mV */
+static const unsigned int fullscale_table[8] = {
+	6144, 4096, 2048, 1024, 512, 256, 256, 256 };
+
+#define ADS1015_CONFIG_CHANNELS 8
+#define ADS1015_DEFAULT_CHANNELS 0xff
+
+struct ads1015_data {
+	struct device *hwmon_dev;
+	struct mutex update_lock; /* mutex protect updates */
+	struct attribute *attr_table[ADS1015_CONFIG_CHANNELS + 1];
+	struct attribute_group attr_group;
+};
+
+static s32 ads1015_read_reg(struct i2c_client *client, unsigned int reg)
+{
+	s32 data = i2c_smbus_read_word_data(client, reg);
+
+	return (data < 0) ? data : swab16(data);
+}
+
+static s32 ads1015_write_reg(struct i2c_client *client, unsigned int reg,
+			     u16 val)
+{
+	return i2c_smbus_write_word_data(client, reg, swab16(val));
+}
+
+static int ads1015_read_value(struct i2c_client *client, unsigned int channel,
+			      int *value)
+{
+	u16 config;
+	s16 conversion;
+	unsigned int pga;
+	int fullscale;
+	unsigned int k;
+	struct ads1015_data *data = i2c_get_clientdata(client);
+	int res;
+
+	mutex_lock(&data->update_lock);
+
+	/* get fullscale voltage */
+	res = ads1015_read_reg(client, ADS1015_CONFIG);
+	if (res < 0)
+		goto err_unlock;
+	config = res;
+	pga = (config >> 9) & 0x0007;
+	fullscale = fullscale_table[pga];
+
+	/* set channel and start single conversion */
+	config &= ~(0x0007 << 12);
+	config |= (1 << 15) | (1 << 8) | (channel & 0x0007) << 12;
+
+	/* wait until conversion finished */
+	res = ads1015_write_reg(client, ADS1015_CONFIG, config);
+	if (res < 0)
+		goto err_unlock;
+	for (k = 0; k < 5; ++k) {
+		schedule_timeout(msecs_to_jiffies(1));
+		res = ads1015_read_reg(client, ADS1015_CONFIG);
+		if (res < 0)
+			goto err_unlock;
+		config = res;
+		if (config & (1 << 15))
+			break;
+	}
+	if (k == 5) {
+		res = -EIO;
+		goto err_unlock;
+	}
+
+	res = ads1015_read_reg(client, ADS1015_CONVERSION);
+	if (res < 0)
+		goto err_unlock;
+	conversion = res;
+
+	mutex_unlock(&data->update_lock);
+
+	*value = DIV_ROUND_CLOSEST(conversion * fullscale, 0x7ff0);
+
+	return 0;
+
+err_unlock:
+	mutex_unlock(&data->update_lock);
+	return res;
+}
+
+/* sysfs callback function */
+static ssize_t show_in(struct device *dev, struct device_attribute *da,
+	char *buf)
+{
+	struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
+	struct i2c_client *client = to_i2c_client(dev);
+	int in;
+	int res;
+
+	res = ads1015_read_value(client, attr->index, &in);
+
+	return (res < 0) ? res : sprintf(buf, "%d\n", in);
+}
+
+#define in_reg(offset)\
+static SENSOR_DEVICE_ATTR(in##offset##_input, S_IRUGO, show_in,\
+	NULL, offset)
+
+in_reg(0);
+in_reg(1);
+in_reg(2);
+in_reg(3);
+in_reg(4);
+in_reg(5);
+in_reg(6);
+in_reg(7);
+
+static struct attribute *all_attributes[] = {
+	&sensor_dev_attr_in0_input.dev_attr.attr,
+	&sensor_dev_attr_in1_input.dev_attr.attr,
+	&sensor_dev_attr_in2_input.dev_attr.attr,
+	&sensor_dev_attr_in3_input.dev_attr.attr,
+	&sensor_dev_attr_in4_input.dev_attr.attr,
+	&sensor_dev_attr_in5_input.dev_attr.attr,
+	&sensor_dev_attr_in6_input.dev_attr.attr,
+	&sensor_dev_attr_in7_input.dev_attr.attr,
+};
+
+/*
+ * Driver interface
+ */
+
+static int ads1015_remove(struct i2c_client *client)
+{
+	struct ads1015_data *data = i2c_get_clientdata(client);
+	hwmon_device_unregister(data->hwmon_dev);
+	sysfs_remove_group(&client->dev.kobj, &data->attr_group);
+	kfree(data);
+	return 0;
+}
+
+static unsigned int ads1015_get_exported_channels(struct i2c_client *client)
+{
+	struct ads1015_platform_data *pdata = dev_get_platdata(&client->dev);
+#ifdef CONFIG_OF
+	struct device_node *np = client->dev.of_node;
+	const __be32 *of_channels;
+	int of_channels_size;
+#endif
+
+	/* prefer platform data */
+	if (pdata)
+		return pdata->exported_channels;
+
+#ifdef CONFIG_OF
+	/* fallback on OF */
+	of_channels = of_get_property(np, "exported-channels",
+				      &of_channels_size);
+	if (of_channels && (of_channels_size == sizeof(*of_channels)))
+		return be32_to_cpup(of_channels);
+#endif
+
+	/* fallback on default configuration */
+	return ADS1015_DEFAULT_CHANNELS;
+}
+
+static int ads1015_probe(struct i2c_client *client,
+			 const struct i2c_device_id *id)
+{
+	struct ads1015_data *data;
+	int err;
+	unsigned int exported_channels;
+	unsigned int k;
+	unsigned int n = 0;
+
+	data = kzalloc(sizeof(struct ads1015_data), GFP_KERNEL);
+	if (!data) {
+		err = -ENOMEM;
+		goto exit;
+	}
+
+	i2c_set_clientdata(client, data);
+	mutex_init(&data->update_lock);
+
+	/* build sysfs attribute group */
+	data->attr_group.attrs = data->attr_table;
+	exported_channels = ads1015_get_exported_channels(client);
+	for (k = 0; k < ADS1015_CONFIG_CHANNELS; ++k) {
+		if (!(exported_channels & (1<<k)))
+			continue;
+		data->attr_table[n++] =
+			all_attributes[k];
+	}
+	err = sysfs_create_group(&client->dev.kobj, &data->attr_group);
+	if (err)
+		goto exit_free;
+
+	data->hwmon_dev = hwmon_device_register(&client->dev);
+	if (IS_ERR(data->hwmon_dev)) {
+		err = PTR_ERR(data->hwmon_dev);
+		goto exit_remove;
+	}
+
+	return 0;
+
+exit_remove:
+	sysfs_remove_group(&client->dev.kobj, &data->attr_group);
+exit_free:
+	kfree(data);
+exit:
+	return err;
+}
+
+static const struct i2c_device_id ads1015_id[] = {
+	{ "ads1015", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, ads1015_id);
+
+static struct i2c_driver ads1015_driver = {
+	.driver = {
+		.name = "ads1015",
+	},
+	.probe = ads1015_probe,
+	.remove = ads1015_remove,
+	.id_table = ads1015_id,
+};
+
+static int __init sensors_ads1015_init(void)
+{
+	return i2c_add_driver(&ads1015_driver);
+}
+
+static void __exit sensors_ads1015_exit(void)
+{
+	i2c_del_driver(&ads1015_driver);
+}
+
+MODULE_AUTHOR("Dirk Eibach <eibach@gdsys.de>");
+MODULE_DESCRIPTION("ADS1015 driver");
+MODULE_LICENSE("GPL");
+
+module_init(sensors_ads1015_init);
+module_exit(sensors_ads1015_exit);
diff --git a/include/linux/i2c/ads1015.h b/include/linux/i2c/ads1015.h
new file mode 100644
index 0000000..8541c6a
--- /dev/null
+++ b/include/linux/i2c/ads1015.h
@@ -0,0 +1,28 @@
+/*
+ * Platform Data for ADS1015 12-bit 4-input ADC
+ * (C) Copyright 2010
+ * Dirk Eibach, Guntermann & Drunck GmbH <eibach@gdsys.de>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+#ifndef LINUX_ADS1015_H
+#define LINUX_ADS1015_H
+
+struct ads1015_platform_data {
+	unsigned int exported_channels;
+};
+
+#endif /* LINUX_ADS1015_H */
-- 
1.5.6.5

Re: [lm-sensors] [PATCH v4] hwmon: Add support for Texas

[Jean Delvare]

Hi Dirk,

On Fri, 25 Feb 2011 14:18:17 +0100, Dirk Eibach wrote:
> Signed-off-by: Dirk Eibach <eibach@gdsys.de>
> ---
> Changes since v1:
> - fixed/extended Documentation
> - removed unused register definitions
> - hardcoded PGA fullscale table size
> - made sure patch applies against v2.6.38-rc4
> - reordered functions to avoid forward declaration
> - results from i2c_smbus_read_word_data() are handled correctly
> - moved locking into ads1015_read_value()
> - removed unnecessray clearing of bit
> - proper error handling in ads1015_read_value()
> - use DIV_ROUND_CLOSEST for scaling result
> - removed detect()
> 
> Changes since v2:
> - removed *all* leftovers from detect()
> - fixed return with mutex held
> - made sysfs representation configurable
>   (hope this will be the reference implementation for generations to come ;)
> 
> Changes since v3:
> - included linux/of.h
> - remove linux/types.h from header file
> - sysfs is now configured with a bitmask
> - assume big-endian of-properties

Patch applied. Two things I'd still like to comment on:

> (...)
> +static unsigned int ads1015_get_exported_channels(struct i2c_client *client)
> +{
> +	struct ads1015_platform_data *pdata = dev_get_platdata(&client->dev);
> +#ifdef CONFIG_OF
> +	struct device_node *np = client->dev.of_node;
> +	const __be32 *of_channels;
> +	int of_channels_size;
> +#endif
> +
> +	/* prefer platform data */
> +	if (pdata)
> +		return pdata->exported_channels;
> +
> +#ifdef CONFIG_OF
> +	/* fallback on OF */
> +	of_channels = of_get_property(np, "exported-channels",
> +				      &of_channels_size);
> +	if (of_channels && (of_channels_size = sizeof(*of_channels)))
> +		return be32_to_cpup(of_channels);
> +#endif

The be32 thing looks odd. I don't get the idea, but as I don't know
much about devicetree, I'll trust you.

> +
> +	/* fallback on default configuration */
> +	return ADS1015_DEFAULT_CHANNELS;
> +}
> +
> +static int ads1015_probe(struct i2c_client *client,
> +			 const struct i2c_device_id *id)
> +{
> +	struct ads1015_data *data;
> +	int err;
> +	unsigned int exported_channels;
> +	unsigned int k;
> +	unsigned int n = 0;
> +
> +	data = kzalloc(sizeof(struct ads1015_data), GFP_KERNEL);
> +	if (!data) {
> +		err = -ENOMEM;
> +		goto exit;
> +	}
> +
> +	i2c_set_clientdata(client, data);
> +	mutex_init(&data->update_lock);
> +
> +	/* build sysfs attribute group */
> +	data->attr_group.attrs = data->attr_table;
> +	exported_channels = ads1015_get_exported_channels(client);
> +	for (k = 0; k < ADS1015_CONFIG_CHANNELS; ++k) {
> +		if (!(exported_channels & (1<<k)))
> +			continue;
> +		data->attr_table[n++] > +			all_attributes[k];

There was no reason to split this statement, so I've put it back on a
single line.

> +	}

Besides this, there is still more dynamic attribute handling than I
expected. It looks OK, but I'll propose a patch making it more static.
You'll tell me what you think.

-- 
Jean Delvare

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [PATCH v4] hwmon: Add support for Texas Instruments ADS1015

[Jean Delvare]

Hi Dirk,

On Fri, 25 Feb 2011 14:18:17 +0100, Dirk Eibach wrote:
> Signed-off-by: Dirk Eibach <eibach@gdsys.de>
> ---
> Changes since v1:
> - fixed/extended Documentation
> - removed unused register definitions
> - hardcoded PGA fullscale table size
> - made sure patch applies against v2.6.38-rc4
> - reordered functions to avoid forward declaration
> - results from i2c_smbus_read_word_data() are handled correctly
> - moved locking into ads1015_read_value()
> - removed unnecessray clearing of bit
> - proper error handling in ads1015_read_value()
> - use DIV_ROUND_CLOSEST for scaling result
> - removed detect()
> 
> Changes since v2:
> - removed *all* leftovers from detect()
> - fixed return with mutex held
> - made sysfs representation configurable
>   (hope this will be the reference implementation for generations to come ;)
> 
> Changes since v3:
> - included linux/of.h
> - remove linux/types.h from header file
> - sysfs is now configured with a bitmask
> - assume big-endian of-properties

Patch applied. Two things I'd still like to comment on:

> (...)
> +static unsigned int ads1015_get_exported_channels(struct i2c_client *client)
> +{
> +	struct ads1015_platform_data *pdata = dev_get_platdata(&client->dev);
> +#ifdef CONFIG_OF
> +	struct device_node *np = client->dev.of_node;
> +	const __be32 *of_channels;
> +	int of_channels_size;
> +#endif
> +
> +	/* prefer platform data */
> +	if (pdata)
> +		return pdata->exported_channels;
> +
> +#ifdef CONFIG_OF
> +	/* fallback on OF */
> +	of_channels = of_get_property(np, "exported-channels",
> +				      &of_channels_size);
> +	if (of_channels && (of_channels_size == sizeof(*of_channels)))
> +		return be32_to_cpup(of_channels);
> +#endif

The be32 thing looks odd. I don't get the idea, but as I don't know
much about devicetree, I'll trust you.

> +
> +	/* fallback on default configuration */
> +	return ADS1015_DEFAULT_CHANNELS;
> +}
> +
> +static int ads1015_probe(struct i2c_client *client,
> +			 const struct i2c_device_id *id)
> +{
> +	struct ads1015_data *data;
> +	int err;
> +	unsigned int exported_channels;
> +	unsigned int k;
> +	unsigned int n = 0;
> +
> +	data = kzalloc(sizeof(struct ads1015_data), GFP_KERNEL);
> +	if (!data) {
> +		err = -ENOMEM;
> +		goto exit;
> +	}
> +
> +	i2c_set_clientdata(client, data);
> +	mutex_init(&data->update_lock);
> +
> +	/* build sysfs attribute group */
> +	data->attr_group.attrs = data->attr_table;
> +	exported_channels = ads1015_get_exported_channels(client);
> +	for (k = 0; k < ADS1015_CONFIG_CHANNELS; ++k) {
> +		if (!(exported_channels & (1<<k)))
> +			continue;
> +		data->attr_table[n++] =
> +			all_attributes[k];

There was no reason to split this statement, so I've put it back on a
single line.

> +	}

Besides this, there is still more dynamic attribute handling than I
expected. It looks OK, but I'll propose a patch making it more static.
You'll tell me what you think.

-- 
Jean Delvare

Re: [lm-sensors] [PATCH v4] hwmon: Add support for Texas

[Wolfram Sang]

[-- Attachment #1.1: Type: text/plain, Size: 949 bytes --]


> > +#ifdef CONFIG_OF
> > +	/* fallback on OF */
> > +	of_channels = of_get_property(np, "exported-channels",
> > +				      &of_channels_size);
> > +	if (of_channels && (of_channels_size == sizeof(*of_channels)))
> > +		return be32_to_cpup(of_channels);
> > +#endif
> 
> The be32 thing looks odd. I don't get the idea, but as I don't know
> much about devicetree, I'll trust you.

That's okay. The properties are be32 (coming from powerpc).

Still, there is a new property defined which _always_ needs

a) CCing devicetree-discuss (get_maintainer helps here)
b) Documentation in Documentation/devicetree/bindings

because it needs to be a lot more stable than platform_data.

(I already lost the original mail, so I sadly can't forward it)

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

[-- Attachment #2: Type: text/plain, Size: 153 bytes --]

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [PATCH v4] hwmon: Add support for Texas Instruments ADS1015

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 949 bytes --]


> > +#ifdef CONFIG_OF
> > +	/* fallback on OF */
> > +	of_channels = of_get_property(np, "exported-channels",
> > +				      &of_channels_size);
> > +	if (of_channels && (of_channels_size == sizeof(*of_channels)))
> > +		return be32_to_cpup(of_channels);
> > +#endif
> 
> The be32 thing looks odd. I don't get the idea, but as I don't know
> much about devicetree, I'll trust you.

That's okay. The properties are be32 (coming from powerpc).

Still, there is a new property defined which _always_ needs

a) CCing devicetree-discuss (get_maintainer helps here)
b) Documentation in Documentation/devicetree/bindings

because it needs to be a lot more stable than platform_data.

(I already lost the original mail, so I sadly can't forward it)

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [lm-sensors] (WARNING!!! PGP with incorrect signature) Re:

[Eibach, Dirk]

 

> Still, there is a new property defined which _always_ needs
> 
> a) CCing devicetree-discuss (get_maintainer helps here)
> b) Documentation in Documentation/devicetree/bindings
> 
> because it needs to be a lot more stable than platform_data.

Jean, should I supply a v5 with Documentation or should I supply a
separate patch?

> (I already lost the original mail, so I sadly can't forward it)

I have forwarded it to devicetree-discuss.
 
> Regards,
> 
>    Wolfram

Cheers
Dirk



_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

RE: (WARNING!!! PGP with incorrect signature) Re: [PATCH v4] hwmon: Add support for Texas Instruments ADS1015

[Eibach, Dirk]

 

> Still, there is a new property defined which _always_ needs
> 
> a) CCing devicetree-discuss (get_maintainer helps here)
> b) Documentation in Documentation/devicetree/bindings
> 
> because it needs to be a lot more stable than platform_data.

Jean, should I supply a v5 with Documentation or should I supply a
separate patch?

> (I already lost the original mail, so I sadly can't forward it)

I have forwarded it to devicetree-discuss.
 
> Regards,
> 
>    Wolfram

Cheers
Dirk

Re: [lm-sensors] [PATCH v4] hwmon: Add support for Texas

[Eibach, Dirk]

Hi Jean,

> Patch applied. Two things I'd still like to comment on:

Ahh, good news :)

> Besides this, there is still more dynamic attribute handling 
> than I expected. It looks OK, but I'll propose a patch making 
> it more static.
> You'll tell me what you think.

I put the attributes in a group because I thought handling single
attributes is clumsy and error prone.
Anyway I am looking forward to your proposal.
Will this be a v5 or a separate patch?

> --
> Jean Delvare

Chhers
Dirk
------------------------------------------------------------------------------------------------
Messe-Highlights 2011. Wir freuen uns auf Ihren Besuch.                                         

CeBIT 2011 
In Hannover - 01.03. bis 05.03.2011 - Halle 12, Stand C50

ATC Global 2011 
Amsterdam - 08.03. bis 10.03.2011 - Halle 9, Stand R404
------------------------------------------------------------------------------------------------
Guntermann & Drunck GmbH Systementwicklung 
Dortmunder Str. 4a 
D-57234 Wilnsdorf - Germany 
Tel: +49 (0) 27 39 / 89 01 - 100  Fax: +49 (0) 27 39 / 89 01 - 120 
E-Mail: sales@gdsys.de - Web: www.gdsys.de
------------------------------------------------------------------------------------------------
Geschaftsfuhrer: 
Udo Guntermann - Martin Drunck - Reiner Ruelmann - Klaus Tocke
HRB 2884, Amtsgericht Siegen - WEEE-Reg.-Nr. DE30763240
USt.-Id.-Nr. DE 126575222 - Steuer-Nr. 342 / 5835 / 1041
------------------------------------------------------------------------------------------------
DQS-zertifiziert nach ISO 9001:2000
------------------------------------------------------------------------------------------------



_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

RE: [PATCH v4] hwmon: Add support for Texas Instruments ADS1015

[Eibach, Dirk]

Hi Jean,

> Patch applied. Two things I'd still like to comment on:

Ahh, good news :)

> Besides this, there is still more dynamic attribute handling 
> than I expected. It looks OK, but I'll propose a patch making 
> it more static.
> You'll tell me what you think.

I put the attributes in a group because I thought handling single
attributes is clumsy and error prone.
Anyway I am looking forward to your proposal.
Will this be a v5 or a separate patch?

> --
> Jean Delvare

Chhers
Dirk
------------------------------------------------------------------------------------------------
Messe-Highlights 2011. Wir freuen uns auf Ihren Besuch.                                         

CeBIT 2011 
In Hannover - 01.03. bis 05.03.2011 - Halle 12, Stand C50

ATC Global 2011 
Amsterdam - 08.03. bis 10.03.2011 - Halle 9, Stand R404
------------------------------------------------------------------------------------------------
Guntermann & Drunck GmbH Systementwicklung 
Dortmunder Str. 4a 
D-57234 Wilnsdorf - Germany 
Tel: +49 (0) 27 39 / 89 01 - 100  Fax: +49 (0) 27 39 / 89 01 - 120 
E-Mail: sales@gdsys.de - Web: www.gdsys.de
------------------------------------------------------------------------------------------------
Geschaftsfuhrer: 
Udo Guntermann - Martin Drunck - Reiner Ruelmann - Klaus Tocke
HRB 2884, Amtsgericht Siegen - WEEE-Reg.-Nr. DE30763240
USt.-Id.-Nr. DE 126575222 - Steuer-Nr. 342 / 5835 / 1041
------------------------------------------------------------------------------------------------
DQS-zertifiziert nach ISO 9001:2000
------------------------------------------------------------------------------------------------

Re: [lm-sensors] [PATCH v4] hwmon: Add support for Texas

[Jean Delvare]

On Thu, 3 Mar 2011 08:49:13 +0100, Eibach, Dirk wrote:
>  
> 
> > Still, there is a new property defined which _always_ needs
> > 
> > a) CCing devicetree-discuss (get_maintainer helps here)
> > b) Documentation in Documentation/devicetree/bindings
> > 
> > because it needs to be a lot more stable than platform_data.
> 
> Jean, should I supply a v5 with Documentation or should I supply a
> separate patch?

Separate patch. I'm not even the one who will review and apply it.

> > (I already lost the original mail, so I sadly can't forward it)
> 
> I have forwarded it to devicetree-discuss.

This is where the review will happen, and where hopefully someone will
pick up the patch.

-- 
Jean Delvare

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [PATCH v4] hwmon: Add support for Texas Instruments ADS1015

[Jean Delvare]

On Thu, 3 Mar 2011 08:49:13 +0100, Eibach, Dirk wrote:
>  
> 
> > Still, there is a new property defined which _always_ needs
> > 
> > a) CCing devicetree-discuss (get_maintainer helps here)
> > b) Documentation in Documentation/devicetree/bindings
> > 
> > because it needs to be a lot more stable than platform_data.
> 
> Jean, should I supply a v5 with Documentation or should I supply a
> separate patch?

Separate patch. I'm not even the one who will review and apply it.

> > (I already lost the original mail, so I sadly can't forward it)
> 
> I have forwarded it to devicetree-discuss.

This is where the review will happen, and where hopefully someone will
pick up the patch.

-- 
Jean Delvare

Re: [lm-sensors] [PATCH v4] hwmon: Add support for Texas

[Eibach, Dirk]

> Hi Dirk,
> thanks for your work! 
>  
> I've used your code in my system and it works fine, except 
> for a small bug when the code checks for the end of 
> conversion flag (it should be negated)
>  
> I've extended it to work with ADS1115 (the 16 bit version) 
> and below there is the "patch of patch v4"
>  
> I hope it would be useful for someone else.
>  
> Thanks again
>  
> Emiliano.
>  
> <<<<<< start of patch >>>>>>

We should mention ADS1115 support in all the appropriate places. Grep
for ADS1015 to get an idea.

> diff --git a/drivers/hwmon/ads1015.c 
> b/drivers/hwmon/ads1015.c index 4572024..6025a90 100644
> --- a/drivers/hwmon/ads1015.c
> +++ b/drivers/hwmon/ads1015.c
> @@ -53,6 +53,12 @@ struct ads1015_data {
>   struct mutex update_lock; /* mutex protect updates */
>   struct attribute *attr_table[ADS1015_CONFIG_CHANNELS + 1];
>   struct attribute_group attr_group;
> + int   id;
> +};
> +
> +enum ads1015_num_id {
> + ADS1015 = 0,
> + ADS1115,
>  };

The first constant in an enumeration is 0 by default.

>  static s32 ads1015_read_reg(struct i2c_client *client, 
> unsigned int reg) @@ -78,6 +84,7 @@ static int 
> ads1015_read_value(struct i2c_client *client, unsigned int channel,
>   unsigned int k;
>   struct ads1015_data *data = i2c_get_clientdata(client);
>   int res;
> + int msec;
>  
>   mutex_lock(&data->update_lock);
>  
> @@ -89,6 +96,14 @@ static int ads1015_read_value(struct 
> i2c_client *client, unsigned int channel,
>   pga = (config >> 9) & 0x0007;
>   fullscale = fullscale_table[pga];
>  
> + /* for ADS1115, get the conversion time */ if(data->id = 
> ADS1115) {  
> + msec = (config >> 5) & 0x0007;  msec = 128 >> msec; } else  
> msec = 1;

Wouldn't it make sense to make sure msec is at least 1?

>   /* set channel and start single conversion */
>   config &= ~(0x0007 << 12);
>   config |= (1 << 15) | (1 << 8) | (channel & 0x0007) << 12; 
> @@ -98,12 +113,12 @@ static int ads1015_read_value(struct 
> i2c_client *client, unsigned int channel,
>   if (res < 0)
>    goto err_unlock;
>   for (k = 0; k < 5; ++k) {
> -  schedule_timeout(msecs_to_jiffies(1));
> +  schedule_timeout(msecs_to_jiffies(msec));
>    res = ads1015_read_reg(client, ADS1015_CONFIG);
>    if (res < 0)
>     goto err_unlock;
>    config = res;
> -  if (config & (1 << 15))
> +  if (~(config) & (1 << 15))
>     break;

Hmm, datasheet says bit 15 is 0 when device is performing a conversion
and 1 when it is finished. So exiting on Bit 15 set seems right to me
(and I verfified it works). Why do you think it should be negated?

>   }
>   if (k = 5) {
> @@ -118,7 +133,10 @@ static int ads1015_read_value(struct 
> i2c_client *client, unsigned int channel,
>  
>   mutex_unlock(&data->update_lock);
>  
> - *value = DIV_ROUND_CLOSEST(conversion * fullscale, 0x7ff0);
> + if(data->id = ADS1115)
> +  *value = DIV_ROUND_CLOSEST(conversion * fullscale, 0x7fff); else  
> + *value = DIV_ROUND_CLOSEST(conversion * fullscale, 0x7ff0);

I would find it more appropriate to store the fullscale value in
ads1015_data.

>   return 0;
>  
> @@ -133,7 +151,7 @@ static ssize_t show_in(struct device 
> *dev, struct device_attribute *da,  {
>   struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
>   struct i2c_client *client = to_i2c_client(dev);
> - int in;
> + int in = 0;

No need to initialize this.

>   int res;
>  
>   res = ads1015_read_value(client, attr->index, &in); @@ 
> -239,7 +257,9 @@ static int ads1015_probe(struct i2c_client *client,
>    err = PTR_ERR(data->hwmon_dev);
>    goto exit_remove;
>   }
> -
> + 
> + data->id = id->driver_data;
> + 
>   return 0;
>  
>  exit_remove:
> @@ -251,7 +271,8 @@ exit:
>  }
>  
>  static const struct i2c_device_id ads1015_id[] = {
> - { "ads1015", 0 },
> + { "ads1015", ADS1015 },
> + { "ads1115", ADS1115 },
>   { }
>  };
>  MODULE_DEVICE_TABLE(i2c, ads1015_id);
> <<<<<< end of patch >>>>>>
> 
> 
> -----------------------------
> 
> 
> Emiliano Carnati

Cheers
Dirk



_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH v4] hwmon: Add support for Texas

[Emiliano Carnati]

>
> We should mention ADS1115 support in all the appropriate places. Grep
> for ADS1015 to get an idea.
>

From what I can see, the other changes could be


<<<< Addendum >>>>
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 9abcc6b..9b3e3e9 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -856,7 +856,7 @@ config SENSORS_ADS1015
 	depends on I2C
 	help
 	  If you say yes here you get support for Texas Instruments ADS1015
-	  12-bit 4-input ADC device.
+	  & ADS1115 12/16-bit 4-input ADC devices.

 	  This driver can also be built as a module.  If so, the module
 	  will be called ads1015.
diff --git a/Documentation/hwmon/ads1015 b/Documentation/hwmon/ads1015
index 85ffd77..e12fd1c 100644
--- a/Documentation/hwmon/ads1015
+++ b/Documentation/hwmon/ads1015
@@ -6,6 +6,10 @@ Supported chips:
     Prefix: 'ads1015'
     Datasheet: Publicly available at the Texas Instruments website :
                http://focus.ti.com/lit/ds/symlink/ads1015.pdf
+  * Texas Instruments ADS1115
+    Prefix: 'ads1115'
+    Datasheet: Publicly available at the Texas Instruments website :
+               http://focus.ti.com/lit/ds/symlink/ads1115.pdf

 Authors:
         Dirk Eibach, Guntermann & Drunck GmbH <eibach <at> gdsys.de>
@@ -13,9 +17,11 @@ Authors:
 Description
 -----------

-This driver implements support for the Texas Instruments ADS1015.
+This driver implements support for the Texas Instruments ADS1015 and
+ADS1115.

-This device is a 12-bit A-D converter with 4 inputs.
+ADS1015 is a 12-bit A-D converter with 4 inputs.
+ADS1115 is a 16-bit A-D converter with 4 inputs.

 The inputs can be used single ended or in certain differential 
combinations.

<<<< end of addendum >>>> 


_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH v4] hwmon: Add support for Texas

[Guenter Roeck]

Hi Emiliano,

On Tue, Mar 08, 2011 at 05:04:20AM -0500, Emiliano Carnati wrote:
> Hi Dirk,
> thanks for your work!
> 
> I've used your code in my system and it works fine, except for a small bug when the
> code checks for the end of conversion flag (it should be negated)
> 
If this is really a bug, there should be two patches - one for the bug fix,
the other for ads1115 support.

> I've extended it to work with ADS1115 (the 16 bit version) and below there
> is the "patch of patch v4"
> 
Couple of comments below.

> I hope it would be useful for someone else.
> 
> Thanks again
> 
> Emiliano.
> 
> <<<<<< start of patch >>>>>>
> diff --git a/drivers/hwmon/ads1015.c b/drivers/hwmon/ads1015.c
> index 4572024..6025a90 100644
> --- a/drivers/hwmon/ads1015.c
> +++ b/drivers/hwmon/ads1015.c
> @@ -53,6 +53,12 @@ struct ads1015_data {
>   struct mutex update_lock; /* mutex protect updates */
>   struct attribute *attr_table[ADS1015_CONFIG_CHANNELS + 1];
>   struct attribute_group attr_group;
> + int   id;
> +};
> +
> +enum ads1015_num_id {
> + ADS1015 = 0,
> + ADS1115,
>  };

We commonly use lowercase here.

> 
>  static s32 ads1015_read_reg(struct i2c_client *client, unsigned int reg)
> @@ -78,6 +84,7 @@ static int ads1015_read_value(struct i2c_client *client, unsigned int channel,
>   unsigned int k;
>   struct ads1015_data *data = i2c_get_clientdata(client);
>   int res;
> + int msec;
> 
>   mutex_lock(&data->update_lock);
> 
> @@ -89,6 +96,14 @@ static int ads1015_read_value(struct i2c_client *client, unsigned int channel,
>   pga = (config >> 9) & 0x0007;
>   fullscale = fullscale_table[pga];
> 
> + /* for ADS1115, get the conversion time */
> + if(data->id = ADS1115)
> + {
> +  msec = (config >> 5) & 0x0007;
> +  msec = 128 >> msec;
> + }
> + else
> +  msec = 1;

Please follow Linux coding style.
	(Location of { }, and use { } in both branches of if/else)

>   /* set channel and start single conversion */
>   config &= ~(0x0007 << 12);
>   config |= (1 << 15) | (1 << 8) | (channel & 0x0007) << 12;
> @@ -98,12 +113,12 @@ static int ads1015_read_value(struct i2c_client *client, unsigned int channel,
>   if (res < 0)
>    goto err_unlock;
>   for (k = 0; k < 5; ++k) {
> -  schedule_timeout(msecs_to_jiffies(1));
> +  schedule_timeout(msecs_to_jiffies(msec));
>    res = ads1015_read_reg(client, ADS1015_CONFIG);
>    if (res < 0)
>     goto err_unlock;
>    config = res;
> -  if (config & (1 << 15))
> +  if (~(config) & (1 << 15))

Again, please follow Linux coding style (no unnecessary () )

>     break;
>   }
>   if (k = 5) {
> @@ -118,7 +133,10 @@ static int ads1015_read_value(struct i2c_client *client, unsigned int channel,
> 
>   mutex_unlock(&data->update_lock);
> 
> - *value = DIV_ROUND_CLOSEST(conversion * fullscale, 0x7ff0);
> + if(data->id = ADS1115)
> +  *value = DIV_ROUND_CLOSEST(conversion * fullscale, 0x7fff);
> + else
> +  *value = DIV_ROUND_CLOSEST(conversion * fullscale, 0x7ff0);
> 
>   return 0;
> 
> @@ -133,7 +151,7 @@ static ssize_t show_in(struct device *dev, struct device_attribute *da,
>  {
>   struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
>   struct i2c_client *client = to_i2c_client(dev);
> - int in;
> + int in = 0;
>   int res;
> 
>   res = ads1015_read_value(client, attr->index, &in);
> @@ -239,7 +257,9 @@ static int ads1015_probe(struct i2c_client *client,
>    err = PTR_ERR(data->hwmon_dev);
>    goto exit_remove;
>   }
> -
> +
> + data->id = id->driver_data;
> +
>   return 0;
> 
>  exit_remove:
> @@ -251,7 +271,8 @@ exit:
>  }
> 
>  static const struct i2c_device_id ads1015_id[] = {
> - { "ads1015", 0 },
> + { "ads1015", ADS1015 },
> + { "ads1115", ADS1115 },
>   { }
>  };
>  MODULE_DEVICE_TABLE(i2c, ads1015_id);
> <<<<<< end of patch >>>>>>
> 
> -----------------------------
> 
> Emiliano Carnati

> _______________________________________________
> lm-sensors mailing list
> lm-sensors@lm-sensors.org
> http://lists.lm-sensors.org/mailman/listinfo/lm-sensors


_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH v4] hwmon: Add support for Texas

[Emiliano Carnati]

It make sense...

Please correct me if I'm wrong. I should:

- start from Dirk's patch v4
- create a patch that fixes the bug
- apply this patch
- create a patch that adds the support for 1115

It's right?


> Hi Emiliano,
>
> On Tue, Mar 08, 2011 at 05:04:20AM -0500, Emiliano Carnati wrote:
>> Hi Dirk,
>> thanks for your work!
>>
>> I've used your code in my system and it works fine, except for a small 
>> bug when the
>> code checks for the end of conversion flag (it should be negated)
>>
> If this is really a bug, there should be two patches - one for the bug 
> fix,
> the other for ads1115 support.
>
>> I've extended it to work with ADS1115 (the 16 bit version) and below 
>> there
>> is the "patch of patch v4"
>>
> Couple of comments below.
>
>> I hope it would be useful for someone else.
>>
>> Thanks again
>>
>> Emiliano.
>>
>> <<<<<< start of patch >>>>>>
>> diff --git a/drivers/hwmon/ads1015.c b/drivers/hwmon/ads1015.c
>> index 4572024..6025a90 100644
>> --- a/drivers/hwmon/ads1015.c
>> +++ b/drivers/hwmon/ads1015.c
>> @@ -53,6 +53,12 @@ struct ads1015_data {
>>   struct mutex update_lock; /* mutex protect updates */
>>   struct attribute *attr_table[ADS1015_CONFIG_CHANNELS + 1];
>>   struct attribute_group attr_group;
>> + int   id;
>> +};
>> +
>> +enum ads1015_num_id {
>> + ADS1015 = 0,
>> + ADS1115,
>>  };
>
> We commonly use lowercase here.
>
>>
>>  static s32 ads1015_read_reg(struct i2c_client *client, unsigned int reg)
>> @@ -78,6 +84,7 @@ static int ads1015_read_value(struct i2c_client 
>> *client, unsigned int channel,
>>   unsigned int k;
>>   struct ads1015_data *data = i2c_get_clientdata(client);
>>   int res;
>> + int msec;
>>
>>   mutex_lock(&data->update_lock);
>>
>> @@ -89,6 +96,14 @@ static int ads1015_read_value(struct i2c_client 
>> *client, unsigned int channel,
>>   pga = (config >> 9) & 0x0007;
>>   fullscale = fullscale_table[pga];
>>
>> + /* for ADS1115, get the conversion time */
>> + if(data->id = ADS1115)
>> + {
>> +  msec = (config >> 5) & 0x0007;
>> +  msec = 128 >> msec;
>> + }
>> + else
>> +  msec = 1;
>
> Please follow Linux coding style.
> (Location of { }, and use { } in both branches of if/else)
>
>>   /* set channel and start single conversion */
>>   config &= ~(0x0007 << 12);
>>   config |= (1 << 15) | (1 << 8) | (channel & 0x0007) << 12;
>> @@ -98,12 +113,12 @@ static int ads1015_read_value(struct i2c_client 
>> *client, unsigned int channel,
>>   if (res < 0)
>>    goto err_unlock;
>>   for (k = 0; k < 5; ++k) {
>> -  schedule_timeout(msecs_to_jiffies(1));
>> +  schedule_timeout(msecs_to_jiffies(msec));
>>    res = ads1015_read_reg(client, ADS1015_CONFIG);
>>    if (res < 0)
>>     goto err_unlock;
>>    config = res;
>> -  if (config & (1 << 15))
>> +  if (~(config) & (1 << 15))
>
> Again, please follow Linux coding style (no unnecessary () )
>
>>     break;
>>   }
>>   if (k = 5) {
>> @@ -118,7 +133,10 @@ static int ads1015_read_value(struct i2c_client 
>> *client, unsigned int channel,
>>
>>   mutex_unlock(&data->update_lock);
>>
>> - *value = DIV_ROUND_CLOSEST(conversion * fullscale, 0x7ff0);
>> + if(data->id = ADS1115)
>> +  *value = DIV_ROUND_CLOSEST(conversion * fullscale, 0x7fff);
>> + else
>> +  *value = DIV_ROUND_CLOSEST(conversion * fullscale, 0x7ff0);
>>
>>   return 0;
>>
>> @@ -133,7 +151,7 @@ static ssize_t show_in(struct device *dev, struct 
>> device_attribute *da,
>>  {
>>   struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
>>   struct i2c_client *client = to_i2c_client(dev);
>> - int in;
>> + int in = 0;
>>   int res;
>>
>>   res = ads1015_read_value(client, attr->index, &in);
>> @@ -239,7 +257,9 @@ static int ads1015_probe(struct i2c_client *client,
>>    err = PTR_ERR(data->hwmon_dev);
>>    goto exit_remove;
>>   }
>> -
>> +
>> + data->id = id->driver_data;
>> +
>>   return 0;
>>
>>  exit_remove:
>> @@ -251,7 +271,8 @@ exit:
>>  }
>>
>>  static const struct i2c_device_id ads1015_id[] = {
>> - { "ads1015", 0 },
>> + { "ads1015", ADS1015 },
>> + { "ads1115", ADS1115 },
>>   { }
>>  };
>>  MODULE_DEVICE_TABLE(i2c, ads1015_id);
>> <<<<<< end of patch >>>>>>
>>
>> -----------------------------
>>
>> Emiliano Carnati
>
>> _______________________________________________
>> lm-sensors mailing list
>> lm-sensors@lm-sensors.org
>> http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
>
>
 


_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH v4] hwmon: Add support for Texas

[Emiliano Carnati]

Sorry to all,
it's the first time I contribute to the kernel and I've done a bit of 
confusion...

First of all, I apologize:  there is no bug in Dirk's code.

What I've found is that I need to call set_current_state before 
schedule_timeout,
otherways the system doesn't wait at all.


Below there is the patch to patch v4  with the changes pointed by Dirk and 
Guenter.
- the {} in the if clause are in kernel style
- the enum is lowercase and is not initialized
- the msec variable is always >= 1 (because 128 >> 7 = 1)
- I've initialized in=0 because otherways I get a compiler warning

Thaks for your patience
Emiliano.

<<<< START >>>>
diff --git a/Documentation/hwmon/ads1015 b/Documentation/hwmon/ads1015
index 85ffd77..e12fd1c 100644
--- a/Documentation/hwmon/ads1015
+++ b/Documentation/hwmon/ads1015
@@ -6,6 +6,10 @@ Supported chips:
     Prefix: 'ads1015'
     Datasheet: Publicly available at the Texas Instruments website :
                http://focus.ti.com/lit/ds/symlink/ads1015.pdf
+  * Texas Instruments ADS1115
+    Prefix: 'ads1115'
+    Datasheet: Publicly available at the Texas Instruments website :
+               http://focus.ti.com/lit/ds/symlink/ads1115.pdf

 Authors:
         Dirk Eibach, Guntermann & Drunck GmbH <eibach <at> gdsys.de>
@@ -13,9 +17,11 @@ Authors:
 Description
 -----------

-This driver implements support for the Texas Instruments ADS1015.
+This driver implements support for the Texas Instruments ADS1015 and
+ADS1115.

-This device is a 12-bit A-D converter with 4 inputs.
+ADS1015 is a 12-bit A-D converter with 4 inputs.
+ADS1115 is a 16-bit A-D converter with 4 inputs.

 The inputs can be used single ended or in certain differential 
combinations.

diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 9abcc6b..9b3e3e9 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -856,7 +856,7 @@ config SENSORS_ADS1015
 	depends on I2C
 	help
 	  If you say yes here you get support for Texas Instruments ADS1015
-	  12-bit 4-input ADC device.
+	  & ADS1115 12/16-bit 4-input ADC devices.

 	  This driver can also be built as a module.  If so, the module
 	  will be called ads1015.
diff --git a/drivers/hwmon/ads1015.c b/drivers/hwmon/ads1015.c
index 4572024..9a607b0 100644
--- a/drivers/hwmon/ads1015.c
+++ b/drivers/hwmon/ads1015.c
@@ -53,6 +53,12 @@ struct ads1015_data {
 	struct mutex update_lock; /* mutex protect updates */
 	struct attribute *attr_table[ADS1015_CONFIG_CHANNELS + 1];
 	struct attribute_group attr_group;
+	int   id;
+};
+
+enum ads1015_num_id {
+	ads1015,
+	ads1115,
 };

 static s32 ads1015_read_reg(struct i2c_client *client, unsigned int reg)
@@ -78,6 +84,7 @@ static int ads1015_read_value(struct i2c_client *client, 
unsigned int channel,
 	unsigned int k;
 	struct ads1015_data *data = i2c_get_clientdata(client);
 	int res;
+	int msec;

 	mutex_lock(&data->update_lock);

@@ -89,6 +96,13 @@ static int ads1015_read_value(struct i2c_client *client, 
unsigned int channel,
 	pga = (config >> 9) & 0x0007;
 	fullscale = fullscale_table[pga];

+	/* for ADS1115, get the conversion time */
+	if(data->id = ads1115) {
+		msec = (config >> 5) & 0x0007;
+		msec = 128 >> msec;
+	}
+	else
+		msec = 1;
 	/* set channel and start single conversion */
 	config &= ~(0x0007 << 12);
 	config |= (1 << 15) | (1 << 8) | (channel & 0x0007) << 12;
@@ -98,7 +112,8 @@ static int ads1015_read_value(struct i2c_client *client, 
unsigned int channel,
 	if (res < 0)
 		goto err_unlock;
 	for (k = 0; k < 5; ++k) {
-		schedule_timeout(msecs_to_jiffies(1));
+		set_current_state(TASK_INTERRUPTIBLE);
+		schedule_timeout(msecs_to_jiffies(msec));
 		res = ads1015_read_reg(client, ADS1015_CONFIG);
 		if (res < 0)
 			goto err_unlock;
@@ -118,7 +133,10 @@ static int ads1015_read_value(struct i2c_client 
*client, unsigned int channel,

 	mutex_unlock(&data->update_lock);

-	*value = DIV_ROUND_CLOSEST(conversion * fullscale, 0x7ff0);
+	if(data->id = ads1115)
+		*value = DIV_ROUND_CLOSEST(conversion * fullscale, 0x7fff);
+	else
+		*value = DIV_ROUND_CLOSEST(conversion * fullscale, 0x7ff0);

 	return 0;

@@ -133,7 +151,7 @@ static ssize_t show_in(struct device *dev, struct 
device_attribute *da,
 {
 	struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
 	struct i2c_client *client = to_i2c_client(dev);
-	int in;
+	int in = 0;
 	int res;

 	res = ads1015_read_value(client, attr->index, &in);
@@ -239,7 +257,9 @@ static int ads1015_probe(struct i2c_client *client,
 		err = PTR_ERR(data->hwmon_dev);
 		goto exit_remove;
 	}
-
+
+	data->id = id->driver_data;
+
 	return 0;

 exit_remove:
@@ -251,7 +271,8 @@ exit:
 }

 static const struct i2c_device_id ads1015_id[] = {
-	{ "ads1015", 0 },
+	{ "ads1015", ads1015 },
+	{ "ads1115", ads1115 },
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, ads1015_id);
<<<< END >>>>
 


_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH v4] hwmon: Add support for Texas

[Guenter Roeck]

Hi Emiliano,

On Tue, Mar 08, 2011 at 12:43:17PM -0500, Emiliano Carnati wrote:
> Sorry to all,
> it's the first time I contribute to the kernel and I've done a bit of 
> confusion...
> 
> First of all, I apologize:  there is no bug in Dirk's code.
> 
> What I've found is that I need to call set_current_state before 
> schedule_timeout,
> otherways the system doesn't wait at all.
> 
> 
> Below there is the patch to patch v4  with the changes pointed by Dirk and 
> Guenter.
> - the {} in the if clause are in kernel style
> - the enum is lowercase and is not initialized
> - the msec variable is always >= 1 (because 128 >> 7 = 1)
> - I've initialized in=0 because otherways I get a compiler warning
> 
> Thaks for your patience
> Emiliano.
> 
> <<<< START >>>>
> diff --git a/Documentation/hwmon/ads1015 b/Documentation/hwmon/ads1015
> index 85ffd77..e12fd1c 100644
> --- a/Documentation/hwmon/ads1015
> +++ b/Documentation/hwmon/ads1015
> @@ -6,6 +6,10 @@ Supported chips:
>      Prefix: 'ads1015'
>      Datasheet: Publicly available at the Texas Instruments website :
>                 http://focus.ti.com/lit/ds/symlink/ads1015.pdf
> +  * Texas Instruments ADS1115
> +    Prefix: 'ads1115'
> +    Datasheet: Publicly available at the Texas Instruments website :
> +               http://focus.ti.com/lit/ds/symlink/ads1115.pdf
> 
>  Authors:
>          Dirk Eibach, Guntermann & Drunck GmbH <eibach <at> gdsys.de>
> @@ -13,9 +17,11 @@ Authors:
>  Description
>  -----------
> 
> -This driver implements support for the Texas Instruments ADS1015.
> +This driver implements support for the Texas Instruments ADS1015 and
> +ADS1115.
> 
> -This device is a 12-bit A-D converter with 4 inputs.
> +ADS1015 is a 12-bit A-D converter with 4 inputs.
> +ADS1115 is a 16-bit A-D converter with 4 inputs.
> 
>  The inputs can be used single ended or in certain differential 
> combinations.
> 
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 9abcc6b..9b3e3e9 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -856,7 +856,7 @@ config SENSORS_ADS1015
>  	depends on I2C
>  	help
>  	  If you say yes here you get support for Texas Instruments ADS1015
> -	  12-bit 4-input ADC device.
> +	  & ADS1115 12/16-bit 4-input ADC devices.
> 
>  	  This driver can also be built as a module.  If so, the module
>  	  will be called ads1015.
> diff --git a/drivers/hwmon/ads1015.c b/drivers/hwmon/ads1015.c
> index 4572024..9a607b0 100644
> --- a/drivers/hwmon/ads1015.c
> +++ b/drivers/hwmon/ads1015.c
> @@ -53,6 +53,12 @@ struct ads1015_data {
>  	struct mutex update_lock; /* mutex protect updates */
>  	struct attribute *attr_table[ADS1015_CONFIG_CHANNELS + 1];
>  	struct attribute_group attr_group;
> +	int   id;
> +};
> +
> +enum ads1015_num_id {
> +	ads1015,
> +	ads1115,
>  };
> 
>  static s32 ads1015_read_reg(struct i2c_client *client, unsigned int reg)
> @@ -78,6 +84,7 @@ static int ads1015_read_value(struct i2c_client *client, 
> unsigned int channel,
>  	unsigned int k;
>  	struct ads1015_data *data = i2c_get_clientdata(client);
>  	int res;
> +	int msec;
> 
>  	mutex_lock(&data->update_lock);
> 
> @@ -89,6 +96,13 @@ static int ads1015_read_value(struct i2c_client *client, 
> unsigned int channel,
>  	pga = (config >> 9) & 0x0007;
>  	fullscale = fullscale_table[pga];
> 
> +	/* for ADS1115, get the conversion time */
> +	if(data->id = ads1115) {
> +		msec = (config >> 5) & 0x0007;
> +		msec = 128 >> msec;
> +	}
> +	else
> +		msec = 1;

Actually, the rule is to either use { } in both branches of an if statement,
or not at all. So here you would use it in both branches, or rewrite it to

+	if(data->id = ads1115)
+		msec = 128 >> ((config >> 5) & 0x0007);
+	else
+		msec = 1;

As mentioned before, with this you end up waiting up to 128 * 5 ms which seems to be
a bit long. However, I wonder if this is necessary in the first place. From the datasheet
it seems to be used for continuous mode only, and it reflects the number of samples
per second. Are you sure you need it, or did you have a problem because set_current_state()
was missing ?

Another question - is it ok to keep the thread state as TASK_INTERRUPTIBLE after
the wait is complete ?

Thanks,
Guenter

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH v4] hwmon: Add support for Texas

[Emiliano Carnati]

Hi Guenter,

> Hi Emiliano,
>
> On Tue, Mar 08, 2011 at 12:43:17PM -0500, Emiliano Carnati wrote:
>> Sorry to all,
>> it's the first time I contribute to the kernel and I've done a bit of
>> confusion...
>>
>> First of all, I apologize:  there is no bug in Dirk's code.
>>
>> What I've found is that I need to call set_current_state before
>> schedule_timeout,
>> otherways the system doesn't wait at all.
>>
>>
>> Below there is the patch to patch v4  with the changes pointed by Dirk 
>> and
>> Guenter.
>> - the {} in the if clause are in kernel style
>> - the enum is lowercase and is not initialized
>> - the msec variable is always >= 1 (because 128 >> 7 = 1)
>> - I've initialized in=0 because otherways I get a compiler warning
>>
>> Thaks for your patience
>> Emiliano.
>>
>> <<<< START >>>>
>> diff --git a/Documentation/hwmon/ads1015 b/Documentation/hwmon/ads1015
>> index 85ffd77..e12fd1c 100644
>> --- a/Documentation/hwmon/ads1015
>> +++ b/Documentation/hwmon/ads1015
>> @@ -6,6 +6,10 @@ Supported chips:
>>      Prefix: 'ads1015'
>>      Datasheet: Publicly available at the Texas Instruments website :
>>                 http://focus.ti.com/lit/ds/symlink/ads1015.pdf
>> +  * Texas Instruments ADS1115
>> +    Prefix: 'ads1115'
>> +    Datasheet: Publicly available at the Texas Instruments website :
>> +               http://focus.ti.com/lit/ds/symlink/ads1115.pdf
>>
>>  Authors:
>>          Dirk Eibach, Guntermann & Drunck GmbH <eibach <at> gdsys.de>
>> @@ -13,9 +17,11 @@ Authors:
>>  Description
>>  -----------
>>
>> -This driver implements support for the Texas Instruments ADS1015.
>> +This driver implements support for the Texas Instruments ADS1015 and
>> +ADS1115.
>>
>> -This device is a 12-bit A-D converter with 4 inputs.
>> +ADS1015 is a 12-bit A-D converter with 4 inputs.
>> +ADS1115 is a 16-bit A-D converter with 4 inputs.
>>
>>  The inputs can be used single ended or in certain differential
>> combinations.
>>
>> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
>> index 9abcc6b..9b3e3e9 100644
>> --- a/drivers/hwmon/Kconfig
>> +++ b/drivers/hwmon/Kconfig
>> @@ -856,7 +856,7 @@ config SENSORS_ADS1015
>>  depends on I2C
>>  help
>>    If you say yes here you get support for Texas Instruments ADS1015
>> -   12-bit 4-input ADC device.
>> +   & ADS1115 12/16-bit 4-input ADC devices.
>>
>>    This driver can also be built as a module.  If so, the module
>>    will be called ads1015.
>> diff --git a/drivers/hwmon/ads1015.c b/drivers/hwmon/ads1015.c
>> index 4572024..9a607b0 100644
>> --- a/drivers/hwmon/ads1015.c
>> +++ b/drivers/hwmon/ads1015.c
>> @@ -53,6 +53,12 @@ struct ads1015_data {
>>  struct mutex update_lock; /* mutex protect updates */
>>  struct attribute *attr_table[ADS1015_CONFIG_CHANNELS + 1];
>>  struct attribute_group attr_group;
>> + int   id;
>> +};
>> +
>> +enum ads1015_num_id {
>> + ads1015,
>> + ads1115,
>>  };
>>
>>  static s32 ads1015_read_reg(struct i2c_client *client, unsigned int reg)
>> @@ -78,6 +84,7 @@ static int ads1015_read_value(struct i2c_client 
>> *client,
>> unsigned int channel,
>>  unsigned int k;
>>  struct ads1015_data *data = i2c_get_clientdata(client);
>>  int res;
>> + int msec;
>>
>>  mutex_lock(&data->update_lock);
>>
>> @@ -89,6 +96,13 @@ static int ads1015_read_value(struct i2c_client 
>> *client,
>> unsigned int channel,
>>  pga = (config >> 9) & 0x0007;
>>  fullscale = fullscale_table[pga];
>>
>> + /* for ADS1115, get the conversion time */
>> + if(data->id = ads1115) {
>> + msec = (config >> 5) & 0x0007;
>> + msec = 128 >> msec;
>> + }
>> + else
>> + msec = 1;
>
> Actually, the rule is to either use { } in both branches of an if 
> statement,
> or not at all. So here you would use it in both branches, or rewrite it to

Ok. I'll write it with  {} in the else branch

>
> + if(data->id = ads1115)
> + msec = 128 >> ((config >> 5) & 0x0007);
> + else
> + msec = 1;
>
> As mentioned before, with this you end up waiting up to 128 * 5 ms which 
> seems to be
> a bit long. However, I wonder if this is necessary in the first place. 
> From the datasheet
> it seems to be used for continuous mode only, and it reflects the number 
> of samples
> per second. Are you sure you need it, or did you have a problem because 
> set_current_state()
> was missing ?

In this way, it waits something between 1 and 128ms, depending on the sps 
field in the config register.
The datasheet is not clear about this, it gives only the timing for the 
highest speed,
but it's exactly 1/rate and the converters are sigma delta, so the 
conversion time should be
proportional to the sampling time

What the driver lacks is a way to set the adc parameters (sps, gain)

>
> Another question - is it ok to keep the thread state as TASK_INTERRUPTIBLE 
> after
> the wait is complete ?

Well, maybe it's better to put a set_current_state(TASK_RUNNING) after the 
wait loop.

>
> Thanks,
> Guenter
>
 


_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH v4] hwmon: Add support for Texas

[Jean Delvare]

Emiliano, Guenter, Dirk,

On Wed, 9 Mar 2011 11:05:52 +0100, Emiliano Carnati wrote:
> > Another question - is it ok to keep the thread state as TASK_INTERRUPTIBLE 
> > after the wait is complete ?
> 
> Well, maybe it's better to put a set_current_state(TASK_RUNNING) after the 
> wait loop.

LDD3 doesn't say anything about setting TASK_RUNNING after
schedule_timeout(), and looking at other drivers, most don't do it, so
I am reasonably certain that we don't have to care.

OTOH, I believe that TASK_INTERRUPTIBLE is not appropriate here. A
received signal would shorten the wait time, meaning that the driver
would no longer wait for the maximum time and may thus return an error.
TASK_UNINTERRUPTIBLE is what we want here, I think, and as a matter of
fact this is what the abituguru driver is doing. TASK_INTERRUPTIBLE
would only be acceptable if the control loop was time-based rather than
count-based.

The missing set_current_state() in the original driver is a genuine
bug, so I'll merge the fix directly in the patch which adds the driver.
Thanks for noticing and reporting.

For reference, here is the change I applied:

--- linux-2.6.38.orig/drivers/hwmon/ads1015.c	2011-03-16 16:49:29.000000000 +0100
+++ linux-2.6.38/drivers/hwmon/ads1015.c	2011-03-16 16:45:04.000000000 +0100
@@ -98,6 +98,7 @@ static int ads1015_read_value(struct i2c
 	if (res < 0)
 		goto err_unlock;
 	for (k = 0; k < 5; ++k) {
+		set_current_state(TASK_UNINTERRUPTIBLE);
 		schedule_timeout(msecs_to_jiffies(1));
 		res = ads1015_read_reg(client, ADS1015_CONFIG);
 		if (res < 0)

If anyone has a problem with this, please speak up.

-- 
Jean Delvare

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH v4] hwmon: Add support for Texas

[Jean Delvare]

Hi Emiliano,

On Tue, 8 Mar 2011 16:36:37 +0100, Emiliano Carnati wrote:
> It make sense...
> 
> Please correct me if I'm wrong. I should:
> 
> - start from Dirk's patch v4

Almost. There is a patch of mine on top of it already, you can get both
patches here:
  ftp://ftp.kernel.org/pub/linux/kernel/people/jdelvare/linux-2.6/jdelvare-hwmon/hwmon-add-support-for-texas-instruments-ads1015.patch
  ftp://ftp.kernel.org/pub/linux/kernel/people/jdelvare/linux-2.6/jdelvare-hwmon/hwmon-ads1015-drop-dynamic-attribute-group.patch
Your own work should go on top of this.

> - create a patch that fixes the bug
> - apply this patch

No longer needed, as I've already merged the fix in the initial patch.

> - create a patch that adds the support for 1115
> 
> It's right?

Yes. Please make sure to run scripts/checkpatch.pl on your patch before
you send it.

Thanks,
-- 
Jean Delvare

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH v4] hwmon: Add support for Texas

[Eibach, Dirk]

> Emiliano, Guenter, Dirk,
> 
> On Wed, 9 Mar 2011 11:05:52 +0100, Emiliano Carnati wrote:
> > > Another question - is it ok to keep the thread state as 
> > > TASK_INTERRUPTIBLE after the wait is complete ?
> > 
> > Well, maybe it's better to put a 
> set_current_state(TASK_RUNNING) after 
> > the wait loop.
> 
> LDD3 doesn't say anything about setting TASK_RUNNING after 
> schedule_timeout(), and looking at other drivers, most don't 
> do it, so I am reasonably certain that we don't have to care.
> 
> OTOH, I believe that TASK_INTERRUPTIBLE is not appropriate 
> here. A received signal would shorten the wait time, meaning 
> that the driver would no longer wait for the maximum time and 
> may thus return an error.
> TASK_UNINTERRUPTIBLE is what we want here, I think, and as a 
> matter of fact this is what the abituguru driver is doing. 
> TASK_INTERRUPTIBLE would only be acceptable if the control 
> loop was time-based rather than count-based.
> 
> The missing set_current_state() in the original driver is a 
> genuine bug, so I'll merge the fix directly in the patch 
> which adds the driver.
> Thanks for noticing and reporting.
> 
> For reference, here is the change I applied:
> 
> --- linux-2.6.38.orig/drivers/hwmon/ads1015.c	2011-03-16 
> 16:49:29.000000000 +0100
> +++ linux-2.6.38/drivers/hwmon/ads1015.c	2011-03-16 
> 16:45:04.000000000 +0100
> @@ -98,6 +98,7 @@ static int ads1015_read_value(struct i2c
>  	if (res < 0)
>  		goto err_unlock;
>  	for (k = 0; k < 5; ++k) {
> +		set_current_state(TASK_UNINTERRUPTIBLE);
>  		schedule_timeout(msecs_to_jiffies(1));
>  		res = ads1015_read_reg(client, ADS1015_CONFIG);
>  		if (res < 0)
> 
> If anyone has a problem with this, please speak up.

In "hwmon: (ads1015) Make gain and datarate configurable" I did

> -		schedule_timeout(msecs_to_jiffies(1));
> +		msleep(k ? 1 : conversion_time_ms);

which should solve this.

Jean, will you merge
hwmon-Add-support-for-Texas-Instruments-ADS1015.patch
hwmon-ads1015-Drop-dynamic-attribute-group.patch
hwmon-ads1015-Add-MAINTAINERS-entry.patch
hwmon-ads1015-Add-devicetree-documentation.patch
hwmon-ads1015-Make-gain-and-datarate-configurable.patch

Grant recently gave his ACK.

Emiliano can then rebase his work on top of those.

Cheers
Dirk
--------------------------------------------------------------------------
Guntermann & Drunck GmbH Systementwicklung 
Dortmunder Str. 4a 
D-57234 Wilnsdorf - Germany 
Tel: +49 (0) 27 39 / 89 01 - 100  Fax: +49 (0) 27 39 / 89 01 - 120 
E-Mail: mailto:sales@gdsys.de Web: www.gdsys.de
--------------------------------------------------------------------------
Geschaeftsfuehrer: 
Udo Guntermann - Martin Drunck - Reiner Ruelmann - Klaus Tocke
HRB 2884, Amtsgericht Siegen - WEEE-Reg.-Nr. DE30763240
USt.-Id.-Nr. DE 126575222 - Steuer-Nr. 342 / 5835 / 1041
--------------------------------------------------------------------------
DQS-zertifiziert nach ISO 9001:2008
--------------------------------------------------------------------------



_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH v4] hwmon: Add support for Texas

[Jean Delvare]

On Thu, 17 Mar 2011 08:24:42 +0100, Eibach, Dirk wrote:
> > LDD3 doesn't say anything about setting TASK_RUNNING after 
> > schedule_timeout(), and looking at other drivers, most don't 
> > do it, so I am reasonably certain that we don't have to care.
> > 
> > OTOH, I believe that TASK_INTERRUPTIBLE is not appropriate 
> > here. A received signal would shorten the wait time, meaning 
> > that the driver would no longer wait for the maximum time and 
> > may thus return an error.
> > TASK_UNINTERRUPTIBLE is what we want here, I think, and as a 
> > matter of fact this is what the abituguru driver is doing. 
> > TASK_INTERRUPTIBLE would only be acceptable if the control 
> > loop was time-based rather than count-based.
> > 
> > The missing set_current_state() in the original driver is a 
> > genuine bug, so I'll merge the fix directly in the patch 
> > which adds the driver.
> > Thanks for noticing and reporting.
> > 
> > For reference, here is the change I applied:
> > 
> > --- linux-2.6.38.orig/drivers/hwmon/ads1015.c	2011-03-16 
> > 16:49:29.000000000 +0100
> > +++ linux-2.6.38/drivers/hwmon/ads1015.c	2011-03-16 
> > 16:45:04.000000000 +0100
> > @@ -98,6 +98,7 @@ static int ads1015_read_value(struct i2c
> >  	if (res < 0)
> >  		goto err_unlock;
> >  	for (k = 0; k < 5; ++k) {
> > +		set_current_state(TASK_UNINTERRUPTIBLE);
> >  		schedule_timeout(msecs_to_jiffies(1));
> >  		res = ads1015_read_reg(client, ADS1015_CONFIG);
> >  		if (res < 0)
> > 
> > If anyone has a problem with this, please speak up.
> 
> In "hwmon: (ads1015) Make gain and datarate configurable" I did
> 
> -		schedule_timeout(msecs_to_jiffies(1));
> +		msleep(k ? 1 : conversion_time_ms);
> 
> which should solve this.

Well, I don't want to commit a patch with a known bug, as it makes
bisecting more difficult. Your patch ("hwmon: (ads1015) Make gain and
datarate configurable") adds a feature, it shouldn't silently fix a bug
too.

If you prefer, I can change the original code to:

	for (k = 0; k < 5; ++k) {
		msleep(1);
		res = ads1015_read_reg(client, ADS1015_CONFIG);

Just let me know.

> Jean, will you merge
> hwmon-Add-support-for-Texas-Instruments-ADS1015.patch
> hwmon-ads1015-Drop-dynamic-attribute-group.patch
> hwmon-ads1015-Add-MAINTAINERS-entry.patch

Already in my tree and ready to be sent to Linus (modulo the possible
change discussed above.)

> hwmon-ads1015-Add-devicetree-documentation.patch

I thought we agreed that this should be merged by whoever is
responsible for this part of the kernel tree, i.e. not me? Also, last
time I looked, this was still work in progress.

> hwmon-ads1015-Make-gain-and-datarate-configurable.patch
> 
> Grant recently gave his ACK.

I'll try to find some time to review this one today. But it may be too
late for kernel 2.6.39.

> Emiliano can then rebase his work on top of those.

Fine with me.

-- 
Jean Delvare

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH v4] hwmon: Add support for Texas

[Eibach, Dirk]

 

> --------------------------------------------------------------------------
Guntermann & Drunck GmbH Systementwicklung 
Dortmunder Str. 4a 
D-57234 Wilnsdorf - Germany 
Tel: +49 (0) 27 39 / 89 01 - 100  Fax: +49 (0) 27 39 / 89 01 - 120 
E-Mail: mailto:sales@gdsys.de Web: www.gdsys.de
--------------------------------------------------------------------------
Geschaeftsfuehrer: 
Udo Guntermann - Martin Drunck - Reiner Ruelmann - Klaus Tocke
HRB 2884, Amtsgericht Siegen - WEEE-Reg.-Nr. DE30763240
USt.-Id.-Nr. DE 126575222 - Steuer-Nr. 342 / 5835 / 1041
--------------------------------------------------------------------------
DQS-zertifiziert nach ISO 9001:2008
--------------------------------------------------------------------------

-----Original Message-----
> From: Jean Delvare [mailto:khali@linux-fr.org] 
> Sent: Thursday, March 17, 2011 10:12 AM
> To: Eibach, Dirk
> Cc: Emiliano Carnati; Guenter Roeck; lm-sensors@lm-sensors.org
> Subject: Re: [lm-sensors] [PATCH v4] hwmon: Add support for 
> Texas Instruments ADS1015
> 
> On Thu, 17 Mar 2011 08:24:42 +0100, Eibach, Dirk wrote:
> > > LDD3 doesn't say anything about setting TASK_RUNNING after 
> > > schedule_timeout(), and looking at other drivers, most 
> don't do it, 
> > > so I am reasonably certain that we don't have to care.
> > > 
> > > OTOH, I believe that TASK_INTERRUPTIBLE is not 
> appropriate here. A 
> > > received signal would shorten the wait time, meaning that 
> the driver 
> > > would no longer wait for the maximum time and may thus return an 
> > > error.
> > > TASK_UNINTERRUPTIBLE is what we want here, I think, and 
> as a matter 
> > > of fact this is what the abituguru driver is doing.
> > > TASK_INTERRUPTIBLE would only be acceptable if the 
> control loop was 
> > > time-based rather than count-based.
> > > 
> > > The missing set_current_state() in the original driver is 
> a genuine 
> > > bug, so I'll merge the fix directly in the patch which adds the 
> > > driver.
> > > Thanks for noticing and reporting.
> > > 
> > > For reference, here is the change I applied:
> > > 
> > > --- linux-2.6.38.orig/drivers/hwmon/ads1015.c	2011-03-16 
> > > 16:49:29.000000000 +0100
> > > +++ linux-2.6.38/drivers/hwmon/ads1015.c	2011-03-16 
> > > 16:45:04.000000000 +0100
> > > @@ -98,6 +98,7 @@ static int ads1015_read_value(struct i2c
> > >  	if (res < 0)
> > >  		goto err_unlock;
> > >  	for (k = 0; k < 5; ++k) {
> > > +		set_current_state(TASK_UNINTERRUPTIBLE);
> > >  		schedule_timeout(msecs_to_jiffies(1));
> > >  		res = ads1015_read_reg(client, ADS1015_CONFIG);
> > >  		if (res < 0)
> > > 
> > > If anyone has a problem with this, please speak up.
> > 
> > In "hwmon: (ads1015) Make gain and datarate configurable" I did
> > 
> > -		schedule_timeout(msecs_to_jiffies(1));
> > +		msleep(k ? 1 : conversion_time_ms);
> > 
> > which should solve this.
> 
> Well, I don't want to commit a patch with a known bug, as it 
> makes bisecting more difficult. Your patch ("hwmon: (ads1015) 
> Make gain and datarate configurable") adds a feature, it 
> shouldn't silently fix a bug too.
> 
> If you prefer, I can change the original code to:
> 
> 	for (k = 0; k < 5; ++k) {
> 		msleep(1);
> 		res = ads1015_read_reg(client, ADS1015_CONFIG);
> 
> Just let me know.

I like msleep because it is more readable.
set_current_state(TASK_UNINTERRUPTIBLE) has some kind of voodoo-flavour
;)

> > Jean, will you merge
> > hwmon-Add-support-for-Texas-Instruments-ADS1015.patch
> > hwmon-ads1015-Drop-dynamic-attribute-group.patch
> > hwmon-ads1015-Add-MAINTAINERS-entry.patch
> 
> Already in my tree and ready to be sent to Linus (modulo the 
> possible change discussed above.)
> 
> > hwmon-ads1015-Add-devicetree-documentation.patch
> 
> I thought we agreed that this should be merged by whoever is 
> responsible for this part of the kernel tree, i.e. not me? 
> Also, last time I looked, this was still work in progress.

I hoped review by the devicetree-discuss folks would be sufficient.

> > hwmon-ads1015-Make-gain-and-datarate-configurable.patch
> > 
> > Grant recently gave his ACK.
> 
> I'll try to find some time to review this one today. But it 
> may be too late for kernel 2.6.39.

A pity. I hoped we could get in the final configuration in the first
shot.

> > Emiliano can then rebase his work on top of those.
> 
> Fine with me.
> 
> --
> Jean Delvare

Cheers
Dirk



_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH v4] hwmon: Add support for Texas

[Jean Delvare]

On Thu, 17 Mar 2011 10:41:41 +0100, Eibach, Dirk wrote:
> > From: Jean Delvare [mailto:khali@linux-fr.org] 
> > Sent: Thursday, March 17, 2011 10:12 AM
> > To: Eibach, Dirk
> > Cc: Emiliano Carnati; Guenter Roeck; lm-sensors@lm-sensors.org
> > Subject: Re: [lm-sensors] [PATCH v4] hwmon: Add support for 
> > Texas Instruments ADS1015
> > 
> > On Thu, 17 Mar 2011 08:24:42 +0100, Eibach, Dirk wrote:
> > > In "hwmon: (ads1015) Make gain and datarate configurable" I did
> > > 
> > > -		schedule_timeout(msecs_to_jiffies(1));
> > > +		msleep(k ? 1 : conversion_time_ms);
> > > 
> > > which should solve this.
> > 
> > Well, I don't want to commit a patch with a known bug, as it 
> > makes bisecting more difficult. Your patch ("hwmon: (ads1015) 
> > Make gain and datarate configurable") adds a feature, it 
> > shouldn't silently fix a bug too.
> > 
> > If you prefer, I can change the original code to:
> > 
> > 	for (k = 0; k < 5; ++k) {
> > 		msleep(1);
> > 		res = ads1015_read_reg(client, ADS1015_CONFIG);
> > 
> > Just let me know.
> 
> I like msleep because it is more readable.
> set_current_state(TASK_UNINTERRUPTIBLE) has some kind of voodoo-flavour
> ;)

OK, changed.

> > > Jean, will you merge
> > > hwmon-Add-support-for-Texas-Instruments-ADS1015.patch
> > > hwmon-ads1015-Drop-dynamic-attribute-group.patch
> > > hwmon-ads1015-Add-MAINTAINERS-entry.patch
> > 
> > Already in my tree and ready to be sent to Linus (modulo the 
> > possible change discussed above.)
> > 
> > > hwmon-ads1015-Add-devicetree-documentation.patch
> > 
> > I thought we agreed that this should be merged by whoever is 
> > responsible for this part of the kernel tree, i.e. not me? 
> > Also, last time I looked, this was still work in progress.
> 
> I hoped review by the devicetree-discuss folks would be sufficient.

I don't really care, I just don't want to step on anyone's toes, nor
give the feeling that we're sneaking things in. I'll ask Grant how we
should proceed.

> > > hwmon-ads1015-Make-gain-and-datarate-configurable.patch
> > > 
> > > Grant recently gave his ACK.
> > 
> > I'll try to find some time to review this one today. But it 
> > may be too late for kernel 2.6.39.
> 
> A pity. I hoped we could get in the final configuration in the first
> shot.

Only code which is in linux-next days before the merge window opens, is
guaranteed to go in.

-- 
Jean Delvare

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH v4] hwmon: Add support for Texas

[Emiliano Carnati]

>
>> > Emiliano can then rebase his work on top of those.
>>
>> Fine with me.
>>


Yes, once Dirk's job is over it will be easy to extend it to the other 
device. 


_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors