Re: [RFC,PATCH v1] hwmon: ADC124S501 generic driver

Re: [RFC,PATCH v1] hwmon: ADC124S501 generic driver

[Tobias Himmer]

Hi Marc,

I just wrote a similar driver for the ADC088S052.
Yesterday I noticed your patch and successfully tested a slightly
modified version of it that supports 8 channels.
The patch below goes on top of yours.

Bye

Tobias

Signed-off-by: Tobias Himmer <tobias@himmer-online.de>
---

Index: linux-2.6.26-rc/drivers/hwmon/adcxx.c
===================================================================
--- linux-2.6.26-rc.orig/drivers/hwmon/adcxx.c
+++ linux-2.6.26-rc/drivers/hwmon/adcxx.c
@@ -5,13 +5,15 @@
  *
  * Copyright (c) 2008 Marc Pignat <marc.pignat@hevs.ch>
  *
+ * 8 channel support added by Tobias Himmer <tobias@himmer-online.de>
+ *
  * The adcxx4s communicates with a host processor via an SPI/Microwire Bus
  * interface. This driver supports the whole family of devices with name
  * ADC<bb><c>S<sss>, where
  * * bb is the resolution in number of bits (8, 10, 12)
- * * c is the number of channels (1, 2, 4)
- * * sss is the maximum conversion speed (021 for 200 kSPS, 051 for 500 kSPS
- *   and 101 for 1 MSPS)
+ * * c is the number of channels (1, 2, 4, 8)
+ * * sss is the maximum conversion speed (021 for 200 kSPS, 051 / 052 for
+ *   500 kSPS and 101 for 1 MSPS)
  *
  * Complete datasheets are available at National's website here:
  * http://www.national.com/ds/DC/ADC<bb><c>S<sss>.pdf
@@ -155,6 +157,10 @@ static struct sensor_device_attribute ad
 	SENSOR_ATTR(in1_input, S_IRUGO, adcxx_read, NULL, 1),
 	SENSOR_ATTR(in2_input, S_IRUGO, adcxx_read, NULL, 2),
 	SENSOR_ATTR(in3_input, S_IRUGO, adcxx_read, NULL, 3),
+	SENSOR_ATTR(in4_input, S_IRUGO, adcxx_read, NULL, 4),
+	SENSOR_ATTR(in5_input, S_IRUGO, adcxx_read, NULL, 5),
+	SENSOR_ATTR(in6_input, S_IRUGO, adcxx_read, NULL, 6),
+	SENSOR_ATTR(in7_input, S_IRUGO, adcxx_read, NULL, 7),
};
 
 /*----------------------------------------------------------------------*/
@@ -218,6 +224,11 @@ static int __devinit adcxx4s_probe(struc
 	return adcxx_probe(spi, 4);
}
 
+static int __devinit adcxx8s_probe(struct spi_device *spi)
+{
+	return adcxx_probe(spi, 8);
+}
+
 static int __devexit adcxx_remove(struct spi_device *spi)
{
 	struct adcxx *adc = dev_get_drvdata(&spi->dev);
@@ -259,6 +270,15 @@ static struct spi_driver adcxx4s_driver 
 	.remove	= __devexit_p(adcxx_remove),
};
 
+static struct spi_driver adcxx8s_driver = {
+	.driver = {
+		.name	= "adcxx8s",
+		.owner	= THIS_MODULE,
+	},
+	.probe	= adcxx8s_probe,
+	.remove	= __devexit_p(adcxx_remove),
+};
+
 static int __init init_adcxx(void)
{
 	int status;
@@ -270,7 +290,11 @@ static int __init init_adcxx(void)
 	if (status)
 		return status;
 
-	return spi_register_driver(&adcxx4s_driver);
+	status = spi_register_driver(&adcxx4s_driver);
+	if (status)
+		return status;
+
+	return spi_register_driver(&adcxx8s_driver);
}
 
 static void __exit exit_adcxx(void)
@@ -278,6 +302,7 @@ static void __exit exit_adcxx(void)
 	spi_unregister_driver(&adcxx1s_driver);
 	spi_unregister_driver(&adcxx2s_driver);
 	spi_unregister_driver(&adcxx4s_driver);
