[lm-sensors] [PATCH] hwmon: Add MCP3021 hardware monitoring driver

[lm-sensors] [PATCH] hwmon: Add MCP3021 hardware monitoring driver

[Xie Xiaobo]

Add I2C driver for MCP3021 that is an ADC chip from Microchip.
On the MPC8536DS board, there are two MCP3021 chips to monitor the
core and platform current individually. The current is converted
to voltage through a current shunt monitor(INA196) and the voltage
then connected to MCP3021's analog input through a voltage-divider
network. On the board, the relationship between the core/platform
current and the MCP3021's input voltage is as follows:
Icore = 6.0 * Vin
Iplat = 5.5 * Vin
The driver export the value of Vin to sysfs, the voltage unit is
mV. Through the sysfs interface, lm-sensors tool can also display
Vin voltage.

Signed-off-by: Mingkai Hu <Mingkai.hu@freescale.com>
Signed-off-by: Xie Xiaobo <X.Xie@freescale.com>
---
 Documentation/hwmon/mcp3021 |   20 ++++
 drivers/hwmon/Kconfig       |   12 +++
 drivers/hwmon/Makefile      |    1 +
 drivers/hwmon/mcp3021.c     |  223 +++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 256 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/hwmon/mcp3021
 create mode 100644 drivers/hwmon/mcp3021.c

diff --git a/Documentation/hwmon/mcp3021 b/Documentation/hwmon/mcp3021
new file mode 100644
index 0000000..e5d67d7
--- /dev/null
+++ b/Documentation/hwmon/mcp3021
@@ -0,0 +1,20 @@
+Kernel driver MCP3021
+===========
+
+Supported chips:
+  * Microchip Technology MCP3021
+    Prefix: 'mcp3021'
+    Addresses scanned: I2C 0x4d
+    Datasheet: http://ww1.microchip.com/downloads/en/DeviceDoc/21805a.pdf
+
+Author: Mingkai Hu
+
+Description
+-----------
+
+This driver implements support for the Microchip Technology MCP3021 chip.
+
+The Microchip Technology Inc. MCP3021 is a successive
+approximation A/D converter (ADC) with 10-bit resolution.
+This device provides one single-ended input with very low
+power consumption.
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 91be41f..5af42db 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1360,6 +1360,18 @@ config SENSORS_MC13783_ADC
         help
           Support for the A/D converter on MC13783 PMIC.
 
+config SENSORS_MCP3021
+	tristate "Microchip MCP3021"
+	depends on I2C
+	select HWMON_VID
+	default n
+	help
+	  If you say yes here you get support for the hardware
+	  monitoring functionality of the MCP3021 chip.
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called mcp3021.
+
 if ACPI
 
 comment "ACPI drivers"
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index 8251ce8..3912557 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -125,6 +125,7 @@ obj-$(CONFIG_SENSORS_W83L785TS)	+= w83l785ts.o
 obj-$(CONFIG_SENSORS_W83L786NG)	+= w83l786ng.o
 obj-$(CONFIG_SENSORS_WM831X)	+= wm831x-hwmon.o
 obj-$(CONFIG_SENSORS_WM8350)	+= wm8350-hwmon.o
+obj-$(CONFIG_SENSORS_MCP3021)	+= mcp3021.o
 
 obj-$(CONFIG_PMBUS)		+= pmbus/
 
