[lm-sensors] [PATCH] hwmon: Driver for as1531,

[lm-sensors] [PATCH] hwmon: Driver for as1531,

[fabien.marteau]

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

From: Fabien Marteau <fabien.marteau@armadeus.com>


Signed-off-by: Fabien Marteau <fabien.marteau@armadeus.com>
---
 drivers/hwmon/Kconfig  |   10 ++
 drivers/hwmon/Makefile |    1 +
 drivers/hwmon/as1531.c |  297 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 308 insertions(+), 0 deletions(-)
 create mode 100644 drivers/hwmon/as1531.c

diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 50e40db..d2ba655 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -104,6 +104,16 @@ config SENSORS_ADCXX
 	  This driver can also be built as a module.  If so, the module
 	  will be called adcxx.
 
+config SENSORS_AS1531
+	tristate "Austria Microsystems AS1531 Analog to Digital Converter"
+	depends on SPI_MASTER
+	help
+	  If you say yes here you get support for Austria Microsystems AS1531.
+		AS1531 is a 12������bits Analog to digitals converter with 8 channels
+		provided by Austria-Microsystems company.
+	  This driver can also be built as a module.  If so, the module
+	  will be called as1531.
+
 config SENSORS_ADM1021
 	tristate "Analog Devices ADM1021 and compatibles"
 	depends on I2C
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index 967d0ea..8a304b3 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -23,6 +23,7 @@ obj-$(CONFIG_SENSORS_ABITUGURU3)+= abituguru3.o
 obj-$(CONFIG_SENSORS_AD7414)	+= ad7414.o
 obj-$(CONFIG_SENSORS_AD7418)	+= ad7418.o
 obj-$(CONFIG_SENSORS_ADCXX)	+= adcxx.o
+obj-$(CONFIG_SENSORS_AS1531)	+= as1531.o
 obj-$(CONFIG_SENSORS_ADM1021)	+= adm1021.o
 obj-$(CONFIG_SENSORS_ADM1025)	+= adm1025.o
 obj-$(CONFIG_SENSORS_ADM1026)	+= adm1026.o
diff --git a/drivers/hwmon/as1531.c b/drivers/hwmon/as1531.c
new file mode 100644
index 0000000..3384663
--- /dev/null
+++ b/drivers/hwmon/as1531.c
@@ -0,0 +1,297 @@
+/*
+ * as1531.c
+ *
+ * Driver for Austria-Microsystem Analog to Digital Converter.
+ *
+ * Copyright (c) 2010, 2011 Fabien Marteau <fabien.marteau@armadeus.com>
+ * Driver based on Marc Pignat <marc.pignat@hevs.ch> adcxx.c driver.
+ *
+ * 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 AS1531_SPI_SPEED 64941
+#define AS1531_MAX_VALUE 2500
+
+#define AS1531_START_BIT	(0x80)
+#define AS1531_CHAN0		(0<<4)
+#define AS1531_CHAN1		(4<<4)
+#define AS1531_CHAN2		(1<<4)
+#define AS1531_CHAN3		(5<<4)
+#define AS1531_CHAN4		(2<<4)
+#define AS1531_CHAN5		(6<<4)
+#define AS1531_CHAN6		(3<<4)
+#define AS1531_CHAN7		(7<<4)
+
+#define AS1531_RANGE_0_TO_VREF			(1<<3)
+#define AS1531_RANGE_HALFVREF_TO_HALFVREF	(0<<3)
+
+#define AS1531_MODE_COM		(1<<2)
+#define AS1531_MODE_DIFF	(0<<2)
+
+#define AS1531_POWER_DOWN		0x0
+#define AS1531_POWER_REDUCED		0x1
+#define AS1531_POWER_REDUCED_BIS	0x2
+#define AS1531_POWER_NORMAL		0x3
+
+struct as1531 {
+	struct device *hwmon_dev;
+	struct mutex lock;
+};
+
+static int as1531_message(struct spi_device *spi, int cmd, int *ret_value)
+{
+	struct spi_message	message;
+	struct spi_transfer	x[1];
+	int status, i;
+	u8	cmd_send;
+	unsigned char buf[64];
+	unsigned char buf_read[64];
+
+	cmd_send = cmd;
+
+	spi_message_init(&message);
+	memset(x, 0, sizeof x);
+	memset(buf, 0, sizeof(buf));
+	memset(buf_read, 0, sizeof(buf_read));
+
+	for (i = 0; i < 8; i++) {
+		buf[i] = ((cmd_send & 0x80)>>7);
+		cmd_send = cmd_send << 1;
+	}
+
+	x[0].tx_buf = buf;
+	x[0].len = 24;
+	x[0].rx_buf = buf_read;
+	x[0].speed_hz = AS1531_SPI_SPEED;
+	x[0].bits_per_word = 1;
+	spi_message_add_tail(&x[0], &message);
+
+	status = spi_sync(spi, &message);
+	if (status < 0)
+		return status;
+
+	*ret_value = buf_read[11] & 0x01;
+	for (i = 12; i < 23 ; i++) {
+		*ret_value = *ret_value << 1;
+		*ret_value = *ret_value | (buf_read[i]&0x01);
+	}
+
+	return 0;
+}
+
+static ssize_t as1531_read(struct device *dev,
+			struct device_attribute *devattr, char *buf)
+{
+	struct spi_device *spi = to_spi_device(dev);
+	struct as1531 *adc = dev_get_drvdata(&spi->dev);
+	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
+	int status = 0;
+	int ret_value;
+	int32_t cmd;
+
+
+	if (mutex_lock_interruptible(&adc->lock))
+		return -ERESTARTSYS;
+
+	switch (attr->index) {
+	case 0:
+		cmd = AS1531_START_BIT | AS1531_CHAN0 |
+		AS1531_RANGE_0_TO_VREF | AS1531_MODE_COM |
+		AS1531_POWER_NORMAL;
+		break;
+	case 1:
+		cmd = AS1531_START_BIT | AS1531_CHAN1 |
+		AS1531_RANGE_0_TO_VREF | AS1531_MODE_COM |
+		AS1531_POWER_NORMAL;
+		break;
+	case 2:
+		cmd = AS1531_START_BIT | AS1531_CHAN2 |
+		AS1531_RANGE_0_TO_VREF | AS1531_MODE_COM |
+		AS1531_POWER_NORMAL;
+		break;
+	case 3:
+		cmd = AS1531_START_BIT | AS1531_CHAN3 |
+		AS1531_RANGE_0_TO_VREF | AS1531_MODE_COM |
+		AS1531_POWER_NORMAL;
+		break;
+	case 4:
+		cmd = AS1531_START_BIT | AS1531_CHAN4 |
+		AS1531_RANGE_0_TO_VREF | AS1531_MODE_COM |
+		AS1531_POWER_NORMAL;
+		break;
+	case 5:
+		cmd = AS1531_START_BIT | AS1531_CHAN5 |
+		AS1531_RANGE_0_TO_VREF | AS1531_MODE_COM |
+		AS1531_POWER_NORMAL;
+		break;
+	case 6:
+		cmd = AS1531_START_BIT | AS1531_CHAN6 |
+		AS1531_RANGE_0_TO_VREF | AS1531_MODE_COM |
+		AS1531_POWER_NORMAL;
+		break;
+	case 7:
+		cmd = AS1531_START_BIT | AS1531_CHAN7 |
+		AS1531_RANGE_0_TO_VREF | AS1531_MODE_COM |
+		AS1531_POWER_NORMAL;
+		break;
+	default:
+		status = -EINVAL;
+		goto out;
+	}
+	status = as1531_message(spi, cmd, &ret_value);
+	if (status < 0)
+		goto out;
+
+	status = sprintf(buf, "%d\n", ret_value*2500/4096);
+out:
+	mutex_unlock(&adc->lock);
+	return status;
+}
+
+
+static ssize_t as1531_show_min(struct device *dev,
+		struct device_attribute *devattr, char *buf)
+{
+	return sprintf(buf, "0\n");
+}
+
+static ssize_t as1531_show_max(struct device *dev,
+		struct device_attribute *devattr, char *buf)
+{
+	return sprintf(buf, "%d\n", AS1531_MAX_VALUE);
+}
+
+static ssize_t as1531_show_name(struct device *dev, struct device_attribute
+				*devattr, char *buf)
+{
+	return sprintf(buf, "as1531\n");
+}
+
+static struct sensor_device_attribute as1531_input[] = {
+	SENSOR_ATTR(name, S_IRUGO, as1531_show_name, NULL, 0),
+	SENSOR_ATTR(in_min, S_IRUGO, as1531_show_min, NULL, 0),
+	SENSOR_ATTR(in_max, S_IRUGO, as1531_show_max, NULL, 0),
+	SENSOR_ATTR(in0_input, S_IRUGO, as1531_read, NULL, 0),
+	SENSOR_ATTR(in1_input, S_IRUGO, as1531_read, NULL, 1),
+	SENSOR_ATTR(in2_input, S_IRUGO, as1531_read, NULL, 2),
+	SENSOR_ATTR(in3_input, S_IRUGO, as1531_read, NULL, 3),
+	SENSOR_ATTR(in4_input, S_IRUGO, as1531_read, NULL, 4),
+	SENSOR_ATTR(in5_input, S_IRUGO, as1531_read, NULL, 5),
+	SENSOR_ATTR(in6_input, S_IRUGO, as1531_read, NULL, 6),
+	SENSOR_ATTR(in7_input, S_IRUGO, as1531_read, NULL, 7),
+};
+
+/*----------------------------------------------------------------------*/
+
+static int __devinit as1531_probe(struct spi_device *spi)
+{
+	struct as1531 *adc;
+	int status;
+	int i;
+
+	adc = kzalloc(sizeof(struct as1531), GFP_KERNEL);
+	if (adc == NULL)
+		return -ENOMEM;
+
+	mutex_init(&adc->lock);
+	mutex_lock(&adc->lock);
+
+	dev_set_drvdata(&spi->dev, adc);
+
+	for (i = 0; i < 11; i++) {
+		status = device_create_file(&spi->dev,
+						&as1531_input[i].dev_attr);
+		if (status < 0) {
+			dev_err(&spi->dev, "device_create_file failed.\n");
+			goto out_err;
+		}
+	}
+
+	adc->hwmon_dev = hwmon_device_register(&spi->dev);
+	if (IS_ERR(adc->hwmon_dev)) {
+		dev_err(&spi->dev, "hwmon_device_register failed.\n");
+		status = PTR_ERR(adc->hwmon_dev);
+		goto out_err;
+	}
+
+	mutex_unlock(&adc->lock);
+	return 0;
+
+out_err:
+	for (i--; i >= 0; i--)
+		device_remove_file(&spi->dev, &as1531_input[i].dev_attr);
+	dev_set_drvdata(&spi->dev, NULL);
+	mutex_unlock(&adc->lock);
+	kfree(adc);
+	return status;
+}
+
+static int __devexit as1531_remove(struct spi_device *spi)
+{
+	struct as1531 *adc = dev_get_drvdata(&spi->dev);
+	int i;
+
+	mutex_lock(&adc->lock);
+	hwmon_device_unregister(adc->hwmon_dev);
+	for (i = 0; i < 8; i++)
+		device_remove_file(&spi->dev, &as1531_input[i].dev_attr);
+
+	dev_set_drvdata(&spi->dev, NULL);
+	mutex_unlock(&adc->lock);
+	kfree(adc);
+
+	return 0;
+}
+
+/* SPI structures */
+
+static struct spi_driver as1531_driver = {
+	.driver = {
+		.name	= "as1531",
+		.owner	= THIS_MODULE,
+	},
+	.probe	= as1531_probe,
+	.remove	= __devexit_p(as1531_remove),
+};
+
+/* Init module */
+
+static int __init init_as1531(void)
+{
+	return spi_register_driver(&as1531_driver);
+}
+
+static void __exit exit_as1531(void)
+{
+	spi_unregister_driver(&as1531_driver);
+}
+
+module_init(init_as1531);
+module_exit(exit_as1531);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Fabien Marteau <fabien.marteau@armadeus.com>");
+MODULE_DESCRIPTION("Driver for AS1531 ADC");
-- 
1.7.0.4



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

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

[PATCH] hwmon: Driver for as1531, Austria-Microsystem Analog to Digital Converter.

[fabien.marteau]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 9513 bytes --]

From: Fabien Marteau <fabien.marteau@armadeus.com>


Signed-off-by: Fabien Marteau <fabien.marteau@armadeus.com>
---
 drivers/hwmon/Kconfig  |   10 ++
 drivers/hwmon/Makefile |    1 +
 drivers/hwmon/as1531.c |  297 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 308 insertions(+), 0 deletions(-)
 create mode 100644 drivers/hwmon/as1531.c

diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 50e40db..d2ba655 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -104,6 +104,16 @@ config SENSORS_ADCXX
 	  This driver can also be built as a module.  If so, the module
 	  will be called adcxx.
 
+config SENSORS_AS1531
+	tristate "Austria Microsystems AS1531 Analog to Digital Converter"
+	depends on SPI_MASTER
+	help
+	  If you say yes here you get support for Austria Microsystems AS1531.
+		AS1531 is a 12 bits Analog to digitals converter with 8 channels
+		provided by Austria-Microsystems company.
+	  This driver can also be built as a module.  If so, the module
+	  will be called as1531.
+
 config SENSORS_ADM1021
 	tristate "Analog Devices ADM1021 and compatibles"
 	depends on I2C
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index 967d0ea..8a304b3 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -23,6 +23,7 @@ obj-$(CONFIG_SENSORS_ABITUGURU3)+= abituguru3.o
 obj-$(CONFIG_SENSORS_AD7414)	+= ad7414.o
 obj-$(CONFIG_SENSORS_AD7418)	+= ad7418.o
 obj-$(CONFIG_SENSORS_ADCXX)	+= adcxx.o
+obj-$(CONFIG_SENSORS_AS1531)	+= as1531.o
 obj-$(CONFIG_SENSORS_ADM1021)	+= adm1021.o
 obj-$(CONFIG_SENSORS_ADM1025)	+= adm1025.o
 obj-$(CONFIG_SENSORS_ADM1026)	+= adm1026.o
diff --git a/drivers/hwmon/as1531.c b/drivers/hwmon/as1531.c
new file mode 100644
index 0000000..3384663
--- /dev/null
+++ b/drivers/hwmon/as1531.c
@@ -0,0 +1,297 @@
+/*
+ * as1531.c
+ *
+ * Driver for Austria-Microsystem Analog to Digital Converter.
+ *
+ * Copyright (c) 2010, 2011 Fabien Marteau <fabien.marteau@armadeus.com>
+ * Driver based on Marc Pignat <marc.pignat@hevs.ch> adcxx.c driver.
+ *
+ * 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 AS1531_SPI_SPEED 64941
+#define AS1531_MAX_VALUE 2500
+
+#define AS1531_START_BIT	(0x80)
+#define AS1531_CHAN0		(0<<4)
+#define AS1531_CHAN1		(4<<4)
+#define AS1531_CHAN2		(1<<4)
+#define AS1531_CHAN3		(5<<4)
+#define AS1531_CHAN4		(2<<4)
+#define AS1531_CHAN5		(6<<4)
+#define AS1531_CHAN6		(3<<4)
+#define AS1531_CHAN7		(7<<4)
+
+#define AS1531_RANGE_0_TO_VREF			(1<<3)
+#define AS1531_RANGE_HALFVREF_TO_HALFVREF	(0<<3)
+
+#define AS1531_MODE_COM		(1<<2)
+#define AS1531_MODE_DIFF	(0<<2)
+
+#define AS1531_POWER_DOWN		0x0
+#define AS1531_POWER_REDUCED		0x1
+#define AS1531_POWER_REDUCED_BIS	0x2
+#define AS1531_POWER_NORMAL		0x3
+
+struct as1531 {
+	struct device *hwmon_dev;
+	struct mutex lock;
+};
+
+static int as1531_message(struct spi_device *spi, int cmd, int *ret_value)
+{
+	struct spi_message	message;
+	struct spi_transfer	x[1];
+	int status, i;
+	u8	cmd_send;
+	unsigned char buf[64];
+	unsigned char buf_read[64];
+
+	cmd_send = cmd;
+
+	spi_message_init(&message);
+	memset(x, 0, sizeof x);
+	memset(buf, 0, sizeof(buf));
+	memset(buf_read, 0, sizeof(buf_read));
+
+	for (i = 0; i < 8; i++) {
+		buf[i] = ((cmd_send & 0x80)>>7);
+		cmd_send = cmd_send << 1;
+	}
+
+	x[0].tx_buf = buf;
+	x[0].len = 24;
+	x[0].rx_buf = buf_read;
+	x[0].speed_hz = AS1531_SPI_SPEED;
+	x[0].bits_per_word = 1;
+	spi_message_add_tail(&x[0], &message);
+
+	status = spi_sync(spi, &message);
+	if (status < 0)
+		return status;
+
+	*ret_value = buf_read[11] & 0x01;
+	for (i = 12; i < 23 ; i++) {
+		*ret_value = *ret_value << 1;
+		*ret_value = *ret_value | (buf_read[i]&0x01);
+	}
+
+	return 0;
+}
+
+static ssize_t as1531_read(struct device *dev,
+			struct device_attribute *devattr, char *buf)
+{
+	struct spi_device *spi = to_spi_device(dev);
+	struct as1531 *adc = dev_get_drvdata(&spi->dev);
+	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
+	int status = 0;
+	int ret_value;
+	int32_t cmd;
+
+
+	if (mutex_lock_interruptible(&adc->lock))
+		return -ERESTARTSYS;
+
+	switch (attr->index) {
+	case 0:
+		cmd = AS1531_START_BIT | AS1531_CHAN0 |
+		AS1531_RANGE_0_TO_VREF | AS1531_MODE_COM |
+		AS1531_POWER_NORMAL;
+		break;
+	case 1:
+		cmd = AS1531_START_BIT | AS1531_CHAN1 |
+		AS1531_RANGE_0_TO_VREF | AS1531_MODE_COM |
+		AS1531_POWER_NORMAL;
+		break;
+	case 2:
+		cmd = AS1531_START_BIT | AS1531_CHAN2 |
+		AS1531_RANGE_0_TO_VREF | AS1531_MODE_COM |
+		AS1531_POWER_NORMAL;
+		break;
+	case 3:
+		cmd = AS1531_START_BIT | AS1531_CHAN3 |
+		AS1531_RANGE_0_TO_VREF | AS1531_MODE_COM |
+		AS1531_POWER_NORMAL;
+		break;
+	case 4:
+		cmd = AS1531_START_BIT | AS1531_CHAN4 |
+		AS1531_RANGE_0_TO_VREF | AS1531_MODE_COM |
+		AS1531_POWER_NORMAL;
+		break;
+	case 5:
+		cmd = AS1531_START_BIT | AS1531_CHAN5 |
+		AS1531_RANGE_0_TO_VREF | AS1531_MODE_COM |
+		AS1531_POWER_NORMAL;
+		break;
+	case 6:
+		cmd = AS1531_START_BIT | AS1531_CHAN6 |
+		AS1531_RANGE_0_TO_VREF | AS1531_MODE_COM |
+		AS1531_POWER_NORMAL;
+		break;
+	case 7:
+		cmd = AS1531_START_BIT | AS1531_CHAN7 |
+		AS1531_RANGE_0_TO_VREF | AS1531_MODE_COM |
+		AS1531_POWER_NORMAL;
+		break;
+	default:
+		status = -EINVAL;
+		goto out;
+	}
+	status = as1531_message(spi, cmd, &ret_value);
+	if (status < 0)
+		goto out;
+
+	status = sprintf(buf, "%d\n", ret_value*2500/4096);
+out:
+	mutex_unlock(&adc->lock);
+	return status;
+}
+
+
+static ssize_t as1531_show_min(struct device *dev,
+		struct device_attribute *devattr, char *buf)
+{
+	return sprintf(buf, "0\n");
+}
+
+static ssize_t as1531_show_max(struct device *dev,
+		struct device_attribute *devattr, char *buf)
+{
+	return sprintf(buf, "%d\n", AS1531_MAX_VALUE);
+}
+
+static ssize_t as1531_show_name(struct device *dev, struct device_attribute
+				*devattr, char *buf)
+{
+	return sprintf(buf, "as1531\n");
+}
+
+static struct sensor_device_attribute as1531_input[] = {
+	SENSOR_ATTR(name, S_IRUGO, as1531_show_name, NULL, 0),
+	SENSOR_ATTR(in_min, S_IRUGO, as1531_show_min, NULL, 0),
+	SENSOR_ATTR(in_max, S_IRUGO, as1531_show_max, NULL, 0),
+	SENSOR_ATTR(in0_input, S_IRUGO, as1531_read, NULL, 0),
+	SENSOR_ATTR(in1_input, S_IRUGO, as1531_read, NULL, 1),
+	SENSOR_ATTR(in2_input, S_IRUGO, as1531_read, NULL, 2),
+	SENSOR_ATTR(in3_input, S_IRUGO, as1531_read, NULL, 3),
+	SENSOR_ATTR(in4_input, S_IRUGO, as1531_read, NULL, 4),
+	SENSOR_ATTR(in5_input, S_IRUGO, as1531_read, NULL, 5),
+	SENSOR_ATTR(in6_input, S_IRUGO, as1531_read, NULL, 6),
+	SENSOR_ATTR(in7_input, S_IRUGO, as1531_read, NULL, 7),
+};
+
+/*----------------------------------------------------------------------*/
+
+static int __devinit as1531_probe(struct spi_device *spi)
+{
+	struct as1531 *adc;
+	int status;
+	int i;
+
+	adc = kzalloc(sizeof(struct as1531), GFP_KERNEL);
+	if (adc == NULL)
+		return -ENOMEM;
+
+	mutex_init(&adc->lock);
+	mutex_lock(&adc->lock);
+
+	dev_set_drvdata(&spi->dev, adc);
+
+	for (i = 0; i < 11; i++) {
+		status = device_create_file(&spi->dev,
+						&as1531_input[i].dev_attr);
+		if (status < 0) {
+			dev_err(&spi->dev, "device_create_file failed.\n");
+			goto out_err;
+		}
+	}
+
+	adc->hwmon_dev = hwmon_device_register(&spi->dev);
+	if (IS_ERR(adc->hwmon_dev)) {
+		dev_err(&spi->dev, "hwmon_device_register failed.\n");
+		status = PTR_ERR(adc->hwmon_dev);
+		goto out_err;
+	}
+
+	mutex_unlock(&adc->lock);
+	return 0;
+
+out_err:
+	for (i--; i >= 0; i--)
+		device_remove_file(&spi->dev, &as1531_input[i].dev_attr);
+	dev_set_drvdata(&spi->dev, NULL);
+	mutex_unlock(&adc->lock);
+	kfree(adc);
+	return status;
+}
+
+static int __devexit as1531_remove(struct spi_device *spi)
+{
+	struct as1531 *adc = dev_get_drvdata(&spi->dev);
+	int i;
+
+	mutex_lock(&adc->lock);
+	hwmon_device_unregister(adc->hwmon_dev);
+	for (i = 0; i < 8; i++)
+		device_remove_file(&spi->dev, &as1531_input[i].dev_attr);
+
+	dev_set_drvdata(&spi->dev, NULL);
+	mutex_unlock(&adc->lock);
+	kfree(adc);
+
+	return 0;
+}
+
+/* SPI structures */
+
+static struct spi_driver as1531_driver = {
+	.driver = {
+		.name	= "as1531",
+		.owner	= THIS_MODULE,
+	},
+	.probe	= as1531_probe,
+	.remove	= __devexit_p(as1531_remove),
+};
+
+/* Init module */
+
+static int __init init_as1531(void)
+{
+	return spi_register_driver(&as1531_driver);
+}
+
+static void __exit exit_as1531(void)
+{
+	spi_unregister_driver(&as1531_driver);
+}
+
+module_init(init_as1531);
+module_exit(exit_as1531);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Fabien Marteau <fabien.marteau@armadeus.com>");
+MODULE_DESCRIPTION("Driver for AS1531 ADC");
-- 
1.7.0.4