+	spi_unregister_driver(&adcxx8s_driver);
}
 
 module_init(init_adcxx);
@@ -290,3 +315,4 @@ MODULE_LICENSE("GPL");
 MODULE_ALIAS("adcxx1s");
 MODULE_ALIAS("adcxx2s");
 MODULE_ALIAS("adcxx4s");
+MODULE_ALIAS("adcxx8s");
Index: linux-2.6.26-rc/drivers/hwmon/Kconfig
===================================================================
--- linux-2.6.26-rc.orig/drivers/hwmon/Kconfig
+++ linux-2.6.26-rc/drivers/hwmon/Kconfig
@@ -397,9 +397,9 @@ config SENSORS_ADCXX
 	  If you say yes here you get support for the National Semiconductor
 	  ADC<bb><c>S<sss> chip family, where
 	  * bb  is the resolution in number of bits (8, 10, 12)
-	  * c   is the number of channels (1, 2, 4)
-	  * sss is the maximum conversion speed (021 for 200 kSPS, 051 for 500
-	    kSPS and 101 for 1 MSPS)
+	  * c   is the number of channels (1, 2, 4, 8)
+	  * sss is the maximum conversion speed (021 for 200 kSPS, 051 / 052 for
+	    500 kSPS and 101 for 1 MSPS)
 
 	  Examples : ADC081S101, ADC124S501, ...
 

Re: [RFC,PATCH v1] hwmon: ADC124S501 generic driver

[Hans J. Koch]

On Thu, Jul 24, 2008 at 11:51:50AM +0200, Tobias Himmer wrote:
> Hi Marc,
> 
> I just wrote a similar driver for the ADC088S052.

Bad luck ;-)

> Yesterday I noticed your patch and successfully tested a slightly
> modified version of it that supports 8 channels.
> The patch below goes on top of yours.

Nice, both patches combined in mainline would be nice. Works for me.

Thanks,
Hans

> 
> Bye
> 
> Tobias
> 
> Signed-off-by: Tobias Himmer <tobias@himmer-online.de>

Acked-by: Hans J. Koch <hjk@linutronix.de>