diff --git a/drivers/hwmon/mcp3021.c b/drivers/hwmon/mcp3021.c
new file mode 100644
index 0000000..e2ebb8c
--- /dev/null
+++ b/drivers/hwmon/mcp3021.c
@@ -0,0 +1,223 @@
+/*
+ * mcp3021.c - driver for the Microchip MCP3021 chip
+ *
+ * Copyright (C) 2008-2009, 2011 Freescale Semiconductor, Inc.
+ * Author: Mingkai Hu <Mingkai.hu@freescale.com>
+ *
+ * This driver export the value of analog input voltage to sysfs, the
+ * voltage unit is mV. Through the sysfs interface, lm-sensors tool
+ * can also display the input voltage.
+ *
+ * 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.
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/jiffies.h>
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
+#include <linux/slab.h>
+#include <linux/i2c.h>
+#include <linux/err.h>
+#include <linux/mutex.h>
+#include <linux/device.h>
+
+/*
+ * MCP3021 macros
+ */
+/* Vdd info */
+#define MCP3021_VDD_MAX		5500
+#define MCP3021_VDD_MIN		2700
+#define MCP3021_VDD_REF		3300
+
+/* output format */
+#define MCP3021_SAR_SHIFT	2
+#define MCP3021_SAR_MASK	0x3ff
+
+#define MCP3021_OUTPUT_RES	10	/* 10-bit resolution */
+#define MCP3021_OUTPUT_SCALE	4
+#define MCP3021_OUTPUT_MAX_VAL	((1 << MCP3021_OUTPUT_RES) - 1)
+
+/*
+ * Client data (each client gets its own)
+ */
+struct mcp3021_data {
+	struct device *hwmon_dev;
+	struct mutex update_lock;	/* protect register access */
+
+	/* Register values */
+	u16 in_input;
+};
+
+/*
+ * Driver data (common to all clients)
+ */
+static int mcp3021_probe(struct i2c_client *client,
+				const struct i2c_device_id *id);
+static int mcp3021_remove(struct i2c_client *client);
+
+enum chips { mcp3021 };
+
+static int mcp3021_read16(struct i2c_client *client, u16 *data)
+{
+	int ret;
+	int reg;
+	uint8_t buf[2];
+
+	ret = i2c_master_recv(client, buf, 2);
+	if (ret != 2)
+		return ret;
+
+	/* The output code of the MCP3021 is transmitted with MSB first. */
+	reg = (buf[0] << 8) | buf[1];
+
+	/* The ten-bit output code is composed of the lower 4-bit of the
+	 * first byte and the upper 6-bit of the second byte.
+	 */
+	reg = (reg >> MCP3021_SAR_SHIFT) & MCP3021_SAR_MASK;
+	*data = reg;
+
+	return 0;
+}
+
+/*
+ * The basic transfer function is volts = vdd * val / 1024, except
+ * that at val's limits (0 and 1023), val should have 0.25 added to it.
+ * We scale val by a factor of four so we can add one instead of 0.25.
+ */
+static inline u16 volts_from_reg(u16 vdd, u16 val)
+{
+	val = val * MCP3021_OUTPUT_SCALE;
+
+	if ((val = 0) ||
+	    (val = MCP3021_OUTPUT_MAX_VAL * MCP3021_OUTPUT_SCALE))
+		val++;
+
+	return  val * vdd / ((1 << MCP3021_OUTPUT_RES)
+			* MCP3021_OUTPUT_SCALE);
+}
+
+static struct mcp3021_data *mcp3021_update_device(struct device *dev)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct mcp3021_data *data = i2c_get_clientdata(client);
+	u16 vdd = MCP3021_VDD_REF;
+
+	mutex_lock(&data->update_lock);
+	mcp3021_read16(client, &data->in_input);
+	data->in_input = volts_from_reg(vdd, data->in_input);
+	mutex_unlock(&data->update_lock);
+
+	return data;
+}
+
+static ssize_t show_in_input(struct device *dev, struct device_attribute *attr,
+		char *buf)
+{
+	struct mcp3021_data *data = mcp3021_update_device(dev);
+	return sprintf(buf, "%d\n", data->in_input);
+}
+
+static DEVICE_ATTR(in0_input, S_IRUGO, show_in_input, NULL);
+
+static struct attribute *mcp3021_attributes[] = {
+	&dev_attr_in0_input.attr,
+	NULL
+};
+
+static const struct attribute_group mcp3021_group = {
+	.attrs = mcp3021_attributes,
+};
+
+static int mcp3021_check_status(struct i2c_client *client)
+{
+	u16 reg;
+	return  mcp3021_read16(client, &reg);
+}
+
+static int mcp3021_probe(struct i2c_client *client,
+				const struct i2c_device_id *id)
+{
+	int err;
+	struct mcp3021_data *data = NULL;
+
+	if (mcp3021_check_status(client))
+		return -ENXIO;
+	if (!i2c_check_functionality(client->adapter,
+			I2C_FUNC_SMBUS_WORD_DATA))
+		return -EIO;
+
+	data = kzalloc(sizeof(struct mcp3021_data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	i2c_set_clientdata(client, data);
+
+	mutex_init(&data->update_lock);
+
+	err = sysfs_create_group(&client->dev.kobj, &mcp3021_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, &mcp3021_group);
+exit_free:
+	kfree(data);
+	i2c_set_clientdata(client, NULL);
+	return err;
+}
+
+static int mcp3021_remove(struct i2c_client *client)
+{
+	struct mcp3021_data *data = i2c_get_clientdata(client);
+
+	hwmon_device_unregister(data->hwmon_dev);
+	sysfs_remove_group(&client->dev.kobj, &mcp3021_group);
+	kfree(data);
+	i2c_set_clientdata(client, NULL);
+
+	return 0;
+}
+
+static const struct i2c_device_id mcp3021_id[] = {
+	{ "mcp3021", mcp3021 },
+	{ }
+};
+
+static struct i2c_driver mcp3021_driver = {
+	.driver = {
+		.name = "mcp3021",
+	},
+	.probe = mcp3021_probe,
+	.remove = mcp3021_remove,
+	.id_table = mcp3021_id,
+};
+
+static int __init mcp3021_init(void)
+{
+	return i2c_add_driver(&mcp3021_driver);
+}
+
+static void __exit mcp3021_exit(void)
+{
+	i2c_del_driver(&mcp3021_driver);
+}
+
+MODULE_AUTHOR("Mingkai Hu <Mingkai.hu@freescale.com>");
+MODULE_DESCRIPTION("Microchip MCP3021 driver");
+MODULE_LICENSE("GPL");
+
+module_init(mcp3021_init);
+module_exit(mcp3021_exit);
-- 
1.7.5.1



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

Re: [lm-sensors] [PATCH] hwmon: Add MCP3021 hardware monitoring

[Guenter Roeck]

Hi,

On Tue, Dec 27, 2011 at 04:51:02AM -0500, Xie Xiaobo wrote:
> Add I2C driver for MCP3021 that is an ADC chip from Microchip.
> On the MPC8536DS board, there are two MCP3021 chips to monitor the
> core and platform current individually. The current is converted
> to voltage through a current shunt monitor(INA196) and the voltage
> then connected to MCP3021's analog input through a voltage-divider
> network. On the board, the relationship between the core/platform
> current and the MCP3021's input voltage is as follows:
> Icore = 6.0 * Vin
> Iplat = 5.5 * Vin
> The driver export the value of Vin to sysfs, the voltage unit is
> mV. Through the sysfs interface, lm-sensors tool can also display
> Vin voltage.
> 
Please refrain from adding board details to the driver commit log.

> Signed-off-by: Mingkai Hu <Mingkai.hu@freescale.com>
> Signed-off-by: Xie Xiaobo <X.Xie@freescale.com>
> ---
>  Documentation/hwmon/mcp3021 |   20 ++++
>  drivers/hwmon/Kconfig       |   12 +++
>  drivers/hwmon/Makefile      |    1 +
>  drivers/hwmon/mcp3021.c     |  223 +++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 256 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/hwmon/mcp3021
>  create mode 100644 drivers/hwmon/mcp3021.c
> 
> diff --git a/Documentation/hwmon/mcp3021 b/Documentation/hwmon/mcp3021
> new file mode 100644
> index 0000000..e5d67d7
> --- /dev/null
> +++ b/Documentation/hwmon/mcp3021
> @@ -0,0 +1,20 @@
> +Kernel driver MCP3021
> +===========
> +
> +Supported chips:
> +  * Microchip Technology MCP3021
> +    Prefix: 'mcp3021'
> +    Addresses scanned: I2C 0x4d

There is no _detect function, nor has the chip any means to detect it.
So there are no addresses to scan. Besides, the chip supports other addresses
as well.

> +    Datasheet: http://ww1.microchip.com/downloads/en/DeviceDoc/21805a.pdf
> +
> +Author: Mingkai Hu
> +
> +Description
> +-----------
> +
> +This driver implements support for the Microchip Technology MCP3021 chip.
> +
> +The Microchip Technology Inc. MCP3021 is a successive
> +approximation A/D converter (ADC) with 10-bit resolution.
> +This device provides one single-ended input with very low
> +power consumption.
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 91be41f..5af42db 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -1360,6 +1360,18 @@ config SENSORS_MC13783_ADC
>          help
>            Support for the A/D converter on MC13783 PMIC.
>  
> +config SENSORS_MCP3021
> +	tristate "Microchip MCP3021"
> +	depends on I2C

Should also depend on EXPERIMENTAL.

> +	select HWMON_VID

Why do you select HWMON_VID ? I do not see a reason for doing it, as the chip
does not support any VID lines.

> +	default n
> +	help
> +	  If you say yes here you get support for the hardware
> +	  monitoring functionality of the MCP3021 chip.
> +
> +	  This driver can also be built as a module.  If so, the module
> +	  will be called mcp3021.
> +
>  if ACPI
>  
>  comment "ACPI drivers"
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index 8251ce8..3912557 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -125,6 +125,7 @@ obj-$(CONFIG_SENSORS_W83L785TS)	+= w83l785ts.o
>  obj-$(CONFIG_SENSORS_W83L786NG)	+= w83l786ng.o
>  obj-$(CONFIG_SENSORS_WM831X)	+= wm831x-hwmon.o
>  obj-$(CONFIG_SENSORS_WM8350)	+= wm8350-hwmon.o
> +obj-$(CONFIG_SENSORS_MCP3021)	+= mcp3021.o
>  
Please retain alphabetical order.

>  obj-$(CONFIG_PMBUS)		+= pmbus/
>  
> diff --git a/drivers/hwmon/mcp3021.c b/drivers/hwmon/mcp3021.c
> new file mode 100644
> index 0000000..e2ebb8c
> --- /dev/null
> +++ b/drivers/hwmon/mcp3021.c
> @@ -0,0 +1,223 @@
> +/*
> + * mcp3021.c - driver for the Microchip MCP3021 chip
> + *
> + * Copyright (C) 2008-2009, 2011 Freescale Semiconductor, Inc.
> + * Author: Mingkai Hu <Mingkai.hu@freescale.com>
> + *
> + * This driver export the value of analog input voltage to sysfs, the
> + * voltage unit is mV. Through the sysfs interface, lm-sensors tool
> + * can also display the input voltage.
> + *
> + * 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.
> + *
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/jiffies.h>
> +#include <linux/hwmon.h>
> +#include <linux/hwmon-sysfs.h>
> +#include <linux/slab.h>
> +#include <linux/i2c.h>
> +#include <linux/err.h>
> +#include <linux/mutex.h>
> +#include <linux/device.h>
> +
> +/*
> + * MCP3021 macros
> + */
> +/* Vdd info */
> +#define MCP3021_VDD_MAX		5500
> +#define MCP3021_VDD_MIN		2700

The above defines are not used and thus unnecessary (unless used for VDD
validation - see below).

> +#define MCP3021_VDD_REF		3300
> +
> +/* output format */
> +#define MCP3021_SAR_SHIFT	2
> +#define MCP3021_SAR_MASK	0x3ff
> +
> +#define MCP3021_OUTPUT_RES	10	/* 10-bit resolution */
> +#define MCP3021_OUTPUT_SCALE	4
> +#define MCP3021_OUTPUT_MAX_VAL	((1 << MCP3021_OUTPUT_RES) - 1)
> +
> +/*
> + * Client data (each client gets its own)
> + */
> +struct mcp3021_data {
> +	struct device *hwmon_dev;
> +	struct mutex update_lock;	/* protect register access */
> +
> +	/* Register values */
> +	u16 in_input;

update_lock and in_input are unnecessary. Please see below.

Consider adding vdd, to be passed as platform data.

> +};
> +
> +/*
> + * Driver data (common to all clients)
> + */
> +static int mcp3021_probe(struct i2c_client *client,
> +				const struct i2c_device_id *id);
> +static int mcp3021_remove(struct i2c_client *client);
> +

Please no forward declarations. Rearrange the code to avoid it.

> +enum chips { mcp3021 };

The driver only supports a single chip. While the define is used to initialize mcp3021_id,
the value is never used, and the enum thus unnecessary.

> +
> +static int mcp3021_read16(struct i2c_client *client, u16 *data)
> +{

There is no need to return the value in a u16 * and an error code 
as return value. Please merge as done in other drivers to simplify the code.

> +	int ret;
> +	int reg;
> +	uint8_t buf[2];
> +
> +	ret = i2c_master_recv(client, buf, 2);
> +	if (ret != 2)
> +		return ret;
> +
With this also have positive error return values, which makes the code much
more difficult to understand, and doesn't even cover the case where 
i2c_master_recv() for unexplicable reasons returns 0. If you don't trust
i2c_master_recv() to return 2 or an error, please at least make it clean.

	ret = i2c_master_recv(client, buf, 2);
	if (ret < 0)
		return ret;
	if (ret != 2)
		return -ENXIO;
	...
	return reg;

> +	/* The output code of the MCP3021 is transmitted with MSB first. */
> +	reg = (buf[0] << 8) | buf[1];
> +
> +	/* The ten-bit output code is composed of the lower 4-bit of the
> +	 * first byte and the upper 6-bit of the second byte.
> +	 */
> +	reg = (reg >> MCP3021_SAR_SHIFT) & MCP3021_SAR_MASK;
> +	*data = reg;
> +
> +	return 0;
> +}
> +
> +/*
> + * The basic transfer function is volts = vdd * val / 1024, except
> + * that at val's limits (0 and 1023), val should have 0.25 added to it.
> + * We scale val by a factor of four so we can add one instead of 0.25.

Please provide a pointer to the datasheet mentioning this requirement.
I don't find it, and it does not make much sense to me since it results 
in never being able to measure 0.

Per datasheet, a measured value of 0 means that the measured voltage
is below 0.5 * LSB, which should return 0, not arbitrarily 0.25 * LSB.
The same applies for the maximum measurement of 1023; all you know
is that the measured voltage is above VDD - 1.5*LSB, and you should
not arbitrarily add 0.25 * LSB to the result.

Besides, this makes the code more complex for no or very little gain.

> + */
> +static inline u16 volts_from_reg(u16 vdd, u16 val)
> +{
> +	val = val * MCP3021_OUTPUT_SCALE;
> +
> +	if ((val = 0) ||
> +	    (val = MCP3021_OUTPUT_MAX_VAL * MCP3021_OUTPUT_SCALE))
> +		val++;
> +
Unnecessary extra ( ).

> +	return  val * vdd / ((1 << MCP3021_OUTPUT_RES)

Unnecessary extra ' '.

> +			* MCP3021_OUTPUT_SCALE);
> +}
> +
> +static struct mcp3021_data *mcp3021_update_device(struct device *dev)
> +{
> +	struct i2c_client *client = to_i2c_client(dev);
> +	struct mcp3021_data *data = i2c_get_clientdata(client);
> +	u16 vdd = MCP3021_VDD_REF;
> +
vdd is only used to pass a constant to volts_from_reg(). This is unnecessary.

On the other side, VDD is platform dependent. Consider adding platform data
to provide it, and store it in data->vdd or similar. You can still initialize
data->vdd with MCP3021_VDD_REF if there is no platform data (and actually use
of MCP3021_VDD_MAX and MCP3021_VDD_MIN to validate the platform data range).

> +	mutex_lock(&data->update_lock);
> +	mcp3021_read16(client, &data->in_input);
> +	data->in_input = volts_from_reg(vdd, data->in_input);
> +	mutex_unlock(&data->update_lock);
> +
> +	return data;
> +}

This function, and storing the value in in_input, is unnecessary.
It would only make sense if the value was cached, which it isn't. 
data->update_lock and data->in_input are thus unnecessary. Just merge
the code into show_in_input().

Also, one variable, one object, please. As currently written, you managed
to introduce a race condition on data->in_input, making the lock quite useless.

> +
> +static ssize_t show_in_input(struct device *dev, struct device_attribute *attr,
> +		char *buf)
> +{
> +	struct mcp3021_data *data = mcp3021_update_device(dev);
> +	return sprintf(buf, "%d\n", data->in_input);
> +}
> +
> +static DEVICE_ATTR(in0_input, S_IRUGO, show_in_input, NULL);
> +
> +static struct attribute *mcp3021_attributes[] = {
> +	&dev_attr_in0_input.attr,
> +	NULL
> +};
> +
> +static const struct attribute_group mcp3021_group = {
> +	.attrs = mcp3021_attributes,
> +};
> +
> +static int mcp3021_check_status(struct i2c_client *client)
> +{
> +	u16 reg;
> +	return  mcp3021_read16(client, &reg);

Unnecessary extra ' '.

> +}
> +
> +static int mcp3021_probe(struct i2c_client *client,
> +				const struct i2c_device_id *id)
> +{
> +	int err;
> +	struct mcp3021_data *data = NULL;
> +
> +	if (mcp3021_check_status(client))
> +		return -ENXIO;

	-ENODEV, please.

> +	if (!i2c_check_functionality(client->adapter,
> +			I2C_FUNC_SMBUS_WORD_DATA))
> +		return -EIO;
> +
	Why -EIO instead of -ENODEV, and why do you check for support of
	I2C_FUNC_SMBUS_WORD_DATA anyway ? You don't call any SMBUS functions.

> +	data = kzalloc(sizeof(struct mcp3021_data), GFP_KERNEL);
> +	if (!data)
> +		return -ENOMEM;
> +
> +	i2c_set_clientdata(client, data);
> +
> +	mutex_init(&data->update_lock);
> +
> +	err = sysfs_create_group(&client->dev.kobj, &mcp3021_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, &mcp3021_group);
> +exit_free:
> +	kfree(data);
> +	i2c_set_clientdata(client, NULL);

Unnecessary.

> +	return err;
> +}
> +
> +static int mcp3021_remove(struct i2c_client *client)
> +{
> +	struct mcp3021_data *data = i2c_get_clientdata(client);
> +
> +	hwmon_device_unregister(data->hwmon_dev);
> +	sysfs_remove_group(&client->dev.kobj, &mcp3021_group);
> +	kfree(data);
> +	i2c_set_clientdata(client, NULL);
> +
Unnecessary.

> +	return 0;
> +}
> +
> +static const struct i2c_device_id mcp3021_id[] = {
> +	{ "mcp3021", mcp3021 },
> +	{ }
> +};
> +
> +static struct i2c_driver mcp3021_driver = {
> +	.driver = {
> +		.name = "mcp3021",
> +	},
> +	.probe = mcp3021_probe,
> +	.remove = mcp3021_remove,
> +	.id_table = mcp3021_id,
> +};
> +
> +static int __init mcp3021_init(void)
> +{
> +	return i2c_add_driver(&mcp3021_driver);
> +}
> +
> +static void __exit mcp3021_exit(void)
> +{
> +	i2c_del_driver(&mcp3021_driver);
> +}
> +
> +MODULE_AUTHOR("Mingkai Hu <Mingkai.hu@freescale.com>");
> +MODULE_DESCRIPTION("Microchip MCP3021 driver");
> +MODULE_LICENSE("GPL");
> +
> +module_init(mcp3021_init);
> +module_exit(mcp3021_exit);
> -- 
> 1.7.5.1
> 
> 

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