Re: [lm-sensors] [PATCH] hwmon: Driver for as1531,

[Randy Dunlap]

On Mon, 16 May 2011 15:39:14 +0200 fabien.marteau@armadeus.com wrote:

> From: Fabien Marteau <fabien.marteau@armadeus.com>
> 
> 
> Signed-off-by: Fabien Marteau <fabien.marteau@armadeus.com>
> ---
>  drivers/hwmon/Kconfig  |   10 ++
>  drivers/hwmon/Makefile |    1 +
>  drivers/hwmon/as1531.c |  297 ++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 308 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/hwmon/as1531.c
> 
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 50e40db..d2ba655 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -104,6 +104,16 @@ config SENSORS_ADCXX
>  	  This driver can also be built as a module.  If so, the module
>  	  will be called adcxx.
>  
> +config SENSORS_AS1531
> +	tristate "Austria Microsystems AS1531 Analog to Digital Converter"
> +	depends on SPI_MASTER
> +	help
> +	  If you say yes here you get support for Austria Microsystems AS1531.
> +		AS1531 is a 12 bits Analog to digitals converter with 8 channels

is a 12? bits

what is the character after the "12", please?

Analog to digital converter

> +		provided by Austria-Microsystems company.
> +	  This driver can also be built as a module.  If so, the module
> +	  will be called as1531.
> +
>  config SENSORS_ADM1021
>  	tristate "Analog Devices ADM1021 and compatibles"
>  	depends on I2C


