Signed-off-by: Dirk Eibach <[email protected]>
---
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 <[email protected]>
+
+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 <[email protected]>
+ *
+ * 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 <[email protected]>");
+MODULE_DESCRIPTION("ADS1015 driver");
+MODULE_LICENSE("GPL");
+
+module_init(sensors_ads1015_init);
+module_exit(sensors_ads1015_exit);
--
1.5.6.5
Hi Dirk,
On Mon, 14 Feb 2011 10:26:21 +0100, Dirk Eibach wrote:
> Signed-off-by: Dirk Eibach <[email protected]>
> ---
> 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 <[email protected]>
> +
> +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 <[email protected]>
> + *
> + * 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 <[email protected]>");
> +MODULE_DESCRIPTION("ADS1015 driver");
> +MODULE_LICENSE("GPL");
> +
> +module_init(sensors_ads1015_init);
> +module_exit(sensors_ads1015_exit);
--
Jean Delvare
Signed-off-by: Dirk Eibach <[email protected]>
---
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 <[email protected]>
+
+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 <[email protected]>
+ *
+ * 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 <[email protected]>");
+MODULE_DESCRIPTION("ADS1015 driver");
+MODULE_LICENSE("GPL");
+
+module_init(sensors_ads1015_init);
+module_exit(sensors_ads1015_exit);
--
1.5.6.5
On Mon, Feb 14, 2011 at 08:21:50AM -0500, Dirk Eibach wrote:
> Signed-off-by: Dirk Eibach <[email protected]>
> ---
> 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 <[email protected]>
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
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 <[email protected]>
> > ---
> > 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 <[email protected]>
>
> 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
Hi Dirk,
On Mon, 14 Feb 2011 14:21:50 +0100, Dirk Eibach wrote:
> Signed-off-by: Dirk Eibach <[email protected]>
> ---
> 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 <[email protected]>
> +
> +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
Signed-off-by: Dirk Eibach <[email protected]>
---
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 <[email protected]>
+
+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 <[email protected]>
+ *
+ * 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 <[email protected]>");
+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 <[email protected]>
+ *
+ * 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
Hi Dirk,
Sorry for the late reply.
On Fri, 18 Feb 2011 11:15:58 +0100, Dirk Eibach wrote:
> Signed-off-by: Dirk Eibach <[email protected]>
> ---
> 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 <[email protected]>
> +
> +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 <[email protected]>
> + *
> + * 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 <[email protected]>");
> +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 <[email protected]>
> + *
> + * 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
Signed-off-by: Dirk Eibach <[email protected]>
---
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 <[email protected]>
+
+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 <[email protected]>
+ *
+ * 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 <[email protected]>");
+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 <[email protected]>
+ *
+ * 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
Hi Dirk,
On Fri, 25 Feb 2011 14:18:17 +0100, Dirk Eibach wrote:
> Signed-off-by: Dirk Eibach <[email protected]>
> ---
> 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
> > +#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/ |
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: [email protected] - Web: http://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
------------------------------------------------------------------------------------------------