> ---
> 
> Index: linux-2.6.26-rc/drivers/hwmon/adcxx.c
> ===================================================================
> --- linux-2.6.26-rc.orig/drivers/hwmon/adcxx.c
> +++ linux-2.6.26-rc/drivers/hwmon/adcxx.c
> @@ -5,13 +5,15 @@
>   *
>   * Copyright (c) 2008 Marc Pignat <marc.pignat@hevs.ch>
>   *
> + * 8 channel support added by Tobias Himmer <tobias@himmer-online.de>
> + *
>   * The adcxx4s communicates with a host processor via an SPI/Microwire Bus
>   * interface. This driver supports the whole family of devices with name
>   * ADC<bb><c>S<sss>, where
>   * * bb is the resolution in number of bits (8, 10, 12)
> - * * c is the number of channels (1, 2, 4)
> - * * sss is the maximum conversion speed (021 for 200 kSPS, 051 for 500 kSPS
> - *   and 101 for 1 MSPS)
> + * * c is the number of channels (1, 2, 4, 8)
> + * * sss is the maximum conversion speed (021 for 200 kSPS, 051 / 052 for
> + *   500 kSPS and 101 for 1 MSPS)
>   *
>   * Complete datasheets are available at National's website here:
>   * http://www.national.com/ds/DC/ADC<bb><c>S<sss>.pdf
> @@ -155,6 +157,10 @@ static struct sensor_device_attribute ad
>  	SENSOR_ATTR(in1_input, S_IRUGO, adcxx_read, NULL, 1),
>  	SENSOR_ATTR(in2_input, S_IRUGO, adcxx_read, NULL, 2),
>  	SENSOR_ATTR(in3_input, S_IRUGO, adcxx_read, NULL, 3),
> +	SENSOR_ATTR(in4_input, S_IRUGO, adcxx_read, NULL, 4),
> +	SENSOR_ATTR(in5_input, S_IRUGO, adcxx_read, NULL, 5),
> +	SENSOR_ATTR(in6_input, S_IRUGO, adcxx_read, NULL, 6),
> +	SENSOR_ATTR(in7_input, S_IRUGO, adcxx_read, NULL, 7),
> };
>  
>  /*----------------------------------------------------------------------*/
> @@ -218,6 +224,11 @@ static int __devinit adcxx4s_probe(struc
>  	return adcxx_probe(spi, 4);
> }
>  
> +static int __devinit adcxx8s_probe(struct spi_device *spi)
> +{
> +	return adcxx_probe(spi, 8);
> +}
> +
>  static int __devexit adcxx_remove(struct spi_device *spi)
> {
>  	struct adcxx *adc = dev_get_drvdata(&spi->dev);
> @@ -259,6 +270,15 @@ static struct spi_driver adcxx4s_driver 
>  	.remove	= __devexit_p(adcxx_remove),
> };
>  
> +static struct spi_driver adcxx8s_driver = {
> +	.driver = {
> +		.name	= "adcxx8s",
> +		.owner	= THIS_MODULE,
> +	},
> +	.probe	= adcxx8s_probe,
> +	.remove	= __devexit_p(adcxx_remove),
> +};
> +
>  static int __init init_adcxx(void)
> {
>  	int status;
> @@ -270,7 +290,11 @@ static int __init init_adcxx(void)
>  	if (status)
>  		return status;
>  
> -	return spi_register_driver(&adcxx4s_driver);
> +	status = spi_register_driver(&adcxx4s_driver);
> +	if (status)
> +		return status;
> +
> +	return spi_register_driver(&adcxx8s_driver);
> }
>  
>  static void __exit exit_adcxx(void)
> @@ -278,6 +302,7 @@ static void __exit exit_adcxx(void)
>  	spi_unregister_driver(&adcxx1s_driver);
>  	spi_unregister_driver(&adcxx2s_driver);
>  	spi_unregister_driver(&adcxx4s_driver);
> +	spi_unregister_driver(&adcxx8s_driver);
> }
>  
>  module_init(init_adcxx);
> @@ -290,3 +315,4 @@ MODULE_LICENSE("GPL");
>  MODULE_ALIAS("adcxx1s");
>  MODULE_ALIAS("adcxx2s");
>  MODULE_ALIAS("adcxx4s");
> +MODULE_ALIAS("adcxx8s");
> Index: linux-2.6.26-rc/drivers/hwmon/Kconfig
> ===================================================================
> --- linux-2.6.26-rc.orig/drivers/hwmon/Kconfig
> +++ linux-2.6.26-rc/drivers/hwmon/Kconfig
> @@ -397,9 +397,9 @@ config SENSORS_ADCXX
>  	  If you say yes here you get support for the National Semiconductor
>  	  ADC<bb><c>S<sss> chip family, where
>  	  * bb  is the resolution in number of bits (8, 10, 12)
> -	  * c   is the number of channels (1, 2, 4)
> -	  * sss is the maximum conversion speed (021 for 200 kSPS, 051 for 500
> -	    kSPS and 101 for 1 MSPS)
> +	  * c   is the number of channels (1, 2, 4, 8)
> +	  * sss is the maximum conversion speed (021 for 200 kSPS, 051 / 052 for
> +	    500 kSPS and 101 for 1 MSPS)
>  
>  	  Examples : ADC081S101, ADC124S501, ...
>  
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC,PATCH v1] hwmon: ADC124S501 generic driver

[Andrew Morton]

On Wed, 23 Jul 2008 13:07:54 +0200 Marc Pignat <marc.pignat@hevs.ch> wrote:

> SPI driver for analog to digital converters national semiconductor ADC081S101,
> ADC124S501, ...
> 
> Signed-off-by: Marc Pignat <marc.pignat@hevs.ch>
> 
> Hi all!
> 
> patch against 2.6.26, tested on a custom arm board and only compil-tested on
> x86.
> 
> This driver add support for National Semiconductor ADC<bb><c>S<sss> chip family,
> where:
>  * bb  is the resolution in number of bits (8, 10, 12)
>  * c   is the number of channels (1, 2, 4)
>  * sss is the maximum conversion speed (021 for 200 kSPS, 051 for 500
>    kSPS and 101 for 1 MSPS)
> 
> Some remarks:
>  * The chip type (-> the number of inputs) are determined by the module alias,
>    is it a good idea? it could be implemented using platform data.
> 
>  * The Makefile seems ordered alphabetically, what order should be used for
>    the Konfig file?
> 

That changelog is a bit of a mess.  I cleaned up up as:

SPI driver for analog to digital converters national semiconductor
ADC081S101, ADC124S501, ...

This driver adds support for National Semiconductor ADC<bb><c>S<sss> chip
family, where:

 * bb  is the resolution in number of bits (8, 10, 12)
 * c   is the number of channels (1, 2, 4)
 * sss is the maximum conversion speed (021 for 200 kSPS, 051 for 500
   kSPS and 101 for 1 MSPS)

Some remarks:

* The chip type (-> the number of inputs) are determined by the module
  alias, is it a good idea?  it could be implemented using platform data.

Signed-off-by: Marc Pignat <marc.pignat@hevs.ch>
Cc: "Mark M. Hoffman" <mhoffman@lightlink.com>
Cc: Jean Delvare <khali@linux-fr.org>
Cc: David Brownell <david-b@pacbell.net>

but then didn't apply the patch.