> +/*----------------------------------------------------------------------*/
> +
> +static int __devinit as1531_probe(struct spi_device *spi)
> +{
> +	struct as1531 *adc;
> +	int status;
> +	int i;
> +
> +	adc = kzalloc(sizeof(struct as1531), GFP_KERNEL);
> +	if (adc = NULL)
> +		return -ENOMEM;
> +
> +	mutex_init(&adc->lock);
> +	mutex_lock(&adc->lock);
> +
> +	dev_set_drvdata(&spi->dev, adc);
> +
> +	for (i = 0; i < 11; i++) {

s/11/ARRAY_SIZE(as1531_input)/

> +		status = device_create_file(&spi->dev,
> +						&as1531_input[i].dev_attr);
> +		if (status < 0) {
> +			dev_err(&spi->dev, "device_create_file failed.\n");
> +			goto out_err;
> +		}
> +	}



---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

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

Re: [PATCH] hwmon: Driver for as1531, Austria-Microsystem Analog to Digital Converter.

[Randy Dunlap]

On Mon, 16 May 2011 15:39:14 +0200 fabien.marteau@armadeus.com wrote:

> From: Fabien Marteau <fabien.marteau@armadeus.com>
> 
> 
> Signed-off-by: Fabien Marteau <fabien.marteau@armadeus.com>
> ---
>  drivers/hwmon/Kconfig  |   10 ++
>  drivers/hwmon/Makefile |    1 +
>  drivers/hwmon/as1531.c |  297 ++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 308 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/hwmon/as1531.c
> 
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 50e40db..d2ba655 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -104,6 +104,16 @@ config SENSORS_ADCXX
>  	  This driver can also be built as a module.  If so, the module
>  	  will be called adcxx.
>  
> +config SENSORS_AS1531
> +	tristate "Austria Microsystems AS1531 Analog to Digital Converter"
> +	depends on SPI_MASTER
> +	help
> +	  If you say yes here you get support for Austria Microsystems AS1531.
> +		AS1531 is a 12 bits Analog to digitals converter with 8 channels

is a 12? bits

what is the character after the "12", please?

Analog to digital converter

> +		provided by Austria-Microsystems company.
> +	  This driver can also be built as a module.  If so, the module
> +	  will be called as1531.
> +
>  config SENSORS_ADM1021
>  	tristate "Analog Devices ADM1021 and compatibles"
>  	depends on I2C


> +/*----------------------------------------------------------------------*/
> +
> +static int __devinit as1531_probe(struct spi_device *spi)
> +{
> +	struct as1531 *adc;
> +	int status;
> +	int i;
> +
> +	adc = kzalloc(sizeof(struct as1531), GFP_KERNEL);
> +	if (adc == NULL)
> +		return -ENOMEM;
> +
> +	mutex_init(&adc->lock);
> +	mutex_lock(&adc->lock);
> +
> +	dev_set_drvdata(&spi->dev, adc);
> +
> +	for (i = 0; i < 11; i++) {

s/11/ARRAY_SIZE(as1531_input)/

> +		status = device_create_file(&spi->dev,
> +						&as1531_input[i].dev_attr);
> +		if (status < 0) {
> +			dev_err(&spi->dev, "device_create_file failed.\n");
> +			goto out_err;
> +		}
> +	}



---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

Re: [lm-sensors] [PATCH] hwmon: Driver for as1531,

[Guenter Roeck]

On Mon, May 16, 2011 at 09:39:14AM -0400, fabien.marteau@armadeus.com wrote:
> From: Fabien Marteau <fabien.marteau@armadeus.com>
> 
> 
Some description, such as "Dhis driver adds support for xxx" would be nice.

Also, I wonder if this driver belongs into hwmon in the first place. It is
a generic ADC chip with high conversion rate. iio would probably be more
appropriate and also much better in supporting high speed readings.

> Signed-off-by: Fabien Marteau <fabien.marteau@armadeus.com>
> ---
>  drivers/hwmon/Kconfig  |   10 ++
>  drivers/hwmon/Makefile |    1 +
>  drivers/hwmon/as1531.c |  297 ++++++++++++++++++++++++++++++++++++++++++++++++

Documentation/hwmon/as1531 is missing.
Do you plan to add yourself into MAINTAINERS ?

>  3 files changed, 308 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/hwmon/as1531.c
> 
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 50e40db..d2ba655 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -104,6 +104,16 @@ config SENSORS_ADCXX
>  	  This driver can also be built as a module.  If so, the module
>  	  will be called adcxx.
>  
> +config SENSORS_AS1531
> +	tristate "Austria Microsystems AS1531 Analog to Digital Converter"
> +	depends on SPI_MASTER
> +	help
> +	  If you say yes here you get support for Austria Microsystems AS1531.
> +		AS1531 is a 12 bits Analog to digitals converter with 8 channels
> +		provided by Austria-Microsystems company.
> +	  This driver can also be built as a module.  If so, the module
> +	  will be called as1531.
> +
Should be inserted in alphabetical order. Should depend on EXPERIMENTAL.

>  config SENSORS_ADM1021
>  	tristate "Analog Devices ADM1021 and compatibles"
>  	depends on I2C
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index 967d0ea..8a304b3 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -23,6 +23,7 @@ obj-$(CONFIG_SENSORS_ABITUGURU3)+= abituguru3.o
>  obj-$(CONFIG_SENSORS_AD7414)	+= ad7414.o
>  obj-$(CONFIG_SENSORS_AD7418)	+= ad7418.o
>  obj-$(CONFIG_SENSORS_ADCXX)	+= adcxx.o
> +obj-$(CONFIG_SENSORS_AS1531)	+= as1531.o

Insert in alphabetical order.

>  obj-$(CONFIG_SENSORS_ADM1021)	+= adm1021.o
>  obj-$(CONFIG_SENSORS_ADM1025)	+= adm1025.o
>  obj-$(CONFIG_SENSORS_ADM1026)	+= adm1026.o
> diff --git a/drivers/hwmon/as1531.c b/drivers/hwmon/as1531.c
> new file mode 100644
> index 0000000..3384663
> --- /dev/null
> +++ b/drivers/hwmon/as1531.c
> @@ -0,0 +1,297 @@
> +/*
> + * as1531.c
> + *
> + * Driver for Austria-Microsystem Analog to Digital Converter.
> + *
> + * Copyright (c) 2010, 2011 Fabien Marteau <fabien.marteau@armadeus.com>
> + * Driver based on Marc Pignat <marc.pignat@hevs.ch> adcxx.c driver.
> + *
> + * 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 AS1531_SPI_SPEED 64941
> +#define AS1531_MAX_VALUE 2500
> +
> +#define AS1531_START_BIT	(0x80)
> +#define AS1531_CHAN0		(0<<4)
> +#define AS1531_CHAN1		(4<<4)
> +#define AS1531_CHAN2		(1<<4)
> +#define AS1531_CHAN3		(5<<4)
> +#define AS1531_CHAN4		(2<<4)
> +#define AS1531_CHAN5		(6<<4)
> +#define AS1531_CHAN6		(3<<4)
> +#define AS1531_CHAN7		(7<<4)
> +
> +#define AS1531_RANGE_0_TO_VREF			(1<<3)
> +#define AS1531_RANGE_HALFVREF_TO_HALFVREF	(0<<3)
> +
> +#define AS1531_MODE_COM		(1<<2)
> +#define AS1531_MODE_DIFF	(0<<2)
> +
> +#define AS1531_POWER_DOWN		0x0
> +#define AS1531_POWER_REDUCED		0x1
> +#define AS1531_POWER_REDUCED_BIS	0x2
> +#define AS1531_POWER_NORMAL		0x3
> +
> +struct as1531 {
> +	struct device *hwmon_dev;
> +	struct mutex lock;
> +};
> +
> +static int as1531_message(struct spi_device *spi, int cmd, int *ret_value)
> +{
> +	struct spi_message	message;
> +	struct spi_transfer	x[1];
> +	int status, i;
> +	u8	cmd_send;
> +	unsigned char buf[64];
> +	unsigned char buf_read[64];
> +
Not that it matters much, but the buffer sizes seem to be a bit on the large side.

> +	cmd_send = cmd;
> +
> +	spi_message_init(&message);
> +	memset(x, 0, sizeof x);
> +	memset(buf, 0, sizeof(buf));
> +	memset(buf_read, 0, sizeof(buf_read));
> +
Are those memsets really needed ?

> +	for (i = 0; i < 8; i++) {
> +		buf[i] = ((cmd_send & 0x80)>>7);

Missing spaces. Wonder why checkpatch doesn't complain about this anymore.

> +		cmd_send = cmd_send << 1;
> +	}
> +
> +	x[0].tx_buf = buf;
> +	x[0].len = 24;
> +	x[0].rx_buf = buf_read;
> +	x[0].speed_hz = AS1531_SPI_SPEED;
> +	x[0].bits_per_word = 1;
> +	spi_message_add_tail(&x[0], &message);
> +
> +	status = spi_sync(spi, &message);
> +	if (status < 0)
> +		return status;
> +
> +	*ret_value = buf_read[11] & 0x01;
> +	for (i = 12; i < 23 ; i++) {
> +		*ret_value = *ret_value << 1;
> +		*ret_value = *ret_value | (buf_read[i]&0x01);

More missing spaces.

> +	}
> +
> +	return 0;
> +}
> +
> +static ssize_t as1531_read(struct device *dev,
> +			struct device_attribute *devattr, char *buf)
> +{
> +	struct spi_device *spi = to_spi_device(dev);
> +	struct as1531 *adc = dev_get_drvdata(&spi->dev);
> +	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
> +	int status = 0;
> +	int ret_value;
> +	int32_t cmd;
> +
> +
Extra blank line.

> +	if (mutex_lock_interruptible(&adc->lock))
> +		return -ERESTARTSYS;
> +
> +	switch (attr->index) {
> +	case 0:
> +		cmd = AS1531_START_BIT | AS1531_CHAN0 |
> +		AS1531_RANGE_0_TO_VREF | AS1531_MODE_COM |
> +		AS1531_POWER_NORMAL;
> +		break;
> +	case 1:
> +		cmd = AS1531_START_BIT | AS1531_CHAN1 |
> +		AS1531_RANGE_0_TO_VREF | AS1531_MODE_COM |
> +		AS1531_POWER_NORMAL;
> +		break;
> +	case 2:
> +		cmd = AS1531_START_BIT | AS1531_CHAN2 |
> +		AS1531_RANGE_0_TO_VREF | AS1531_MODE_COM |
> +		AS1531_POWER_NORMAL;
> +		break;
> +	case 3:
> +		cmd = AS1531_START_BIT | AS1531_CHAN3 |
> +		AS1531_RANGE_0_TO_VREF | AS1531_MODE_COM |
> +		AS1531_POWER_NORMAL;
> +		break;
> +	case 4:
> +		cmd = AS1531_START_BIT | AS1531_CHAN4 |
> +		AS1531_RANGE_0_TO_VREF | AS1531_MODE_COM |
> +		AS1531_POWER_NORMAL;
> +		break;
> +	case 5:
> +		cmd = AS1531_START_BIT | AS1531_CHAN5 |
> +		AS1531_RANGE_0_TO_VREF | AS1531_MODE_COM |
> +		AS1531_POWER_NORMAL;
> +		break;
> +	case 6:
> +		cmd = AS1531_START_BIT | AS1531_CHAN6 |
> +		AS1531_RANGE_0_TO_VREF | AS1531_MODE_COM |
> +		AS1531_POWER_NORMAL;
> +		break;
> +	case 7:
> +		cmd = AS1531_START_BIT | AS1531_CHAN7 |
> +		AS1531_RANGE_0_TO_VREF | AS1531_MODE_COM |
> +		AS1531_POWER_NORMAL;
> +		break;
> +	default:
> +		status = -EINVAL;
> +		goto out;
> +	}

The lock is really only needed here.

> +	status = as1531_message(spi, cmd, &ret_value);
> +	if (status < 0)
> +		goto out;
> +
> +	status = sprintf(buf, "%d\n", ret_value*2500/4096);

Missing spaces. The conversion seems to be arbitrary. You might want to return the raw value 
and convert via sensors.conf.

> +out:
> +	mutex_unlock(&adc->lock);
> +	return status;
> +}
> +
> +
> +static ssize_t as1531_show_min(struct device *dev,
> +		struct device_attribute *devattr, char *buf)
> +{
> +	return sprintf(buf, "0\n");
> +}
> +
> +static ssize_t as1531_show_max(struct device *dev,
> +		struct device_attribute *devattr, char *buf)
> +{
> +	return sprintf(buf, "%d\n", AS1531_MAX_VALUE);
> +}
> +

Not sure if reporting constants makes much sense. Opinions, anyone ?
Besides, AS1531_MAX_VALUE is as arbitrary as the conversion function above.

> +static ssize_t as1531_show_name(struct device *dev, struct device_attribute
> +				*devattr, char *buf)
> +{
> +	return sprintf(buf, "as1531\n");
> +}
> +
> +static struct sensor_device_attribute as1531_input[] = {
> +	SENSOR_ATTR(name, S_IRUGO, as1531_show_name, NULL, 0),
> +	SENSOR_ATTR(in_min, S_IRUGO, as1531_show_min, NULL, 0),
> +	SENSOR_ATTR(in_max, S_IRUGO, as1531_show_max, NULL, 0),

Non-standard attributes. Even if there is only one set of values reported per chip, 
reporting it as non-standard attribute doesn't provide much value. Please define 
per-sensor attributes.

> +	SENSOR_ATTR(in0_input, S_IRUGO, as1531_read, NULL, 0),
> +	SENSOR_ATTR(in1_input, S_IRUGO, as1531_read, NULL, 1),
> +	SENSOR_ATTR(in2_input, S_IRUGO, as1531_read, NULL, 2),
> +	SENSOR_ATTR(in3_input, S_IRUGO, as1531_read, NULL, 3),
> +	SENSOR_ATTR(in4_input, S_IRUGO, as1531_read, NULL, 4),
> +	SENSOR_ATTR(in5_input, S_IRUGO, as1531_read, NULL, 5),
> +	SENSOR_ATTR(in6_input, S_IRUGO, as1531_read, NULL, 6),
> +	SENSOR_ATTR(in7_input, S_IRUGO, as1531_read, NULL, 7),
> +};
> +
> +/*----------------------------------------------------------------------*/
> +
> +static int __devinit as1531_probe(struct spi_device *spi)
> +{
> +	struct as1531 *adc;
> +	int status;
> +	int i;
> +
> +	adc = kzalloc(sizeof(struct as1531), GFP_KERNEL);
> +	if (adc = NULL)
> +		return -ENOMEM;
> +
> +	mutex_init(&adc->lock);
> +	mutex_lock(&adc->lock);

Is that lock here really needed ? Should not be necessary.

> +
> +	dev_set_drvdata(&spi->dev, adc);
> +
> +	for (i = 0; i < 11; i++) {
> +		status = device_create_file(&spi->dev,
> +						&as1531_input[i].dev_attr);
> +		if (status < 0) {
> +			dev_err(&spi->dev, "device_create_file failed.\n");
> +			goto out_err;
> +		}
> +	}
> +
> +	adc->hwmon_dev = hwmon_device_register(&spi->dev);
> +	if (IS_ERR(adc->hwmon_dev)) {
> +		dev_err(&spi->dev, "hwmon_device_register failed.\n");
> +		status = PTR_ERR(adc->hwmon_dev);
> +		goto out_err;
> +	}
> +
> +	mutex_unlock(&adc->lock);
> +	return 0;
> +
> +out_err:
> +	for (i--; i >= 0; i--)
> +		device_remove_file(&spi->dev, &as1531_input[i].dev_attr);
> +	dev_set_drvdata(&spi->dev, NULL);
> +	mutex_unlock(&adc->lock);
> +	kfree(adc);
> +	return status;
> +}
> +
> +static int __devexit as1531_remove(struct spi_device *spi)
> +{
> +	struct as1531 *adc = dev_get_drvdata(&spi->dev);
> +	int i;
> +
> +	mutex_lock(&adc->lock);

Lock should not be needed here. If anything, it provides a false sense 
of security: Anything else waiting on it will be surprised to notice that 
the mutex (and memory) it is waiting on has been freed.
In other words, if this lock is really needed, you are in big trouble.

> +	hwmon_device_unregister(adc->hwmon_dev);
> +	for (i = 0; i < 8; i++)
> +		device_remove_file(&spi->dev, &as1531_input[i].dev_attr);
> +
> +	dev_set_drvdata(&spi->dev, NULL);
> +	mutex_unlock(&adc->lock);
> +	kfree(adc);
> +
> +	return 0;
> +}
> +
> +/* SPI structures */
> +
> +static struct spi_driver as1531_driver = {
> +	.driver = {
> +		.name	= "as1531",
> +		.owner	= THIS_MODULE,
> +	},
> +	.probe	= as1531_probe,
> +	.remove	= __devexit_p(as1531_remove),
> +};
> +
> +/* Init module */
> +
> +static int __init init_as1531(void)
> +{
> +	return spi_register_driver(&as1531_driver);
> +}
> +
> +static void __exit exit_as1531(void)
> +{
> +	spi_unregister_driver(&as1531_driver);
> +}
> +
> +module_init(init_as1531);
> +module_exit(exit_as1531);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Fabien Marteau <fabien.marteau@armadeus.com>");
> +MODULE_DESCRIPTION("Driver for AS1531 ADC");
> -- 
> 1.7.0.4
> 

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

Re: [PATCH] hwmon: Driver for as1531, Austria-Microsystem Analog to Digital Converter.

[Guenter Roeck]

On Mon, May 16, 2011 at 09:39:14AM -0400, fabien.marteau@armadeus.com wrote:
> From: Fabien Marteau <fabien.marteau@armadeus.com>
> 
> 
Some description, such as "Dhis driver adds support for xxx" would be nice.

Also, I wonder if this driver belongs into hwmon in the first place. It is
a generic ADC chip with high conversion rate. iio would probably be more
appropriate and also much better in supporting high speed readings.

> Signed-off-by: Fabien Marteau <fabien.marteau@armadeus.com>
> ---
>  drivers/hwmon/Kconfig  |   10 ++
>  drivers/hwmon/Makefile |    1 +
>  drivers/hwmon/as1531.c |  297 ++++++++++++++++++++++++++++++++++++++++++++++++

Documentation/hwmon/as1531 is missing.
Do you plan to add yourself into MAINTAINERS ?

>  3 files changed, 308 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/hwmon/as1531.c
> 
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 50e40db..d2ba655 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -104,6 +104,16 @@ config SENSORS_ADCXX
>  	  This driver can also be built as a module.  If so, the module
>  	  will be called adcxx.
>  
> +config SENSORS_AS1531
> +	tristate "Austria Microsystems AS1531 Analog to Digital Converter"
> +	depends on SPI_MASTER
> +	help
> +	  If you say yes here you get support for Austria Microsystems AS1531.
> +		AS1531 is a 12 bits Analog to digitals converter with 8 channels
> +		provided by Austria-Microsystems company.
> +	  This driver can also be built as a module.  If so, the module
> +	  will be called as1531.
> +
Should be inserted in alphabetical order. Should depend on EXPERIMENTAL.

>  config SENSORS_ADM1021
>  	tristate "Analog Devices ADM1021 and compatibles"
>  	depends on I2C
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index 967d0ea..8a304b3 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -23,6 +23,7 @@ obj-$(CONFIG_SENSORS_ABITUGURU3)+= abituguru3.o
>  obj-$(CONFIG_SENSORS_AD7414)	+= ad7414.o
>  obj-$(CONFIG_SENSORS_AD7418)	+= ad7418.o
>  obj-$(CONFIG_SENSORS_ADCXX)	+= adcxx.o
> +obj-$(CONFIG_SENSORS_AS1531)	+= as1531.o

Insert in alphabetical order.

>  obj-$(CONFIG_SENSORS_ADM1021)	+= adm1021.o
>  obj-$(CONFIG_SENSORS_ADM1025)	+= adm1025.o
>  obj-$(CONFIG_SENSORS_ADM1026)	+= adm1026.o
> diff --git a/drivers/hwmon/as1531.c b/drivers/hwmon/as1531.c
> new file mode 100644
> index 0000000..3384663
> --- /dev/null
> +++ b/drivers/hwmon/as1531.c
> @@ -0,0 +1,297 @@
> +/*
> + * as1531.c
> + *
> + * Driver for Austria-Microsystem Analog to Digital Converter.
> + *
> + * Copyright (c) 2010, 2011 Fabien Marteau <fabien.marteau@armadeus.com>
> + * Driver based on Marc Pignat <marc.pignat@hevs.ch> adcxx.c driver.
> + *
> + * 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 AS1531_SPI_SPEED 64941
> +#define AS1531_MAX_VALUE 2500
> +
> +#define AS1531_START_BIT	(0x80)
> +#define AS1531_CHAN0		(0<<4)
> +#define AS1531_CHAN1		(4<<4)
> +#define AS1531_CHAN2		(1<<4)
> +#define AS1531_CHAN3		(5<<4)
> +#define AS1531_CHAN4		(2<<4)
> +#define AS1531_CHAN5		(6<<4)
> +#define AS1531_CHAN6		(3<<4)
> +#define AS1531_CHAN7		(7<<4)
> +
> +#define AS1531_RANGE_0_TO_VREF			(1<<3)
> +#define AS1531_RANGE_HALFVREF_TO_HALFVREF	(0<<3)
> +
> +#define AS1531_MODE_COM		(1<<2)
> +#define AS1531_MODE_DIFF	(0<<2)
> +
> +#define AS1531_POWER_DOWN		0x0
> +#define AS1531_POWER_REDUCED		0x1
> +#define AS1531_POWER_REDUCED_BIS	0x2
> +#define AS1531_POWER_NORMAL		0x3
> +
> +struct as1531 {
> +	struct device *hwmon_dev;
> +	struct mutex lock;
> +};
> +
> +static int as1531_message(struct spi_device *spi, int cmd, int *ret_value)
> +{
> +	struct spi_message	message;
> +	struct spi_transfer	x[1];
> +	int status, i;
> +	u8	cmd_send;
> +	unsigned char buf[64];
> +	unsigned char buf_read[64];
> +
Not that it matters much, but the buffer sizes seem to be a bit on the large side.

> +	cmd_send = cmd;
> +
> +	spi_message_init(&message);
> +	memset(x, 0, sizeof x);
> +	memset(buf, 0, sizeof(buf));
> +	memset(buf_read, 0, sizeof(buf_read));
> +
Are those memsets really needed ?

> +	for (i = 0; i < 8; i++) {
> +		buf[i] = ((cmd_send & 0x80)>>7);

Missing spaces. Wonder why checkpatch doesn't complain about this anymore.

> +		cmd_send = cmd_send << 1;
> +	}
> +
> +	x[0].tx_buf = buf;
> +	x[0].len = 24;
> +	x[0].rx_buf = buf_read;
> +	x[0].speed_hz = AS1531_SPI_SPEED;
> +	x[0].bits_per_word = 1;
> +	spi_message_add_tail(&x[0], &message);
> +
> +	status = spi_sync(spi, &message);
> +	if (status < 0)
> +		return status;
> +
> +	*ret_value = buf_read[11] & 0x01;
> +	for (i = 12; i < 23 ; i++) {
> +		*ret_value = *ret_value << 1;
> +		*ret_value = *ret_value | (buf_read[i]&0x01);

More missing spaces.

> +	}
> +
> +	return 0;
> +}
> +
> +static ssize_t as1531_read(struct device *dev,
> +			struct device_attribute *devattr, char *buf)
> +{
> +	struct spi_device *spi = to_spi_device(dev);
> +	struct as1531 *adc = dev_get_drvdata(&spi->dev);
> +	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
> +	int status = 0;
> +	int ret_value;
> +	int32_t cmd;
> +
> +
Extra blank line.

> +	if (mutex_lock_interruptible(&adc->lock))
> +		return -ERESTARTSYS;
> +
> +	switch (attr->index) {
> +	case 0:
> +		cmd = AS1531_START_BIT | AS1531_CHAN0 |
> +		AS1531_RANGE_0_TO_VREF | AS1531_MODE_COM |
> +		AS1531_POWER_NORMAL;
> +		break;
> +	case 1:
> +		cmd = AS1531_START_BIT | AS1531_CHAN1 |
> +		AS1531_RANGE_0_TO_VREF | AS1531_MODE_COM |
> +		AS1531_POWER_NORMAL;
> +		break;
> +	case 2:
> +		cmd = AS1531_START_BIT | AS1531_CHAN2 |
> +		AS1531_RANGE_0_TO_VREF | AS1531_MODE_COM |
> +		AS1531_POWER_NORMAL;
> +		break;
> +	case 3:
> +		cmd = AS1531_START_BIT | AS1531_CHAN3 |
> +		AS1531_RANGE_0_TO_VREF | AS1531_MODE_COM |
> +		AS1531_POWER_NORMAL;
> +		break;
> +	case 4:
> +		cmd = AS1531_START_BIT | AS1531_CHAN4 |
> +		AS1531_RANGE_0_TO_VREF | AS1531_MODE_COM |
> +		AS1531_POWER_NORMAL;
> +		break;
> +	case 5:
> +		cmd = AS1531_START_BIT | AS1531_CHAN5 |
> +		AS1531_RANGE_0_TO_VREF | AS1531_MODE_COM |
> +		AS1531_POWER_NORMAL;
> +		break;
> +	case 6:
> +		cmd = AS1531_START_BIT | AS1531_CHAN6 |
> +		AS1531_RANGE_0_TO_VREF | AS1531_MODE_COM |
> +		AS1531_POWER_NORMAL;
> +		break;
> +	case 7:
> +		cmd = AS1531_START_BIT | AS1531_CHAN7 |
> +		AS1531_RANGE_0_TO_VREF | AS1531_MODE_COM |
> +		AS1531_POWER_NORMAL;
> +		break;
> +	default:
> +		status = -EINVAL;
> +		goto out;
> +	}

The lock is really only needed here.

> +	status = as1531_message(spi, cmd, &ret_value);
> +	if (status < 0)
> +		goto out;
> +
> +	status = sprintf(buf, "%d\n", ret_value*2500/4096);

Missing spaces. The conversion seems to be arbitrary. You might want to return the raw value 
and convert via sensors.conf.

> +out:
> +	mutex_unlock(&adc->lock);
> +	return status;
> +}
> +
> +
> +static ssize_t as1531_show_min(struct device *dev,
> +		struct device_attribute *devattr, char *buf)
> +{
> +	return sprintf(buf, "0\n");
> +}
> +
> +static ssize_t as1531_show_max(struct device *dev,
> +		struct device_attribute *devattr, char *buf)
> +{
> +	return sprintf(buf, "%d\n", AS1531_MAX_VALUE);
> +}
> +

Not sure if reporting constants makes much sense. Opinions, anyone ?
Besides, AS1531_MAX_VALUE is as arbitrary as the conversion function above.

> +static ssize_t as1531_show_name(struct device *dev, struct device_attribute
> +				*devattr, char *buf)
> +{
> +	return sprintf(buf, "as1531\n");
> +}
> +
> +static struct sensor_device_attribute as1531_input[] = {
> +	SENSOR_ATTR(name, S_IRUGO, as1531_show_name, NULL, 0),
> +	SENSOR_ATTR(in_min, S_IRUGO, as1531_show_min, NULL, 0),
> +	SENSOR_ATTR(in_max, S_IRUGO, as1531_show_max, NULL, 0),

Non-standard attributes. Even if there is only one set of values reported per chip, 
reporting it as non-standard attribute doesn't provide much value. Please define 
per-sensor attributes.

> +	SENSOR_ATTR(in0_input, S_IRUGO, as1531_read, NULL, 0),
> +	SENSOR_ATTR(in1_input, S_IRUGO, as1531_read, NULL, 1),
> +	SENSOR_ATTR(in2_input, S_IRUGO, as1531_read, NULL, 2),
> +	SENSOR_ATTR(in3_input, S_IRUGO, as1531_read, NULL, 3),
> +	SENSOR_ATTR(in4_input, S_IRUGO, as1531_read, NULL, 4),
> +	SENSOR_ATTR(in5_input, S_IRUGO, as1531_read, NULL, 5),
> +	SENSOR_ATTR(in6_input, S_IRUGO, as1531_read, NULL, 6),
> +	SENSOR_ATTR(in7_input, S_IRUGO, as1531_read, NULL, 7),
> +};
> +
> +/*----------------------------------------------------------------------*/
> +
> +static int __devinit as1531_probe(struct spi_device *spi)
> +{
> +	struct as1531 *adc;
> +	int status;
> +	int i;
> +
> +	adc = kzalloc(sizeof(struct as1531), GFP_KERNEL);
> +	if (adc == NULL)
> +		return -ENOMEM;
> +
> +	mutex_init(&adc->lock);
> +	mutex_lock(&adc->lock);

Is that lock here really needed ? Should not be necessary.

> +
> +	dev_set_drvdata(&spi->dev, adc);
> +
> +	for (i = 0; i < 11; i++) {
> +		status = device_create_file(&spi->dev,
> +						&as1531_input[i].dev_attr);
> +		if (status < 0) {
> +			dev_err(&spi->dev, "device_create_file failed.\n");
> +			goto out_err;
> +		}
> +	}
> +
> +	adc->hwmon_dev = hwmon_device_register(&spi->dev);
> +	if (IS_ERR(adc->hwmon_dev)) {
> +		dev_err(&spi->dev, "hwmon_device_register failed.\n");
> +		status = PTR_ERR(adc->hwmon_dev);
> +		goto out_err;
> +	}
> +
> +	mutex_unlock(&adc->lock);
> +	return 0;
> +
> +out_err:
> +	for (i--; i >= 0; i--)
> +		device_remove_file(&spi->dev, &as1531_input[i].dev_attr);
> +	dev_set_drvdata(&spi->dev, NULL);
> +	mutex_unlock(&adc->lock);
> +	kfree(adc);
> +	return status;
> +}
> +
> +static int __devexit as1531_remove(struct spi_device *spi)
> +{
> +	struct as1531 *adc = dev_get_drvdata(&spi->dev);
> +	int i;
> +
> +	mutex_lock(&adc->lock);

Lock should not be needed here. If anything, it provides a false sense 
of security: Anything else waiting on it will be surprised to notice that 
the mutex (and memory) it is waiting on has been freed.
In other words, if this lock is really needed, you are in big trouble.

> +	hwmon_device_unregister(adc->hwmon_dev);
> +	for (i = 0; i < 8; i++)
> +		device_remove_file(&spi->dev, &as1531_input[i].dev_attr);
> +
> +	dev_set_drvdata(&spi->dev, NULL);
> +	mutex_unlock(&adc->lock);
> +	kfree(adc);
> +
> +	return 0;
> +}
> +
> +/* SPI structures */
> +
> +static struct spi_driver as1531_driver = {
> +	.driver = {
> +		.name	= "as1531",
> +		.owner	= THIS_MODULE,
> +	},
> +	.probe	= as1531_probe,
> +	.remove	= __devexit_p(as1531_remove),
> +};
> +
> +/* Init module */
> +
> +static int __init init_as1531(void)
> +{
> +	return spi_register_driver(&as1531_driver);
> +}
> +
> +static void __exit exit_as1531(void)
> +{
> +	spi_unregister_driver(&as1531_driver);
> +}
> +
> +module_init(init_as1531);
> +module_exit(exit_as1531);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Fabien Marteau <fabien.marteau@armadeus.com>");
> +MODULE_DESCRIPTION("Driver for AS1531 ADC");
> -- 
> 1.7.0.4
> 

Re: [lm-sensors] [PATCH] hwmon: Driver for as1531,

[Fabien Marteau]

Hi Guenter,

Thanks for the review.

On 16/05/2011 17:39, Guenter Roeck wrote:
> On Mon, May 16, 2011 at 09:39:14AM -0400, fabien.marteau@armadeus.com wrote:
>> From: Fabien Marteau <fabien.marteau@armadeus.com>
>>
>>
> Some description, such as "Dhis driver adds support for xxx" would be nice.
> 
> Also, I wonder if this driver belongs into hwmon in the first place. It is
> a generic ADC chip with high conversion rate. iio would probably be more
> appropriate and also much better in supporting high speed readings.
I provided this driver "as is" because it's a driver that work well on
our platform. I thought that iio was not stable enough driver framework
to be used.
I can rewrite it under iio framework but I have no time for this moment
to do that. You think it's better to wait for an iio driver or to
continue commiting this ?

regards,
Fabien M

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

Re: [PATCH] hwmon: Driver for as1531, Austria-Microsystem Analog to Digital Converter.

[Fabien Marteau]

Hi Guenter,

Thanks for the review.

On 16/05/2011 17:39, Guenter Roeck wrote:
> On Mon, May 16, 2011 at 09:39:14AM -0400, fabien.marteau@armadeus.com wrote:
>> From: Fabien Marteau <fabien.marteau@armadeus.com>
>>
>>
> Some description, such as "Dhis driver adds support for xxx" would be nice.
> 
> Also, I wonder if this driver belongs into hwmon in the first place. It is
> a generic ADC chip with high conversion rate. iio would probably be more
> appropriate and also much better in supporting high speed readings.
I provided this driver "as is" because it's a driver that work well on
our platform. I thought that iio was not stable enough driver framework
to be used.
I can rewrite it under iio framework but I have no time for this moment
to do that. You think it's better to wait for an iio driver or to
continue commiting this ?

regards,
Fabien M

Re: [lm-sensors] [PATCH] hwmon: Driver for as1531,

[Jonathan Cameron]

On 05/17/11 08:06, Fabien Marteau wrote:
> Hi Guenter,
> 
> Thanks for the review.
> 
> On 16/05/2011 17:39, Guenter Roeck wrote:
>> On Mon, May 16, 2011 at 09:39:14AM -0400, fabien.marteau@armadeus.com wrote:
>>> From: Fabien Marteau <fabien.marteau@armadeus.com>
>>>
>>>
>> Some description, such as "Dhis driver adds support for xxx" would be nice.
>>
>> Also, I wonder if this driver belongs into hwmon in the first place. It is
>> a generic ADC chip with high conversion rate. iio would probably be more
>> appropriate and also much better in supporting high speed readings.
> I provided this driver "as is" because it's a driver that work well on
> our platform. I thought that iio was not stable enough driver framework
> to be used.
> I can rewrite it under iio framework but I have no time for this moment
> to do that. You think it's better to wait for an iio driver or to
> continue commiting this ?
I'd say that if you primary use is hwmon, put it there for now and we can think
about moving it at a later date depending on how people are actually using it.
Guenter, would that be ok for you?

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

Re: [PATCH] hwmon: Driver for as1531, Austria-Microsystem Analog to Digital Converter.

[Jonathan Cameron]

On 05/17/11 08:06, Fabien Marteau wrote:
> Hi Guenter,
> 
> Thanks for the review.
> 
> On 16/05/2011 17:39, Guenter Roeck wrote:
>> On Mon, May 16, 2011 at 09:39:14AM -0400, fabien.marteau@armadeus.com wrote:
>>> From: Fabien Marteau <fabien.marteau@armadeus.com>
>>>
>>>
>> Some description, such as "Dhis driver adds support for xxx" would be nice.
>>
>> Also, I wonder if this driver belongs into hwmon in the first place. It is
>> a generic ADC chip with high conversion rate. iio would probably be more
>> appropriate and also much better in supporting high speed readings.
> I provided this driver "as is" because it's a driver that work well on
> our platform. I thought that iio was not stable enough driver framework
> to be used.
> I can rewrite it under iio framework but I have no time for this moment
> to do that. You think it's better to wait for an iio driver or to
> continue commiting this ?
I'd say that if you primary use is hwmon, put it there for now and we can think
about moving it at a later date depending on how people are actually using it.
Guenter, would that be ok for you?

Re: [lm-sensors] [PATCH] hwmon: Driver for as1531,

[Jonathan Cameron]

On 05/17/11 10:25, Jonathan Cameron wrote:
> On 05/17/11 08:06, Fabien Marteau wrote:
>> Hi Guenter,
>>
>> Thanks for the review.
>>
>> On 16/05/2011 17:39, Guenter Roeck wrote:
>>> On Mon, May 16, 2011 at 09:39:14AM -0400, fabien.marteau@armadeus.com wrote:
>>>> From: Fabien Marteau <fabien.marteau@armadeus.com>
>>>>
>>>>
>>> Some description, such as "Dhis driver adds support for xxx" would be nice.
>>>
>>> Also, I wonder if this driver belongs into hwmon in the first place. It is
>>> a generic ADC chip with high conversion rate. iio would probably be more
>>> appropriate and also much better in supporting high speed readings.
>> I provided this driver "as is" because it's a driver that work well on
>> our platform. I thought that iio was not stable enough driver framework
>> to be used.
>> I can rewrite it under iio framework but I have no time for this moment
>> to do that. You think it's better to wait for an iio driver or to
>> continue commiting this ?
> I'd say that if you primary use is hwmon, put it there for now and we can think
> about moving it at a later date depending on how people are actually using it.
> Guenter, would that be ok for you?
Having actually taken a look at the code, it's straight forward, so if you
are using it as a general purpose adc then I'm happy to port to IIO
sometimes soonish...  Will need some testing at somepoint though.

Jonathan

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

Re: [PATCH] hwmon: Driver for as1531, Austria-Microsystem Analog to Digital Converter.

[Jonathan Cameron]

On 05/17/11 10:25, Jonathan Cameron wrote:
> On 05/17/11 08:06, Fabien Marteau wrote:
>> Hi Guenter,
>>
>> Thanks for the review.
>>
>> On 16/05/2011 17:39, Guenter Roeck wrote:
>>> On Mon, May 16, 2011 at 09:39:14AM -0400, fabien.marteau@armadeus.com wrote:
>>>> From: Fabien Marteau <fabien.marteau@armadeus.com>
>>>>
>>>>
>>> Some description, such as "Dhis driver adds support for xxx" would be nice.
>>>
>>> Also, I wonder if this driver belongs into hwmon in the first place. It is
>>> a generic ADC chip with high conversion rate. iio would probably be more
>>> appropriate and also much better in supporting high speed readings.
>> I provided this driver "as is" because it's a driver that work well on
>> our platform. I thought that iio was not stable enough driver framework
>> to be used.
>> I can rewrite it under iio framework but I have no time for this moment
>> to do that. You think it's better to wait for an iio driver or to
>> continue commiting this ?
> I'd say that if you primary use is hwmon, put it there for now and we can think
> about moving it at a later date depending on how people are actually using it.
> Guenter, would that be ok for you?
Having actually taken a look at the code, it's straight forward, so if you
are using it as a general purpose adc then I'm happy to port to IIO
sometimes soonish...  Will need some testing at somepoint though.

Jonathan

Re: [lm-sensors] [PATCH] hwmon: Driver for as1531,

[Fabien Marteau]

On 17/05/2011 11:34, Jonathan Cameron wrote:
> On 05/17/11 10:25, Jonathan Cameron wrote:
>> On 05/17/11 08:06, Fabien Marteau wrote:
>>> Hi Guenter,
>>>
>>> Thanks for the review.
>>>
>>> On 16/05/2011 17:39, Guenter Roeck wrote:
>>>> On Mon, May 16, 2011 at 09:39:14AM -0400, fabien.marteau@armadeus.com wrote:
>>>>> From: Fabien Marteau <fabien.marteau@armadeus.com>
>>>>>
>>>>>
>>>> Some description, such as "Dhis driver adds support for xxx" would be nice.
>>>>
>>>> Also, I wonder if this driver belongs into hwmon in the first place. It is
>>>> a generic ADC chip with high conversion rate. iio would probably be more
>>>> appropriate and also much better in supporting high speed readings.
>>> I provided this driver "as is" because it's a driver that work well on
>>> our platform. I thought that iio was not stable enough driver framework
>>> to be used.
>>> I can rewrite it under iio framework but I have no time for this moment
>>> to do that. You think it's better to wait for an iio driver or to
>>> continue commiting this ?
>> I'd say that if you primary use is hwmon, put it there for now and we can think
>> about moving it at a later date depending on how people are actually using it.
>> Guenter, would that be ok for you?
> Having actually taken a look at the code, it's straight forward, so if you
> are using it as a general purpose adc then I'm happy to port to IIO
> sometimes soonish...  Will need some testing at somepoint though.
I've got it on my platform, then I can test it without problem if you want.

Fabien
> 
> Jonathan
> 


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

Re: [PATCH] hwmon: Driver for as1531, Austria-Microsystem Analog to Digital Converter.

[Fabien Marteau]

On 17/05/2011 11:34, Jonathan Cameron wrote:
> On 05/17/11 10:25, Jonathan Cameron wrote:
>> On 05/17/11 08:06, Fabien Marteau wrote:
>>> Hi Guenter,
>>>
>>> Thanks for the review.
>>>
>>> On 16/05/2011 17:39, Guenter Roeck wrote:
>>>> On Mon, May 16, 2011 at 09:39:14AM -0400, fabien.marteau@armadeus.com wrote:
>>>>> From: Fabien Marteau <fabien.marteau@armadeus.com>
>>>>>
>>>>>
>>>> Some description, such as "Dhis driver adds support for xxx" would be nice.
>>>>
>>>> Also, I wonder if this driver belongs into hwmon in the first place. It is
>>>> a generic ADC chip with high conversion rate. iio would probably be more
>>>> appropriate and also much better in supporting high speed readings.
>>> I provided this driver "as is" because it's a driver that work well on
>>> our platform. I thought that iio was not stable enough driver framework
>>> to be used.
>>> I can rewrite it under iio framework but I have no time for this moment
>>> to do that. You think it's better to wait for an iio driver or to
>>> continue commiting this ?
>> I'd say that if you primary use is hwmon, put it there for now and we can think
>> about moving it at a later date depending on how people are actually using it.
>> Guenter, would that be ok for you?
> Having actually taken a look at the code, it's straight forward, so if you
> are using it as a general purpose adc then I'm happy to port to IIO
> sometimes soonish...  Will need some testing at somepoint though.
I've got it on my platform, then I can test it without problem if you want.

Fabien
> 
> Jonathan
> 

[PATCH 0/2] staging:iio:adc:as1531 driver port from hwmon driver.

[Jonathan Cameron]

Dear All,

This is a quick initial port of the as1531 driver Fabien submitted
to the lm-sensors mailing list.

Based on top of a large series I just sent to Greg KH and posted to
linux-iio. That has a few other dependencies, so might be easier
to just to wait for them all to be in a standard tree (linux-next
or mainline).

Dropped from hwmon driver - max and min values.

Stuff to add at some point (i.e. when needed)

1) Differential channels - trivial to do, just add a few more entries to
the channels array and some signed bits in read_raw

2) Buffered read support - straight forward case where all channels have
to be explicitly requested anyway.

3) Other devices trivially added:
as1530 is directly compatible.
as1532/1533 look like 4 channel equivalents (add iio_chan_spec arrays).

The other similarly numbered parts are different enough to need their
own drivers.

Fabien, as more of the code in the first patch is yours than mine,
I've put you as author of that one.

The second is mostly stuff you'd have gotten in review to the hwmon
driver anyway.


Fabien Marteau (1):
  staging:iio:adc: as1531 driver initial conversion from hwmon
    submission.

Jonathan Cameron (1):
  staging:iio:adc:as1351 general cleanup and conversion to standard
    functions.

 drivers/staging/iio/adc/Kconfig  |   10 ++
 drivers/staging/iio/adc/Makefile |    2 +
 drivers/staging/iio/adc/as1531.c |  168 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 180 insertions(+), 0 deletions(-)
 create mode 100644 drivers/staging/iio/adc/as1531.c

-- 
1.7.3.4

[lm-sensors] [PATCH 0/2] staging:iio:adc:as1531 driver port from

[Jonathan Cameron]

Dear All,

This is a quick initial port of the as1531 driver Fabien submitted
to the lm-sensors mailing list.

Based on top of a large series I just sent to Greg KH and posted to
linux-iio. That has a few other dependencies, so might be easier
to just to wait for them all to be in a standard tree (linux-next
or mainline).

Dropped from hwmon driver - max and min values.

Stuff to add at some point (i.e. when needed)

1) Differential channels - trivial to do, just add a few more entries to
the channels array and some signed bits in read_raw

2) Buffered read support - straight forward case where all channels have
to be explicitly requested anyway.

3) Other devices trivially added:
as1530 is directly compatible.
as1532/1533 look like 4 channel equivalents (add iio_chan_spec arrays).

The other similarly numbered parts are different enough to need their
own drivers.

Fabien, as more of the code in the first patch is yours than mine,
I've put you as author of that one.

The second is mostly stuff you'd have gotten in review to the hwmon
driver anyway.


Fabien Marteau (1):
  staging:iio:adc: as1531 driver initial conversion from hwmon
    submission.

Jonathan Cameron (1):
  staging:iio:adc:as1351 general cleanup and conversion to standard
    functions.

 drivers/staging/iio/adc/Kconfig  |   10 ++
 drivers/staging/iio/adc/Makefile |    2 +
 drivers/staging/iio/adc/as1531.c |  168 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 180 insertions(+), 0 deletions(-)
 create mode 100644 drivers/staging/iio/adc/as1531.c

-- 
1.7.3.4


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

[PATCH V2 0/2] staging:iio:adc:as1531 driver port from hwmon driver.

[Jonathan Cameron]

V2: Fix the various issues Guenter found.

Original message -

Dear All,

This is a quick initial port of the as1531 driver Fabien submitted
to the lm-sensors mailing list.

Based on top of a large series I just sent to Greg KH and posted to
linux-iio. That has a few other dependencies, so might be easier
to just to wait for them all to be in a standard tree (linux-next
or mainline).

Dropped from hwmon driver - max and min values.

Stuff to add at some point (i.e. when needed)

1) Differential channels - trivial to do, just add a few more entries to
the channels array and some signed bits in read_raw

2) Buffered read support - straight forward case where all channels have
to be explicitly requested anyway.

3) Other devices trivially added:
as1530 is directly compatible.
as1532/1533 look like 4 channel equivalents (add iio_chan_spec arrays).

The other similarly numbered parts are different enough to need their
own drivers.

Fabien, as more of the code in the first patch is yours than mine,
I've put you as author of that one.

The second is mostly stuff you'd have gotten in review to the hwmon
driver anyway.

Jonathan

Fabien Marteau (1):
  staging:iio:adc: as1531 driver initial conversion from hwmon
    submission.

Jonathan Cameron (1):
  staging:iio:adc:as1351 general cleanup and conversion to standard
    functions.

 drivers/staging/iio/adc/Kconfig  |   10 +++
 drivers/staging/iio/adc/Makefile |    2 +
 drivers/staging/iio/adc/as1531.c |  166 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 178 insertions(+), 0 deletions(-)
 create mode 100644 drivers/staging/iio/adc/as1531.c

-- 
1.7.3.4

[lm-sensors] [PATCH V2 0/2] staging:iio:adc:as1531 driver port from

[Jonathan Cameron]

V2: Fix the various issues Guenter found.

Original message -

Dear All,

This is a quick initial port of the as1531 driver Fabien submitted
to the lm-sensors mailing list.

Based on top of a large series I just sent to Greg KH and posted to
linux-iio. That has a few other dependencies, so might be easier
to just to wait for them all to be in a standard tree (linux-next
or mainline).

Dropped from hwmon driver - max and min values.

Stuff to add at some point (i.e. when needed)

1) Differential channels - trivial to do, just add a few more entries to
the channels array and some signed bits in read_raw

2) Buffered read support - straight forward case where all channels have
to be explicitly requested anyway.

3) Other devices trivially added:
as1530 is directly compatible.
as1532/1533 look like 4 channel equivalents (add iio_chan_spec arrays).

The other similarly numbered parts are different enough to need their
own drivers.

Fabien, as more of the code in the first patch is yours than mine,
I've put you as author of that one.

The second is mostly stuff you'd have gotten in review to the hwmon
driver anyway.

Jonathan

Fabien Marteau (1):
  staging:iio:adc: as1531 driver initial conversion from hwmon
    submission.

Jonathan Cameron (1):
  staging:iio:adc:as1351 general cleanup and conversion to standard
    functions.

 drivers/staging/iio/adc/Kconfig  |   10 +++
 drivers/staging/iio/adc/Makefile |    2 +
 drivers/staging/iio/adc/as1531.c |  166 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 178 insertions(+), 0 deletions(-)
 create mode 100644 drivers/staging/iio/adc/as1531.c

-- 
1.7.3.4


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

Re: [PATCH V2 0/2] staging:iio:adc:as1531 driver port from hwmon driver.

[Fabien Marteau]

Dear Jonathan,

Thanks for all the work you done to re-write this driver under iio. I
will try to find time this week to test this new version under my platform.

It was just a little tricky hwmon driver, it's really nice that you
spend time to write a good driver.

Fabien M

On 18/05/2011 18:16, Jonathan Cameron wrote:
> V2: Fix the various issues Guenter found.
> 
> Original message -
> 
> Dear All,
> 
> This is a quick initial port of the as1531 driver Fabien submitted
> to the lm-sensors mailing list.
> 
> Based on top of a large series I just sent to Greg KH and posted to
> linux-iio. That has a few other dependencies, so might be easier
> to just to wait for them all to be in a standard tree (linux-next
> or mainline).
> 
> Dropped from hwmon driver - max and min values.
> 
> Stuff to add at some point (i.e. when needed)
> 
> 1) Differential channels - trivial to do, just add a few more entries to
> the channels array and some signed bits in read_raw
> 
> 2) Buffered read support - straight forward case where all channels have
> to be explicitly requested anyway.
> 
> 3) Other devices trivially added:
> as1530 is directly compatible.
> as1532/1533 look like 4 channel equivalents (add iio_chan_spec arrays).
> 
> The other similarly numbered parts are different enough to need their
> own drivers.
> 
> Fabien, as more of the code in the first patch is yours than mine,
> I've put you as author of that one.
> 
> The second is mostly stuff you'd have gotten in review to the hwmon
> driver anyway.
> 
> Jonathan
> 
> Fabien Marteau (1):
>   staging:iio:adc: as1531 driver initial conversion from hwmon
>     submission.
> 
> Jonathan Cameron (1):
>   staging:iio:adc:as1351 general cleanup and conversion to standard
>     functions.
> 
>  drivers/staging/iio/adc/Kconfig  |   10 +++
>  drivers/staging/iio/adc/Makefile |    2 +
>  drivers/staging/iio/adc/as1531.c |  166 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 178 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/staging/iio/adc/as1531.c
> 

Re: [lm-sensors] [PATCH V2 0/2] staging:iio:adc:as1531 driver port

[Fabien Marteau]

Dear Jonathan,

Thanks for all the work you done to re-write this driver under iio. I
will try to find time this week to test this new version under my platform.

It was just a little tricky hwmon driver, it's really nice that you
spend time to write a good driver.

Fabien M

On 18/05/2011 18:16, Jonathan Cameron wrote:
> V2: Fix the various issues Guenter found.
> 
> Original message -
> 
> Dear All,
> 
> This is a quick initial port of the as1531 driver Fabien submitted
> to the lm-sensors mailing list.
> 
> Based on top of a large series I just sent to Greg KH and posted to
> linux-iio. That has a few other dependencies, so might be easier
> to just to wait for them all to be in a standard tree (linux-next
> or mainline).
> 
> Dropped from hwmon driver - max and min values.
> 
> Stuff to add at some point (i.e. when needed)
> 
> 1) Differential channels - trivial to do, just add a few more entries to
> the channels array and some signed bits in read_raw
> 
> 2) Buffered read support - straight forward case where all channels have
> to be explicitly requested anyway.
> 
> 3) Other devices trivially added:
> as1530 is directly compatible.
> as1532/1533 look like 4 channel equivalents (add iio_chan_spec arrays).
> 
> The other similarly numbered parts are different enough to need their
> own drivers.
> 
> Fabien, as more of the code in the first patch is yours than mine,
> I've put you as author of that one.
> 
> The second is mostly stuff you'd have gotten in review to the hwmon
> driver anyway.
> 
> Jonathan
> 
> Fabien Marteau (1):
>   staging:iio:adc: as1531 driver initial conversion from hwmon
>     submission.
> 
> Jonathan Cameron (1):
>   staging:iio:adc:as1351 general cleanup and conversion to standard
>     functions.
> 
>  drivers/staging/iio/adc/Kconfig  |   10 +++
>  drivers/staging/iio/adc/Makefile |    2 +
>  drivers/staging/iio/adc/as1531.c |  166 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 178 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/staging/iio/adc/as1531.c
> 


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

[PATCH 1/2] staging:iio:adc: as1531 driver initial conversion from hwmon submission.

[Jonathan Cameron]

From: Fabien Marteau <fabien.marteau@armadeus.com>

V2: Fix double mutex unlock and whitespace (Guenter Roeck).
Signed-off-by: Jonathan Cameron <jic23@cam.ac.uk>
---
 drivers/staging/iio/adc/Kconfig  |   10 ++
 drivers/staging/iio/adc/Makefile |    2 +
 drivers/staging/iio/adc/as1531.c |  209 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 221 insertions(+), 0 deletions(-)

diff --git a/drivers/staging/iio/adc/Kconfig b/drivers/staging/iio/adc/Kconfig
index 8c751c4..4af730a 100644
--- a/drivers/staging/iio/adc/Kconfig
+++ b/drivers/staging/iio/adc/Kconfig
@@ -168,6 +168,16 @@ config ADT7410
 	  Say yes here to build support for Analog Devices ADT7410
 	  temperature sensors.
 
+config AS1531
+tristate "Austria Microsystems AS1531 Analog to Digital Converter"
+	depends on SPI_MASTER
+	help
+	  If you say yes here you get support for Austria Microsystems AS1531.
+		AS1531 is a 12 bits Analog to digitals converter with 8 channels
+		provided by Austria-Microsystems company.
+	  This driver can also be built as a module.  If so, the module
+	  will be called as1531.
+
 config MAX1363
 	tristate "Maxim max1363 ADC driver"
 	depends on I2C
diff --git a/drivers/staging/iio/adc/Makefile b/drivers/staging/iio/adc/Makefile
index 1d9b3f5..027b94e2 100644
--- a/drivers/staging/iio/adc/Makefile
+++ b/drivers/staging/iio/adc/Makefile
@@ -39,3 +39,5 @@ obj-$(CONFIG_AD7816) += ad7816.o
 obj-$(CONFIG_ADT75) += adt75.o
 obj-$(CONFIG_ADT7310) += adt7310.o
 obj-$(CONFIG_ADT7410) += adt7410.o