>
> ...
>
> +/* sysfs hook function */
> +static ssize_t adcxx_read(struct device *dev,
> +		struct device_attribute *devattr, char *buf)
> +{
> +	struct spi_device *spi = to_spi_device(dev);
> +	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
> +	struct adcxx *adc = dev_get_drvdata(&spi->dev);
> +
> +	u8 tx_buf[2] = { attr->index << 3 }; /* other bits are don't care */
> +	u8 rx_buf[2];
> +	int status;
> +	int value;

The driver has rather a lot of inexplicable blank lines in the middle
of the automatic variable definitions.  The preferred style is no blank
lines within the definitions and a single blank line after them all,
thanks.

>
> ...
>
> +static int __devinit adcxx_probe(struct spi_device *spi, int channels)
> +{
> +	struct adcxx *adc;
> +	int status;
> +	int i;
> +
> +	adc = kzalloc(sizeof *adc, GFP_KERNEL);
> +	if (!adc)
> +		return -ENOMEM;
> +
> +	/* set a default value for the reference */
> +	adc->reference = 3300;
> +
> +	adc->channels = channels;
> +
> +	mutex_init(&adc->lock);
> +
> +	dev_set_drvdata(&spi->dev, adc);
> +
> +	for (i = 0; i < 3 + adc->channels; i++) {
> +		status = device_create_file(&spi->dev, &ad_input[i].dev_attr);
> +		if (status)
> +			goto out_dev_create_file_failed;
> +	}
> +
> +	adc->hwmon_dev = hwmon_device_register(&spi->dev);
> +	if (IS_ERR(adc->hwmon_dev)) {
> +		dev_dbg(&spi->dev, "hwmon_device_register failed.\n");
> +		status = PTR_ERR(adc->hwmon_dev);
> +		goto out_dev_reg_failed;
> +	}
> +
> +	return 0;
> +
> +out_dev_create_file_failed:
> +	hwmon_device_unregister(adc->hwmon_dev);
> +	for (i = 0; i < 3 + adc->channels; i++)
> +		device_remove_file(&spi->dev, &ad_input[i].dev_attr);
> +out_dev_reg_failed:
> +	dev_set_drvdata(&spi->dev, NULL);
> +	kfree(adc);
> +	return status;
> +}

The error recovery here is messed up.  The targets of the `goto's are
reversed.  But even if that is fixed, we can end up doing
device_remove_file() of objects which weren't successfully created. 
That might work, or it might generate runtime warnings or it might
crash.  I don't know.  It'd be best to just avoid doing it?

[RFC,PATCH v1] hwmon: ADC124S501 generic driver

[Marc Pignat]

SPI driver for analog to digital converters national semiconductor ADC081S101,
ADC124S501, ...

Signed-off-by: Marc Pignat <marc.pignat@hevs.ch>

Hi all!

patch against 2.6.26, tested on a custom arm board and only compil-tested on
x86.

This driver add support for National Semiconductor ADC<bb><c>S<sss> chip family,
where:
 * bb  is the resolution in number of bits (8, 10, 12)
 * c   is the number of channels (1, 2, 4)
 * sss is the maximum conversion speed (021 for 200 kSPS, 051 for 500
   kSPS and 101 for 1 MSPS)

Some remarks:
 * The chip type (-> the number of inputs) are determined by the module alias,
   is it a good idea? it could be implemented using platform data.

 * The Makefile seems ordered alphabetically, what order should be used for
   the Konfig file?

Best regards

Marc


diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 00ff533..9c4a579 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -390,6 +390,22 @@ config SENSORS_LM70
 	  This driver can also be built as a module.  If so, the module
 	  will be called lm70.
 
+config SENSORS_ADCXX
+	tristate "National Semiconductor ADCxxxSxxx"
+	depends on SPI_MASTER && EXPERIMENTAL
+	help
+	  If you say yes here you get support for the National Semiconductor
+	  ADC<bb><c>S<sss> chip family, where
+	  * bb  is the resolution in number of bits (8, 10, 12)
+	  * c   is the number of channels (1, 2, 4)
+	  * sss is the maximum conversion speed (021 for 200 kSPS, 051 for 500
+	    kSPS and 101 for 1 MSPS)
+
+	  Examples : ADC081S101, ADC124S501, ...
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called adcxx.
+
 config SENSORS_LM75
 	tristate "National Semiconductor LM75 and compatibles"
 	depends on I2C
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index d098677..026ba1e 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -16,6 +16,7 @@ obj-$(CONFIG_SENSORS_W83791D)	+= w83791d.o
 obj-$(CONFIG_SENSORS_ABITUGURU)	+= abituguru.o
 obj-$(CONFIG_SENSORS_ABITUGURU3)+= abituguru3.o
 obj-$(CONFIG_SENSORS_AD7418)	+= ad7418.o
+obj-$(CONFIG_SENSORS_ADCXX)	+= adcxx.o
 obj-$(CONFIG_SENSORS_ADM1021)	+= adm1021.o
 obj-$(CONFIG_SENSORS_ADM1025)	+= adm1025.o
 obj-$(CONFIG_SENSORS_ADM1026)	+= adm1026.o
diff --git a/drivers/hwmon/adcxx.c b/drivers/hwmon/adcxx.c
new file mode 100644
index 0000000..d920fe5
--- /dev/null
+++ b/drivers/hwmon/adcxx.c
@@ -0,0 +1,292 @@
+/*
+ * adcxx.c
+ *
+ * The adcxx4s is an AD converter family from National Semiconductor (NS).
+ *
+ * Copyright (c) 2008 Marc Pignat <marc.pignat@hevs.ch>
+ *
+ * The adcxx4s communicates with a host processor via an SPI/Microwire Bus
+ * interface. This driver supports the whole family of devices with name
+ * ADC<bb><c>S<sss>, where
+ * * bb is the resolution in number of bits (8, 10, 12)
+ * * c is the number of channels (1, 2, 4)
+ * * sss is the maximum conversion speed (021 for 200 kSPS, 051 for 500 kSPS
+ *   and 101 for 1 MSPS)
+ *
+ * Complete datasheets are available at National's website here:
+ * http://www.national.com/ds/DC/ADC<bb><c>S<sss>.pdf
+ *
+ * Handling of 8, 10 and 12 bits converters are the same, the
+ * unavailable bits are 0 :)
+ *
+ * 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/init.h>
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/sysfs.h>
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
+#include <linux/mutex.h>
+#include <linux/spi/spi.h>
+
+#define DRVNAME		"adcxx"
+
+struct adcxx {
+	struct device *hwmon_dev;
+	struct mutex lock;
+	u32 channels;
+	u32 reference; /* in millivolts */
+};
+
+/* sysfs hook function */
+static ssize_t adcxx_read(struct device *dev,
+		struct device_attribute *devattr, char *buf)
+{
+	struct spi_device *spi = to_spi_device(dev);
+	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
+	struct adcxx *adc = dev_get_drvdata(&spi->dev);
+
+	u8 tx_buf[2] = { attr->index << 3 }; /* other bits are don't care */
+	u8 rx_buf[2];
+	int status;
+	int value;
+
+	if (mutex_lock_interruptible(&adc->lock))
+		return -ERESTARTSYS;
+
+	status = spi_write_then_read(spi, tx_buf, sizeof(tx_buf),
+					rx_buf, sizeof(rx_buf));
+	if (status < 0) {
+		dev_warn(dev, "spi_write_then_read failed with status %d\n",
+				status);
+		goto out;
+	}
+
+	value = (rx_buf[0] << 8) + rx_buf[1];
+	dev_dbg(dev, "raw value = 0x%x\n", value);
+
+	value = value * adc->reference >> 12;
+	status = sprintf(buf, "%d\n", value);
+out:
+	mutex_unlock(&adc->lock);
+	return status;
+}
+
+static ssize_t adcxx_show_min(struct device *dev,
+		struct device_attribute *devattr, char *buf)
+{
+	/* The minimum reference is 0 for this chip family */
+	return sprintf(buf, "0\n");
+}
+
+static ssize_t adcxx_show_max(struct device *dev,
+		struct device_attribute *devattr, char *buf)
+{
+	struct spi_device *spi = to_spi_device(dev);
+	struct adcxx *adc = dev_get_drvdata(&spi->dev);
+
+	u32 reference;
+
+	if (mutex_lock_interruptible(&adc->lock))
+		return -ERESTARTSYS;
+
+	reference = adc->reference;
+
+	mutex_unlock(&adc->lock);
+
+	return sprintf(buf, "%d\n", reference);
+}
+
+static ssize_t adcxx_set_max(struct device *dev,
+	struct device_attribute *devattr, const char *buf, size_t count)
+{
+	struct spi_device *spi = to_spi_device(dev);
+	struct adcxx *adc = dev_get_drvdata(&spi->dev);
+
+	unsigned long value;
+
+	if (strict_strtoul(buf, 10, &value))
+		return -EINVAL;
+
+	if (mutex_lock_interruptible(&adc->lock))
+		return -ERESTARTSYS;
+
+	adc->reference = value;
+
+	mutex_unlock(&adc->lock);
+
+	return count;
+}
+
+static ssize_t adcxx_show_name(struct device *dev, struct device_attribute
+			      *devattr, char *buf)
+{
+	struct spi_device *spi = to_spi_device(dev);
+	struct adcxx *adc = dev_get_drvdata(&spi->dev);
+
+	return sprintf(buf, "adcxx%ds\n", adc->channels);
+}
+
+static struct sensor_device_attribute ad_input[] = {
+	SENSOR_ATTR(name, S_IRUGO, adcxx_show_name, NULL, 0),
+
+	/* The reference is the same for all channels */
+	SENSOR_ATTR(in_min, S_IRUGO, adcxx_show_min, NULL, 0),
+	SENSOR_ATTR(in_max, S_IWUSR | S_IRUGO, adcxx_show_max,
+					adcxx_set_max, 0),
+
+	SENSOR_ATTR(in0_input, S_IRUGO, adcxx_read, NULL, 0),
+	SENSOR_ATTR(in1_input, S_IRUGO, adcxx_read, NULL, 1),
+	SENSOR_ATTR(in2_input, S_IRUGO, adcxx_read, NULL, 2),
+	SENSOR_ATTR(in3_input, S_IRUGO, adcxx_read, NULL, 3),
+};
+
+/*----------------------------------------------------------------------*/
+
+static int __devinit adcxx_probe(struct spi_device *spi, int channels)
+{
+	struct adcxx *adc;
+	int status;
+	int i;
+
+	adc = kzalloc(sizeof *adc, GFP_KERNEL);
+	if (!adc)
+		return -ENOMEM;
+
+	/* set a default value for the reference */
+	adc->reference = 3300;
+
+	adc->channels = channels;
+
+	mutex_init(&adc->lock);
+
+	dev_set_drvdata(&spi->dev, adc);
+
+	for (i = 0; i < 3 + adc->channels; i++) {
+		status = device_create_file(&spi->dev, &ad_input[i].dev_attr);
+		if (status)
+			goto out_dev_create_file_failed;
+	}
+
+	adc->hwmon_dev = hwmon_device_register(&spi->dev);
+	if (IS_ERR(adc->hwmon_dev)) {
+		dev_dbg(&spi->dev, "hwmon_device_register failed.\n");
+		status = PTR_ERR(adc->hwmon_dev);
+		goto out_dev_reg_failed;
+	}
+
+	return 0;
+
+out_dev_create_file_failed:
+	hwmon_device_unregister(adc->hwmon_dev);
+	for (i = 0; i < 3 + adc->channels; i++)
+		device_remove_file(&spi->dev, &ad_input[i].dev_attr);
+out_dev_reg_failed:
+	dev_set_drvdata(&spi->dev, NULL);
+	kfree(adc);
+	return status;
+}
+
+static int __devinit adcxx1s_probe(struct spi_device *spi)
+{
+	return adcxx_probe(spi, 1);
+}
+
+static int __devinit adcxx2s_probe(struct spi_device *spi)
+{
+	return adcxx_probe(spi, 2);
+}
+
+static int __devinit adcxx4s_probe(struct spi_device *spi)
+{
+	return adcxx_probe(spi, 4);
+}
+
+static int __devexit adcxx_remove(struct spi_device *spi)
+{
+	struct adcxx *adc = dev_get_drvdata(&spi->dev);
+	int i;
+
+	hwmon_device_unregister(adc->hwmon_dev);
+	for (i = 0; i < 3 + adc->channels; i++)
+		device_remove_file(&spi->dev, &ad_input[i].dev_attr);
+	dev_set_drvdata(&spi->dev, NULL);
+	kfree(adc);
+
+	return 0;
+}
+
+static struct spi_driver adcxx1s_driver = {
+	.driver = {
+		.name	= "adcxx1s",
+		.owner	= THIS_MODULE,
+	},
+	.probe	= adcxx1s_probe,
+	.remove	= __devexit_p(adcxx_remove),
+};
+
+static struct spi_driver adcxx2s_driver = {
+	.driver = {
+		.name	= "adcxx2s",
+		.owner	= THIS_MODULE,
+	},
+	.probe	= adcxx2s_probe,
+	.remove	= __devexit_p(adcxx_remove),
+};
+
+static struct spi_driver adcxx4s_driver = {
+	.driver = {
+		.name	= "adcxx4s",
+		.owner	= THIS_MODULE,
+	},
+	.probe	= adcxx4s_probe,
+	.remove	= __devexit_p(adcxx_remove),
+};
+
+static int __init init_adcxx(void)
+{
+	int status;
+	status = spi_register_driver(&adcxx1s_driver);
+	if (status)
+		return status;
+
+	status = spi_register_driver(&adcxx2s_driver);
+	if (status)
+		return status;
+
+	return spi_register_driver(&adcxx4s_driver);
+}
+
+static void __exit exit_adcxx(void)
+{
+	spi_unregister_driver(&adcxx1s_driver);
+	spi_unregister_driver(&adcxx2s_driver);
+	spi_unregister_driver(&adcxx4s_driver);
+}
+
+module_init(init_adcxx);
+module_exit(exit_adcxx);
+
+MODULE_AUTHOR("Marc Pignat");
+MODULE_DESCRIPTION("National Semiconductor adcxx4sxxx Linux driver");
+MODULE_LICENSE("GPL");
+
+MODULE_ALIAS("adcxx1s");
+MODULE_ALIAS("adcxx2s");
+MODULE_ALIAS("adcxx4s");