+
+obj-$(CONFIG_AS1531) += as1531.o
\ No newline at end of file
diff --git a/drivers/staging/iio/adc/as1531.c b/drivers/staging/iio/adc/as1531.c
new file mode 100644
index 0000000..1724c39
--- /dev/null
+++ b/drivers/staging/iio/adc/as1531.c
@@ -0,0 +1,209 @@
+/*
+ * as1531.c
+ *
+ * Driver for Austria-Microsystem Analog to Digital Converter.
+ *
+ * Copyright (c) 2010, 2011 Fabien Marteau <fabien.marteau@armadeus.com>
+ * Driver based on Marc Pignat <marc.pignat@hevs.ch> adcxx.c driver.
+ *
+ * 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/device.h>
+#include <linux/kernel.h>
+#include <linux/sysfs.h>
+#include <linux/spi/spi.h>
+
+#include "../iio.h"
+#include "../sysfs.h"
+#include "adc.h"
+
+#define AS1531_SPI_SPEED 64941
+#define AS1531_MAX_VALUE 2500
+
+#define AS1531_START_BIT	(0x80)
+#define AS1531_CHAN0		(0<<4)
+#define AS1531_CHAN1		(4<<4)
+#define AS1531_CHAN2		(1<<4)
+#define AS1531_CHAN3		(5<<4)
+#define AS1531_CHAN4		(2<<4)
+#define AS1531_CHAN5		(6<<4)
+#define AS1531_CHAN6		(3<<4)
+#define AS1531_CHAN7		(7<<4)
+
+#define AS1531_RANGE_0_TO_VREF			(1<<3)
+#define AS1531_RANGE_HALFVREF_TO_HALFVREF	(0<<3)
+
+#define AS1531_MODE_COM		(1<<2)
+#define AS1531_MODE_DIFF	(0<<2)
+
+#define AS1531_POWER_DOWN		0x0
+#define AS1531_POWER_REDUCED		0x1
+#define AS1531_POWER_REDUCED_BIS	0x2
+#define AS1531_POWER_NORMAL		0x3
+
+struct as1531_state {
+	struct spi_device *spi;
+	struct mutex lock;
+};
+
+static int as1531_message(struct spi_device *spi, int cmd, int *ret_value)
+{
+	struct spi_message	message;
+	struct spi_transfer	x[1];
+	int status, i;
+	u8	cmd_send;
+	unsigned char buf[64];
+	unsigned char buf_read[64];
+
+	cmd_send = cmd;
+
+	spi_message_init(&message);
+	memset(x, 0, sizeof x);
+	memset(buf, 0, sizeof(buf));
+	memset(buf_read, 0, sizeof(buf_read));
+
+	for (i = 0; i < 8; i++) {
+		buf[i] = ((cmd_send & 0x80)>>7);
+		cmd_send = cmd_send << 1;
+	}
+
+	x[0].tx_buf = buf;
+	x[0].len = 24;
+	x[0].rx_buf = buf_read;
+	x[0].speed_hz = AS1531_SPI_SPEED;
+	x[0].bits_per_word = 1;
+	spi_message_add_tail(&x[0], &message);
+
+	status = spi_sync(spi, &message);
+	if (status < 0)
+		return status;
+
+	*ret_value = buf_read[11] & 0x01;
+	for (i = 12; i < 23 ; i++) {
+		*ret_value = *ret_value << 1;
+		*ret_value = *ret_value | (buf_read[i]&0x01);
+	}
+
+	return 0;
+}
+
+static int as1531_read_raw(struct iio_dev *indio_dev,
+			   struct iio_chan_spec const *chan,
+			   int *val,
+			   int *val2,
+			   long m)
+{
+	int status = 0;
+	int ret_value = 0;
+	struct as1531_state *st = iio_priv(indio_dev);
+
+	if (mutex_lock_interruptible(&st->lock))
+		return -ERESTARTSYS;
+
+	status = as1531_message(st->spi,
+				AS1531_START_BIT | chan->address |
+				AS1531_RANGE_0_TO_VREF | AS1531_MODE_COM |
+				AS1531_POWER_NORMAL,
+				&ret_value);
+	mutex_unlock(&st->lock);
+	if (status < 0)
+		return status;
+
+	*val = ret_value*2500/4096;
+
+	return IIO_VAL_INT;
+}
+
+static const struct iio_chan_spec as1531_channels[] = {
+	IIO_CHAN(IIO_IN, 0, 1, 1, NULL, 0, 1, 0, AS1531_CHAN0, 0, {}, 0),
+	IIO_CHAN(IIO_IN, 0, 1, 1, NULL, 1, 1, 0, AS1531_CHAN1, 1, {}, 0),
+	IIO_CHAN(IIO_IN, 0, 1, 1, NULL, 2, 1, 0, AS1531_CHAN2, 2, {}, 0),
+	IIO_CHAN(IIO_IN, 0, 1, 1, NULL, 3, 1, 0, AS1531_CHAN3, 3, {}, 0),
+	IIO_CHAN(IIO_IN, 0, 1, 1, NULL, 4, 1, 0, AS1531_CHAN4, 4, {}, 0),
+	IIO_CHAN(IIO_IN, 0, 1, 1, NULL, 5, 1, 0, AS1531_CHAN5, 5, {}, 0),
+	IIO_CHAN(IIO_IN, 0, 1, 1, NULL, 6, 1, 0, AS1531_CHAN6, 6, {}, 0),
+	IIO_CHAN(IIO_IN, 0, 1, 1, NULL, 7, 1, 0, AS1531_CHAN7, 7, {}, 0),
+};
+
+static const struct iio_info as1531_info = {
+	.read_raw = &as1531_read_raw,
+	.driver_module = THIS_MODULE,
+};
+
+static int __devinit as1531_probe(struct spi_device *spi)
+{
+	int ret;
+	struct as1531_state *st;
+	struct iio_dev *indio_dev = iio_allocate_device(sizeof(*st));
+
+	if (indio_dev == NULL)
+		return -ENOMEM;
+
+	st = iio_priv(indio_dev);
+	mutex_init(&st->lock);
+	st->spi = spi;
+
+	spi_set_drvdata(spi, indio_dev);
+	indio_dev->dev.parent = &spi->dev;
+	indio_dev->name = spi_get_device_id(spi)->name;
+	indio_dev->info = &as1531_info;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->channels = as1531_channels;
+	indio_dev->num_channels = ARRAY_SIZE(as1531_channels);
+
+	ret = iio_device_register(indio_dev);
+	if (ret < 0)
+		iio_free_device(indio_dev);
+	return ret;
+}
+
+static int as1531_remove(struct spi_device *spi)
+{
+	struct iio_dev *indio_dev = spi_get_drvdata(spi);
+
+	iio_device_unregister(indio_dev);
+	return 0;
+}
+
+static const struct spi_device_id as1531_id[] = {
+	{"as1531", 0},
+	{}
+};
+
+static struct spi_driver as1531_driver = {
+	.driver = {
+		.name	= "as1531",
+		.owner	= THIS_MODULE,
+	},
+	.probe	= as1531_probe,
+	.remove	= __devexit_p(as1531_remove),
+	.id_table = as1531_id,
+};
+
+static int __init init_as1531(void)
+{
+	return spi_register_driver(&as1531_driver);
+}
+module_init(init_as1531);
+
+static void __exit exit_as1531(void)
+{
+	spi_unregister_driver(&as1531_driver);
+}
+module_exit(exit_as1531);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Fabien Marteau <fabien.marteau@armadeus.com>");
+MODULE_DESCRIPTION("Driver for AS1531 ADC");
-- 
1.7.3.4

[lm-sensors] [PATCH 1/2] staging:iio:adc: as1

[Jonathan Cameron]

From: Fabien Marteau <fabien.marteau@armadeus.com>

V2: Fix double mutex unlock and whitespace (Guenter Roeck).
Signed-off-by: Jonathan Cameron <jic23@cam.ac.uk>
---
 drivers/staging/iio/adc/Kconfig  |   10 ++
 drivers/staging/iio/adc/Makefile |    2 +
 drivers/staging/iio/adc/as1531.c |  209 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 221 insertions(+), 0 deletions(-)

diff --git a/drivers/staging/iio/adc/Kconfig b/drivers/staging/iio/adc/Kconfig
index 8c751c4..4af730a 100644
--- a/drivers/staging/iio/adc/Kconfig
+++ b/drivers/staging/iio/adc/Kconfig
@@ -168,6 +168,16 @@ config ADT7410
 	  Say yes here to build support for Analog Devices ADT7410
 	  temperature sensors.
 
+config AS1531
+tristate "Austria Microsystems AS1531 Analog to Digital Converter"
+	depends on SPI_MASTER
+	help
+	  If you say yes here you get support for Austria Microsystems AS1531.
+		AS1531 is a 12 bits Analog to digitals converter with 8 channels
+		provided by Austria-Microsystems company.
+	  This driver can also be built as a module.  If so, the module
+	  will be called as1531.
+
 config MAX1363
 	tristate "Maxim max1363 ADC driver"
 	depends on I2C
diff --git a/drivers/staging/iio/adc/Makefile b/drivers/staging/iio/adc/Makefile
index 1d9b3f5..027b94e2 100644
--- a/drivers/staging/iio/adc/Makefile
+++ b/drivers/staging/iio/adc/Makefile
@@ -39,3 +39,5 @@ obj-$(CONFIG_AD7816) += ad7816.o
 obj-$(CONFIG_ADT75) += adt75.o
 obj-$(CONFIG_ADT7310) += adt7310.o
 obj-$(CONFIG_ADT7410) += adt7410.o
+
+obj-$(CONFIG_AS1531) += as1531.o
\ No newline at end of file
diff --git a/drivers/staging/iio/adc/as1531.c b/drivers/staging/iio/adc/as1531.c
new file mode 100644
index 0000000..1724c39
--- /dev/null
+++ b/drivers/staging/iio/adc/as1531.c
@@ -0,0 +1,209 @@
+/*
+ * as1531.c
+ *
+ * Driver for Austria-Microsystem Analog to Digital Converter.
+ *
+ * Copyright (c) 2010, 2011 Fabien Marteau <fabien.marteau@armadeus.com>
+ * Driver based on Marc Pignat <marc.pignat@hevs.ch> adcxx.c driver.
+ *
+ * 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/device.h>
+#include <linux/kernel.h>
+#include <linux/sysfs.h>
+#include <linux/spi/spi.h>
+
+#include "../iio.h"
+#include "../sysfs.h"
+#include "adc.h"
+
+#define AS1531_SPI_SPEED 64941
+#define AS1531_MAX_VALUE 2500
+
+#define AS1531_START_BIT	(0x80)
+#define AS1531_CHAN0		(0<<4)
+#define AS1531_CHAN1		(4<<4)
+#define AS1531_CHAN2		(1<<4)
+#define AS1531_CHAN3		(5<<4)
+#define AS1531_CHAN4		(2<<4)
+#define AS1531_CHAN5		(6<<4)
+#define AS1531_CHAN6		(3<<4)
+#define AS1531_CHAN7		(7<<4)
+
+#define AS1531_RANGE_0_TO_VREF			(1<<3)
+#define AS1531_RANGE_HALFVREF_TO_HALFVREF	(0<<3)
+
+#define AS1531_MODE_COM		(1<<2)
+#define AS1531_MODE_DIFF	(0<<2)
+
+#define AS1531_POWER_DOWN		0x0
+#define AS1531_POWER_REDUCED		0x1
+#define AS1531_POWER_REDUCED_BIS	0x2
+#define AS1531_POWER_NORMAL		0x3
+
+struct as1531_state {
+	struct spi_device *spi;
+	struct mutex lock;
+};
+
+static int as1531_message(struct spi_device *spi, int cmd, int *ret_value)
+{
+	struct spi_message	message;
+	struct spi_transfer	x[1];
+	int status, i;
+	u8	cmd_send;
+	unsigned char buf[64];
+	unsigned char buf_read[64];
+
+	cmd_send = cmd;
+
+	spi_message_init(&message);
+	memset(x, 0, sizeof x);
+	memset(buf, 0, sizeof(buf));
+	memset(buf_read, 0, sizeof(buf_read));
+
+	for (i = 0; i < 8; i++) {
+		buf[i] = ((cmd_send & 0x80)>>7);
+		cmd_send = cmd_send << 1;
+	}
+
+	x[0].tx_buf = buf;
+	x[0].len = 24;
+	x[0].rx_buf = buf_read;
+	x[0].speed_hz = AS1531_SPI_SPEED;
+	x[0].bits_per_word = 1;
+	spi_message_add_tail(&x[0], &message);
+
+	status = spi_sync(spi, &message);
+	if (status < 0)
+		return status;
+
+	*ret_value = buf_read[11] & 0x01;
+	for (i = 12; i < 23 ; i++) {
+		*ret_value = *ret_value << 1;
+		*ret_value = *ret_value | (buf_read[i]&0x01);
+	}
+
+	return 0;
+}
+
+static int as1531_read_raw(struct iio_dev *indio_dev,
+			   struct iio_chan_spec const *chan,
+			   int *val,
+			   int *val2,
+			   long m)
+{
+	int status = 0;
+	int ret_value = 0;
+	struct as1531_state *st = iio_priv(indio_dev);
+
+	if (mutex_lock_interruptible(&st->lock))
+		return -ERESTARTSYS;
+
+	status = as1531_message(st->spi,
+				AS1531_START_BIT | chan->address |
+				AS1531_RANGE_0_TO_VREF | AS1531_MODE_COM |
+				AS1531_POWER_NORMAL,
+				&ret_value);
+	mutex_unlock(&st->lock);
+	if (status < 0)
+		return status;
+
+	*val = ret_value*2500/4096;
+
+	return IIO_VAL_INT;
+}
+
+static const struct iio_chan_spec as1531_channels[] = {
+	IIO_CHAN(IIO_IN, 0, 1, 1, NULL, 0, 1, 0, AS1531_CHAN0, 0, {}, 0),
+	IIO_CHAN(IIO_IN, 0, 1, 1, NULL, 1, 1, 0, AS1531_CHAN1, 1, {}, 0),
+	IIO_CHAN(IIO_IN, 0, 1, 1, NULL, 2, 1, 0, AS1531_CHAN2, 2, {}, 0),
+	IIO_CHAN(IIO_IN, 0, 1, 1, NULL, 3, 1, 0, AS1531_CHAN3, 3, {}, 0),
+	IIO_CHAN(IIO_IN, 0, 1, 1, NULL, 4, 1, 0, AS1531_CHAN4, 4, {}, 0),
+	IIO_CHAN(IIO_IN, 0, 1, 1, NULL, 5, 1, 0, AS1531_CHAN5, 5, {}, 0),
+	IIO_CHAN(IIO_IN, 0, 1, 1, NULL, 6, 1, 0, AS1531_CHAN6, 6, {}, 0),
+	IIO_CHAN(IIO_IN, 0, 1, 1, NULL, 7, 1, 0, AS1531_CHAN7, 7, {}, 0),
+};
+
+static const struct iio_info as1531_info = {
+	.read_raw = &as1531_read_raw,
+	.driver_module = THIS_MODULE,
+};
+
+static int __devinit as1531_probe(struct spi_device *spi)
+{
+	int ret;
+	struct as1531_state *st;
+	struct iio_dev *indio_dev = iio_allocate_device(sizeof(*st));
+
+	if (indio_dev = NULL)
+		return -ENOMEM;
+
+	st = iio_priv(indio_dev);
+	mutex_init(&st->lock);
+	st->spi = spi;
+
+	spi_set_drvdata(spi, indio_dev);
+	indio_dev->dev.parent = &spi->dev;
+	indio_dev->name = spi_get_device_id(spi)->name;
+	indio_dev->info = &as1531_info;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->channels = as1531_channels;
+	indio_dev->num_channels = ARRAY_SIZE(as1531_channels);
+
+	ret = iio_device_register(indio_dev);
+	if (ret < 0)
+		iio_free_device(indio_dev);
+	return ret;
+}
+
+static int as1531_remove(struct spi_device *spi)
+{
+	struct iio_dev *indio_dev = spi_get_drvdata(spi);
+
+	iio_device_unregister(indio_dev);
+	return 0;
+}
+
+static const struct spi_device_id as1531_id[] = {
+	{"as1531", 0},
+	{}
+};
+
+static struct spi_driver as1531_driver = {
+	.driver = {
+		.name	= "as1531",
+		.owner	= THIS_MODULE,
+	},
+	.probe	= as1531_probe,
+	.remove	= __devexit_p(as1531_remove),
+	.id_table = as1531_id,
+};
+
+static int __init init_as1531(void)
+{
+	return spi_register_driver(&as1531_driver);
+}
+module_init(init_as1531);
+
+static void __exit exit_as1531(void)
+{
+	spi_unregister_driver(&as1531_driver);
+}
+module_exit(exit_as1531);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Fabien Marteau <fabien.marteau@armadeus.com>");
+MODULE_DESCRIPTION("Driver for AS1531 ADC");
-- 
1.7.3.4


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

Re: [PATCH 1/2] staging:iio:adc: as1531 driver initial conversion from hwmon submission.

[Guenter Roeck]

On Wed, May 18, 2011 at 12:16:59PM -0400, Jonathan Cameron wrote:
> From: Fabien Marteau <fabien.marteau@armadeus.com>
>=20
> V2: Fix double mutex unlock and whitespace (Guenter Roeck).

Should be below ---

> Signed-off-by: Jonathan Cameron <jic23@cam.ac.uk>

and nitpick below, otherwise=20

Revewed-by: Guenter Roeck <guenter.roeck@ericsson.com>

> ---
>  drivers/staging/iio/adc/Kconfig  |   10 ++
>  drivers/staging/iio/adc/Makefile |    2 +
>  drivers/staging/iio/adc/as1531.c |  209 ++++++++++++++++++++++++++++++=
++++++++
>  3 files changed, 221 insertions(+), 0 deletions(-)
>=20
> diff --git a/drivers/staging/iio/adc/Kconfig b/drivers/staging/iio/adc/=
Kconfig
> index 8c751c4..4af730a 100644
> --- a/drivers/staging/iio/adc/Kconfig
> +++ b/drivers/staging/iio/adc/Kconfig
> @@ -168,6 +168,16 @@ config ADT7410
>  	  Say yes here to build support for Analog Devices ADT7410
>  	  temperature sensors.
> =20
> +config AS1531
> +tristate "Austria Microsystems AS1531 Analog to Digital Converter"
> +	depends on SPI_MASTER
> +	help
> +	  If you say yes here you get support for Austria Microsystems AS1531.
> +		AS1531 is a 12=A0bits Analog to digitals converter with 8 channels
> +		provided by Austria-Microsystems company.
> +	  This driver can also be built as a module.  If so, the module
> +	  will be called as1531.
> +
>  config MAX1363
>  	tristate "Maxim max1363 ADC driver"
>  	depends on I2C
> diff --git a/drivers/staging/iio/adc/Makefile b/drivers/staging/iio/adc=
/Makefile
> index 1d9b3f5..027b94e2 100644
> --- a/drivers/staging/iio/adc/Makefile
> +++ b/drivers/staging/iio/adc/Makefile
> @@ -39,3 +39,5 @@ obj-$(CONFIG_AD7816) +=3D ad7816.o
>  obj-$(CONFIG_ADT75) +=3D adt75.o
>  obj-$(CONFIG_ADT7310) +=3D adt7310.o
>  obj-$(CONFIG_ADT7410) +=3D adt7410.o
> +
> +obj-$(CONFIG_AS1531) +=3D as1531.o
> \ No newline at end of file

What does this mean ? Is there a missing newline ?

Thanks,
Guenter

Re: [lm-sensors] [PATCH 1/2] staging:iio:adc: as1531 driver initial

[Guenter Roeck]

On Wed, May 18, 2011 at 12:16:59PM -0400, Jonathan Cameron wrote:
> From: Fabien Marteau <fabien.marteau@armadeus.com>
> 
> V2: Fix double mutex unlock and whitespace (Guenter Roeck).

Should be below ---

> Signed-off-by: Jonathan Cameron <jic23@cam.ac.uk>

and nitpick below, otherwise 

Revewed-by: Guenter Roeck <guenter.roeck@ericsson.com>

> ---
>  drivers/staging/iio/adc/Kconfig  |   10 ++
>  drivers/staging/iio/adc/Makefile |    2 +
>  drivers/staging/iio/adc/as1531.c |  209 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 221 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/staging/iio/adc/Kconfig b/drivers/staging/iio/adc/Kconfig
> index 8c751c4..4af730a 100644
> --- a/drivers/staging/iio/adc/Kconfig
> +++ b/drivers/staging/iio/adc/Kconfig
> @@ -168,6 +168,16 @@ config ADT7410
>  	  Say yes here to build support for Analog Devices ADT7410
>  	  temperature sensors.
>  
> +config AS1531
> +tristate "Austria Microsystems AS1531 Analog to Digital Converter"
> +	depends on SPI_MASTER
> +	help
> +	  If you say yes here you get support for Austria Microsystems AS1531.
> +		AS1531 is a 12 bits Analog to digitals converter with 8 channels
> +		provided by Austria-Microsystems company.
> +	  This driver can also be built as a module.  If so, the module
> +	  will be called as1531.
> +
>  config MAX1363
>  	tristate "Maxim max1363 ADC driver"
>  	depends on I2C
> diff --git a/drivers/staging/iio/adc/Makefile b/drivers/staging/iio/adc/Makefile
> index 1d9b3f5..027b94e2 100644
> --- a/drivers/staging/iio/adc/Makefile
> +++ b/drivers/staging/iio/adc/Makefile
> @@ -39,3 +39,5 @@ obj-$(CONFIG_AD7816) += ad7816.o
>  obj-$(CONFIG_ADT75) += adt75.o
>  obj-$(CONFIG_ADT7310) += adt7310.o
>  obj-$(CONFIG_ADT7410) += adt7410.o
> +
> +obj-$(CONFIG_AS1531) += as1531.o
> \ No newline at end of file

What does this mean ? Is there a missing newline ?

Thanks,
Guenter

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

[PATCH 2/2] staging:iio:adc:as1351 general cleanup and conversion to standard functions.

[Jonathan Cameron]

V2: Fix the precedence issue Guenter Roeck pointed out.

Signed-off-by: Jonathan Cameron <jic23@cam.ac.uk>
---
 drivers/staging/iio/adc/as1531.c |   59 +++++--------------------------------
 1 files changed, 8 insertions(+), 51 deletions(-)

diff --git a/drivers/staging/iio/adc/as1531.c b/drivers/staging/iio/adc/as1531.c
index 1724c39..24a1ac0 100644
--- a/drivers/staging/iio/adc/as1531.c
+++ b/drivers/staging/iio/adc/as1531.c
@@ -58,47 +58,6 @@ struct as1531_state {
 	struct mutex lock;
 };
 
-static int as1531_message(struct spi_device *spi, int cmd, int *ret_value)
-{
-	struct spi_message	message;
-	struct spi_transfer	x[1];
-	int status, i;
-	u8	cmd_send;
-	unsigned char buf[64];
-	unsigned char buf_read[64];
-
-	cmd_send = cmd;
-
-	spi_message_init(&message);
-	memset(x, 0, sizeof x);
-	memset(buf, 0, sizeof(buf));
-	memset(buf_read, 0, sizeof(buf_read));
-
-	for (i = 0; i < 8; i++) {
-		buf[i] = ((cmd_send & 0x80)>>7);
-		cmd_send = cmd_send << 1;
-	}
-
-	x[0].tx_buf = buf;
-	x[0].len = 24;
-	x[0].rx_buf = buf_read;
-	x[0].speed_hz = AS1531_SPI_SPEED;
-	x[0].bits_per_word = 1;
-	spi_message_add_tail(&x[0], &message);
-
-	status = spi_sync(spi, &message);
-	if (status < 0)
-		return status;
-
-	*ret_value = buf_read[11] & 0x01;
-	for (i = 12; i < 23 ; i++) {
-		*ret_value = *ret_value << 1;
-		*ret_value = *ret_value | (buf_read[i]&0x01);
-	}
-
-	return 0;
-}
-
 static int as1531_read_raw(struct iio_dev *indio_dev,
 			   struct iio_chan_spec const *chan,
 			   int *val,
@@ -106,22 +65,22 @@ static int as1531_read_raw(struct iio_dev *indio_dev,
 			   long m)
 {
 	int status = 0;
-	int ret_value = 0;
+	u16 ret_value = 0;
 	struct as1531_state *st = iio_priv(indio_dev);
 
 	if (mutex_lock_interruptible(&st->lock))
 		return -ERESTARTSYS;
 
-	status = as1531_message(st->spi,
-				AS1531_START_BIT | chan->address |
-				AS1531_RANGE_0_TO_VREF | AS1531_MODE_COM |
-				AS1531_POWER_NORMAL,
-				&ret_value);
+	status = spi_w8r16(st->spi,
+			   AS1531_START_BIT | chan->address |
+			   AS1531_RANGE_0_TO_VREF | AS1531_MODE_COM |
+			   AS1531_POWER_NORMAL);
 	mutex_unlock(&st->lock);
 	if (status < 0)
 		return status;
 
-	*val = ret_value*2500/4096;
+	ret_value = status;
+	*val = ((be16_to_cpu(ret_value) >> 2) & 0xFFF)*2500/4096;
 
 	return IIO_VAL_INT;
 }
@@ -171,9 +130,7 @@ static int __devinit as1531_probe(struct spi_device *spi)
 
 static int as1531_remove(struct spi_device *spi)
 {
-	struct iio_dev *indio_dev = spi_get_drvdata(spi);
-
-	iio_device_unregister(indio_dev);
+	iio_device_unregister(spi_get_drvdata(spi));
 	return 0;
 }
 
-- 
1.7.3.4

[lm-sensors] [PATCH 2/2] staging:iio:adc:as1351 general cleanup and

[Jonathan Cameron]

V2: Fix the precedence issue Guenter Roeck pointed out.

Signed-off-by: Jonathan Cameron <jic23@cam.ac.uk>
---
 drivers/staging/iio/adc/as1531.c |   59 +++++--------------------------------
 1 files changed, 8 insertions(+), 51 deletions(-)

diff --git a/drivers/staging/iio/adc/as1531.c b/drivers/staging/iio/adc/as1531.c
index 1724c39..24a1ac0 100644
--- a/drivers/staging/iio/adc/as1531.c
+++ b/drivers/staging/iio/adc/as1531.c
@@ -58,47 +58,6 @@ struct as1531_state {
 	struct mutex lock;
 };
 
-static int as1531_message(struct spi_device *spi, int cmd, int *ret_value)
-{
-	struct spi_message	message;
-	struct spi_transfer	x[1];
-	int status, i;
-	u8	cmd_send;
-	unsigned char buf[64];
-	unsigned char buf_read[64];
-
-	cmd_send = cmd;
-
-	spi_message_init(&message);
-	memset(x, 0, sizeof x);
-	memset(buf, 0, sizeof(buf));
-	memset(buf_read, 0, sizeof(buf_read));
-
-	for (i = 0; i < 8; i++) {
-		buf[i] = ((cmd_send & 0x80)>>7);
-		cmd_send = cmd_send << 1;
-	}
-
-	x[0].tx_buf = buf;
-	x[0].len = 24;
-	x[0].rx_buf = buf_read;
-	x[0].speed_hz = AS1531_SPI_SPEED;
-	x[0].bits_per_word = 1;
-	spi_message_add_tail(&x[0], &message);
-
-	status = spi_sync(spi, &message);
-	if (status < 0)
-		return status;
-
-	*ret_value = buf_read[11] & 0x01;
-	for (i = 12; i < 23 ; i++) {
-		*ret_value = *ret_value << 1;
-		*ret_value = *ret_value | (buf_read[i]&0x01);
-	}
-
-	return 0;
-}
-
 static int as1531_read_raw(struct iio_dev *indio_dev,
 			   struct iio_chan_spec const *chan,
 			   int *val,
@@ -106,22 +65,22 @@ static int as1531_read_raw(struct iio_dev *indio_dev,
 			   long m)
 {
 	int status = 0;
-	int ret_value = 0;
+	u16 ret_value = 0;
 	struct as1531_state *st = iio_priv(indio_dev);
 
 	if (mutex_lock_interruptible(&st->lock))
 		return -ERESTARTSYS;
 
-	status = as1531_message(st->spi,
-				AS1531_START_BIT | chan->address |
-				AS1531_RANGE_0_TO_VREF | AS1531_MODE_COM |
-				AS1531_POWER_NORMAL,
-				&ret_value);
+	status = spi_w8r16(st->spi,
+			   AS1531_START_BIT | chan->address |
+			   AS1531_RANGE_0_TO_VREF | AS1531_MODE_COM |
+			   AS1531_POWER_NORMAL);
 	mutex_unlock(&st->lock);
 	if (status < 0)
 		return status;
 
-	*val = ret_value*2500/4096;
+	ret_value = status;
+	*val = ((be16_to_cpu(ret_value) >> 2) & 0xFFF)*2500/4096;
 
 	return IIO_VAL_INT;
 }
@@ -171,9 +130,7 @@ static int __devinit as1531_probe(struct spi_device *spi)
 
 static int as1531_remove(struct spi_device *spi)
 {
-	struct iio_dev *indio_dev = spi_get_drvdata(spi);
-
-	iio_device_unregister(indio_dev);
+	iio_device_unregister(spi_get_drvdata(spi));
 	return 0;
 }
 
-- 
1.7.3.4


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

Re: [PATCH 2/2] staging:iio:adc:as1351 general cleanup and conversion to standard functions.

[Guenter Roeck]

On Wed, May 18, 2011 at 12:17:00PM -0400, Jonathan Cameron wrote:
> V2: Fix the precedence issue Guenter Roeck pointed out.
> 
Should be below ---, unless you want it in the commit log.

> Signed-off-by: Jonathan Cameron <jic23@cam.ac.uk>

Nitpick below, otherwise

Reviewed-by: Guenter Roeck <guenter.roeck@ericsson.com>

> ---
>  drivers/staging/iio/adc/as1531.c |   59 +++++--------------------------------
>  1 files changed, 8 insertions(+), 51 deletions(-)
> 
> diff --git a/drivers/staging/iio/adc/as1531.c b/drivers/staging/iio/adc/as1531.c
> index 1724c39..24a1ac0 100644
> --- a/drivers/staging/iio/adc/as1531.c
> +++ b/drivers/staging/iio/adc/as1531.c
> @@ -58,47 +58,6 @@ struct as1531_state {
>  	struct mutex lock;
>  };
>  
> -static int as1531_message(struct spi_device *spi, int cmd, int *ret_value)
> -{
> -	struct spi_message	message;
> -	struct spi_transfer	x[1];
> -	int status, i;
> -	u8	cmd_send;
> -	unsigned char buf[64];
> -	unsigned char buf_read[64];
> -
> -	cmd_send = cmd;
> -
> -	spi_message_init(&message);
> -	memset(x, 0, sizeof x);
> -	memset(buf, 0, sizeof(buf));
> -	memset(buf_read, 0, sizeof(buf_read));
> -
> -	for (i = 0; i < 8; i++) {
> -		buf[i] = ((cmd_send & 0x80)>>7);
> -		cmd_send = cmd_send << 1;
> -	}
> -
> -	x[0].tx_buf = buf;
> -	x[0].len = 24;
> -	x[0].rx_buf = buf_read;
> -	x[0].speed_hz = AS1531_SPI_SPEED;
> -	x[0].bits_per_word = 1;
> -	spi_message_add_tail(&x[0], &message);
> -
> -	status = spi_sync(spi, &message);
> -	if (status < 0)
> -		return status;
> -
> -	*ret_value = buf_read[11] & 0x01;
> -	for (i = 12; i < 23 ; i++) {
> -		*ret_value = *ret_value << 1;
> -		*ret_value = *ret_value | (buf_read[i]&0x01);
> -	}
> -
> -	return 0;
> -}
> -
>  static int as1531_read_raw(struct iio_dev *indio_dev,
>  			   struct iio_chan_spec const *chan,
>  			   int *val,
> @@ -106,22 +65,22 @@ static int as1531_read_raw(struct iio_dev *indio_dev,
>  			   long m)
>  {
>  	int status = 0;
> -	int ret_value = 0;
> +	u16 ret_value = 0;
>  	struct as1531_state *st = iio_priv(indio_dev);
>  
>  	if (mutex_lock_interruptible(&st->lock))
>  		return -ERESTARTSYS;
>  
> -	status = as1531_message(st->spi,
> -				AS1531_START_BIT | chan->address |
> -				AS1531_RANGE_0_TO_VREF | AS1531_MODE_COM |
> -				AS1531_POWER_NORMAL,
> -				&ret_value);
> +	status = spi_w8r16(st->spi,
> +			   AS1531_START_BIT | chan->address |
> +			   AS1531_RANGE_0_TO_VREF | AS1531_MODE_COM |
> +			   AS1531_POWER_NORMAL);
>  	mutex_unlock(&st->lock);
>  	if (status < 0)
>  		return status;
>  
> -	*val = ret_value*2500/4096;
> +	ret_value = status;
> +	*val = ((be16_to_cpu(ret_value) >> 2) & 0xFFF)*2500/4096;
>  
Even though checkpatch doesn't complain anymore (why ?), the rule to put spaces
around * and / still applies:

Use one space around (on each side of) most binary and ternary operators,
such as any of these:

        =  +  -  <  >  *  /  %  |  &  ^  <=  >=  ==  !=  ?  :

Thanks,
Guenter

Re: [lm-sensors] [PATCH 2/2] staging:iio:adc:as1351 general cleanup

[Guenter Roeck]

On Wed, May 18, 2011 at 12:17:00PM -0400, Jonathan Cameron wrote:
> V2: Fix the precedence issue Guenter Roeck pointed out.
> 
Should be below ---, unless you want it in the commit log.

> Signed-off-by: Jonathan Cameron <jic23@cam.ac.uk>

Nitpick below, otherwise

Reviewed-by: Guenter Roeck <guenter.roeck@ericsson.com>

> ---
>  drivers/staging/iio/adc/as1531.c |   59 +++++--------------------------------
>  1 files changed, 8 insertions(+), 51 deletions(-)
> 
> diff --git a/drivers/staging/iio/adc/as1531.c b/drivers/staging/iio/adc/as1531.c
> index 1724c39..24a1ac0 100644
> --- a/drivers/staging/iio/adc/as1531.c
> +++ b/drivers/staging/iio/adc/as1531.c
> @@ -58,47 +58,6 @@ struct as1531_state {
>  	struct mutex lock;
>  };
>  
> -static int as1531_message(struct spi_device *spi, int cmd, int *ret_value)
> -{
> -	struct spi_message	message;
> -	struct spi_transfer	x[1];
> -	int status, i;
> -	u8	cmd_send;
> -	unsigned char buf[64];
> -	unsigned char buf_read[64];
> -
> -	cmd_send = cmd;
> -
> -	spi_message_init(&message);
> -	memset(x, 0, sizeof x);
> -	memset(buf, 0, sizeof(buf));
> -	memset(buf_read, 0, sizeof(buf_read));
> -
> -	for (i = 0; i < 8; i++) {
> -		buf[i] = ((cmd_send & 0x80)>>7);
> -		cmd_send = cmd_send << 1;
> -	}
> -
> -	x[0].tx_buf = buf;
> -	x[0].len = 24;
> -	x[0].rx_buf = buf_read;
> -	x[0].speed_hz = AS1531_SPI_SPEED;
> -	x[0].bits_per_word = 1;
> -	spi_message_add_tail(&x[0], &message);
> -
> -	status = spi_sync(spi, &message);
> -	if (status < 0)
> -		return status;
> -
> -	*ret_value = buf_read[11] & 0x01;
> -	for (i = 12; i < 23 ; i++) {
> -		*ret_value = *ret_value << 1;
> -		*ret_value = *ret_value | (buf_read[i]&0x01);
> -	}
> -
> -	return 0;
> -}
> -
>  static int as1531_read_raw(struct iio_dev *indio_dev,
>  			   struct iio_chan_spec const *chan,
>  			   int *val,
> @@ -106,22 +65,22 @@ static int as1531_read_raw(struct iio_dev *indio_dev,
>  			   long m)
>  {
>  	int status = 0;
> -	int ret_value = 0;
> +	u16 ret_value = 0;
>  	struct as1531_state *st = iio_priv(indio_dev);
>  
>  	if (mutex_lock_interruptible(&st->lock))
>  		return -ERESTARTSYS;
>  
> -	status = as1531_message(st->spi,
> -				AS1531_START_BIT | chan->address |
> -				AS1531_RANGE_0_TO_VREF | AS1531_MODE_COM |
> -				AS1531_POWER_NORMAL,
> -				&ret_value);
> +	status = spi_w8r16(st->spi,
> +			   AS1531_START_BIT | chan->address |
> +			   AS1531_RANGE_0_TO_VREF | AS1531_MODE_COM |
> +			   AS1531_POWER_NORMAL);
>  	mutex_unlock(&st->lock);
>  	if (status < 0)
>  		return status;
>  
> -	*val = ret_value*2500/4096;
> +	ret_value = status;
> +	*val = ((be16_to_cpu(ret_value) >> 2) & 0xFFF)*2500/4096;
>  
Even though checkpatch doesn't complain anymore (why ?), the rule to put spaces
around * and / still applies:

Use one space around (on each side of) most binary and ternary operators,
such as any of these:

        =  +  -  <  >  *  /  %  |  &  ^  <=  >=  =  !=  ?  :

Thanks,
Guenter

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

Re: [PATCH 2/2] staging:iio:adc:as1351 general cleanup and conversion to standard functions.

[Jonathan Cameron]

On 05/18/11 17:35, Guenter Roeck wrote:
> On Wed, May 18, 2011 at 12:17:00PM -0400, Jonathan Cameron wrote:
>> V2: Fix the precedence issue Guenter Roeck pointed out.
>>
> Should be below ---, unless you want it in the commit log.
> 
>> Signed-off-by: Jonathan Cameron <jic23@cam.ac.uk>
> 
> Nitpick below, otherwise
> 
> Reviewed-by: Guenter Roeck <guenter.roeck@ericsson.com>
Thanks,

Fixed up as suggested.  I'll wait for Fabien's test results
before doing any more with this driver.

Re: [lm-sensors] [PATCH 2/2] staging:iio:adc:as1351 general cleanup

[Jonathan Cameron]

On 05/18/11 17:35, Guenter Roeck wrote:
> On Wed, May 18, 2011 at 12:17:00PM -0400, Jonathan Cameron wrote:
>> V2: Fix the precedence issue Guenter Roeck pointed out.
>>
> Should be below ---, unless you want it in the commit log.
> 
>> Signed-off-by: Jonathan Cameron <jic23@cam.ac.uk>
> 
> Nitpick below, otherwise
> 
> Reviewed-by: Guenter Roeck <guenter.roeck@ericsson.com>
Thanks,

Fixed up as suggested.  I'll wait for Fabien's test results
before doing any more with this driver.


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

[PATCH 1/2] staging:iio:adc: as1531 driver initial conversion from hwmon submission.

[Jonathan Cameron]

=46rom: Fabien Marteau <fabien.marteau@armadeus.com>

Signed-off-by: Jonathan Cameron <jic23@cam.ac.uk>
---
 drivers/staging/iio/adc/Kconfig  |   10 ++
 drivers/staging/iio/adc/Makefile |    2 +
 drivers/staging/iio/adc/as1531.c |  212 ++++++++++++++++++++++++++++++=
++++++++
 3 files changed, 224 insertions(+), 0 deletions(-)

diff --git a/drivers/staging/iio/adc/Kconfig b/drivers/staging/iio/adc/=
Kconfig
index 8c751c4..4af730a 100644
--- a/drivers/staging/iio/adc/Kconfig
+++ b/drivers/staging/iio/adc/Kconfig
@@ -168,6 +168,16 @@ config ADT7410
 	  Say yes here to build support for Analog Devices ADT7410
 	  temperature sensors.
=20
+config AS1531
+tristate "Austria Microsystems AS1531 Analog to Digital Converter"
+	depends on SPI_MASTER
+	help
+	  If you say yes here you get support for Austria Microsystems AS1531=
=2E
+		AS1531 is a 12=C2=A0bits Analog to digitals converter with 8 channel=
s
+		provided by Austria-Microsystems company.
+	  This driver can also be built as a module.  If so, the module
+	  will be called as1531.
+
 config MAX1363
 	tristate "Maxim max1363 ADC driver"
 	depends on I2C
diff --git a/drivers/staging/iio/adc/Makefile b/drivers/staging/iio/adc=
/Makefile
index 1d9b3f5..027b94e2 100644
--- a/drivers/staging/iio/adc/Makefile
+++ b/drivers/staging/iio/adc/Makefile
@@ -39,3 +39,5 @@ obj-$(CONFIG_AD7816) +=3D ad7816.o
 obj-$(CONFIG_ADT75) +=3D adt75.o
 obj-$(CONFIG_ADT7310) +=3D adt7310.o
 obj-$(CONFIG_ADT7410) +=3D adt7410.o
+
+obj-$(CONFIG_AS1531) +=3D as1531.o
\ No newline at end of file
diff --git a/drivers/staging/iio/adc/as1531.c b/drivers/staging/iio/adc=
/as1531.c
new file mode 100644
index 0000000..7222549
--- /dev/null
+++ b/drivers/staging/iio/adc/as1531.c
@@ -0,0 +1,212 @@
+/*
+ * as1531.c
+ *
+ * Driver for Austria-Microsystem Analog to Digital Converter.
+ *
+ * Copyright (c) 2010, 2011 Fabien Marteau <fabien.marteau@armadeus.co=
m>
+ * Driver based on Marc Pignat <marc.pignat@hevs.ch> adcxx.c driver.
+ *
+ * This program is free software; you can redistribute it and/or modif=
y
+ * it under the terms of the GNU General Public License as published b=
y
+ * 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/device.h>
+#include <linux/kernel.h>
+#include <linux/sysfs.h>
+#include <linux/spi/spi.h>
+
+#include "../iio.h"
+#include "../sysfs.h"
+#include "adc.h"
+
+#define AS1531_SPI_SPEED 64941
+#define AS1531_MAX_VALUE 2500
+
+#define AS1531_START_BIT	(0x80)
+#define AS1531_CHAN0		(0<<4)
+#define AS1531_CHAN1		(4<<4)
+#define AS1531_CHAN2		(1<<4)
+#define AS1531_CHAN3		(5<<4)
+#define AS1531_CHAN4		(2<<4)
+#define AS1531_CHAN5		(6<<4)
+#define AS1531_CHAN6		(3<<4)
+#define AS1531_CHAN7		(7<<4)
+
+#define AS1531_RANGE_0_TO_VREF			(1<<3)
+#define AS1531_RANGE_HALFVREF_TO_HALFVREF	(0<<3)
+
+#define AS1531_MODE_COM		(1<<2)
+#define AS1531_MODE_DIFF	(0<<2)
+
+#define AS1531_POWER_DOWN		0x0
+#define AS1531_POWER_REDUCED		0x1
+#define AS1531_POWER_REDUCED_BIS	0x2
+#define AS1531_POWER_NORMAL		0x3
+
+struct as1531_state {
+	struct spi_device *spi;
+	struct mutex lock;
+};
+
+static int as1531_message(struct spi_device *spi, int cmd, int *ret_va=
lue)
+{
+	struct spi_message	message;
+	struct spi_transfer	x[1];
+	int status, i;
+	u8	cmd_send;
+	unsigned char buf[64];
+	unsigned char buf_read[64];
+
+	cmd_send =3D cmd;
+
+	spi_message_init(&message);
+	memset(x, 0, sizeof x);
+	memset(buf, 0, sizeof(buf));
+	memset(buf_read, 0, sizeof(buf_read));
+
+	for (i =3D 0; i < 8; i++) {
+		buf[i] =3D ((cmd_send & 0x80)>>7);
+		cmd_send =3D cmd_send << 1;
+	}
+
+	x[0].tx_buf =3D buf;
+	x[0].len =3D 24;
+	x[0].rx_buf =3D buf_read;
+	x[0].speed_hz =3D AS1531_SPI_SPEED;
+	x[0].bits_per_word =3D 1;
+	spi_message_add_tail(&x[0], &message);
+
+	status =3D spi_sync(spi, &message);
+	if (status < 0)
+		return status;
+
+	*ret_value =3D buf_read[11] & 0x01;
+	for (i =3D 12; i < 23 ; i++) {
+		*ret_value =3D *ret_value << 1;
+		*ret_value =3D *ret_value | (buf_read[i]&0x01);
+	}
+
+	return 0;
+}
+
+static int as1531_read_raw(struct iio_dev *indio_dev,
+			   struct iio_chan_spec const *chan,
+			   int *val,
+			   int *val2,
+			   long m)
+{
+
+	int status =3D 0;
+	int ret_value =3D 0;
+	struct as1531_state *st =3D iio_priv(indio_dev);
+	if (mutex_lock_interruptible(&st->lock))
+		return -ERESTARTSYS;
+
+	status =3D as1531_message(st->spi,
+				AS1531_START_BIT | chan->address |
+				AS1531_RANGE_0_TO_VREF | AS1531_MODE_COM |
+				AS1531_POWER_NORMAL,
+				&ret_value);
+	mutex_unlock(&st->lock);
+	if (status < 0)
+		goto out;
+=09
+	*val =3D ret_value*2500/4096;
+
+	return IIO_VAL_INT;
+out:
+	mutex_unlock(&st->lock);
+	return status;
+}
+
+static const struct iio_chan_spec as1531_channels[] =3D {
+	IIO_CHAN(IIO_IN, 0, 1, 1, NULL, 0, 1, 0, AS1531_CHAN0, 0, {}, 0),
+	IIO_CHAN(IIO_IN, 0, 1, 1, NULL, 1, 1, 0, AS1531_CHAN1, 1, {}, 0),
+	IIO_CHAN(IIO_IN, 0, 1, 1, NULL, 2, 1, 0, AS1531_CHAN2, 2, {}, 0),
+	IIO_CHAN(IIO_IN, 0, 1, 1, NULL, 3, 1, 0, AS1531_CHAN3, 3, {}, 0),
+	IIO_CHAN(IIO_IN, 0, 1, 1, NULL, 4, 1, 0, AS1531_CHAN4, 4, {}, 0),
+	IIO_CHAN(IIO_IN, 0, 1, 1, NULL, 5, 1, 0, AS1531_CHAN5, 5, {}, 0),
+	IIO_CHAN(IIO_IN, 0, 1, 1, NULL, 6, 1, 0, AS1531_CHAN6, 6, {}, 0),
+	IIO_CHAN(IIO_IN, 0, 1, 1, NULL, 7, 1, 0, AS1531_CHAN7, 7, {}, 0),
+};
+
+static const struct iio_info as1531_info =3D {
+	.read_raw =3D &as1531_read_raw,
+	.driver_module =3D THIS_MODULE,
+};
+
+static int __devinit as1531_probe(struct spi_device *spi)
+{
+	int ret;
+	struct as1531_state *st;
+	struct iio_dev *indio_dev =3D iio_allocate_device(sizeof(*st));
+
+	if (indio_dev =3D=3D NULL)
+		return -ENOMEM;
+
+	st =3D iio_priv(indio_dev);
+	mutex_init(&st->lock);
+	st->spi =3D spi;
+
+	spi_set_drvdata(spi, indio_dev);
+	indio_dev->dev.parent =3D &spi->dev;
+	indio_dev->name =3D spi_get_device_id(spi)->name;
+	indio_dev->info =3D &as1531_info;
+	indio_dev->modes =3D INDIO_DIRECT_MODE;
+	indio_dev->channels =3D as1531_channels;
+	indio_dev->num_channels =3D ARRAY_SIZE(as1531_channels);
+
+	ret =3D iio_device_register(indio_dev);
+	if (ret < 0)
+		iio_free_device(indio_dev);
+	return ret;
+}
+
+static int as1531_remove(struct spi_device *spi)
+{
+	struct iio_dev *indio_dev =3D spi_get_drvdata(spi);
+
+	iio_device_unregister(indio_dev);
+	return 0;
+}
+
+static const struct spi_device_id as1531_id[] =3D {
+	{"as1531", 0},
+	{}
+};
+
+static struct spi_driver as1531_driver =3D {
+	.driver =3D {
+		.name	=3D "as1531",
+		.owner	=3D THIS_MODULE,
+	},
+	.probe	=3D as1531_probe,
+	.remove	=3D __devexit_p(as1531_remove),
+	.id_table =3D as1531_id,
+};
+
+static int __init init_as1531(void)
+{
+	return spi_register_driver(&as1531_driver);
+}
+module_init(init_as1531);
+
+static void __exit exit_as1531(void)
+{
+	spi_unregister_driver(&as1531_driver);
+}
+module_exit(exit_as1531);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Fabien Marteau <fabien.marteau@armadeus.com>");
+MODULE_DESCRIPTION("Driver for AS1531 ADC");
--=20
1.7.3.4

[lm-sensors] [PATCH 1/2] staging:iio:adc: as1

[Jonathan Cameron]

From: Fabien Marteau <fabien.marteau@armadeus.com>

Signed-off-by: Jonathan Cameron <jic23@cam.ac.uk>
---
 drivers/staging/iio/adc/Kconfig  |   10 ++
 drivers/staging/iio/adc/Makefile |    2 +
 drivers/staging/iio/adc/as1531.c |  212 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 224 insertions(+), 0 deletions(-)

diff --git a/drivers/staging/iio/adc/Kconfig b/drivers/staging/iio/adc/Kconfig
index 8c751c4..4af730a 100644
--- a/drivers/staging/iio/adc/Kconfig
+++ b/drivers/staging/iio/adc/Kconfig
@@ -168,6 +168,16 @@ config ADT7410
 	  Say yes here to build support for Analog Devices ADT7410
 	  temperature sensors.
 
+config AS1531
+tristate "Austria Microsystems AS1531 Analog to Digital Converter"
+	depends on SPI_MASTER
+	help
+	  If you say yes here you get support for Austria Microsystems AS1531.
+		AS1531 is a 12 bits Analog to digitals converter with 8 channels
+		provided by Austria-Microsystems company.
+	  This driver can also be built as a module.  If so, the module
+	  will be called as1531.
+
 config MAX1363
 	tristate "Maxim max1363 ADC driver"
 	depends on I2C
diff --git a/drivers/staging/iio/adc/Makefile b/drivers/staging/iio/adc/Makefile
index 1d9b3f5..027b94e2 100644
--- a/drivers/staging/iio/adc/Makefile
+++ b/drivers/staging/iio/adc/Makefile
@@ -39,3 +39,5 @@ obj-$(CONFIG_AD7816) += ad7816.o
 obj-$(CONFIG_ADT75) += adt75.o
 obj-$(CONFIG_ADT7310) += adt7310.o
 obj-$(CONFIG_ADT7410) += adt7410.o
+
+obj-$(CONFIG_AS1531) += as1531.o
\ No newline at end of file
diff --git a/drivers/staging/iio/adc/as1531.c b/drivers/staging/iio/adc/as1531.c
new file mode 100644
index 0000000..7222549
--- /dev/null
+++ b/drivers/staging/iio/adc/as1531.c
@@ -0,0 +1,212 @@
+/*
+ * as1531.c
+ *
+ * Driver for Austria-Microsystem Analog to Digital Converter.
+ *
+ * Copyright (c) 2010, 2011 Fabien Marteau <fabien.marteau@armadeus.com>
+ * Driver based on Marc Pignat <marc.pignat@hevs.ch> adcxx.c driver.
+ *
+ * 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/device.h>
+#include <linux/kernel.h>
+#include <linux/sysfs.h>
+#include <linux/spi/spi.h>
+
+#include "../iio.h"
+#include "../sysfs.h"
+#include "adc.h"
+
+#define AS1531_SPI_SPEED 64941
+#define AS1531_MAX_VALUE 2500
+
+#define AS1531_START_BIT	(0x80)
+#define AS1531_CHAN0		(0<<4)
+#define AS1531_CHAN1		(4<<4)
+#define AS1531_CHAN2		(1<<4)
+#define AS1531_CHAN3		(5<<4)
+#define AS1531_CHAN4		(2<<4)
+#define AS1531_CHAN5		(6<<4)
+#define AS1531_CHAN6		(3<<4)
+#define AS1531_CHAN7		(7<<4)
+
+#define AS1531_RANGE_0_TO_VREF			(1<<3)
+#define AS1531_RANGE_HALFVREF_TO_HALFVREF	(0<<3)
+
+#define AS1531_MODE_COM		(1<<2)
+#define AS1531_MODE_DIFF	(0<<2)
+
+#define AS1531_POWER_DOWN		0x0
+#define AS1531_POWER_REDUCED		0x1
+#define AS1531_POWER_REDUCED_BIS	0x2
+#define AS1531_POWER_NORMAL		0x3
+
+struct as1531_state {
+	struct spi_device *spi;
+	struct mutex lock;
+};
+
+static int as1531_message(struct spi_device *spi, int cmd, int *ret_value)
+{
+	struct spi_message	message;
+	struct spi_transfer	x[1];
+	int status, i;
+	u8	cmd_send;
+	unsigned char buf[64];
+	unsigned char buf_read[64];
+
+	cmd_send = cmd;
+
+	spi_message_init(&message);
+	memset(x, 0, sizeof x);
+	memset(buf, 0, sizeof(buf));
+	memset(buf_read, 0, sizeof(buf_read));
+
+	for (i = 0; i < 8; i++) {
+		buf[i] = ((cmd_send & 0x80)>>7);
+		cmd_send = cmd_send << 1;
+	}
+
+	x[0].tx_buf = buf;
+	x[0].len = 24;
+	x[0].rx_buf = buf_read;
+	x[0].speed_hz = AS1531_SPI_SPEED;
+	x[0].bits_per_word = 1;
+	spi_message_add_tail(&x[0], &message);
+
+	status = spi_sync(spi, &message);
+	if (status < 0)
+		return status;
+
+	*ret_value = buf_read[11] & 0x01;
+	for (i = 12; i < 23 ; i++) {
+		*ret_value = *ret_value << 1;
+		*ret_value = *ret_value | (buf_read[i]&0x01);
+	}
+
+	return 0;
+}
+
+static int as1531_read_raw(struct iio_dev *indio_dev,
+			   struct iio_chan_spec const *chan,
+			   int *val,
+			   int *val2,
+			   long m)
+{
+
+	int status = 0;
+	int ret_value = 0;
+	struct as1531_state *st = iio_priv(indio_dev);
+	if (mutex_lock_interruptible(&st->lock))
+		return -ERESTARTSYS;
+
+	status = as1531_message(st->spi,
+				AS1531_START_BIT | chan->address |
+				AS1531_RANGE_0_TO_VREF | AS1531_MODE_COM |
+				AS1531_POWER_NORMAL,
+				&ret_value);
+	mutex_unlock(&st->lock);
+	if (status < 0)
+		goto out;
+	
+	*val = ret_value*2500/4096;
+
+	return IIO_VAL_INT;
+out:
+	mutex_unlock(&st->lock);
+	return status;
+}
+
+static const struct iio_chan_spec as1531_channels[] = {
+	IIO_CHAN(IIO_IN, 0, 1, 1, NULL, 0, 1, 0, AS1531_CHAN0, 0, {}, 0),
+	IIO_CHAN(IIO_IN, 0, 1, 1, NULL, 1, 1, 0, AS1531_CHAN1, 1, {}, 0),
+	IIO_CHAN(IIO_IN, 0, 1, 1, NULL, 2, 1, 0, AS1531_CHAN2, 2, {}, 0),
+	IIO_CHAN(IIO_IN, 0, 1, 1, NULL, 3, 1, 0, AS1531_CHAN3, 3, {}, 0),
+	IIO_CHAN(IIO_IN, 0, 1, 1, NULL, 4, 1, 0, AS1531_CHAN4, 4, {}, 0),
+	IIO_CHAN(IIO_IN, 0, 1, 1, NULL, 5, 1, 0, AS1531_CHAN5, 5, {}, 0),
+	IIO_CHAN(IIO_IN, 0, 1, 1, NULL, 6, 1, 0, AS1531_CHAN6, 6, {}, 0),
+	IIO_CHAN(IIO_IN, 0, 1, 1, NULL, 7, 1, 0, AS1531_CHAN7, 7, {}, 0),
+};
+
+static const struct iio_info as1531_info = {
+	.read_raw = &as1531_read_raw,
+	.driver_module = THIS_MODULE,
+};
+
+static int __devinit as1531_probe(struct spi_device *spi)
+{
+	int ret;
+	struct as1531_state *st;
+	struct iio_dev *indio_dev = iio_allocate_device(sizeof(*st));
+
+	if (indio_dev = NULL)
+		return -ENOMEM;
+
+	st = iio_priv(indio_dev);
+	mutex_init(&st->lock);
+	st->spi = spi;
+
+	spi_set_drvdata(spi, indio_dev);
+	indio_dev->dev.parent = &spi->dev;
+	indio_dev->name = spi_get_device_id(spi)->name;
+	indio_dev->info = &as1531_info;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->channels = as1531_channels;
+	indio_dev->num_channels = ARRAY_SIZE(as1531_channels);
+
+	ret = iio_device_register(indio_dev);
+	if (ret < 0)
+		iio_free_device(indio_dev);
+	return ret;
+}
+
+static int as1531_remove(struct spi_device *spi)
+{
+	struct iio_dev *indio_dev = spi_get_drvdata(spi);
+
+	iio_device_unregister(indio_dev);
+	return 0;
+}
+
+static const struct spi_device_id as1531_id[] = {
+	{"as1531", 0},
+	{}
+};
+
+static struct spi_driver as1531_driver = {
+	.driver = {
+		.name	= "as1531",
+		.owner	= THIS_MODULE,
+	},
+	.probe	= as1531_probe,
+	.remove	= __devexit_p(as1531_remove),
+	.id_table = as1531_id,
+};
+
+static int __init init_as1531(void)
+{
+	return spi_register_driver(&as1531_driver);
+}
+module_init(init_as1531);
+
+static void __exit exit_as1531(void)
+{
+	spi_unregister_driver(&as1531_driver);
+}
+module_exit(exit_as1531);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Fabien Marteau <fabien.marteau@armadeus.com>");
+MODULE_DESCRIPTION("Driver for AS1531 ADC");
-- 
1.7.3.4


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

Re: [PATCH 1/2] staging:iio:adc: as1531 driver initial conversion from hwmon submission.

[Guenter Roeck]

On Wed, May 18, 2011 at 11:39:31AM -0400, Jonathan Cameron wrote:
> From: Fabien Marteau <fabien.marteau@armadeus.com>
> 
> Signed-off-by: Jonathan Cameron <jic23@cam.ac.uk>

Hi Jonathan,

nice job. Thanks a lot for the effort.

[ ... ]

> +static int as1531_read_raw(struct iio_dev *indio_dev,
> +			   struct iio_chan_spec const *chan,
> +			   int *val,
> +			   int *val2,
> +			   long m)
> +{
> +
Extra blank line

> +	int status = 0;
> +	int ret_value = 0;
> +	struct as1531_state *st = iio_priv(indio_dev);

Move it to here, maybe ?

> +	if (mutex_lock_interruptible(&st->lock))
> +		return -ERESTARTSYS;
> +
> +	status = as1531_message(st->spi,
> +				AS1531_START_BIT | chan->address |
> +				AS1531_RANGE_0_TO_VREF | AS1531_MODE_COM |
> +				AS1531_POWER_NORMAL,
> +				&ret_value);
> +	mutex_unlock(&st->lock);
> +	if (status < 0)
> +		goto out;
> +	
> +	*val = ret_value*2500/4096;
> +
> +	return IIO_VAL_INT;
> +out:
> +	mutex_unlock(&st->lock);

Mutex was unlocked above already. Maybe just return status above ?

Thanks,
Guenter 

Re: [lm-sensors] [PATCH 1/2] staging:iio:adc: as1531 driver initial

[Guenter Roeck]

On Wed, May 18, 2011 at 11:39:31AM -0400, Jonathan Cameron wrote:
> From: Fabien Marteau <fabien.marteau@armadeus.com>
> 
> Signed-off-by: Jonathan Cameron <jic23@cam.ac.uk>

Hi Jonathan,

nice job. Thanks a lot for the effort.

[ ... ]

> +static int as1531_read_raw(struct iio_dev *indio_dev,
> +			   struct iio_chan_spec const *chan,
> +			   int *val,
> +			   int *val2,
> +			   long m)
> +{
> +
Extra blank line

> +	int status = 0;
> +	int ret_value = 0;
> +	struct as1531_state *st = iio_priv(indio_dev);

Move it to here, maybe ?

> +	if (mutex_lock_interruptible(&st->lock))
> +		return -ERESTARTSYS;
> +
> +	status = as1531_message(st->spi,
> +				AS1531_START_BIT | chan->address |
> +				AS1531_RANGE_0_TO_VREF | AS1531_MODE_COM |
> +				AS1531_POWER_NORMAL,
> +				&ret_value);
> +	mutex_unlock(&st->lock);
> +	if (status < 0)
> +		goto out;
> +	
> +	*val = ret_value*2500/4096;
> +
> +	return IIO_VAL_INT;
> +out:
> +	mutex_unlock(&st->lock);

Mutex was unlocked above already. Maybe just return status above ?

Thanks,
Guenter 

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

Re: [PATCH 1/2] staging:iio:adc: as1531 driver initial conversion from hwmon submission.

[Jonathan Cameron]

On 05/18/11 17:02, Guenter Roeck wrote:
> On Wed, May 18, 2011 at 11:39:31AM -0400, Jonathan Cameron wrote:
>> From: Fabien Marteau <fabien.marteau-d2DlULPkwbNWk0Htik3J/w@public.gmane.org>
>>
>> Signed-off-by: Jonathan Cameron <jic23-KWPb1pKIrIJaa/9Udqfwiw@public.gmane.org>
> 
> Hi Jonathan,
> 
> nice job. Thanks a lot for the effort.
> 
> [ ... ]
> 
>> +static int as1531_read_raw(struct iio_dev *indio_dev,
>> +			   struct iio_chan_spec const *chan,
>> +			   int *val,
>> +			   int *val2,
>> +			   long m)
>> +{
>> +
> Extra blank line
> 
>> +	int status = 0;
>> +	int ret_value = 0;
>> +	struct as1531_state *st = iio_priv(indio_dev);
> 
> Move it to here, maybe ?
> 
>> +	if (mutex_lock_interruptible(&st->lock))
>> +		return -ERESTARTSYS;
>> +
>> +	status = as1531_message(st->spi,
>> +				AS1531_START_BIT | chan->address |
>> +				AS1531_RANGE_0_TO_VREF | AS1531_MODE_COM |
>> +				AS1531_POWER_NORMAL,
>> +				&ret_value);
>> +	mutex_unlock(&st->lock);
>> +	if (status < 0)
>> +		goto out;
>> +	
>> +	*val = ret_value*2500/4096;
>> +
>> +	return IIO_VAL_INT;
>> +out:
>> +	mutex_unlock(&st->lock);
> 
> Mutex was unlocked above already. Maybe just return status above ?
Hmm.. Sometimes working without testing shows just how bad first versions
of code really can be ;)

Thanks for the other review as well.  Have fixed up both.


Thanks.

Jonathan 

Re: [lm-sensors] [PATCH 1/2] staging:iio:adc: as1531 driver initial

[Jonathan Cameron]

On 05/18/11 17:02, Guenter Roeck wrote:
> On Wed, May 18, 2011 at 11:39:31AM -0400, Jonathan Cameron wrote:
>> From: Fabien Marteau <fabien.marteau-d2DlULPkwbNWk0Htik3J/w@public.gmane.org>
>>
>> Signed-off-by: Jonathan Cameron <jic23-KWPb1pKIrIJaa/9Udqfwiw@public.gmane.org>
> 
> Hi Jonathan,
> 
> nice job. Thanks a lot for the effort.
> 
> [ ... ]
> 
>> +static int as1531_read_raw(struct iio_dev *indio_dev,
>> +			   struct iio_chan_spec const *chan,
>> +			   int *val,
>> +			   int *val2,
>> +			   long m)
>> +{
>> +
> Extra blank line
> 
>> +	int status = 0;
>> +	int ret_value = 0;
>> +	struct as1531_state *st = iio_priv(indio_dev);
> 
> Move it to here, maybe ?
> 
>> +	if (mutex_lock_interruptible(&st->lock))
>> +		return -ERESTARTSYS;
>> +
>> +	status = as1531_message(st->spi,
>> +				AS1531_START_BIT | chan->address |
>> +				AS1531_RANGE_0_TO_VREF | AS1531_MODE_COM |
>> +				AS1531_POWER_NORMAL,
>> +				&ret_value);
>> +	mutex_unlock(&st->lock);
>> +	if (status < 0)
>> +		goto out;
>> +	
>> +	*val = ret_value*2500/4096;
>> +
>> +	return IIO_VAL_INT;
>> +out:
>> +	mutex_unlock(&st->lock);
> 
> Mutex was unlocked above already. Maybe just return status above ?
Hmm.. Sometimes working without testing shows just how bad first versions
of code really can be ;)

Thanks for the other review as well.  Have fixed up both.


Thanks.

Jonathan 


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

[PATCH 2/2] staging:iio:adc:as1351 general cleanup and conversion to standard functions.

[Jonathan Cameron]

Signed-off-by: Jonathan Cameron <jic23@cam.ac.uk>
---
 drivers/staging/iio/adc/as1531.c |   60 +++++--------------------------------
 1 files changed, 8 insertions(+), 52 deletions(-)

diff --git a/drivers/staging/iio/adc/as1531.c b/drivers/staging/iio/adc/as1531.c
index 7222549..30119ce 100644
--- a/drivers/staging/iio/adc/as1531.c
+++ b/drivers/staging/iio/adc/as1531.c
@@ -58,47 +58,6 @@ struct as1531_state {
 	struct mutex lock;
 };
 
-static int as1531_message(struct spi_device *spi, int cmd, int *ret_value)
-{
-	struct spi_message	message;
-	struct spi_transfer	x[1];
-	int status, i;
-	u8	cmd_send;
-	unsigned char buf[64];
-	unsigned char buf_read[64];
-
-	cmd_send = cmd;
-
-	spi_message_init(&message);
-	memset(x, 0, sizeof x);
-	memset(buf, 0, sizeof(buf));
-	memset(buf_read, 0, sizeof(buf_read));
-
-	for (i = 0; i < 8; i++) {
-		buf[i] = ((cmd_send & 0x80)>>7);
-		cmd_send = cmd_send << 1;
-	}
-
-	x[0].tx_buf = buf;
-	x[0].len = 24;
-	x[0].rx_buf = buf_read;
-	x[0].speed_hz = AS1531_SPI_SPEED;
-	x[0].bits_per_word = 1;
-	spi_message_add_tail(&x[0], &message);
-
-	status = spi_sync(spi, &message);
-	if (status < 0)
-		return status;
-
-	*ret_value = buf_read[11] & 0x01;
-	for (i = 12; i < 23 ; i++) {
-		*ret_value = *ret_value << 1;
-		*ret_value = *ret_value | (buf_read[i]&0x01);
-	}
-
-	return 0;
-}
-
 static int as1531_read_raw(struct iio_dev *indio_dev,
 			   struct iio_chan_spec const *chan,
 			   int *val,
@@ -107,21 +66,20 @@ static int as1531_read_raw(struct iio_dev *indio_dev,
 {
 
 	int status = 0;
-	int ret_value = 0;
+	u16 ret_value = 0;
 	struct as1531_state *st = iio_priv(indio_dev);
 	if (mutex_lock_interruptible(&st->lock))
 		return -ERESTARTSYS;
 
-	status = as1531_message(st->spi,
-				AS1531_START_BIT | chan->address |
-				AS1531_RANGE_0_TO_VREF | AS1531_MODE_COM |
-				AS1531_POWER_NORMAL,
-				&ret_value);
+	status = spi_w8r16(st->spi,
+			   AS1531_START_BIT | chan->address |
+			   AS1531_RANGE_0_TO_VREF | AS1531_MODE_COM |
+			   AS1531_POWER_NORMAL);
 	mutex_unlock(&st->lock);
 	if (status < 0)
 		goto out;
-	
-	*val = ret_value*2500/4096;
+	ret_value = status;
+	*val = (be16_to_cpu(ret_value) >>2)&0xFFF*2500/4096;
 
 	return IIO_VAL_INT;
 out:
@@ -174,9 +132,7 @@ static int __devinit as1531_probe(struct spi_device *spi)
 
 static int as1531_remove(struct spi_device *spi)
 {
-	struct iio_dev *indio_dev = spi_get_drvdata(spi);
-
-	iio_device_unregister(indio_dev);
+	iio_device_unregister(spi_get_drvdata(spi));
 	return 0;
 }
 
-- 
1.7.3.4

[lm-sensors] [PATCH 2/2] staging:iio:adc:as1351 general cleanup and

[Jonathan Cameron]

Signed-off-by: Jonathan Cameron <jic23@cam.ac.uk>
---
 drivers/staging/iio/adc/as1531.c |   60 +++++--------------------------------
 1 files changed, 8 insertions(+), 52 deletions(-)

diff --git a/drivers/staging/iio/adc/as1531.c b/drivers/staging/iio/adc/as1531.c
index 7222549..30119ce 100644
--- a/drivers/staging/iio/adc/as1531.c
+++ b/drivers/staging/iio/adc/as1531.c
@@ -58,47 +58,6 @@ struct as1531_state {
 	struct mutex lock;
 };
 
-static int as1531_message(struct spi_device *spi, int cmd, int *ret_value)
-{
-	struct spi_message	message;
-	struct spi_transfer	x[1];
-	int status, i;
-	u8	cmd_send;
-	unsigned char buf[64];
-	unsigned char buf_read[64];
-
-	cmd_send = cmd;
-
-	spi_message_init(&message);
-	memset(x, 0, sizeof x);
-	memset(buf, 0, sizeof(buf));
-	memset(buf_read, 0, sizeof(buf_read));
-
-	for (i = 0; i < 8; i++) {
-		buf[i] = ((cmd_send & 0x80)>>7);
-		cmd_send = cmd_send << 1;
-	}
-
-	x[0].tx_buf = buf;
-	x[0].len = 24;
-	x[0].rx_buf = buf_read;
-	x[0].speed_hz = AS1531_SPI_SPEED;
-	x[0].bits_per_word = 1;
-	spi_message_add_tail(&x[0], &message);
-
-	status = spi_sync(spi, &message);
-	if (status < 0)
-		return status;
-
-	*ret_value = buf_read[11] & 0x01;
-	for (i = 12; i < 23 ; i++) {
-		*ret_value = *ret_value << 1;
-		*ret_value = *ret_value | (buf_read[i]&0x01);
-	}
-
-	return 0;
-}
-
 static int as1531_read_raw(struct iio_dev *indio_dev,
 			   struct iio_chan_spec const *chan,
 			   int *val,
@@ -107,21 +66,20 @@ static int as1531_read_raw(struct iio_dev *indio_dev,
 {
 
 	int status = 0;
-	int ret_value = 0;
+	u16 ret_value = 0;
 	struct as1531_state *st = iio_priv(indio_dev);
 	if (mutex_lock_interruptible(&st->lock))
 		return -ERESTARTSYS;
 
-	status = as1531_message(st->spi,
-				AS1531_START_BIT | chan->address |
-				AS1531_RANGE_0_TO_VREF | AS1531_MODE_COM |
-				AS1531_POWER_NORMAL,
-				&ret_value);
+	status = spi_w8r16(st->spi,
+			   AS1531_START_BIT | chan->address |
+			   AS1531_RANGE_0_TO_VREF | AS1531_MODE_COM |
+			   AS1531_POWER_NORMAL);
 	mutex_unlock(&st->lock);
 	if (status < 0)
 		goto out;
-	
-	*val = ret_value*2500/4096;
+	ret_value = status;
+	*val = (be16_to_cpu(ret_value) >>2)&0xFFF*2500/4096;
 
 	return IIO_VAL_INT;
 out:
@@ -174,9 +132,7 @@ static int __devinit as1531_probe(struct spi_device *spi)
 
 static int as1531_remove(struct spi_device *spi)
 {
-	struct iio_dev *indio_dev = spi_get_drvdata(spi);
-
-	iio_device_unregister(indio_dev);
+	iio_device_unregister(spi_get_drvdata(spi));
 	return 0;
 }
 
-- 
1.7.3.4


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

Re: [PATCH 2/2] staging:iio:adc:as1351 general cleanup and conversion to standard functions.

[Guenter Roeck]

Hi Jonathan,

On Wed, May 18, 2011 at 11:39:32AM -0400, Jonathan Cameron wrote:
> Signed-off-by: Jonathan Cameron <jic23@cam.ac.uk>
> ---
>  drivers/staging/iio/adc/as1531.c |   60 +++++--------------------------------
>  1 files changed, 8 insertions(+), 52 deletions(-)
> 
> diff --git a/drivers/staging/iio/adc/as1531.c b/drivers/staging/iio/adc/as1531.c
> index 7222549..30119ce 100644
> --- a/drivers/staging/iio/adc/as1531.c
> +++ b/drivers/staging/iio/adc/as1531.c
> @@ -58,47 +58,6 @@ struct as1531_state {
>  	struct mutex lock;
>  };
>  
> -static int as1531_message(struct spi_device *spi, int cmd, int *ret_value)
> -{
> -	struct spi_message	message;
> -	struct spi_transfer	x[1];
> -	int status, i;
> -	u8	cmd_send;
> -	unsigned char buf[64];
> -	unsigned char buf_read[64];
> -
> -	cmd_send = cmd;
> -
> -	spi_message_init(&message);
> -	memset(x, 0, sizeof x);
> -	memset(buf, 0, sizeof(buf));
> -	memset(buf_read, 0, sizeof(buf_read));
> -
> -	for (i = 0; i < 8; i++) {
> -		buf[i] = ((cmd_send & 0x80)>>7);
> -		cmd_send = cmd_send << 1;
> -	}
> -
> -	x[0].tx_buf = buf;
> -	x[0].len = 24;
> -	x[0].rx_buf = buf_read;
> -	x[0].speed_hz = AS1531_SPI_SPEED;
> -	x[0].bits_per_word = 1;
> -	spi_message_add_tail(&x[0], &message);
> -
> -	status = spi_sync(spi, &message);
> -	if (status < 0)
> -		return status;
> -
> -	*ret_value = buf_read[11] & 0x01;
> -	for (i = 12; i < 23 ; i++) {
> -		*ret_value = *ret_value << 1;
> -		*ret_value = *ret_value | (buf_read[i]&0x01);
> -	}
> -
> -	return 0;
> -}
> -
>  static int as1531_read_raw(struct iio_dev *indio_dev,
>  			   struct iio_chan_spec const *chan,
>  			   int *val,
> @@ -107,21 +66,20 @@ static int as1531_read_raw(struct iio_dev *indio_dev,
>  {
>  
>  	int status = 0;
> -	int ret_value = 0;
> +	u16 ret_value = 0;
>  	struct as1531_state *st = iio_priv(indio_dev);
>  	if (mutex_lock_interruptible(&st->lock))
>  		return -ERESTARTSYS;
>  
> -	status = as1531_message(st->spi,
> -				AS1531_START_BIT | chan->address |
> -				AS1531_RANGE_0_TO_VREF | AS1531_MODE_COM |
> -				AS1531_POWER_NORMAL,
> -				&ret_value);
> +	status = spi_w8r16(st->spi,
> +			   AS1531_START_BIT | chan->address |
> +			   AS1531_RANGE_0_TO_VREF | AS1531_MODE_COM |
> +			   AS1531_POWER_NORMAL);
>  	mutex_unlock(&st->lock);
>  	if (status < 0)
>  		goto out;
> -	
> -	*val = ret_value*2500/4096;
> +	ret_value = status;
> +	*val = (be16_to_cpu(ret_value) >>2)&0xFFF*2500/4096;

checkpatch does complain about this one ... oddly enough, though, only about the missing space
between >> and 2.

& has lower precedence than * and /, so I am not sure if the code here does
what you expect it to do.

Thanks,
Guenter

Re: [lm-sensors] [PATCH 2/2] staging:iio:adc:as1351 general cleanup

[Guenter Roeck]

Hi Jonathan,

On Wed, May 18, 2011 at 11:39:32AM -0400, Jonathan Cameron wrote:
> Signed-off-by: Jonathan Cameron <jic23@cam.ac.uk>
> ---
>  drivers/staging/iio/adc/as1531.c |   60 +++++--------------------------------
>  1 files changed, 8 insertions(+), 52 deletions(-)
> 
> diff --git a/drivers/staging/iio/adc/as1531.c b/drivers/staging/iio/adc/as1531.c
> index 7222549..30119ce 100644
> --- a/drivers/staging/iio/adc/as1531.c
> +++ b/drivers/staging/iio/adc/as1531.c
> @@ -58,47 +58,6 @@ struct as1531_state {
>  	struct mutex lock;
>  };
>  
> -static int as1531_message(struct spi_device *spi, int cmd, int *ret_value)
> -{
> -	struct spi_message	message;
> -	struct spi_transfer	x[1];
> -	int status, i;
> -	u8	cmd_send;
> -	unsigned char buf[64];
> -	unsigned char buf_read[64];
> -
> -	cmd_send = cmd;
> -
> -	spi_message_init(&message);
> -	memset(x, 0, sizeof x);
> -	memset(buf, 0, sizeof(buf));
> -	memset(buf_read, 0, sizeof(buf_read));
> -
> -	for (i = 0; i < 8; i++) {
> -		buf[i] = ((cmd_send & 0x80)>>7);
> -		cmd_send = cmd_send << 1;
> -	}
> -
> -	x[0].tx_buf = buf;
> -	x[0].len = 24;
> -	x[0].rx_buf = buf_read;
> -	x[0].speed_hz = AS1531_SPI_SPEED;
> -	x[0].bits_per_word = 1;
> -	spi_message_add_tail(&x[0], &message);
> -
> -	status = spi_sync(spi, &message);
> -	if (status < 0)
> -		return status;
> -
> -	*ret_value = buf_read[11] & 0x01;
> -	for (i = 12; i < 23 ; i++) {
> -		*ret_value = *ret_value << 1;
> -		*ret_value = *ret_value | (buf_read[i]&0x01);
> -	}
> -
> -	return 0;
> -}
> -
>  static int as1531_read_raw(struct iio_dev *indio_dev,
>  			   struct iio_chan_spec const *chan,
>  			   int *val,
> @@ -107,21 +66,20 @@ static int as1531_read_raw(struct iio_dev *indio_dev,
>  {
>  
>  	int status = 0;
> -	int ret_value = 0;
> +	u16 ret_value = 0;
>  	struct as1531_state *st = iio_priv(indio_dev);
>  	if (mutex_lock_interruptible(&st->lock))
>  		return -ERESTARTSYS;
>  
> -	status = as1531_message(st->spi,
> -				AS1531_START_BIT | chan->address |
> -				AS1531_RANGE_0_TO_VREF | AS1531_MODE_COM |
> -				AS1531_POWER_NORMAL,
> -				&ret_value);
> +	status = spi_w8r16(st->spi,
> +			   AS1531_START_BIT | chan->address |
> +			   AS1531_RANGE_0_TO_VREF | AS1531_MODE_COM |
> +			   AS1531_POWER_NORMAL);
>  	mutex_unlock(&st->lock);
>  	if (status < 0)
>  		goto out;
> -	
> -	*val = ret_value*2500/4096;
> +	ret_value = status;
> +	*val = (be16_to_cpu(ret_value) >>2)&0xFFF*2500/4096;

checkpatch does complain about this one ... oddly enough, though, only about the missing space
between >> and 2.

& has lower precedence than * and /, so I am not sure if the code here does
what you expect it to do.

Thanks,
Guenter

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

Re: [lm-sensors] [PATCH] hwmon: Driver for as1531,

[Guenter Roeck]

On Tue, May 17, 2011 at 05:25:01AM -0400, Jonathan Cameron wrote:
> On 05/17/11 08:06, Fabien Marteau wrote:
> > Hi Guenter,
> > 
> > Thanks for the review.
> > 
> > On 16/05/2011 17:39, Guenter Roeck wrote:
> >> On Mon, May 16, 2011 at 09:39:14AM -0400, fabien.marteau@armadeus.com wrote:
> >>> From: Fabien Marteau <fabien.marteau@armadeus.com>
> >>>
> >>>
> >> Some description, such as "Dhis driver adds support for xxx" would be nice.
> >>
> >> Also, I wonder if this driver belongs into hwmon in the first place. It is
> >> a generic ADC chip with high conversion rate. iio would probably be more
> >> appropriate and also much better in supporting high speed readings.
> > I provided this driver "as is" because it's a driver that work well on
> > our platform. I thought that iio was not stable enough driver framework
> > to be used.
> > I can rewrite it under iio framework but I have no time for this moment
> > to do that. You think it's better to wait for an iio driver or to
> > continue commiting this ?
> I'd say that if you primary use is hwmon, put it there for now and we can think
> about moving it at a later date depending on how people are actually using it.
> Guenter, would that be ok for you?

Yes, that would be ok.

Thanks,
Guenter

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

Re: [PATCH] hwmon: Driver for as1531, Austria-Microsystem Analog to Digital Converter.

[Guenter Roeck]

On Tue, May 17, 2011 at 05:25:01AM -0400, Jonathan Cameron wrote:
> On 05/17/11 08:06, Fabien Marteau wrote:
> > Hi Guenter,
> > 
> > Thanks for the review.
> > 
> > On 16/05/2011 17:39, Guenter Roeck wrote:
> >> On Mon, May 16, 2011 at 09:39:14AM -0400, fabien.marteau@armadeus.com wrote:
> >>> From: Fabien Marteau <fabien.marteau@armadeus.com>
> >>>
> >>>
> >> Some description, such as "Dhis driver adds support for xxx" would be nice.
> >>
> >> Also, I wonder if this driver belongs into hwmon in the first place. It is
> >> a generic ADC chip with high conversion rate. iio would probably be more
> >> appropriate and also much better in supporting high speed readings.
> > I provided this driver "as is" because it's a driver that work well on
> > our platform. I thought that iio was not stable enough driver framework
> > to be used.
> > I can rewrite it under iio framework but I have no time for this moment
> > to do that. You think it's better to wait for an iio driver or to
> > continue commiting this ?
> I'd say that if you primary use is hwmon, put it there for now and we can think
> about moving it at a later date depending on how people are actually using it.
> Guenter, would that be ok for you?

Yes, that would be ok.

Thanks,
Guenter

Re: [lm-sensors] [PATCH] hwmon: Driver for as1531,

[Jonathan Cameron]

On 05/16/11 14:39, fabien.marteau@armadeus.com wrote:
> From: Fabien Marteau <fabien.marteau@armadeus.com>
> 
Had a few minutes so done a quick port to IIO with minimal code changes.
Now looking at cleaning things up so actually reading the code rather
than cut and paste.  The message is somewhat 'novel'. 

...

So we have a spi device that only talks in 1 bit words?
1) I'm amazed any spi controllers support that.
2) Doesn't look necessary from the data sheet google furnished me with.

Looks like it will quite happily work with 8 bit transfers. Suggested replacement code below.
> +static int as1531_message(struct spi_device *spi, int cmd, int *ret_value)
> +{
> +	struct spi_message	message;
> +	struct spi_transfer	x[1];
> +	int status, i;
> +	u8	cmd_send;
> +	unsigned char buf[64];
> +	unsigned char buf_read[64];
> +
> +	cmd_send = cmd;
> +
> +	spi_message_init(&message);
> +	memset(x, 0, sizeof x);
> +	memset(buf, 0, sizeof(buf));
> +	memset(buf_read, 0, sizeof(buf_read));
> +
> +	for (i = 0; i < 8; i++) {
> +		buf[i] = ((cmd_send & 0x80)>>7);
> +		cmd_send = cmd_send << 1;
> +	}
> +
> +	x[0].tx_buf = buf;
> +	x[0].len = 24;
> +	x[0].rx_buf = buf_read;
> +	x[0].speed_hz = AS1531_SPI_SPEED;
> +	x[0].bits_per_word = 1;
> +	spi_message_add_tail(&x[0], &message);
> +
> +	status = spi_sync(spi, &message);
> +	if (status < 0)
> +		return status;
> +
> +	*ret_value = buf_read[11] & 0x01;
> +	for (i = 12; i < 23 ; i++) {
> +		*ret_value = *ret_value << 1;
> +		*ret_value = *ret_value | (buf_read[i]&0x01);
> +	}
> +
> +	return 0;
> +}
static int as1531_message(struct spi_device *spi, int cmd, u16 *ret_value)
{
	int status;
	u8 cmd_send = cmd;
	struct spi_message message;
	struct spi_transfer x[2] = {
		{
			.len = 1,
			/* this should be default anyway - so could drop */
			.bits_per_word = 8,
			/* This should be set in board config really */
			.speed_hz = AS1531_SPI_SPEED,
			.rx_buf = &cmd_send,
		}, {
			.len = 2,
			.bits_per_word = 8,
			.speed_hz = AS1531_SPI_SPEED,
			.tx_buf = ret_value,
		}
	};

	spi_message_init(&message);
	spi_message_add_tail(&x[0], &message);
	spi_message_add_tail(&x[1], &message);

	status = spi_sync(spi, &message);
	if (status < 0)
		return status;

	*ret_value = (be16_to_cpup(ret_value) >> 2) & 0xFFF;

	return 0;
}

Obviously also requires changing the type of the element passed
as ret_value.  Could set the bits_per_word of the second transfer
to 16 and avoid the endianness conversion, but I'm not sure how
many spi masters support that.

Jonathan

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

Re: [PATCH] hwmon: Driver for as1531, Austria-Microsystem Analog to Digital Converter.

[Jonathan Cameron]

On 05/16/11 14:39, fabien.marteau@armadeus.com wrote:
> From: Fabien Marteau <fabien.marteau@armadeus.com>
> 
Had a few minutes so done a quick port to IIO with minimal code changes.
Now looking at cleaning things up so actually reading the code rather
than cut and paste.  The message is somewhat 'novel'. 

...

So we have a spi device that only talks in 1 bit words?
1) I'm amazed any spi controllers support that.
2) Doesn't look necessary from the data sheet google furnished me with.

Looks like it will quite happily work with 8 bit transfers. Suggested replacement code below.
> +static int as1531_message(struct spi_device *spi, int cmd, int *ret_value)
> +{
> +	struct spi_message	message;
> +	struct spi_transfer	x[1];
> +	int status, i;
> +	u8	cmd_send;
> +	unsigned char buf[64];
> +	unsigned char buf_read[64];
> +
> +	cmd_send = cmd;
> +
> +	spi_message_init(&message);
> +	memset(x, 0, sizeof x);
> +	memset(buf, 0, sizeof(buf));
> +	memset(buf_read, 0, sizeof(buf_read));
> +
> +	for (i = 0; i < 8; i++) {
> +		buf[i] = ((cmd_send & 0x80)>>7);
> +		cmd_send = cmd_send << 1;
> +	}
> +
> +	x[0].tx_buf = buf;
> +	x[0].len = 24;
> +	x[0].rx_buf = buf_read;
> +	x[0].speed_hz = AS1531_SPI_SPEED;
> +	x[0].bits_per_word = 1;
> +	spi_message_add_tail(&x[0], &message);
> +
> +	status = spi_sync(spi, &message);
> +	if (status < 0)
> +		return status;
> +
> +	*ret_value = buf_read[11] & 0x01;
> +	for (i = 12; i < 23 ; i++) {
> +		*ret_value = *ret_value << 1;
> +		*ret_value = *ret_value | (buf_read[i]&0x01);
> +	}
> +
> +	return 0;
> +}
static int as1531_message(struct spi_device *spi, int cmd, u16 *ret_value)
{
	int status;
	u8 cmd_send = cmd;
	struct spi_message message;
	struct spi_transfer x[2] = {
		{
			.len = 1,
			/* this should be default anyway - so could drop */
			.bits_per_word = 8,
			/* This should be set in board config really */
			.speed_hz = AS1531_SPI_SPEED,
			.rx_buf = &cmd_send,
		}, {
			.len = 2,
			.bits_per_word = 8,
			.speed_hz = AS1531_SPI_SPEED,
			.tx_buf = ret_value,
		}
	};

	spi_message_init(&message);
	spi_message_add_tail(&x[0], &message);
	spi_message_add_tail(&x[1], &message);

	status = spi_sync(spi, &message);
	if (status < 0)
		return status;

	*ret_value = (be16_to_cpup(ret_value) >> 2) & 0xFFF;

	return 0;
}

Obviously also requires changing the type of the element passed
as ret_value.  Could set the bits_per_word of the second transfer
to 16 and avoid the endianness conversion, but I'm not sure how
many spi masters support that.

Jonathan

Re: [lm-sensors] [PATCH] hwmon: Driver for as1531,

[Jonathan Cameron]

On 05/18/11 14:09, Jonathan Cameron wrote:
> On 05/16/11 14:39, fabien.marteau@armadeus.com wrote:
>> From: Fabien Marteau <fabien.marteau@armadeus.com>
>>
> Had a few minutes so done a quick port to IIO with minimal code changes.
> Now looking at cleaning things up so actually reading the code rather
> than cut and paste.  The message is somewhat 'novel'. 
> 
> ...
> 
> So we have a spi device that only talks in 1 bit words?
> 1) I'm amazed any spi controllers support that.
> 2) Doesn't look necessary from the data sheet google furnished me with.
> 
> Looks like it will quite happily work with 8 bit transfers. Suggested replacement code below.
>> +static int as1531_message(struct spi_device *spi, int cmd, int *ret_value)
>> +{
>> +	struct spi_message	message;
>> +	struct spi_transfer	x[1];
>> +	int status, i;
>> +	u8	cmd_send;
>> +	unsigned char buf[64];
>> +	unsigned char buf_read[64];
>> +
>> +	cmd_send = cmd;
>> +
>> +	spi_message_init(&message);
>> +	memset(x, 0, sizeof x);
>> +	memset(buf, 0, sizeof(buf));
>> +	memset(buf_read, 0, sizeof(buf_read));
>> +
>> +	for (i = 0; i < 8; i++) {
>> +		buf[i] = ((cmd_send & 0x80)>>7);
>> +		cmd_send = cmd_send << 1;
>> +	}
>> +
>> +	x[0].tx_buf = buf;
>> +	x[0].len = 24;
>> +	x[0].rx_buf = buf_read;
>> +	x[0].speed_hz = AS1531_SPI_SPEED;
>> +	x[0].bits_per_word = 1;
>> +	spi_message_add_tail(&x[0], &message);
>> +
>> +	status = spi_sync(spi, &message);
>> +	if (status < 0)
>> +		return status;
>> +
>> +	*ret_value = buf_read[11] & 0x01;
>> +	for (i = 12; i < 23 ; i++) {
>> +		*ret_value = *ret_value << 1;
>> +		*ret_value = *ret_value | (buf_read[i]&0x01);
>> +	}
>> +
>> +	return 0;
>> +}

Or skip the below and use spi_w8r16. This isn't a fast capture route anyway.
> static int as1531_message(struct spi_device *spi, int cmd, u16 *ret_value)
> {
> 	int status;
> 	u8 cmd_send = cmd;
> 	struct spi_message message;
> 	struct spi_transfer x[2] = {
> 		{
> 			.len = 1,
> 			/* this should be default anyway - so could drop */
> 			.bits_per_word = 8,
> 			/* This should be set in board config really */
> 			.speed_hz = AS1531_SPI_SPEED,
> 			.rx_buf = &cmd_send,
> 		}, {
> 			.len = 2,
> 			.bits_per_word = 8,
> 			.speed_hz = AS1531_SPI_SPEED,
> 			.tx_buf = ret_value,
> 		}
> 	};
> 
> 	spi_message_init(&message);
> 	spi_message_add_tail(&x[0], &message);
> 	spi_message_add_tail(&x[1], &message);
> 
> 	status = spi_sync(spi, &message);
> 	if (status < 0)
> 		return status;
> 
> 	*ret_value = (be16_to_cpup(ret_value) >> 2) & 0xFFF;
> 
> 	return 0;
> }
> 
> Obviously also requires changing the type of the element passed
> as ret_value.  Could set the bits_per_word of the second transfer
> to 16 and avoid the endianness conversion, but I'm not sure how
> many spi masters support that.
> 
> Jonathan


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

Re: [PATCH] hwmon: Driver for as1531, Austria-Microsystem Analog to Digital Converter.

[Jonathan Cameron]

On 05/18/11 14:09, Jonathan Cameron wrote:
> On 05/16/11 14:39, fabien.marteau@armadeus.com wrote:
>> From: Fabien Marteau <fabien.marteau@armadeus.com>
>>
> Had a few minutes so done a quick port to IIO with minimal code changes.
> Now looking at cleaning things up so actually reading the code rather
> than cut and paste.  The message is somewhat 'novel'. 
> 
> ...
> 
> So we have a spi device that only talks in 1 bit words?
> 1) I'm amazed any spi controllers support that.
> 2) Doesn't look necessary from the data sheet google furnished me with.
> 
> Looks like it will quite happily work with 8 bit transfers. Suggested replacement code below.
>> +static int as1531_message(struct spi_device *spi, int cmd, int *ret_value)
>> +{
>> +	struct spi_message	message;
>> +	struct spi_transfer	x[1];
>> +	int status, i;
>> +	u8	cmd_send;
>> +	unsigned char buf[64];
>> +	unsigned char buf_read[64];
>> +
>> +	cmd_send = cmd;
>> +
>> +	spi_message_init(&message);
>> +	memset(x, 0, sizeof x);
>> +	memset(buf, 0, sizeof(buf));
>> +	memset(buf_read, 0, sizeof(buf_read));
>> +
>> +	for (i = 0; i < 8; i++) {
>> +		buf[i] = ((cmd_send & 0x80)>>7);
>> +		cmd_send = cmd_send << 1;
>> +	}
>> +
>> +	x[0].tx_buf = buf;
>> +	x[0].len = 24;
>> +	x[0].rx_buf = buf_read;
>> +	x[0].speed_hz = AS1531_SPI_SPEED;
>> +	x[0].bits_per_word = 1;
>> +	spi_message_add_tail(&x[0], &message);
>> +
>> +	status = spi_sync(spi, &message);
>> +	if (status < 0)
>> +		return status;
>> +
>> +	*ret_value = buf_read[11] & 0x01;
>> +	for (i = 12; i < 23 ; i++) {
>> +		*ret_value = *ret_value << 1;
>> +		*ret_value = *ret_value | (buf_read[i]&0x01);
>> +	}
>> +
>> +	return 0;
>> +}

Or skip the below and use spi_w8r16. This isn't a fast capture route anyway.
> static int as1531_message(struct spi_device *spi, int cmd, u16 *ret_value)
> {
> 	int status;
> 	u8 cmd_send = cmd;
> 	struct spi_message message;
> 	struct spi_transfer x[2] = {
> 		{
> 			.len = 1,
> 			/* this should be default anyway - so could drop */
> 			.bits_per_word = 8,
> 			/* This should be set in board config really */
> 			.speed_hz = AS1531_SPI_SPEED,
> 			.rx_buf = &cmd_send,
> 		}, {
> 			.len = 2,
> 			.bits_per_word = 8,
> 			.speed_hz = AS1531_SPI_SPEED,
> 			.tx_buf = ret_value,
> 		}
> 	};
> 
> 	spi_message_init(&message);
> 	spi_message_add_tail(&x[0], &message);
> 	spi_message_add_tail(&x[1], &message);
> 
> 	status = spi_sync(spi, &message);
> 	if (status < 0)
> 		return status;
> 
> 	*ret_value = (be16_to_cpup(ret_value) >> 2) & 0xFFF;
> 
> 	return 0;
> }
> 
> Obviously also requires changing the type of the element passed
> as ret_value.  Could set the bits_per_word of the second transfer
> to 16 and avoid the endianness conversion, but I'm not sure how
> many spi masters support that.
> 
> Jonathan