[PATCH 1/4] iio: core: constitfy available_scan_mask

[PATCH 1/4] iio: core: constitfy available_scan_mask

[michael.hennerich]

From: Michael Hennerich <michael.hennerich@analog.com>

The core must not modify available_scan_mask, because it causes problems
with drivers where multiple instances of the driver share the same mask set.
So make this explicit by marking available scan masks as const.

The max1363 driver needs some minor adjustment to accommodate this change.

Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
---
 drivers/staging/iio/adc/max1363_core.c    |   36 +++++++++++++++++++---------
 drivers/staging/iio/iio.h                 |    4 +-
 drivers/staging/iio/industrialio-buffer.c |    6 ++--
 3 files changed, 29 insertions(+), 17 deletions(-)

diff --git a/drivers/staging/iio/adc/max1363_core.c b/drivers/staging/iio/adc/max1363_core.c
index 3a5bd20..7f39ca3 100644
--- a/drivers/staging/iio/adc/max1363_core.c
+++ b/drivers/staging/iio/adc/max1363_core.c
@@ -1245,10 +1245,31 @@ static int max1363_initial_setup(struct max1363_state *st)
 	return max1363_set_scan_mode(st);
 }
 
+static int __devinit max1363_alloc_scan_masks(struct iio_dev *indio_dev)
+{
+	struct max1363_state *st = iio_priv(indio_dev);
+	unsigned long *masks;
+	int i;
+
+	masks = kzalloc(BITS_TO_LONGS(MAX1363_MAX_CHANNELS)*sizeof(long)*
+			  (st->chip_info->num_modes + 1), GFP_KERNEL);
+	if (!masks)
+		return -ENOMEM;
+
+	for (i = 0; i < st->chip_info->num_modes; i++)
+		bitmap_copy(masks + BITS_TO_LONGS(MAX1363_MAX_CHANNELS)*i,
+			    max1363_mode_table[st->chip_info->mode_list[i]]
+			    .modemask, MAX1363_MAX_CHANNELS);
+
+	indio_dev->available_scan_masks = masks;
+
+	return 0;
+}
+
 static int __devinit max1363_probe(struct i2c_client *client,
 				   const struct i2c_device_id *id)
 {
-	int ret, i;
+	int ret;
 	struct max1363_state *st;
 	struct iio_dev *indio_dev;
 	struct regulator *reg;
@@ -1276,19 +1297,10 @@ static int __devinit max1363_probe(struct i2c_client *client,
 	st->chip_info = &max1363_chip_info_tbl[id->driver_data];
 	st->client = client;
 
-	indio_dev->available_scan_masks
-		= kzalloc(BITS_TO_LONGS(MAX1363_MAX_CHANNELS)*sizeof(long)*
-			  (st->chip_info->num_modes + 1), GFP_KERNEL);
-	if (!indio_dev->available_scan_masks) {
-		ret = -ENOMEM;
+	ret = max1363_alloc_scan_masks(indio_dev);
+	if (ret)
 		goto error_free_device;
-	}
 
-	for (i = 0; i < st->chip_info->num_modes; i++)
-		bitmap_copy(indio_dev->available_scan_masks +
-			    BITS_TO_LONGS(MAX1363_MAX_CHANNELS)*i,
-			    max1363_mode_table[st->chip_info->mode_list[i]]
-			    .modemask, MAX1363_MAX_CHANNELS);
 	/* Estabilish that the iio_dev is a child of the i2c device */
 	indio_dev->dev.parent = &client->dev;
 	indio_dev->name = id->name;
diff --git a/drivers/staging/iio/iio.h b/drivers/staging/iio/iio.h
index be6ced3..42a362c 100644
--- a/drivers/staging/iio/iio.h
+++ b/drivers/staging/iio/iio.h
@@ -327,9 +327,9 @@ struct iio_dev {
 	struct iio_buffer		*buffer;
 	struct mutex			mlock;
 
-	unsigned long			*available_scan_masks;
+	const unsigned long		*available_scan_masks;
 	unsigned			masklength;
-	unsigned long			*active_scan_mask;
+	const unsigned long		*active_scan_mask;
 	struct iio_trigger		*trig;
 	struct iio_poll_func		*pollfunc;
 
diff --git a/drivers/staging/iio/industrialio-buffer.c b/drivers/staging/iio/industrialio-buffer.c
index d7b1e9e..386ba76 100644
--- a/drivers/staging/iio/industrialio-buffer.c
+++ b/drivers/staging/iio/industrialio-buffer.c
@@ -489,9 +489,9 @@ ssize_t iio_buffer_show_enable(struct device *dev,
 EXPORT_SYMBOL(iio_buffer_show_enable);
 
 /* note NULL used as error indicator as it doesn't make sense. */
-static unsigned long *iio_scan_mask_match(unsigned long *av_masks,
+static const unsigned long *iio_scan_mask_match(const unsigned long *av_masks,
 					  unsigned int masklength,
-					  unsigned long *mask)
+					  const unsigned long *mask)
 {
 	if (bitmap_empty(mask, masklength))
 		return NULL;
@@ -554,7 +554,7 @@ EXPORT_SYMBOL(iio_sw_buffer_preenable);
 int iio_scan_mask_set(struct iio_dev *indio_dev,
 		      struct iio_buffer *buffer, int bit)
 {
-	unsigned long *mask;
+	const unsigned long *mask;
 	unsigned long *trialmask;
 
 	trialmask = kmalloc(sizeof(*trialmask)*
-- 
1.7.0.4

[PATCH 2/4] iio: core: Introduce debugfs support, add support for direct register access

[michael.hennerich]

From: Michael Hennerich <michael.hennerich@analog.com>

Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
---
 drivers/staging/iio/iio.h               |    5 +
 drivers/staging/iio/industrialio-core.c |  124 ++++++++++++++++++++++++++++++-
 2 files changed, 128 insertions(+), 1 deletions(-)

diff --git a/drivers/staging/iio/iio.h b/drivers/staging/iio/iio.h
index 42a362c..d0cf71f 100644
--- a/drivers/staging/iio/iio.h
+++ b/drivers/staging/iio/iio.h
@@ -269,6 +269,9 @@ struct iio_info {
 				struct iio_trigger *trig);
 	int (*update_scan_mode)(struct iio_dev *indio_dev,
 				const unsigned long *scan_mask);
+	int (*debugfs_reg_access)(struct iio_dev *indio_dev,
+				  unsigned reg, unsigned writeval,
+				  unsigned *readval);
 };
 
 /**
@@ -347,6 +350,8 @@ struct iio_dev {
 	int				groupcounter;
 
 	unsigned long			flags;
+	struct dentry			*debugfs_dentry;
+	unsigned			cached_reg_addr;
 };
 
 /**
diff --git a/drivers/staging/iio/industrialio-core.c b/drivers/staging/iio/industrialio-core.c
index e4824fe..cf48489 100644
--- a/drivers/staging/iio/industrialio-core.c
+++ b/drivers/staging/iio/industrialio-core.c
@@ -22,6 +22,7 @@
 #include <linux/cdev.h>
 #include <linux/slab.h>
 #include <linux/anon_inodes.h>
+#include <linux/debugfs.h>
 #include "iio.h"
 #include "iio_core.h"
 #include "iio_core_trigger.h"
@@ -39,6 +40,8 @@ struct bus_type iio_bus_type = {
 };
 EXPORT_SYMBOL(iio_bus_type);
 
+static struct dentry *iio_debugfs_dentry;
+
 static const char * const iio_data_type_name[] = {
 	[IIO_RAW] = "raw",
 	[IIO_PROCESSED] = "input",
@@ -129,6 +132,8 @@ static int __init iio_init(void)
 		goto error_unregister_bus_type;
 	}
 
+	iio_debugfs_dentry = debugfs_create_dir("iio", NULL);
+
 	return 0;
 
 error_unregister_bus_type:
@@ -142,6 +147,114 @@ static void __exit iio_exit(void)
 	if (iio_devt)
 		unregister_chrdev_region(iio_devt, IIO_DEV_MAX);
 	bus_unregister(&iio_bus_type);
+	debugfs_remove_recursive(iio_debugfs_dentry);
+}
+
+static int iio_debugfs_open(struct inode *inode, struct file *file)
+{
+	if (inode->i_private)
+		file->private_data = inode->i_private;
+
+	return 0;
+}
+
+static ssize_t iio_debugfs_read_reg(struct file *file, char __user *userbuf,
+			      size_t count, loff_t *ppos)
+{
+	struct iio_dev *indio_dev = file->private_data;
+	char buf[80], *p = buf;
+	unsigned val;
+	int ret;
+
+	ret = indio_dev->info->debugfs_reg_access(indio_dev,
+						  indio_dev->cached_reg_addr,
+						  0, &val);
+	if (ret)
+		dev_err(indio_dev->dev.parent, "%s: read failed\n", __func__);
+
+	p += sprintf(p, "0x%X\n", val);
+
+	return simple_read_from_buffer(userbuf, count, ppos, buf, p - buf);
+}
+
+static ssize_t iio_debugfs_write_reg(struct file *file,
+		     const char __user *userbuf, size_t count, loff_t *ppos)
+{
+	struct iio_dev *indio_dev = file->private_data;
+	unsigned reg, val;
+	char buf[80], *p = buf;
+	int ret;
+
+	count = min_t(size_t, count, (sizeof(buf)-1));
+	if (copy_from_user(p, userbuf, count))
+		return -EFAULT;
+
+	ret = sscanf(p, "%x %x", &reg, &val);
+
+	switch (ret) {
+	case 1:
+		indio_dev->cached_reg_addr = reg;
+		break;
+	case 2:
+		indio_dev->cached_reg_addr = reg;
+		ret = indio_dev->info->debugfs_reg_access(indio_dev, reg,
+							  val, NULL);
+		if (ret) {
+			dev_err(indio_dev->dev.parent, "%s: write failed\n",
+				__func__);
+			return ret;
+		}
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return count;
+}
+
+static const struct file_operations iio_debugfs_reg_fops = {
+	.open = iio_debugfs_open,
+	.read = iio_debugfs_read_reg,
+	.write = iio_debugfs_write_reg,
+};
+
+static void iio_device_unregister_debugfs(struct iio_dev *indio_dev)
+{
+	debugfs_remove_recursive(indio_dev->debugfs_dentry);
+}
+
+static int iio_device_register_debugfs(struct iio_dev *indio_dev)
+{
+	struct dentry *d;
+
+	if (indio_dev->info->debugfs_reg_access == NULL)
+		return 0;
+
+	if (IS_ERR(iio_debugfs_dentry))
+		return 0;
+
+	indio_dev->debugfs_dentry =
+		debugfs_create_dir(dev_name(&indio_dev->dev),
+				   iio_debugfs_dentry);
+	if (IS_ERR(indio_dev->debugfs_dentry))
+		return IS_ERR(indio_dev->debugfs_dentry);
+
+	if (indio_dev->debugfs_dentry == NULL) {
+		dev_warn(indio_dev->dev.parent,
+			 "Failed to create debugfs directory\n");
+		return -EFAULT;
+
+	}
+
+	d = debugfs_create_file("direct_reg_access", 0644,
+				indio_dev->debugfs_dentry,
+				indio_dev, &iio_debugfs_reg_fops);
+	if (!d) {
+		iio_device_unregister_debugfs(indio_dev);
+		return -ENOMEM;
+	}
+
+	return 0;
 }
 
 static ssize_t iio_read_channel_info(struct device *dev,
@@ -565,6 +678,7 @@ static void iio_dev_release(struct device *device)
 		iio_device_unregister_trigger_consumer(indio_dev);
 	iio_device_unregister_eventset(indio_dev);
 	iio_device_unregister_sysfs(indio_dev);
+	iio_device_unregister_debugfs(indio_dev);
 }
 
 static struct device_type iio_dev_type = {
@@ -680,11 +794,17 @@ int iio_device_register(struct iio_dev *indio_dev)
 	/* configure elements for the chrdev */
 	indio_dev->dev.devt = MKDEV(MAJOR(iio_devt), indio_dev->id);
 
+	ret = iio_device_register_debugfs(indio_dev);
+	if (ret) {
+		dev_err(indio_dev->dev.parent,
+			"Failed to register debugfs interfaces\n");
+		goto error_ret;
+	}
 	ret = iio_device_register_sysfs(indio_dev);
 	if (ret) {
 		dev_err(indio_dev->dev.parent,
 			"Failed to register sysfs interfaces\n");
-		goto error_ret;
+		goto error_unreg_debugfs;
 	}
 	ret = iio_device_register_eventset(indio_dev);
 	if (ret) {
@@ -711,6 +831,8 @@ error_unreg_eventset:
 	iio_device_unregister_eventset(indio_dev);
 error_free_sysfs:
 	iio_device_unregister_sysfs(indio_dev);
+error_unreg_debugfs:
+	iio_device_unregister_debugfs(indio_dev);
 error_ret:
 	return ret;
 }
-- 
1.7.0.4

Re: [PATCH 2/4] iio: core: Introduce debugfs support, add support for direct register access

[Jonathan Cameron]

On 2/17/2012 12:46 PM, michael.hennerich@analog.com wrote:
> From: Michael Hennerich<michael.hennerich@analog.com>
Please can we have some documentation for this. We may not 'have'
to keep debugfs interfaces the same, but they are a type of userspace
abi.

Worth having the code disappear if debugfs isn't actually being built?

Otherwise all fine with me.
>
> Signed-off-by: Michael Hennerich<michael.hennerich@analog.com>
> ---
>   drivers/staging/iio/iio.h               |    5 +
>   drivers/staging/iio/industrialio-core.c |  124 ++++++++++++++++++++++++++++++-
>   2 files changed, 128 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/staging/iio/iio.h b/drivers/staging/iio/iio.h
> index 42a362c..d0cf71f 100644
> --- a/drivers/staging/iio/iio.h
> +++ b/drivers/staging/iio/iio.h
> @@ -269,6 +269,9 @@ struct iio_info {
>   				struct iio_trigger *trig);
>   	int (*update_scan_mode)(struct iio_dev *indio_dev,
>   				const unsigned long *scan_mask);
> +	int (*debugfs_reg_access)(struct iio_dev *indio_dev,
> +				  unsigned reg, unsigned writeval,
> +				  unsigned *readval);
>   };
>
>   /**
> @@ -347,6 +350,8 @@ struct iio_dev {
>   	int				groupcounter;
>
>   	unsigned long			flags;
> +	struct dentry			*debugfs_dentry;
> +	unsigned			cached_reg_addr;
>   };
>
>   /**
> diff --git a/drivers/staging/iio/industrialio-core.c b/drivers/staging/iio/industrialio-core.c
> index e4824fe..cf48489 100644
> --- a/drivers/staging/iio/industrialio-core.c
> +++ b/drivers/staging/iio/industrialio-core.c
> @@ -22,6 +22,7 @@
>   #include<linux/cdev.h>
>   #include<linux/slab.h>
>   #include<linux/anon_inodes.h>
> +#include<linux/debugfs.h>
>   #include "iio.h"
>   #include "iio_core.h"
>   #include "iio_core_trigger.h"
> @@ -39,6 +40,8 @@ struct bus_type iio_bus_type = {
>   };
>   EXPORT_SYMBOL(iio_bus_type);
>
> +static struct dentry *iio_debugfs_dentry;
> +
>   static const char * const iio_data_type_name[] = {
>   	[IIO_RAW] = "raw",
>   	[IIO_PROCESSED] = "input",
> @@ -129,6 +132,8 @@ static int __init iio_init(void)
>   		goto error_unregister_bus_type;
>   	}
>
> +	iio_debugfs_dentry = debugfs_create_dir("iio", NULL);
> +
>   	return 0;
>
>   error_unregister_bus_type:
> @@ -142,6 +147,114 @@ static void __exit iio_exit(void)
>   	if (iio_devt)
>   		unregister_chrdev_region(iio_devt, IIO_DEV_MAX);
>   	bus_unregister(&iio_bus_type);
> +	debugfs_remove_recursive(iio_debugfs_dentry);
> +}
> +
> +static int iio_debugfs_open(struct inode *inode, struct file *file)
> +{
> +	if (inode->i_private)
> +		file->private_data = inode->i_private;
> +
> +	return 0;
> +}
> +
> +static ssize_t iio_debugfs_read_reg(struct file *file, char __user *userbuf,
> +			      size_t count, loff_t *ppos)
> +{
> +	struct iio_dev *indio_dev = file->private_data;
> +	char buf[80], *p = buf;
> +	unsigned val;
> +	int ret;
> +
> +	ret = indio_dev->info->debugfs_reg_access(indio_dev,
> +						  indio_dev->cached_reg_addr,
> +						  0,&val);
> +	if (ret)
> +		dev_err(indio_dev->dev.parent, "%s: read failed\n", __func__);
> +
I'd prefer a length parameter and using &buf.  A little easier to read 
to my mind.
> +	p += sprintf(p, "0x%X\n", val);
snprintf.  80 chars seems excessive but it is fixed length.
> +
> +	return simple_read_from_buffer(userbuf, count, ppos, buf, p - buf);
> +}
> +
> +static ssize_t iio_debugfs_write_reg(struct file *file,
> +		     const char __user *userbuf, size_t count, loff_t *ppos)
> +{
> +	struct iio_dev *indio_dev = file->private_data;
> +	unsigned reg, val;
> +	char buf[80], *p = buf;
> +	int ret;
> +
> +	count = min_t(size_t, count, (sizeof(buf)-1));
> +	if (copy_from_user(p, userbuf, count))
> +		return -EFAULT;
> +
> +	ret = sscanf(p, "%x %x",&reg,&val);
> +
> +	switch (ret) {
> +	case 1:
> +		indio_dev->cached_reg_addr = reg;
> +		break;
> +	case 2:
> +		indio_dev->cached_reg_addr = reg;
> +		ret = indio_dev->info->debugfs_reg_access(indio_dev, reg,
> +							  val, NULL);
> +		if (ret) {
> +			dev_err(indio_dev->dev.parent, "%s: write failed\n",
> +				__func__);
> +			return ret;
> +		}
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return count;
> +}
> +
> +static const struct file_operations iio_debugfs_reg_fops = {
> +	.open = iio_debugfs_open,
> +	.read = iio_debugfs_read_reg,
> +	.write = iio_debugfs_write_reg,
> +};
> +
> +static void iio_device_unregister_debugfs(struct iio_dev *indio_dev)
> +{
> +	debugfs_remove_recursive(indio_dev->debugfs_dentry);
> +}
> +
> +static int iio_device_register_debugfs(struct iio_dev *indio_dev)
> +{
> +	struct dentry *d;
> +
> +	if (indio_dev->info->debugfs_reg_access == NULL)
> +		return 0;
> +
> +	if (IS_ERR(iio_debugfs_dentry))
> +		return 0;
> +
> +	indio_dev->debugfs_dentry =
> +		debugfs_create_dir(dev_name(&indio_dev->dev),
> +				   iio_debugfs_dentry);
> +	if (IS_ERR(indio_dev->debugfs_dentry))
> +		return IS_ERR(indio_dev->debugfs_dentry);
> +
> +	if (indio_dev->debugfs_dentry == NULL) {
> +		dev_warn(indio_dev->dev.parent,
> +			 "Failed to create debugfs directory\n");
> +		return -EFAULT;
excess blank line.
> +
> +	}
> +
> +	d = debugfs_create_file("direct_reg_access", 0644,
> +				indio_dev->debugfs_dentry,
> +				indio_dev,&iio_debugfs_reg_fops);
> +	if (!d) {
> +		iio_device_unregister_debugfs(indio_dev);
> +		return -ENOMEM;
> +	}
> +
> +	return 0;
>   }
>
>   static ssize_t iio_read_channel_info(struct device *dev,
> @@ -565,6 +678,7 @@ static void iio_dev_release(struct device *device)
>   		iio_device_unregister_trigger_consumer(indio_dev);
>   	iio_device_unregister_eventset(indio_dev);
>   	iio_device_unregister_sysfs(indio_dev);
> +	iio_device_unregister_debugfs(indio_dev);
>   }
>
>   static struct device_type iio_dev_type = {
> @@ -680,11 +794,17 @@ int iio_device_register(struct iio_dev *indio_dev)
>   	/* configure elements for the chrdev */
>   	indio_dev->dev.devt = MKDEV(MAJOR(iio_devt), indio_dev->id);
>
> +	ret = iio_device_register_debugfs(indio_dev);
> +	if (ret) {
> +		dev_err(indio_dev->dev.parent,
> +			"Failed to register debugfs interfaces\n");
> +		goto error_ret;
> +	}
>   	ret = iio_device_register_sysfs(indio_dev);
>   	if (ret) {
>   		dev_err(indio_dev->dev.parent,
>   			"Failed to register sysfs interfaces\n");
> -		goto error_ret;
> +		goto error_unreg_debugfs;
>   	}
>   	ret = iio_device_register_eventset(indio_dev);
>   	if (ret) {
> @@ -711,6 +831,8 @@ error_unreg_eventset:
>   	iio_device_unregister_eventset(indio_dev);
>   error_free_sysfs:
>   	iio_device_unregister_sysfs(indio_dev);
> +error_unreg_debugfs:
> +	iio_device_unregister_debugfs(indio_dev);
>   error_ret:
>   	return ret;
>   }

[PATCH 3/4] of_spi: New function spi_of_node_to_master

[michael.hennerich]

From: Michael Hennerich <michael.hennerich@analog.com>

look up master associated with of_node

Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
---
 drivers/of/of_spi.c     |   28 ++++++++++++++++++++++++++++
 drivers/spi/spi.c       |    4 ++--
 include/linux/of_spi.h  |    1 +
 include/linux/spi/spi.h |    2 ++
 4 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/drivers/of/of_spi.c b/drivers/of/of_spi.c
index 6dbc074..f3dee01 100644
--- a/drivers/of/of_spi.c
+++ b/drivers/of/of_spi.c
@@ -97,3 +97,31 @@ void of_register_spi_devices(struct spi_master *master)
 	}
 }
 EXPORT_SYMBOL(of_register_spi_devices);
+
+static int __spi_master_of_match(struct device *dev, void *data)
+{
+	struct device_node *of_node = data;
+	return dev->of_node == of_node;
+}
+
+/**
+ * spi_of_node_to_master - look up master associated with of_node
+ * @of_node: pointer to the device tree node.
+ * Context: can sleep
+ *
+ * Returns a pointer to the relevant spi_master, or NULL if there is
+ * no such master registered.
+ */
+struct spi_master *spi_of_node_to_master(struct device_node *of_node)
+{
+	struct device *dev;
+	struct spi_master *master = NULL;
+
+	dev = class_find_device(&spi_master_class, NULL, of_node,
+				__spi_master_of_match);
+	if (dev)
+		master = container_of(dev, struct spi_master, dev);
+
+	return master;
+}
+EXPORT_SYMBOL_GPL(spi_of_node_to_master);
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index b2ccdea..6f63a8e 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -515,12 +515,12 @@ static void spi_master_release(struct device *dev)
 	kfree(master);
 }
 
-static struct class spi_master_class = {
+struct class spi_master_class = {
 	.name		= "spi_master",
 	.owner		= THIS_MODULE,
 	.dev_release	= spi_master_release,
 };
-
+EXPORT_SYMBOL_GPL(spi_master_class);
 
 /**
  * spi_alloc_master - allocate SPI master controller
diff --git a/include/linux/of_spi.h b/include/linux/of_spi.h
index 9e3e70f..e1bafd2 100644
--- a/include/linux/of_spi.h
+++ b/include/linux/of_spi.h
@@ -13,6 +13,7 @@
 
 #if defined(CONFIG_OF_SPI) || defined(CONFIG_OF_SPI_MODULE)
 extern void of_register_spi_devices(struct spi_master *master);
+extern struct spi_master *spi_of_node_to_master(struct device_node *of_node);
 #else
 static inline void of_register_spi_devices(struct spi_master *master)
 {
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index 176fce9..62e566e 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -805,4 +805,6 @@ spi_unregister_device(struct spi_device *spi)
 extern const struct spi_device_id *
 spi_get_device_id(const struct spi_device *sdev);
 
+extern struct class spi_master_class;
+
 #endif /* __LINUX_SPI_H */
-- 
1.7.0.4

[PATCH 4/4] iio: adc: New driver for AD9467 and AD9643 High-Speed LVDS ADCs

[michael.hennerich]

From: Michael Hennerich <michael.hennerich@analog.com>

Suitable FPGA interface HDL is available here:
http://wiki.analog.com/resources/fpga/xilinx/fmc/ad9467

Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
---
 drivers/staging/iio/adc/Kconfig          |   13 +
 drivers/staging/iio/adc/Makefile         |    3 +
 drivers/staging/iio/adc/cf_ad9467.h      |  161 ++++++++
 drivers/staging/iio/adc/cf_ad9467_core.c |  585 ++++++++++++++++++++++++++++++
 drivers/staging/iio/adc/cf_ad9467_ring.c |  214 +++++++++++
 5 files changed, 976 insertions(+), 0 deletions(-)
 create mode 100644 drivers/staging/iio/adc/cf_ad9467.h
 create mode 100644 drivers/staging/iio/adc/cf_ad9467_core.c
 create mode 100644 drivers/staging/iio/adc/cf_ad9467_ring.c

diff --git a/drivers/staging/iio/adc/Kconfig b/drivers/staging/iio/adc/Kconfig
index d9decea..266e807 100644
--- a/drivers/staging/iio/adc/Kconfig
+++ b/drivers/staging/iio/adc/Kconfig
@@ -169,6 +169,19 @@ config AD7280
 	  To compile this driver as a module, choose M here: the
 	  module will be called ad7280a
 
+config CF_AD9467
+	tristate "Analog Devices AD9467 AD9643 High-Speed ADC driver"
+	select IIO_BUFFER
+	help
+	  Say yes here to build support for Analog Devices AD9467 and AD9643,
+	  High-Speed LVDS analog to digital converters (ADC).
+	  FPGA interface HDL is available here:
+	  http://wiki.analog.com/resources/fpga/xilinx/fmc/ad9467
+	  If unsure, say N (but it's safe to say "Y").
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called cf_ad9467.
+
 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 ceee7f3..2f37d85 100644
--- a/drivers/staging/iio/adc/Makefile
+++ b/drivers/staging/iio/adc/Makefile
@@ -29,6 +29,9 @@ ad7298-y := ad7298_core.o
 ad7298-$(CONFIG_IIO_BUFFER) += ad7298_ring.o
 obj-$(CONFIG_AD7298) += ad7298.o
 
+cf_ad9467-y := cf_ad9467_core.o cf_ad9467_ring.o
+obj-$(CONFIG_CF_AD9467) += cf_ad9467.o
+
 obj-$(CONFIG_AD7291) += ad7291.o
 obj-$(CONFIG_AD7780) += ad7780.o
 obj-$(CONFIG_AD7793) += ad7793.o
diff --git a/drivers/staging/iio/adc/cf_ad9467.h b/drivers/staging/iio/adc/cf_ad9467.h
new file mode 100644
index 0000000..1423e4c
--- /dev/null
+++ b/drivers/staging/iio/adc/cf_ad9467.h
@@ -0,0 +1,161 @@
+/*
+ * ADI-AIM ADI ADC Interface Module
+ *
+ * Copyright 2012 Analog Devices Inc.
+ *
+ * Licensed under the GPL-2.
+ *
+ * http://wiki.analog.com/resources/fpga/xilinx/fmc/ad9467
+ */
+
+#ifndef ADI_AIM_H_
+#define ADI_AIM_H_
+
+/* PCORE CoreFPGA register map */
+
+#define AD9467_PCORE_VERSION		0x00
+#define AD9467_PCORE_SPI_CTRL		0x04
+#define AD9467_PCORE_SPI_RDSTAT		0x08
+#define AD9467_PCORE_DMA_CTRL		0x0C
+#define AD9467_PCORE_DMA_STAT		0x10
+#define AD9467_PCORE_ADC_STAT		0x14
+#define AD9467_PCORE_PN_ERR_CTRL	0x24
+
+/* AD9467_PCORE_SPI_CTRL */
+#define AD9647_SPI_START		(1 << 25)
+#define AD9647_SPI_SEL(x)		(((x) & 0x1) << 24)
+#define AD9647_SPI_READ			(1 << 23)
+#define AD9647_SPI_WRITE		(0 << 23)
+#define AD9647_SPI_ADDR(x)		(((x) & 0x1FFF) << 8)
+#define AD9647_SPI_DATA(x)		(((x) & 0xFF) << 0)
+
+/* AD9467_PCORE_SPI_RDSTAT */
+#define AD9647_SPI_IDLE			(1 << 8)
+#define AD9647_SPI_READVAL(x)		((x) & 0xFF)
+
+/* AD9467_PCORE_DMA_CTRL */
+#define AD9647_DMA_CAP_EN		(1 << 16)
+#define AD9647_DMA_CNT(x)		(((x) & 0xFFFF) << 0)
+
+/* AD9467_PCORE_DMA_STAT */
+#define AD9647_DMA_STAT_BUSY		(1 << 0)
+#define AD9647_DMA_STAT_OVF		(1 << 1)
+#define AD9647_DMA_STAT_UNF		(1 << 2)
+
+/* AD9467_PCORE_ADC_STAT */
+#define AD9467_PCORE_ADC_STAT_OVR	(1 << 0)
+#define AD9467_PCORE_ADC_STAT_PN_OOS	(1 << 1) /* W1C */
+#define AD9467_PCORE_ADC_STAT_PN_ERR	(1 << 2) /* W1C */
+
+/* AD9467_PCORE_PN_ERR_CTRL */
+#define AD9467_PN23_EN			(1 << 0)
+#define AD9467_PN9_EN			(0 << 0)
+
+/*
+ * ADI High-Speed ADC common spi interface registers
+ * See Application-Note AN-877
+ */
+
+#define ADC_REG_CHIP_PORT_CONF		0x00
+#define ADC_REG_CHIP_ID			0x01
+#define ADC_REG_CHIP_GRADE		0x02
+#define ADC_REG_TRANSFER		0xFF
+#define ADC_REG_MODES			0x08
+#define ADC_REG_TEST_IO			0x0D
+#define ADC_REG_ADC_INPUT		0x0F
+#define ADC_REG_OFFSET			0x10
+#define ADC_REG_OUTPUT_MODE		0x14
+#define ADC_REG_OUTPUT_ADJUST		0x15
+#define ADC_REG_OUTPUT_PHASE		0x16
+#define ADC_REG_OUTPUT_DELAY		0x17
+#define ADC_REG_VREF			0x18
+#define ADC_REG_ANALOG_INPUT		0x2C
+
+/* ADC_REG_TRANSFER */
+#define TRANSFER_SYNC			0x1
+
+/* ADC_REG_TEST_IO */
+#define TESTMODE_OFF			0x0
+#define TESTMODE_MIDSCALE_SHORT		0x1
+#define TESTMODE_POS_FULLSCALE		0x2
+#define TESTMODE_NEG_FULLSCALE		0x3
+#define TESTMODE_ALT_CHECKERBOARD	0x4
+#define TESTMODE_PN23_SEQ		0x5
+#define TESTMODE_PN9_SEQ		0x6
+#define TESTMODE_ONE_ZERO_TOGGLE	0x7
+
+/* ADC_REG_OUTPUT_MODE */
+#define OUTPUT_MODE_OFFSET_BINARY	0x0
+#define OUTPUT_MODE_TWOS_COMPLEMENT	0x1
+#define OUTPUT_MODE_GRAY_CODE		0x2
+
+/*
+ * Analog Devices AD9467 16-Bit, 200/250 MSPS ADC
+ */
+
+#define AD9467_DEF_OUTPUT_MODE		0x08
+#define AD9467_REG_VREF_MASK		0x0F
+#define CHIPID_AD9467			0x50
+
+/*
+ * Analog Devices AD9643 Dual 14-Bit, 170/210/250 MSPS ADC
+ */
+
+#define CHIPID_AD9643			0x82
+#define AD9643_REG_VREF_MASK		0x1F
+#define AD9643_DEF_OUTPUT_MODE		0x04
+
+enum {
+	ID_AD9467,
+	ID_AD9643,
+};
+
+struct aim_chip_info {
+	char				name[8];
+	unsigned			num_channels;
+	unsigned long			available_scan_masks[2];
+	const int			(*scale_table)[2];
+	int				num_scales;
+	struct iio_chan_spec		channel[2];
+};
+
+struct aim_state {
+	struct spi_device		*spi;
+	struct mutex			lock;
+	struct completion		dma_complete;
+	struct dma_chan			*rx_chan;
+	const struct aim_chip_info	*chip_info;
+	void __iomem			*regs;
+	void				*buf_virt;
+	dma_addr_t			buf_phys;
+	unsigned			spi_ssel;
+	unsigned			ring_lenght;
+	unsigned			bytes_per_datum;
+	unsigned			id;
+	/*
+	 * DMA (thus cache coherency maintenance) requires the
+	 * transfer buffers to live in their own cache lines.
+	 */
+
+	unsigned char			data[3] ____cacheline_aligned;
+};
+
+/*
+ * IO accessors
+ */
+
+static inline void aim_write(struct aim_state *st, unsigned reg, unsigned val)
+{
+	iowrite32(val, st->regs + reg);
+}
+
+static inline unsigned int aim_read(struct aim_state *st, unsigned reg)
+{
+	return ioread32(st->regs + reg);
+}
+
+void aim_register_ring_funcs(struct iio_dev *indio_dev);
+int aim_configure_ring(struct iio_dev *indio_dev);
+void aim_unconfigure_ring(struct iio_dev *indio_dev);
+
+#endif /* ADI_AIM_H_ */
diff --git a/drivers/staging/iio/adc/cf_ad9467_core.c b/drivers/staging/iio/adc/cf_ad9467_core.c
new file mode 100644
index 0000000..811a659
--- /dev/null
+++ b/drivers/staging/iio/adc/cf_ad9467_core.c
@@ -0,0 +1,585 @@
+/*
+ * ADI-AIM ADI ADC Interface Module
+ *
+ * Copyright 2012 Analog Devices Inc.
+ *
+ * Licensed under the GPL-2.
+ *
+ * http://wiki.analog.com/resources/fpga/xilinx/fmc/ad9467
+ */
+
+#include <linux/module.h>
+#include <linux/errno.h>
+#include <linux/slab.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/wait.h>
+#include <linux/spi/spi.h>
+#include <linux/dma-mapping.h>
+#include <linux/dmaengine.h>
+#include <linux/delay.h>
+#include <linux/debugfs.h>
+#include <linux/uaccess.h>
+
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_platform.h>
+#include <linux/of_address.h>
+#include <linux/of_spi.h>
+
+#include "../iio.h"
+#include "../sysfs.h"
+#include "../buffer.h"
+
+#include "cf_ad9467.h"
+
+static int aim_spi_read(struct aim_state *st, unsigned reg)
+{
+	unsigned long timeout = jiffies + HZ / 4;
+	int ret;
+
+	if (st->spi) {
+		unsigned char *buf = st->data;
+		buf[0] = 0x80 | (reg >> 8);
+		buf[1] = reg & 0xFF;
+
+		ret = spi_write_then_read(st->spi, &buf[0], 2, &buf[2], 1);
+		if (ret < 0)
+			return ret;
+
+		return buf[2];
+	}
+
+	aim_write(st, AD9467_PCORE_SPI_CTRL, 0);
+	aim_write(st, AD9467_PCORE_SPI_CTRL,
+		AD9647_SPI_START |
+		AD9647_SPI_READ |
+		AD9647_SPI_SEL(st->spi_ssel) |
+		AD9647_SPI_ADDR(reg));
+
+	while (!(aim_read(st, AD9467_PCORE_SPI_RDSTAT) & AD9647_SPI_IDLE)) {
+		cpu_relax();
+		if (time_after(jiffies, timeout))
+			return -EIO;
+	}
+
+	return AD9647_SPI_READVAL(aim_read(st, AD9467_PCORE_SPI_RDSTAT));
+}
+
+static int aim_spi_write(struct aim_state *st, unsigned reg, unsigned val)
+{
+	unsigned long timeout = jiffies + HZ / 4;
+
+	int ret;
+
+	if (st->spi) {
+		unsigned char *buf = st->data;
+		buf[0] = reg >> 8;
+		buf[1] = reg & 0xFF;
+		buf[2] = val;
+		ret = spi_write(st->spi, buf, 3);
+		if (ret < 0)
+			return ret;
+
+		return 0;
+	}
+
+	aim_write(st, AD9467_PCORE_SPI_CTRL, 0);
+	aim_write(st, AD9467_PCORE_SPI_CTRL,
+		AD9647_SPI_START |
+		AD9647_SPI_WRITE |
+		AD9647_SPI_SEL(st->spi_ssel) |
+		AD9647_SPI_ADDR(reg) |
+		AD9647_SPI_DATA(val));
+
+
+	while (!(aim_read(st, AD9467_PCORE_SPI_RDSTAT) & AD9647_SPI_IDLE)) {
+		cpu_relax();
+		if (time_after(jiffies, timeout))
+			return -EIO;
+	}
+
+	return 0;
+}
+
+static int aim_debugfs_open(struct inode *inode, struct file *file)
+{
+	if (inode->i_private)
+		file->private_data = inode->i_private;
+
+	return 0;
+}
+
+static ssize_t aim_debugfs_pncheck_read(struct file *file, char __user *userbuf,
+			      size_t count, loff_t *ppos)
+{
+	struct iio_dev *indio_dev = file->private_data;
+	struct aim_state *st = iio_priv(indio_dev);
+	char buf[80], *p = buf;
+	unsigned stat;
+
+	stat = aim_read(st, AD9467_PCORE_VERSION);
+	p += sprintf(p, "%s %s\n", stat & AD9467_PCORE_ADC_STAT_PN_OOS ?
+		"Out of Sync" : "", stat & AD9467_PCORE_ADC_STAT_PN_ERR ?
+		"PN Error" : "");
+
+	return simple_read_from_buffer(userbuf, count, ppos, buf, p - buf);
+}
+
+static ssize_t aim_debugfs_pncheck_write(struct file *file,
+		     const char __user *userbuf, size_t count, loff_t *ppos)
+{
+	struct iio_dev *indio_dev = file->private_data;
+	struct aim_state *st = iio_priv(indio_dev);
+	unsigned mode;
+	char buf[80], *p = buf;
+
+	count = min_t(size_t, count, (sizeof(buf)-1));
+	if (copy_from_user(p, userbuf, count))
+		return -EFAULT;
+
+	if (sysfs_streq(p, "PN9"))
+		mode = TESTMODE_PN9_SEQ;
+	else if (sysfs_streq(p, "PN23"))
+		mode = TESTMODE_PN23_SEQ;
+	else
+		mode = TESTMODE_OFF;
+
+	aim_spi_write(st, ADC_REG_TEST_IO, mode);
+	aim_spi_write(st, ADC_REG_TRANSFER, TRANSFER_SYNC);
+
+	aim_write(st, AD9467_PCORE_PN_ERR_CTRL, mode == TESTMODE_PN23_SEQ ?
+		  AD9467_PN23_EN : AD9467_PN9_EN);
+
+	mdelay(1);
+
+	aim_write(st, AD9467_PCORE_ADC_STAT,
+		  AD9467_PCORE_ADC_STAT_PN_OOS |
+		  AD9467_PCORE_ADC_STAT_PN_OOS |
+		  AD9467_PCORE_ADC_STAT_OVR);
+
+	return count;
+}
+
+static const struct file_operations aim_debugfs_pncheck_fops = {
+	.open = aim_debugfs_open,
+	.read = aim_debugfs_pncheck_read,
+	.write = aim_debugfs_pncheck_write,
+};
+
+static int aim_reg_access(struct iio_dev *indio_dev,
+			      unsigned reg, unsigned writeval,
+			      unsigned *readval)
+{
+	struct aim_state *st = iio_priv(indio_dev);
+	int ret;
+
+	mutex_lock(&indio_dev->mlock);
+	if (readval == NULL) {
+		ret = aim_spi_write(st, reg, writeval);
+		aim_spi_write(st, ADC_REG_TRANSFER, TRANSFER_SYNC);
+	} else {
+		ret = aim_spi_read(st, reg);
+		if (ret < 0)
+			return ret;
+		*readval = ret;
+		ret = 0;
+	}
+
+	mutex_unlock(&indio_dev->mlock);
+
+	return ret;
+}
+
+static int aim_read_raw(struct iio_dev *indio_dev,
+			   struct iio_chan_spec const *chan,
+			   int *val,
+			   int *val2,
+			   long m)
+{
+	struct aim_state *st = iio_priv(indio_dev);
+	int i;
+	unsigned vref_val;
+
+	switch (m) {
+	case IIO_CHAN_INFO_SCALE:
+		vref_val = aim_spi_read(st, ADC_REG_VREF) &
+			(st->id == CHIPID_AD9643 ? AD9643_REG_VREF_MASK :
+			AD9467_REG_VREF_MASK);
+
+		for (i = 0; i < st->chip_info->num_scales; i++)
+			if (vref_val == st->chip_info->scale_table[i][1])
+				break;
+
+		*val =  0;
+		*val2 = st->chip_info->scale_table[i][0];
+
+		return IIO_VAL_INT_PLUS_MICRO;
+	}
+	return -EINVAL;
+}
+
+
+static int aim_write_raw(struct iio_dev *indio_dev,
+			       struct iio_chan_spec const *chan,
+			       int val,
+			       int val2,
+			       long mask)
+{
+	struct aim_state *st = iio_priv(indio_dev);
+	int i;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_SCALE:
+		if (val != 0)
+			return -EINVAL;
+
+		for (i = 0; i < st->chip_info->num_scales; i++)
+			if (val2 == st->chip_info->scale_table[i][0])
+				break;
+
+		aim_spi_write(st, ADC_REG_VREF,
+			      st->chip_info->scale_table[i][1]);
+		aim_spi_write(st, ADC_REG_TRANSFER, TRANSFER_SYNC);
+
+		return 0;
+	default:
+		return -EINVAL;
+	}
+}
+
+static ssize_t aim_show_scale_available(struct device *dev,
+			struct device_attribute *attr, char *buf)
+{
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
+	struct aim_state *st = iio_priv(indio_dev);
+	int i, len = 0;
+
+	for (i = 0; i < st->chip_info->num_scales; i++)
+		len += sprintf(buf + len, "0.%06u ",
+			       st->chip_info->scale_table[i][0]);
+
+	len += sprintf(buf + len, "\n");
+
+	return len;
+}
+
+static IIO_DEVICE_ATTR(in_voltage_scale_available, S_IRUGO,
+		       aim_show_scale_available, NULL, 0);
+
+static struct attribute *aim_attributes[] = {
+	&iio_dev_attr_in_voltage_scale_available.dev_attr.attr,
+	NULL,
+};
+
+static const struct attribute_group aim_attribute_group = {
+	.attrs = aim_attributes,
+};
+
+static const int ad9467_scale_table[][2] = {
+	{30517, 0}, {32043, 6}, {33569, 7},
+	{35095, 8}, {36621, 9}, {38146, 10},
+};
+
+static const int ad9643_scale_table[][2] = {
+	{31738, 0xF}, {31403, 0xE}, {31067, 0xD}, {30731, 0xC}, {30396, 0xB},
+	{30060, 0xA}, {29724, 0x9}, {29388, 0x8}, {29053, 0x7}, {28717, 0x6},
+	{28381, 0x5}, {28046, 0x4}, {27710, 0x3}, {27374, 0x2}, {27039, 0x1},
+	{26703, 0x0}, {26367, 0x1F}, {26031, 0x1E}, {25696, 0x1D},
+	{25360, 0x1C}, {25024, 0x1B}, {24689, 0x1A}, {24353, 0x19},
+	{24017, 0x18}, {23682, 0x17}, {23346, 0x16}, {23010, 0x15},
+	{22675, 0x14}, {22339, 0x13}, {22003, 0x12}, {21667, 0x11},
+	{21332, 0x10},
+};
+
+#define AIM_CHAN(_chan, _name, _si, _bits, _sign)			\
+	{ .type = IIO_VOLTAGE,						\
+	  .indexed = 1,							\
+	  .extend_name = _name,						\
+	  .channel = _chan,						\
+	  .info_mask = IIO_CHAN_INFO_SCALE_SHARED_BIT,			\
+	  .scan_index = _si,						\
+	  .scan_type =  IIO_ST(_sign, _bits, 16, 0)}
+
+static const struct aim_chip_info aim_chip_info_tbl[] = {
+	[ID_AD9467] = {
+		.name = "AD9467",
+		.scale_table = ad9467_scale_table,
+		.num_scales = ARRAY_SIZE(ad9467_scale_table),
+		.num_channels = 1,
+		.available_scan_masks[0] = BIT(0),
+		.channel[0] = AIM_CHAN(0, NULL, 0, 16, 's'),
+	},
+	[ID_AD9643] = {
+		.name = "AD9643",
+		.scale_table = ad9643_scale_table,
+		.num_scales = ARRAY_SIZE(ad9643_scale_table),
+		.num_channels = 2,
+		.available_scan_masks[0] = BIT(0) | BIT(1),
+		.channel[0] = AIM_CHAN(0, "I", 0, 14, 'u'),
+		.channel[1] = AIM_CHAN(1, "Q", 1, 14, 'u'),
+	},
+};
+
+static const struct iio_info aim_info = {
+	.driver_module = THIS_MODULE,
+	.read_raw = &aim_read_raw,
+	.write_raw = &aim_write_raw,
+	.attrs = &aim_attribute_group,
+	.debugfs_reg_access = aim_reg_access,
+};
+
+struct aim_dma_params {
+	struct device_node *of_node;
+	int chan_id;
+};
+
+static bool aim_dma_filter(struct dma_chan *chan, void *param)
+{
+	struct aim_dma_params *p = param;
+
+	return chan->device->dev->of_node == p->of_node &&
+		chan->chan_id == p->chan_id;
+}
+
+/**
+ * aim_of_probe - probe method for the AIM device.
+ * @of_dev:	pointer to OF device structure
+ * @match:	pointer to the structure used for matching a device
+ *
+ * This function probes the AIM device in the device tree.
+ * It initializes the driver data structure and the hardware.
+ * It returns 0, if the driver is bound to the AIM device, or a negative
+ * value if there is an error.
+ */
+static int __devinit aim_of_probe(struct platform_device *op)
+{
+	struct iio_dev *indio_dev;
+	struct device *dev = &op->dev;
+	struct aim_state *st;
+	struct resource r_mem; /* IO mem resources */
+	struct spi_master *spi_master;
+	struct device_node *nspi;
+	struct aim_dma_params dma_params;
+	struct of_phandle_args dma_spec;
+	dma_cap_mask_t mask;
+	resource_size_t remap_size, phys_addr;
+	unsigned def_mode;
+	int ret;
+
+	dev_info(dev, "Device Tree Probing \'%s\'\n",
+		 op->dev.of_node->name);
+
+	/* Get iospace for the device */
+	ret = of_address_to_resource(op->dev.of_node, 0, &r_mem);
+	if (ret) {
+		dev_err(dev, "invalid address\n");
+		return ret;
+	}
+
+	indio_dev = iio_allocate_device(sizeof(*st));
+	if (indio_dev == NULL)
+		return -ENOMEM;
+
+	st = iio_priv(indio_dev);
+
+	dev_set_drvdata(dev, indio_dev);
+	mutex_init(&st->lock);
+
+	phys_addr = r_mem.start;
+	remap_size = resource_size(&r_mem);
+	if (!request_mem_region(phys_addr, remap_size, KBUILD_MODNAME)) {
+		dev_err(dev, "Couldn't lock memory region at 0x%08llX\n",
+			(unsigned long long)phys_addr);
+		ret = -EBUSY;
+		goto failed1;
+	}
+
+	st->regs = ioremap(phys_addr, remap_size);
+	if (st->regs == NULL) {
+		dev_err(dev, "Couldn't ioremap memory at 0x%08llX\n",
+			(unsigned long long)phys_addr);
+		ret = -EFAULT;
+		goto failed2;
+	}
+	/* Get dma channel for the device */
+	ret = of_parse_phandle_with_args(op->dev.of_node, "dma-request",
+					 "#dma-cells", 0, &dma_spec);
+	if (ret) {
+		dev_err(dev, "Couldn't parse dma-request\n");
+		goto failed2;
+	}
+
+	dma_params.of_node = dma_spec.np;
+	dma_params.chan_id = dma_spec.args[0];
+
+	dma_cap_zero(mask);
+	dma_cap_set(DMA_SLAVE | DMA_PRIVATE, mask);
+
+	st->rx_chan = dma_request_channel(mask, aim_dma_filter, &dma_params);
+	if (!st->rx_chan) {
+		dev_err(dev, "failed to find rx dma device\n");
+		goto failed2;
+	}
+	/*
+	 * Get SPI configuration interface for the device
+	 * We are platform_driver, so we need other means to get the
+	 * associated control interface.
+	 */
+	ret = of_property_read_u32(op->dev.of_node,
+				   "spibus-slaveselect-connected",
+				   &st->spi_ssel);
+	if (ret) {
+		dev_err(dev, "failed to get connected SPI slave select\n");
+		goto failed3;
+	}
+
+	nspi = of_parse_phandle(op->dev.of_node, "spibus-connected", 0);
+	spi_master = spi_of_node_to_master(nspi);
+
+	if (spi_master != NULL) {
+		struct spi_board_info info = {
+			.modalias = KBUILD_MODNAME,
+			.max_speed_hz = 1000000,
+			.chip_select = st->spi_ssel,
+			.mode = SPI_MODE_0 | SPI_3WIRE,
+		};
+		st->spi = spi_new_device(spi_master, &info);
+		if (st->spi == NULL) {
+			dev_err(dev, "failed to add spi device\n");
+			goto failed3;
+		}
+	} else {
+		dev_dbg(dev, "could not find SPI master node,"
+			"using pcore spi implementation\n");
+	}
+
+	/* Probe device */
+	st->id = aim_spi_read(st, ADC_REG_CHIP_ID);
+
+	switch (st->id) {
+	case CHIPID_AD9467:
+		st->chip_info = &aim_chip_info_tbl[ID_AD9467];
+		def_mode = AD9467_DEF_OUTPUT_MODE | OUTPUT_MODE_TWOS_COMPLEMENT;
+		break;
+	case CHIPID_AD9643:
+		st->chip_info = &aim_chip_info_tbl[ID_AD9643];
+		def_mode = AD9643_DEF_OUTPUT_MODE | OUTPUT_MODE_OFFSET_BINARY;
+		break;
+	default:
+		dev_err(dev, "Unrecognized CHIP_ID 0x%X\n", st->id);
+		ret = -ENODEV;
+		goto failed3;
+	}
+
+	indio_dev->dev.parent = dev;
+	indio_dev->name = op->dev.of_node->name;
+	indio_dev->channels = st->chip_info->channel;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->num_channels = st->chip_info->num_channels;
+	indio_dev->available_scan_masks = st->chip_info->available_scan_masks;
+	indio_dev->info = &aim_info;
+
+	init_completion(&st->dma_complete);
+
+	aim_spi_write(st, ADC_REG_OUTPUT_MODE, def_mode);
+	aim_spi_write(st, ADC_REG_TEST_IO, TESTMODE_OFF);
+	aim_spi_write(st, ADC_REG_TRANSFER, TRANSFER_SYNC);
+
+	aim_configure_ring(indio_dev);
+	aim_register_ring_funcs(indio_dev);
+	ret = iio_buffer_register(indio_dev,
+				  st->chip_info->channel,
+				  ARRAY_SIZE(st->chip_info->channel));
+	if (ret)
+		goto failed4;
+
+	ret = iio_device_register(indio_dev);
+	if (ret)
+		goto failed4;
+
+	dev_info(dev, "ADI AIM (0x%X) at 0x%08llX mapped to 0x%p,"
+		 " DMA-%d probed ADC %s\n",
+		 aim_read(st, AD9467_PCORE_VERSION),
+		 (unsigned long long)phys_addr, st->regs,
+		 st->rx_chan->chan_id, st->chip_info->name);
+
+	if (indio_dev->debugfs_dentry)
+		debugfs_create_file("pseudorandom_err_check", 0644,
+					indio_dev->debugfs_dentry,
+					indio_dev, &aim_debugfs_pncheck_fops);
+
+	return 0;
+
+failed4:
+	aim_unconfigure_ring(indio_dev);
+failed3:
+	dma_release_channel(st->rx_chan);
+failed2:
+	release_mem_region(phys_addr, remap_size);
+failed1:
+	iio_free_device(indio_dev);
+	dev_set_drvdata(dev, NULL);
+
+	return ret;
+}
+
+/**
+ * aim_of_remove - unbinds the driver from the AIM device.
+ * @of_dev:	pointer to OF device structure
+ *
+ * This function is called if a device is physically removed from the system or
+ * if the driver module is being unloaded. It frees any resources allocated to
+ * the device.
+ */
+static int __devexit aim_of_remove(struct platform_device *op)
+{
+	struct device *dev = &op->dev;
+	struct resource r_mem; /* IO mem resources */
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
+	struct aim_state *st = iio_priv(indio_dev);
+
+	iio_device_unregister(indio_dev);
+	iio_buffer_unregister(indio_dev);
+	aim_unconfigure_ring(indio_dev);
+
+	dma_release_channel(st->rx_chan);
+
+	iounmap(st->regs);
+
+	/* Get iospace of the device */
+	if (of_address_to_resource(op->dev.of_node, 0, &r_mem))
+		dev_err(dev, "invalid address\n");
+	else
+		release_mem_region(r_mem.start, resource_size(&r_mem));
+
+	iio_free_device(indio_dev);
+
+	dev_set_drvdata(dev, NULL);
+
+	return 0;
+}
+
+/* Match table for of_platform binding */
+static const struct of_device_id aim_of_match[] __devinitconst = {
+	{ .compatible = "xlnx,cf-ad9467-core-1.00.a", },
+	{ .compatible = "xlnx,cf-ad9643-core-1.00.a", },
+	{ /* end of list */ },
+};
+MODULE_DEVICE_TABLE(of, aim_of_match);
+
+static struct platform_driver aim_of_driver = {
+	.driver = {
+		.name = KBUILD_MODNAME,
+		.owner = THIS_MODULE,
+		.of_match_table = aim_of_match,
+	},
+	.probe		= aim_of_probe,
+	.remove		= __devexit_p(aim_of_remove),
+};
+
+module_platform_driver(aim_of_driver);
+
+MODULE_AUTHOR("Michael Hennerich <hennerich@blackfin.uclinux.org>");
+MODULE_DESCRIPTION("Analog Devices ADI-AIM");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/staging/iio/adc/cf_ad9467_ring.c b/drivers/staging/iio/adc/cf_ad9467_ring.c
new file mode 100644
index 0000000..c6b2230
--- /dev/null
+++ b/drivers/staging/iio/adc/cf_ad9467_ring.c
@@ -0,0 +1,214 @@
+/*
+ * ADI-AIM ADC Interface Module
+ *
+ * Copyright 2012 Analog Devices Inc.
+ *
+ * Licensed under the GPL-2 or later.
+ */
+
+#include <linux/slab.h>
+#include <linux/kernel.h>
+#include <linux/poll.h>
+#include <linux/io.h>
+#include <linux/dma-mapping.h>
+#include <linux/dmaengine.h>
+
+#include "../iio.h"
+#include "../sysfs.h"
+#include "../buffer.h"
+#include "../ring_hw.h"
+#include "cf_ad9467.h"
+
+static int aim_read_first_n_hw_rb(struct iio_buffer *r,
+				      size_t count, char __user *buf)
+{
+	struct iio_hw_buffer *hw_ring = iio_to_hw_buf(r);
+	struct iio_dev *indio_dev = hw_ring->private;
+	struct aim_state *st = iio_priv(indio_dev);
+	struct dma_async_tx_descriptor *desc;
+	dma_cookie_t cookie;
+	unsigned rcount;
+	int ret;
+
+	mutex_lock(&st->lock);
+	if (count == 0) {
+		ret = -EINVAL;
+		goto error_ret;
+	}
+
+	if (count % 8)
+		rcount = (count + 8) & 0xFFFFFFF8;
+	else
+		rcount = count;
+
+	st->buf_virt = dma_alloc_coherent(indio_dev->dev.parent,
+					  PAGE_ALIGN(rcount), &st->buf_phys,
+					  GFP_KERNEL);
+	if (st->buf_virt == NULL) {
+		ret = -ENOMEM;
+		goto error_ret;
+	}
+
+	desc = dmaengine_prep_slave_single(st->rx_chan, st->buf_phys, rcount,
+					   DMA_FROM_DEVICE, DMA_PREP_INTERRUPT);
+	if (!desc) {
+		dev_err(indio_dev->dev.parent,
+			"Failed to allocate a dma descriptor\n");
+		ret = -ENOMEM;
+		goto error_free;
+	}
+
+	desc->callback = (dma_async_tx_callback) complete;
+	desc->callback_param = &st->dma_complete;
+
+	cookie = dmaengine_submit(desc);
+	if (cookie < 0) {
+		dev_err(indio_dev->dev.parent,
+			"Failed to submit a dma transfer\n");
+		ret = cookie;
+		goto error_free;
+	}
+
+	dma_async_issue_pending(st->rx_chan);
+
+	aim_write(st, AD9467_PCORE_DMA_CTRL, 0);
+	aim_read(st, AD9467_PCORE_DMA_STAT);
+	aim_write(st, AD9467_PCORE_DMA_CTRL,
+		  AD9647_DMA_CAP_EN | AD9647_DMA_CNT((rcount / 8) - 1));
+
+	ret = wait_for_completion_interruptible_timeout(&st->dma_complete,
+							2 * HZ);
+	if (ret == 0) {
+		ret = -ETIMEDOUT;
+		goto error_free;
+	} else if (ret < 0) {
+		goto error_free;
+	}
+
+	if (copy_to_user(buf, st->buf_virt, count))
+		ret = -EFAULT;
+
+error_free:
+	dma_free_coherent(indio_dev->dev.parent, PAGE_ALIGN(rcount),
+			  st->buf_virt, st->buf_phys);
+	r->stufftoread = 0;
+error_ret:
+	mutex_unlock(&st->lock);
+
+	return ret ? ret : count;
+}
+
+static int aim_ring_get_length(struct iio_buffer *r)
+{
+	struct iio_hw_buffer *hw_ring = iio_to_hw_buf(r);
+	struct iio_dev *indio_dev = hw_ring->private;
+	struct aim_state *st = iio_priv(indio_dev);
+
+	return st->ring_lenght;
+}
+
+static int aim_ring_set_length(struct iio_buffer *r, int length)
+{
+	struct iio_hw_buffer *hw_ring = iio_to_hw_buf(r);
+	struct iio_dev *indio_dev = hw_ring->private;
+	struct aim_state *st = iio_priv(indio_dev);
+
+	st->ring_lenght = length;
+
+	return 0;
+}
+
+static int aim_ring_get_bytes_per_datum(struct iio_buffer *r)
+{
+	struct iio_hw_buffer *hw_ring = iio_to_hw_buf(r);
+	struct iio_dev *indio_dev = hw_ring->private;
+	struct aim_state *st = iio_priv(indio_dev);
+
+	return st->bytes_per_datum;
+}
+
+static IIO_BUFFER_ENABLE_ATTR;
+static IIO_BUFFER_LENGTH_ATTR;
+
+static struct attribute *aim_ring_attributes[] = {
+	&dev_attr_length.attr,
+	&dev_attr_enable.attr,
+	NULL,
+};
+
+static struct attribute_group aim_ring_attr = {
+	.attrs = aim_ring_attributes,
+	.name = "buffer",
+};
+
+static struct iio_buffer *aim_rb_allocate(struct iio_dev *indio_dev)
+{
+	struct iio_buffer *buf;
+	struct iio_hw_buffer *ring;
+
+	ring = kzalloc(sizeof *ring, GFP_KERNEL);
+	if (!ring)
+		return NULL;
+
+	ring->private = indio_dev;
+	buf = &ring->buf;
+	buf->stufftoread = 0;
+	buf->attrs = &aim_ring_attr;
+	iio_buffer_init(buf);
+
+	return buf;
+}
+
+static inline void aim_rb_free(struct iio_buffer *r)
+{
+	kfree(iio_to_hw_buf(r));
+}
+
+static const struct iio_buffer_access_funcs aim_ring_access_funcs = {
+	.read_first_n = &aim_read_first_n_hw_rb,
+	.get_length = &aim_ring_get_length,
+	.set_length = &aim_ring_set_length,
+	.get_bytes_per_datum = &aim_ring_get_bytes_per_datum,
+};
+
+int aim_configure_ring(struct iio_dev *indio_dev)
+{
+	indio_dev->buffer = aim_rb_allocate(indio_dev);
+	if (indio_dev->buffer == NULL)
+		return -ENOMEM;
+
+	indio_dev->modes |= INDIO_BUFFER_HARDWARE;
+	indio_dev->buffer->access = &aim_ring_access_funcs;
+
+	return 0;
+}
+
+void aim_unconfigure_ring(struct iio_dev *indio_dev)
+{
+	aim_rb_free(indio_dev->buffer);
+}
+
+static inline int __aim_hw_ring_state_set(struct iio_dev *indio_dev, bool state)
+{
+	return 0;
+}
+
+static int aim_hw_ring_preenable(struct iio_dev *indio_dev)
+{
+	return __aim_hw_ring_state_set(indio_dev, 1);
+}
+
+static int aim_hw_ring_postdisable(struct iio_dev *indio_dev)
+{
+	return __aim_hw_ring_state_set(indio_dev, 0);
+}
+
+static const struct iio_buffer_setup_ops aim_ring_setup_ops = {
+	.preenable = &aim_hw_ring_preenable,
+	.postdisable = &aim_hw_ring_postdisable,
+};
+
+void aim_register_ring_funcs(struct iio_dev *indio_dev)
+{
+	indio_dev->setup_ops = &aim_ring_setup_ops;
+}
-- 
1.7.0.4

Re: [PATCH 4/4] iio: adc: New driver for AD9467 and AD9643 High-Speed LVDS ADCs

[Jonathan Cameron]

On 02/17/2012 12:46 PM, michael.hennerich@analog.com wrote:
> From: Michael Hennerich <michael.hennerich@analog.com>
> 
> Suitable FPGA interface HDL is available here:
> http://wiki.analog.com/resources/fpga/xilinx/fmc/ad9467

I don't know my way around the dma stuff so would appreciate review
for someone who does (if any such person is reading!)


Mostly for this one I would like a little more explanation of some
of the design descisions.
1) Registering as platform device rather than spi with extra bits.
    May well be cleaner this way around. If so please just briefly
    explain why.

2) Effectively embedded spi implementation rather than a separate
spi driver for the aim to spi stuff.
    I can see it would be fairly clunky given this only does very
    very special forms of spi, but it would cleanup this end by
    removing the special casing.

3) Separation (or lack there of) for the adc interface and the hardware
buffer implemented in the fpga.
    Probably tricky to pull off but a few lines saying where the problems
    lie would be informative.

A nice interest example and well written driver otherwise.  Couple
of things like documentation requests and small tidy ups inline.

> 
> Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
> ---
>  drivers/staging/iio/adc/Kconfig          |   13 +
>  drivers/staging/iio/adc/Makefile         |    3 +
>  drivers/staging/iio/adc/cf_ad9467.h      |  161 ++++++++
>  drivers/staging/iio/adc/cf_ad9467_core.c |  585 ++++++++++++++++++++++++++++++
>  drivers/staging/iio/adc/cf_ad9467_ring.c |  214 +++++++++++
>  5 files changed, 976 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/staging/iio/adc/cf_ad9467.h
>  create mode 100644 drivers/staging/iio/adc/cf_ad9467_core.c
>  create mode 100644 drivers/staging/iio/adc/cf_ad9467_ring.c
> 
> diff --git a/drivers/staging/iio/adc/Kconfig b/drivers/staging/iio/adc/Kconfig
> index d9decea..266e807 100644
> --- a/drivers/staging/iio/adc/Kconfig
> +++ b/drivers/staging/iio/adc/Kconfig
> @@ -169,6 +169,19 @@ config AD7280
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called ad7280a
>  
> +config CF_AD9467
> +	tristate "Analog Devices AD9467 AD9643 High-Speed ADC driver"
> +	select IIO_BUFFER
> +	help
> +	  Say yes here to build support for Analog Devices AD9467 and AD9643,
> +	  High-Speed LVDS analog to digital converters (ADC).
> +	  FPGA interface HDL is available here:
> +	  http://wiki.analog.com/resources/fpga/xilinx/fmc/ad9467
> +	  If unsure, say N (but it's safe to say "Y").
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called cf_ad9467.
> +
>  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 ceee7f3..2f37d85 100644
> --- a/drivers/staging/iio/adc/Makefile
> +++ b/drivers/staging/iio/adc/Makefile
> @@ -29,6 +29,9 @@ ad7298-y := ad7298_core.o
>  ad7298-$(CONFIG_IIO_BUFFER) += ad7298_ring.o
>  obj-$(CONFIG_AD7298) += ad7298.o
>  
> +cf_ad9467-y := cf_ad9467_core.o cf_ad9467_ring.o
> +obj-$(CONFIG_CF_AD9467) += cf_ad9467.o
> +
>  obj-$(CONFIG_AD7291) += ad7291.o
>  obj-$(CONFIG_AD7780) += ad7780.o
>  obj-$(CONFIG_AD7793) += ad7793.o
> diff --git a/drivers/staging/iio/adc/cf_ad9467.h b/drivers/staging/iio/adc/cf_ad9467.h
> new file mode 100644
> index 0000000..1423e4c
> --- /dev/null
> +++ b/drivers/staging/iio/adc/cf_ad9467.h
> @@ -0,0 +1,161 @@
> +/*
> + * ADI-AIM ADI ADC Interface Module
> + *
> + * Copyright 2012 Analog Devices Inc.
> + *
> + * Licensed under the GPL-2.
> + *
> + * http://wiki.analog.com/resources/fpga/xilinx/fmc/ad9467
> + */
> +
> +#ifndef ADI_AIM_H_
> +#define ADI_AIM_H_
> +
> +/* PCORE CoreFPGA register map */
> +
> +#define AD9467_PCORE_VERSION		0x00
> +#define AD9467_PCORE_SPI_CTRL		0x04
> +#define AD9467_PCORE_SPI_RDSTAT		0x08
> +#define AD9467_PCORE_DMA_CTRL		0x0C
> +#define AD9467_PCORE_DMA_STAT		0x10
> +#define AD9467_PCORE_ADC_STAT		0x14
> +#define AD9467_PCORE_PN_ERR_CTRL	0x24
> +
> +/* AD9467_PCORE_SPI_CTRL */
> +#define AD9647_SPI_START		(1 << 25)
> +#define AD9647_SPI_SEL(x)		(((x) & 0x1) << 24)
> +#define AD9647_SPI_READ			(1 << 23)
> +#define AD9647_SPI_WRITE		(0 << 23)
> +#define AD9647_SPI_ADDR(x)		(((x) & 0x1FFF) << 8)
> +#define AD9647_SPI_DATA(x)		(((x) & 0xFF) << 0)
> +
> +/* AD9467_PCORE_SPI_RDSTAT */
> +#define AD9647_SPI_IDLE			(1 << 8)
> +#define AD9647_SPI_READVAL(x)		((x) & 0xFF)
> +
> +/* AD9467_PCORE_DMA_CTRL */
> +#define AD9647_DMA_CAP_EN		(1 << 16)
> +#define AD9647_DMA_CNT(x)		(((x) & 0xFFFF) << 0)
> +
> +/* AD9467_PCORE_DMA_STAT */
> +#define AD9647_DMA_STAT_BUSY		(1 << 0)
> +#define AD9647_DMA_STAT_OVF		(1 << 1)
> +#define AD9647_DMA_STAT_UNF		(1 << 2)
> +
> +/* AD9467_PCORE_ADC_STAT */
> +#define AD9467_PCORE_ADC_STAT_OVR	(1 << 0)
> +#define AD9467_PCORE_ADC_STAT_PN_OOS	(1 << 1) /* W1C */
> +#define AD9467_PCORE_ADC_STAT_PN_ERR	(1 << 2) /* W1C */
> +
> +/* AD9467_PCORE_PN_ERR_CTRL */
> +#define AD9467_PN23_EN			(1 << 0)
> +#define AD9467_PN9_EN			(0 << 0)
> +
> +/*
> + * ADI High-Speed ADC common spi interface registers
> + * See Application-Note AN-877
> + */
> +
> +#define ADC_REG_CHIP_PORT_CONF		0x00
> +#define ADC_REG_CHIP_ID			0x01
> +#define ADC_REG_CHIP_GRADE		0x02
> +#define ADC_REG_TRANSFER		0xFF
> +#define ADC_REG_MODES			0x08
> +#define ADC_REG_TEST_IO			0x0D
> +#define ADC_REG_ADC_INPUT		0x0F
> +#define ADC_REG_OFFSET			0x10
> +#define ADC_REG_OUTPUT_MODE		0x14
> +#define ADC_REG_OUTPUT_ADJUST		0x15
> +#define ADC_REG_OUTPUT_PHASE		0x16
> +#define ADC_REG_OUTPUT_DELAY		0x17
> +#define ADC_REG_VREF			0x18
> +#define ADC_REG_ANALOG_INPUT		0x2C
> +
> +/* ADC_REG_TRANSFER */
> +#define TRANSFER_SYNC			0x1
> +
> +/* ADC_REG_TEST_IO */
> +#define TESTMODE_OFF			0x0
> +#define TESTMODE_MIDSCALE_SHORT		0x1
> +#define TESTMODE_POS_FULLSCALE		0x2
> +#define TESTMODE_NEG_FULLSCALE		0x3
> +#define TESTMODE_ALT_CHECKERBOARD	0x4
> +#define TESTMODE_PN23_SEQ		0x5
> +#define TESTMODE_PN9_SEQ		0x6
> +#define TESTMODE_ONE_ZERO_TOGGLE	0x7
> +
> +/* ADC_REG_OUTPUT_MODE */
> +#define OUTPUT_MODE_OFFSET_BINARY	0x0
> +#define OUTPUT_MODE_TWOS_COMPLEMENT	0x1
> +#define OUTPUT_MODE_GRAY_CODE		0x2
> +
> +/*
> + * Analog Devices AD9467 16-Bit, 200/250 MSPS ADC
> + */
> +
> +#define AD9467_DEF_OUTPUT_MODE		0x08
> +#define AD9467_REG_VREF_MASK		0x0F
> +#define CHIPID_AD9467			0x50
> +
> +/*
> + * Analog Devices AD9643 Dual 14-Bit, 170/210/250 MSPS ADC
> + */
> +
> +#define CHIPID_AD9643			0x82
> +#define AD9643_REG_VREF_MASK		0x1F
> +#define AD9643_DEF_OUTPUT_MODE		0x04
> +
> +enum {
> +	ID_AD9467,
> +	ID_AD9643,
> +};
> +
> +struct aim_chip_info {
> +	char				name[8];
> +	unsigned			num_channels;
> +	unsigned long			available_scan_masks[2];
> +	const int			(*scale_table)[2];
> +	int				num_scales;
> +	struct iio_chan_spec		channel[2];
> +};
> +
> +struct aim_state {
> +	struct spi_device		*spi;
> +	struct mutex			lock;
> +	struct completion		dma_complete;
> +	struct dma_chan			*rx_chan;
> +	const struct aim_chip_info	*chip_info;
> +	void __iomem			*regs;
> +	void				*buf_virt;
> +	dma_addr_t			buf_phys;
> +	unsigned			spi_ssel;
> +	unsigned			ring_lenght;
> +	unsigned			bytes_per_datum;
> +	unsigned			id;
> +	/*
> +	 * DMA (thus cache coherency maintenance) requires the
> +	 * transfer buffers to live in their own cache lines.
> +	 */
no blank line here and one above the comment perhaps?
> +
> +	unsigned char			data[3] ____cacheline_aligned;
> +};
> +
> +/*
> + * IO accessors
> + */
> +
> +static inline void aim_write(struct aim_state *st, unsigned reg, unsigned val)
> +{
> +	iowrite32(val, st->regs + reg);
> +}
> +
> +static inline unsigned int aim_read(struct aim_state *st, unsigned reg)
> +{
> +	return ioread32(st->regs + reg);
> +}
> +
> +void aim_register_ring_funcs(struct iio_dev *indio_dev);
> +int aim_configure_ring(struct iio_dev *indio_dev);
> +void aim_unconfigure_ring(struct iio_dev *indio_dev);
> +
> +#endif /* ADI_AIM_H_ */
> diff --git a/drivers/staging/iio/adc/cf_ad9467_core.c b/drivers/staging/iio/adc/cf_ad9467_core.c
> new file mode 100644
> index 0000000..811a659
> --- /dev/null
> +++ b/drivers/staging/iio/adc/cf_ad9467_core.c
> @@ -0,0 +1,585 @@
> +/*
> + * ADI-AIM ADI ADC Interface Module
> + *
> + * Copyright 2012 Analog Devices Inc.
> + *
> + * Licensed under the GPL-2.
> + *
> + * http://wiki.analog.com/resources/fpga/xilinx/fmc/ad9467
> + */
> +
> +#include <linux/module.h>
> +#include <linux/errno.h>
> +#include <linux/slab.h>
> +#include <linux/init.h>
> +#include <linux/io.h>
> +#include <linux/wait.h>
> +#include <linux/spi/spi.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/dmaengine.h>
> +#include <linux/delay.h>
> +#include <linux/debugfs.h>
> +#include <linux/uaccess.h>
> +
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/of_platform.h>
> +#include <linux/of_address.h>
> +#include <linux/of_spi.h>
> +
> +#include "../iio.h"
> +#include "../sysfs.h"
> +#include "../buffer.h"
> +
> +#include "cf_ad9467.h"
> +
> +static int aim_spi_read(struct aim_state *st, unsigned reg)
> +{
> +	unsigned long timeout = jiffies + HZ / 4;
> +	int ret;
> +
> +	if (st->spi) {
> +		unsigned char *buf = st->data;
> +		buf[0] = 0x80 | (reg >> 8);
> +		buf[1] = reg & 0xFF;
> +
> +		ret = spi_write_then_read(st->spi, &buf[0], 2, &buf[2], 1);
> +		if (ret < 0)
> +			return ret;
> +
> +		return buf[2];
> +	}
> +
> +	aim_write(st, AD9467_PCORE_SPI_CTRL, 0);
> +	aim_write(st, AD9467_PCORE_SPI_CTRL,
> +		AD9647_SPI_START |
> +		AD9647_SPI_READ |
> +		AD9647_SPI_SEL(st->spi_ssel) |
> +		AD9647_SPI_ADDR(reg));
> +
> +	while (!(aim_read(st, AD9467_PCORE_SPI_RDSTAT) & AD9647_SPI_IDLE)) {
> +		cpu_relax();
> +		if (time_after(jiffies, timeout))
> +			return -EIO;
> +	}
> +
> +	return AD9647_SPI_READVAL(aim_read(st, AD9467_PCORE_SPI_RDSTAT));
> +}
> +
> +static int aim_spi_write(struct aim_state *st, unsigned reg, unsigned val)
> +{
> +	unsigned long timeout = jiffies + HZ / 4;
> +
> +	int ret;
> +
> +	if (st->spi) {
> +		unsigned char *buf = st->data;
> +		buf[0] = reg >> 8;
> +		buf[1] = reg & 0xFF;
> +		buf[2] = val;
> +		ret = spi_write(st->spi, buf, 3);
> +		if (ret < 0)
> +			return ret;
> +
> +		return 0;
> +	}
> +
This is 'unusual'.  Perhaps it really should be a limited functionality
spi driver?
> +	aim_write(st, AD9467_PCORE_SPI_CTRL, 0);
> +	aim_write(st, AD9467_PCORE_SPI_CTRL,
> +		AD9647_SPI_START |
> +		AD9647_SPI_WRITE |
> +		AD9647_SPI_SEL(st->spi_ssel) |
> +		AD9647_SPI_ADDR(reg) |
> +		AD9647_SPI_DATA(val));
> +
> +
> +	while (!(aim_read(st, AD9467_PCORE_SPI_RDSTAT) & AD9647_SPI_IDLE)) {
> +		cpu_relax();
> +		if (time_after(jiffies, timeout))
> +			return -EIO;
> +	}
> +
> +	return 0;
> +}
> +
> +static int aim_debugfs_open(struct inode *inode, struct file *file)
> +{
> +	if (inode->i_private)
> +		file->private_data = inode->i_private;
> +
> +	return 0;
> +}
> +
> +static ssize_t aim_debugfs_pncheck_read(struct file *file, char __user *userbuf,
> +			      size_t count, loff_t *ppos)
> +{
> +	struct iio_dev *indio_dev = file->private_data;
> +	struct aim_state *st = iio_priv(indio_dev);
> +	char buf[80], *p = buf;
> +	unsigned stat;
> +
> +	stat = aim_read(st, AD9467_PCORE_VERSION);
> +	p += sprintf(p, "%s %s\n", stat & AD9467_PCORE_ADC_STAT_PN_OOS ?
> +		"Out of Sync" : "", stat & AD9467_PCORE_ADC_STAT_PN_ERR ?
> +		"PN Error" : "");
> +
> +	return simple_read_from_buffer(userbuf, count, ppos, buf, p - buf);
> +}
> +
> +static ssize_t aim_debugfs_pncheck_write(struct file *file,
> +		     const char __user *userbuf, size_t count, loff_t *ppos)
> +{
> +	struct iio_dev *indio_dev = file->private_data;
> +	struct aim_state *st = iio_priv(indio_dev);
> +	unsigned mode;
> +	char buf[80], *p = buf;
> +
> +	count = min_t(size_t, count, (sizeof(buf)-1));
> +	if (copy_from_user(p, userbuf, count))
> +		return -EFAULT;
> +
> +	if (sysfs_streq(p, "PN9"))
> +		mode = TESTMODE_PN9_SEQ;
> +	else if (sysfs_streq(p, "PN23"))
> +		mode = TESTMODE_PN23_SEQ;
> +	else
> +		mode = TESTMODE_OFF;
> +
> +	aim_spi_write(st, ADC_REG_TEST_IO, mode);
> +	aim_spi_write(st, ADC_REG_TRANSFER, TRANSFER_SYNC);
> +
> +	aim_write(st, AD9467_PCORE_PN_ERR_CTRL, mode == TESTMODE_PN23_SEQ ?
> +		  AD9467_PN23_EN : AD9467_PN9_EN);
> +
> +	mdelay(1);
document why please.
> +
> +	aim_write(st, AD9467_PCORE_ADC_STAT,
> +		  AD9467_PCORE_ADC_STAT_PN_OOS |
> +		  AD9467_PCORE_ADC_STAT_PN_OOS |
> +		  AD9467_PCORE_ADC_STAT_OVR);
> +
> +	return count;
> +}
> +
> +static const struct file_operations aim_debugfs_pncheck_fops = {
> +	.open = aim_debugfs_open,
> +	.read = aim_debugfs_pncheck_read,
> +	.write = aim_debugfs_pncheck_write,
> +};
> +
> +static int aim_reg_access(struct iio_dev *indio_dev,
> +			      unsigned reg, unsigned writeval,
> +			      unsigned *readval)
> +{
> +	struct aim_state *st = iio_priv(indio_dev);
> +	int ret;
> +
> +	mutex_lock(&indio_dev->mlock);
> +	if (readval == NULL) {
> +		ret = aim_spi_write(st, reg, writeval);
> +		aim_spi_write(st, ADC_REG_TRANSFER, TRANSFER_SYNC);
> +	} else {
> +		ret = aim_spi_read(st, reg);
> +		if (ret < 0)
> +			return ret;
> +		*readval = ret;
> +		ret = 0;
> +	}
Seems like an excess blank line....
> +
> +	mutex_unlock(&indio_dev->mlock);
> +
> +	return ret;
> +}
> +
> +static int aim_read_raw(struct iio_dev *indio_dev,
> +			   struct iio_chan_spec const *chan,
> +			   int *val,
> +			   int *val2,
> +			   long m)
> +{
> +	struct aim_state *st = iio_priv(indio_dev);
> +	int i;
> +	unsigned vref_val;
> +
> +	switch (m) {
> +	case IIO_CHAN_INFO_SCALE:
> +		vref_val = aim_spi_read(st, ADC_REG_VREF) &
> +			(st->id == CHIPID_AD9643 ? AD9643_REG_VREF_MASK :
> +			AD9467_REG_VREF_MASK);
> +
> +		for (i = 0; i < st->chip_info->num_scales; i++)
> +			if (vref_val == st->chip_info->scale_table[i][1])
> +				break;
> +
> +		*val =  0;
> +		*val2 = st->chip_info->scale_table[i][0];
> +
> +		return IIO_VAL_INT_PLUS_MICRO;
> +	}
> +	return -EINVAL;
> +}
> +
> +
> +static int aim_write_raw(struct iio_dev *indio_dev,
> +			       struct iio_chan_spec const *chan,
> +			       int val,
> +			       int val2,
> +			       long mask)
> +{
> +	struct aim_state *st = iio_priv(indio_dev);
> +	int i;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_SCALE:
> +		if (val != 0)
> +			return -EINVAL;
> +
> +		for (i = 0; i < st->chip_info->num_scales; i++)
> +			if (val2 == st->chip_info->scale_table[i][0])
> +				break;
what if it isn't any of them?
> +
> +		aim_spi_write(st, ADC_REG_VREF,
> +			      st->chip_info->scale_table[i][1]);
> +		aim_spi_write(st, ADC_REG_TRANSFER, TRANSFER_SYNC);
> +
> +		return 0;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static ssize_t aim_show_scale_available(struct device *dev,
> +			struct device_attribute *attr, char *buf)
> +{
> +	struct iio_dev *indio_dev = dev_get_drvdata(dev);
> +	struct aim_state *st = iio_priv(indio_dev);
> +	int i, len = 0;
> +
> +	for (i = 0; i < st->chip_info->num_scales; i++)
> +		len += sprintf(buf + len, "0.%06u ",
> +			       st->chip_info->scale_table[i][0]);
> +
> +	len += sprintf(buf + len, "\n");
> +
> +	return len;
> +}
> +
> +static IIO_DEVICE_ATTR(in_voltage_scale_available, S_IRUGO,
> +		       aim_show_scale_available, NULL, 0);
> +
> +static struct attribute *aim_attributes[] = {
> +	&iio_dev_attr_in_voltage_scale_available.dev_attr.attr,
> +	NULL,
> +};
> +
> +static const struct attribute_group aim_attribute_group = {
> +	.attrs = aim_attributes,
> +};
> +
> +static const int ad9467_scale_table[][2] = {
> +	{30517, 0}, {32043, 6}, {33569, 7},
> +	{35095, 8}, {36621, 9}, {38146, 10},
> +};
> +
> +static const int ad9643_scale_table[][2] = {
> +	{31738, 0xF}, {31403, 0xE}, {31067, 0xD}, {30731, 0xC}, {30396, 0xB},
> +	{30060, 0xA}, {29724, 0x9}, {29388, 0x8}, {29053, 0x7}, {28717, 0x6},
> +	{28381, 0x5}, {28046, 0x4}, {27710, 0x3}, {27374, 0x2}, {27039, 0x1},
> +	{26703, 0x0}, {26367, 0x1F}, {26031, 0x1E}, {25696, 0x1D},
> +	{25360, 0x1C}, {25024, 0x1B}, {24689, 0x1A}, {24353, 0x19},
> +	{24017, 0x18}, {23682, 0x17}, {23346, 0x16}, {23010, 0x15},
> +	{22675, 0x14}, {22339, 0x13}, {22003, 0x12}, {21667, 0x11},
> +	{21332, 0x10},
> +};
> +
> +#define AIM_CHAN(_chan, _name, _si, _bits, _sign)			\
> +	{ .type = IIO_VOLTAGE,						\
> +	  .indexed = 1,							\
> +	  .extend_name = _name,						\
> +	  .channel = _chan,						\
> +	  .info_mask = IIO_CHAN_INFO_SCALE_SHARED_BIT,			\
> +	  .scan_index = _si,						\
> +	  .scan_type =  IIO_ST(_sign, _bits, 16, 0)}
> +
> +static const struct aim_chip_info aim_chip_info_tbl[] = {
> +	[ID_AD9467] = {
> +		.name = "AD9467",
> +		.scale_table = ad9467_scale_table,
> +		.num_scales = ARRAY_SIZE(ad9467_scale_table),
> +		.num_channels = 1,
> +		.available_scan_masks[0] = BIT(0),
> +		.channel[0] = AIM_CHAN(0, NULL, 0, 16, 's'),
> +	},
> +	[ID_AD9643] = {
> +		.name = "AD9643",
> +		.scale_table = ad9643_scale_table,
> +		.num_scales = ARRAY_SIZE(ad9643_scale_table),
> +		.num_channels = 2,
> +		.available_scan_masks[0] = BIT(0) | BIT(1),
> +		.channel[0] = AIM_CHAN(0, "I", 0, 14, 'u'),
> +		.channel[1] = AIM_CHAN(1, "Q", 1, 14, 'u'),
> +	},
Why I and Q?  I clearly could be a quadrature pair of channels but
is there anything in inherent in the device forcing this?  Also
is this documented?
> +};
> +
> +static const struct iio_info aim_info = {
> +	.driver_module = THIS_MODULE,
> +	.read_raw = &aim_read_raw,
> +	.write_raw = &aim_write_raw,
> +	.attrs = &aim_attribute_group,
> +	.debugfs_reg_access = aim_reg_access,
> +};
> +
> +struct aim_dma_params {
> +	struct device_node *of_node;
> +	int chan_id;
> +};
> +
> +static bool aim_dma_filter(struct dma_chan *chan, void *param)
> +{
> +	struct aim_dma_params *p = param;
> +
> +	return chan->device->dev->of_node == p->of_node &&
> +		chan->chan_id == p->chan_id;
> +}
> +
> +/**
> + * aim_of_probe - probe method for the AIM device.
> + * @of_dev:	pointer to OF device structure
> + * @match:	pointer to the structure used for matching a device
> + *
> + * This function probes the AIM device in the device tree.
> + * It initializes the driver data structure and the hardware.
> + * It returns 0, if the driver is bound to the AIM device, or a negative
> + * value if there is an error.
> + */
> +static int __devinit aim_of_probe(struct platform_device *op)
> +{
> +	struct iio_dev *indio_dev;
> +	struct device *dev = &op->dev;
> +	struct aim_state *st;
> +	struct resource r_mem; /* IO mem resources */
> +	struct spi_master *spi_master;
> +	struct device_node *nspi;
> +	struct aim_dma_params dma_params;
> +	struct of_phandle_args dma_spec;
> +	dma_cap_mask_t mask;
> +	resource_size_t remap_size, phys_addr;
> +	unsigned def_mode;
> +	int ret;
> +
> +	dev_info(dev, "Device Tree Probing \'%s\'\n",
> +		 op->dev.of_node->name);
> +
> +	/* Get iospace for the device */
> +	ret = of_address_to_resource(op->dev.of_node, 0, &r_mem);
> +	if (ret) {
> +		dev_err(dev, "invalid address\n");
> +		return ret;
> +	}
> +
> +	indio_dev = iio_allocate_device(sizeof(*st));
> +	if (indio_dev == NULL)
> +		return -ENOMEM;
> +
> +	st = iio_priv(indio_dev);
> +
> +	dev_set_drvdata(dev, indio_dev);
> +	mutex_init(&st->lock);
> +
> +	phys_addr = r_mem.start;
> +	remap_size = resource_size(&r_mem);
> +	if (!request_mem_region(phys_addr, remap_size, KBUILD_MODNAME)) {
> +		dev_err(dev, "Couldn't lock memory region at 0x%08llX\n",
> +			(unsigned long long)phys_addr);
> +		ret = -EBUSY;
> +		goto failed1;
> +	}
> +
> +	st->regs = ioremap(phys_addr, remap_size);
> +	if (st->regs == NULL) {
> +		dev_err(dev, "Couldn't ioremap memory at 0x%08llX\n",
> +			(unsigned long long)phys_addr);
> +		ret = -EFAULT;
> +		goto failed2;
> +	}
> +	/* Get dma channel for the device */
> +	ret = of_parse_phandle_with_args(op->dev.of_node, "dma-request",
> +					 "#dma-cells", 0, &dma_spec);
> +	if (ret) {
> +		dev_err(dev, "Couldn't parse dma-request\n");
> +		goto failed2;
> +	}
> +
> +	dma_params.of_node = dma_spec.np;
> +	dma_params.chan_id = dma_spec.args[0];
> +
> +	dma_cap_zero(mask);
> +	dma_cap_set(DMA_SLAVE | DMA_PRIVATE, mask);
> +
> +	st->rx_chan = dma_request_channel(mask, aim_dma_filter, &dma_params);
> +	if (!st->rx_chan) {
> +		dev_err(dev, "failed to find rx dma device\n");
> +		goto failed2;
> +	}
> +	/*
> +	 * Get SPI configuration interface for the device
> +	 * We are platform_driver, so we need other means to get the
> +	 * associated control interface.
> +	 */
> +	ret = of_property_read_u32(op->dev.of_node,
> +				   "spibus-slaveselect-connected",
> +				   &st->spi_ssel);
> +	if (ret) {
> +		dev_err(dev, "failed to get connected SPI slave select\n");
> +		goto failed3;
> +	}
> +
> +	nspi = of_parse_phandle(op->dev.of_node, "spibus-connected", 0);
Could you explain decision to do registration like this.
(platform device and manual handling of spi register?)
Could do spi device with resources for platform pass into it for example.
I'm not arguing one way or the other, just would like to see your
thinking on this.

> +	spi_master = spi_of_node_to_master(nspi);
> +
> +	if (spi_master != NULL) {
> +		struct spi_board_info info = {
> +			.modalias = KBUILD_MODNAME,
> +			.max_speed_hz = 1000000,
> +			.chip_select = st->spi_ssel,
> +			.mode = SPI_MODE_0 | SPI_3WIRE,
> +		};
> +		st->spi = spi_new_device(spi_master, &info);
> +		if (st->spi == NULL) {
> +			dev_err(dev, "failed to add spi device\n");
> +			goto failed3;
> +		}
> +	} else {
> +		dev_dbg(dev, "could not find SPI master node,"
> +			"using pcore spi implementation\n");
> +	}
> +
> +	/* Probe device */
> +	st->id = aim_spi_read(st, ADC_REG_CHIP_ID);
> +
> +	switch (st->id) {
> +	case CHIPID_AD9467:
> +		st->chip_info = &aim_chip_info_tbl[ID_AD9467];
> +		def_mode = AD9467_DEF_OUTPUT_MODE | OUTPUT_MODE_TWOS_COMPLEMENT;
> +		break;
> +	case CHIPID_AD9643:
> +		st->chip_info = &aim_chip_info_tbl[ID_AD9643];
> +		def_mode = AD9643_DEF_OUTPUT_MODE | OUTPUT_MODE_OFFSET_BINARY;
> +		break;
> +	default:
> +		dev_err(dev, "Unrecognized CHIP_ID 0x%X\n", st->id);
> +		ret = -ENODEV;
> +		goto failed3;
> +	}
> +
> +	indio_dev->dev.parent = dev;
> +	indio_dev->name = op->dev.of_node->name;
> +	indio_dev->channels = st->chip_info->channel;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->num_channels = st->chip_info->num_channels;
> +	indio_dev->available_scan_masks = st->chip_info->available_scan_masks;
> +	indio_dev->info = &aim_info;
> +
> +	init_completion(&st->dma_complete);
> +
> +	aim_spi_write(st, ADC_REG_OUTPUT_MODE, def_mode);
> +	aim_spi_write(st, ADC_REG_TEST_IO, TESTMODE_OFF);
> +	aim_spi_write(st, ADC_REG_TRANSFER, TRANSFER_SYNC);
> +
> +	aim_configure_ring(indio_dev);
> +	aim_register_ring_funcs(indio_dev);
> +	ret = iio_buffer_register(indio_dev,
> +				  st->chip_info->channel,
> +				  ARRAY_SIZE(st->chip_info->channel));
> +	if (ret)
> +		goto failed4;
> +
> +	ret = iio_device_register(indio_dev);
> +	if (ret)
> +		goto failed4;
> +
> +	dev_info(dev, "ADI AIM (0x%X) at 0x%08llX mapped to 0x%p,"
> +		 " DMA-%d probed ADC %s\n",
> +		 aim_read(st, AD9467_PCORE_VERSION),
> +		 (unsigned long long)phys_addr, st->regs,
> +		 st->rx_chan->chan_id, st->chip_info->name);
> +
documented anywhere?
> +	if (indio_dev->debugfs_dentry)
> +		debugfs_create_file("pseudorandom_err_check", 0644,
> +					indio_dev->debugfs_dentry,
> +					indio_dev, &aim_debugfs_pncheck_fops);
> +
> +	return 0;
> +
> +failed4:
> +	aim_unconfigure_ring(indio_dev);
> +failed3:
> +	dma_release_channel(st->rx_chan);
> +failed2:
> +	release_mem_region(phys_addr, remap_size);
> +failed1:
> +	iio_free_device(indio_dev);
> +	dev_set_drvdata(dev, NULL);
> +
> +	return ret;
> +}
> +
> +/**
> + * aim_of_remove - unbinds the driver from the AIM device.
> + * @of_dev:	pointer to OF device structure
> + *
> + * This function is called if a device is physically removed from the system or
> + * if the driver module is being unloaded. It frees any resources allocated to
> + * the device.
> + */
> +static int __devexit aim_of_remove(struct platform_device *op)
> +{
> +	struct device *dev = &op->dev;
> +	struct resource r_mem; /* IO mem resources */
> +	struct iio_dev *indio_dev = dev_get_drvdata(dev);
> +	struct aim_state *st = iio_priv(indio_dev);
> +
> +	iio_device_unregister(indio_dev);
> +	iio_buffer_unregister(indio_dev);
> +	aim_unconfigure_ring(indio_dev);
> +
> +	dma_release_channel(st->rx_chan);
> +
> +	iounmap(st->regs);
> +
> +	/* Get iospace of the device */
> +	if (of_address_to_resource(op->dev.of_node, 0, &r_mem))
> +		dev_err(dev, "invalid address\n");
> +	else
> +		release_mem_region(r_mem.start, resource_size(&r_mem));
> +
> +	iio_free_device(indio_dev);
> +
> +	dev_set_drvdata(dev, NULL);
> +
> +	return 0;
> +}
> +
> +/* Match table for of_platform binding */
> +static const struct of_device_id aim_of_match[] __devinitconst = {
> +	{ .compatible = "xlnx,cf-ad9467-core-1.00.a", },
> +	{ .compatible = "xlnx,cf-ad9643-core-1.00.a", },
> +	{ /* end of list */ },
> +};
> +MODULE_DEVICE_TABLE(of, aim_of_match);
> +
> +static struct platform_driver aim_of_driver = {
> +	.driver = {
> +		.name = KBUILD_MODNAME,
> +		.owner = THIS_MODULE,
> +		.of_match_table = aim_of_match,
> +	},
> +	.probe		= aim_of_probe,
> +	.remove		= __devexit_p(aim_of_remove),
> +};
> +
> +module_platform_driver(aim_of_driver);
> +
> +MODULE_AUTHOR("Michael Hennerich <hennerich@blackfin.uclinux.org>");
> +MODULE_DESCRIPTION("Analog Devices ADI-AIM");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/staging/iio/adc/cf_ad9467_ring.c b/drivers/staging/iio/adc/cf_ad9467_ring.c
> new file mode 100644
> index 0000000..c6b2230
> --- /dev/null
> +++ b/drivers/staging/iio/adc/cf_ad9467_ring.c
> @@ -0,0 +1,214 @@
> +/*
> + * ADI-AIM ADC Interface Module
> + *
> + * Copyright 2012 Analog Devices Inc.
> + *
> + * Licensed under the GPL-2 or later.
> + */
> +
> +#include <linux/slab.h>
> +#include <linux/kernel.h>
> +#include <linux/poll.h>
> +#include <linux/io.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/dmaengine.h>
> +
> +#include "../iio.h"
> +#include "../sysfs.h"
> +#include "../buffer.h"
> +#include "../ring_hw.h"
> +#include "cf_ad9467.h"
> +
> +static int aim_read_first_n_hw_rb(struct iio_buffer *r,
> +				      size_t count, char __user *buf)
> +{
> +	struct iio_hw_buffer *hw_ring = iio_to_hw_buf(r);
> +	struct iio_dev *indio_dev = hw_ring->private;
> +	struct aim_state *st = iio_priv(indio_dev);
> +	struct dma_async_tx_descriptor *desc;
> +	dma_cookie_t cookie;
> +	unsigned rcount;
> +	int ret;
> +
> +	mutex_lock(&st->lock);
> +	if (count == 0) {
> +		ret = -EINVAL;
> +		goto error_ret;
> +	}
> +
> +	if (count % 8)
> +		rcount = (count + 8) & 0xFFFFFFF8;
> +	else
> +		rcount = count;
> +
> +	st->buf_virt = dma_alloc_coherent(indio_dev->dev.parent,
> +					  PAGE_ALIGN(rcount), &st->buf_phys,
> +					  GFP_KERNEL);
> +	if (st->buf_virt == NULL) {
> +		ret = -ENOMEM;
> +		goto error_ret;
> +	}
> +
> +	desc = dmaengine_prep_slave_single(st->rx_chan, st->buf_phys, rcount,
> +					   DMA_FROM_DEVICE, DMA_PREP_INTERRUPT);
> +	if (!desc) {
> +		dev_err(indio_dev->dev.parent,
> +			"Failed to allocate a dma descriptor\n");
> +		ret = -ENOMEM;
> +		goto error_free;
> +	}
> +
> +	desc->callback = (dma_async_tx_callback) complete;
> +	desc->callback_param = &st->dma_complete;
> +
> +	cookie = dmaengine_submit(desc);
> +	if (cookie < 0) {
> +		dev_err(indio_dev->dev.parent,
> +			"Failed to submit a dma transfer\n");
> +		ret = cookie;
> +		goto error_free;
> +	}
> +
> +	dma_async_issue_pending(st->rx_chan);
> +
> +	aim_write(st, AD9467_PCORE_DMA_CTRL, 0);
> +	aim_read(st, AD9467_PCORE_DMA_STAT);
> +	aim_write(st, AD9467_PCORE_DMA_CTRL,
> +		  AD9647_DMA_CAP_EN | AD9647_DMA_CNT((rcount / 8) - 1));
> +
> +	ret = wait_for_completion_interruptible_timeout(&st->dma_complete,
> +							2 * HZ);
> +	if (ret == 0) {
> +		ret = -ETIMEDOUT;
> +		goto error_free;
> +	} else if (ret < 0) {
> +		goto error_free;
> +	}
> +
> +	if (copy_to_user(buf, st->buf_virt, count))
> +		ret = -EFAULT;
> +
> +error_free:
> +	dma_free_coherent(indio_dev->dev.parent, PAGE_ALIGN(rcount),
> +			  st->buf_virt, st->buf_phys);
> +	r->stufftoread = 0;
> +error_ret:
> +	mutex_unlock(&st->lock);
> +
> +	return ret ? ret : count;
> +}
> +
> +static int aim_ring_get_length(struct iio_buffer *r)
> +{
> +	struct iio_hw_buffer *hw_ring = iio_to_hw_buf(r);
> +	struct iio_dev *indio_dev = hw_ring->private;
> +	struct aim_state *st = iio_priv(indio_dev);
> +
> +	return st->ring_lenght;
> +}
> +
> +static int aim_ring_set_length(struct iio_buffer *r, int length)
> +{
> +	struct iio_hw_buffer *hw_ring = iio_to_hw_buf(r);
> +	struct iio_dev *indio_dev = hw_ring->private;
> +	struct aim_state *st = iio_priv(indio_dev);
could squish last two lines together without it getting harder
to follow.
> +
> +	st->ring_lenght = length;
lenght?
> +
> +	return 0;
> +}
> +
> +static int aim_ring_get_bytes_per_datum(struct iio_buffer *r)
> +{
> +	struct iio_hw_buffer *hw_ring = iio_to_hw_buf(r);
> +	struct iio_dev *indio_dev = hw_ring->private;
> +	struct aim_state *st = iio_priv(indio_dev);
> +
> +	return st->bytes_per_datum;
> +}
> +
> +static IIO_BUFFER_ENABLE_ATTR;
> +static IIO_BUFFER_LENGTH_ATTR;
> +
> +static struct attribute *aim_ring_attributes[] = {
> +	&dev_attr_length.attr,
> +	&dev_attr_enable.attr,
> +	NULL,
> +};
> +
> +static struct attribute_group aim_ring_attr = {
> +	.attrs = aim_ring_attributes,
> +	.name = "buffer",
> +};
> +
> +static struct iio_buffer *aim_rb_allocate(struct iio_dev *indio_dev)
> +{
> +	struct iio_buffer *buf;
> +	struct iio_hw_buffer *ring;
> +
> +	ring = kzalloc(sizeof *ring, GFP_KERNEL);
> +	if (!ring)
> +		return NULL;
> +
> +	ring->private = indio_dev;
> +	buf = &ring->buf;
> +	buf->stufftoread = 0;
technically no need to zero as you just kzalloc'd this
> +	buf->attrs = &aim_ring_attr;
> +	iio_buffer_init(buf);
> +
> +	return buf;
> +}
> +
> +static inline void aim_rb_free(struct iio_buffer *r)
> +{
> +	kfree(iio_to_hw_buf(r));
> +}
> +
> +static const struct iio_buffer_access_funcs aim_ring_access_funcs = {
> +	.read_first_n = &aim_read_first_n_hw_rb,
> +	.get_length = &aim_ring_get_length,
> +	.set_length = &aim_ring_set_length,
> +	.get_bytes_per_datum = &aim_ring_get_bytes_per_datum,
> +};
> +
> +int aim_configure_ring(struct iio_dev *indio_dev)
> +{
> +	indio_dev->buffer = aim_rb_allocate(indio_dev);
> +	if (indio_dev->buffer == NULL)
> +		return -ENOMEM;
> +
> +	indio_dev->modes |= INDIO_BUFFER_HARDWARE;
> +	indio_dev->buffer->access = &aim_ring_access_funcs;
> +
> +	return 0;
> +}
> +
> +void aim_unconfigure_ring(struct iio_dev *indio_dev)
> +{
> +	aim_rb_free(indio_dev->buffer);
> +}
> +
These next three seem a little 'odd'.  Why have them at all?
I don't think you are obliged to provide any of the callbacks...
> +static inline int __aim_hw_ring_state_set(struct iio_dev *indio_dev, bool state)
> +{
> +	return 0;
> +}
> +
> +static int aim_hw_ring_preenable(struct iio_dev *indio_dev)
> +{
> +	return __aim_hw_ring_state_set(indio_dev, 1);
> +}
> +
> +static int aim_hw_ring_postdisable(struct iio_dev *indio_dev)
> +{
> +	return __aim_hw_ring_state_set(indio_dev, 0);
> +}
> +
> +static const struct iio_buffer_setup_ops aim_ring_setup_ops = {
> +	.preenable = &aim_hw_ring_preenable,
> +	.postdisable = &aim_hw_ring_postdisable,
> +};
> +
> +void aim_register_ring_funcs(struct iio_dev *indio_dev)
> +{
> +	indio_dev->setup_ops = &aim_ring_setup_ops;
> +}

Re: [PATCH 4/4] iio: adc: New driver for AD9467 and AD9643 High-Speed LVDS ADCs

[Michael Hennerich]

On 02/19/2012 05:09 PM, Jonathan Cameron wrote:
> On 02/17/2012 12:46 PM, michael.hennerich@analog.com wrote:
>> From: Michael Hennerich<michael.hennerich@analog.com>
>>
>> Suitable FPGA interface HDL is available here:
>> http://wiki.analog.com/resources/fpga/xilinx/fmc/ad9467
> I don't know my way around the dma stuff so would appreciate review
> for someone who does (if any such person is reading!)
>
>
> Mostly for this one I would like a little more explanation of some
> of the design descisions.
> 1) Registering as platform device rather than spi with extra bits.
>      May well be cleaner this way around. If so please just briefly
>      explain why.

Hi Jonathan,

thanks for the review.

Well - this driver targets an 'on-chip' memory mapped peripheral,
sitting on an AMBA AXI Bus Interface. Main purpose is to interface
with high speed ADCs using an LVDS physical external interface.
SPI is only used to query the connected converter and do some
one time configuration setup.

This approach is not different from other drivers in other subsystems,
where the target device lives on an e.g. PCI bus and the driver
also registers an I2C or SPI client.
See drivers/drm, V4l drivers/media/video or also drivers/sound.


> 2) Effectively embedded spi implementation rather than a separate
> spi driver for the aim to spi stuff.
>      I can see it would be fairly clunky given this only does very
>      very special forms of spi, but it would cleanup this end by
>      removing the special casing.
Not sure it really makes sense here. The PCORE/HDL internal
SPI interface is only meant to interface with converters
covered by  AN-877. It only supports one flavor of SPI register
access. For example part of the transfer is fixed in hardware.
Only 3-WIRE mode, without interrupt capabilities.
A separate SPI master would only make sense if there were other
potential users for it.

> 3) Separation (or lack there of) for the adc interface and the hardware
> buffer implemented in the fpga.
>      Probably tricky to pull off but a few lines saying where the problems
>      lie would be informative.
You probably noticed; to-date there is no real buffer involved.
Given the sampling speed of 250MSPS every software manged buffer
mechanism might overflow. Given the fact that the writer reader
problem can't be solved in real-time. So right now, we do
blocking reads. The AXI S2MM DMA reads the requested number of
samples directly from the peripheral. Given the high sample rate,
there wouldn't be any difference whether the data would come
from an FPGA/peripheral internal buffer, or directly from the ADC.

> A nice interest example and well written driver otherwise.  Couple
> of things like documentation requests and small tidy ups inline.
>
>> Signed-off-by: Michael Hennerich<michael.hennerich@analog.com>
>> ---
>>   drivers/staging/iio/adc/Kconfig          |   13 +
>>   drivers/staging/iio/adc/Makefile         |    3 +
>>   drivers/staging/iio/adc/cf_ad9467.h      |  161 ++++++++
>>   drivers/staging/iio/adc/cf_ad9467_core.c |  585 ++++++++++++++++++++++++++++++
>>   drivers/staging/iio/adc/cf_ad9467_ring.c |  214 +++++++++++
>>   5 files changed, 976 insertions(+), 0 deletions(-)
>>   create mode 100644 drivers/staging/iio/adc/cf_ad9467.h
>>   create mode 100644 drivers/staging/iio/adc/cf_ad9467_core.c
>>   create mode 100644 drivers/staging/iio/adc/cf_ad9467_ring.c
>>
>> diff --git a/drivers/staging/iio/adc/Kconfig b/drivers/staging/iio/adc/Kconfig
>> index d9decea..266e807 100644
>> --- a/drivers/staging/iio/adc/Kconfig
>> +++ b/drivers/staging/iio/adc/Kconfig
>> @@ -169,6 +169,19 @@ config AD7280
>>          To compile this driver as a module, choose M here: the
>>          module will be called ad7280a
>>
>> +config CF_AD9467
>> +     tristate "Analog Devices AD9467 AD9643 High-Speed ADC driver"
>> +     select IIO_BUFFER
>> +     help
>> +       Say yes here to build support for Analog Devices AD9467 and AD9643,
>> +       High-Speed LVDS analog to digital converters (ADC).
>> +       FPGA interface HDL is available here:
>> +       http://wiki.analog.com/resources/fpga/xilinx/fmc/ad9467
>> +       If unsure, say N (but it's safe to say "Y").
>> +
>> +       To compile this driver as a module, choose M here: the
>> +       module will be called cf_ad9467.
>> +
>>   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 ceee7f3..2f37d85 100644
>> --- a/drivers/staging/iio/adc/Makefile
>> +++ b/drivers/staging/iio/adc/Makefile
>> @@ -29,6 +29,9 @@ ad7298-y := ad7298_core.o
>>   ad7298-$(CONFIG_IIO_BUFFER) += ad7298_ring.o
>>   obj-$(CONFIG_AD7298) += ad7298.o
>>
>> +cf_ad9467-y := cf_ad9467_core.o cf_ad9467_ring.o
>> +obj-$(CONFIG_CF_AD9467) += cf_ad9467.o
>> +
>>   obj-$(CONFIG_AD7291) += ad7291.o
>>   obj-$(CONFIG_AD7780) += ad7780.o
>>   obj-$(CONFIG_AD7793) += ad7793.o
>> diff --git a/drivers/staging/iio/adc/cf_ad9467.h b/drivers/staging/iio/adc/cf_ad9467.h
>> new file mode 100644
>> index 0000000..1423e4c
>> --- /dev/null
>> +++ b/drivers/staging/iio/adc/cf_ad9467.h
>> @@ -0,0 +1,161 @@
>> +/*
>> + * ADI-AIM ADI ADC Interface Module
>> + *
>> + * Copyright 2012 Analog Devices Inc.
>> + *
>> + * Licensed under the GPL-2.
>> + *
>> + * http://wiki.analog.com/resources/fpga/xilinx/fmc/ad9467
>> + */
>> +
>> +#ifndef ADI_AIM_H_
>> +#define ADI_AIM_H_
>> +
>> +/* PCORE CoreFPGA register map */
>> +
>> +#define AD9467_PCORE_VERSION         0x00
>> +#define AD9467_PCORE_SPI_CTRL                0x04
>> +#define AD9467_PCORE_SPI_RDSTAT              0x08
>> +#define AD9467_PCORE_DMA_CTRL                0x0C
>> +#define AD9467_PCORE_DMA_STAT                0x10
>> +#define AD9467_PCORE_ADC_STAT                0x14
>> +#define AD9467_PCORE_PN_ERR_CTRL     0x24
>> +
>> +/* AD9467_PCORE_SPI_CTRL */
>> +#define AD9647_SPI_START             (1<<  25)
>> +#define AD9647_SPI_SEL(x)            (((x)&  0x1)<<  24)
>> +#define AD9647_SPI_READ                      (1<<  23)
>> +#define AD9647_SPI_WRITE             (0<<  23)
>> +#define AD9647_SPI_ADDR(x)           (((x)&  0x1FFF)<<  8)
>> +#define AD9647_SPI_DATA(x)           (((x)&  0xFF)<<  0)
>> +
>> +/* AD9467_PCORE_SPI_RDSTAT */
>> +#define AD9647_SPI_IDLE                      (1<<  8)
>> +#define AD9647_SPI_READVAL(x)                ((x)&  0xFF)
>> +
>> +/* AD9467_PCORE_DMA_CTRL */
>> +#define AD9647_DMA_CAP_EN            (1<<  16)
>> +#define AD9647_DMA_CNT(x)            (((x)&  0xFFFF)<<  0)
>> +
>> +/* AD9467_PCORE_DMA_STAT */
>> +#define AD9647_DMA_STAT_BUSY         (1<<  0)
>> +#define AD9647_DMA_STAT_OVF          (1<<  1)
>> +#define AD9647_DMA_STAT_UNF          (1<<  2)
>> +
>> +/* AD9467_PCORE_ADC_STAT */
>> +#define AD9467_PCORE_ADC_STAT_OVR    (1<<  0)
>> +#define AD9467_PCORE_ADC_STAT_PN_OOS (1<<  1) /* W1C */
>> +#define AD9467_PCORE_ADC_STAT_PN_ERR (1<<  2) /* W1C */
>> +
>> +/* AD9467_PCORE_PN_ERR_CTRL */
>> +#define AD9467_PN23_EN                       (1<<  0)
>> +#define AD9467_PN9_EN                        (0<<  0)
>> +
>> +/*
>> + * ADI High-Speed ADC common spi interface registers
>> + * See Application-Note AN-877
>> + */
>> +
>> +#define ADC_REG_CHIP_PORT_CONF               0x00
>> +#define ADC_REG_CHIP_ID                      0x01
>> +#define ADC_REG_CHIP_GRADE           0x02
>> +#define ADC_REG_TRANSFER             0xFF
>> +#define ADC_REG_MODES                        0x08
>> +#define ADC_REG_TEST_IO                      0x0D
>> +#define ADC_REG_ADC_INPUT            0x0F
>> +#define ADC_REG_OFFSET                       0x10
>> +#define ADC_REG_OUTPUT_MODE          0x14
>> +#define ADC_REG_OUTPUT_ADJUST                0x15
>> +#define ADC_REG_OUTPUT_PHASE         0x16
>> +#define ADC_REG_OUTPUT_DELAY         0x17
>> +#define ADC_REG_VREF                 0x18
>> +#define ADC_REG_ANALOG_INPUT         0x2C
>> +
>> +/* ADC_REG_TRANSFER */
>> +#define TRANSFER_SYNC                        0x1
>> +
>> +/* ADC_REG_TEST_IO */
>> +#define TESTMODE_OFF                 0x0
>> +#define TESTMODE_MIDSCALE_SHORT              0x1
>> +#define TESTMODE_POS_FULLSCALE               0x2
>> +#define TESTMODE_NEG_FULLSCALE               0x3
>> +#define TESTMODE_ALT_CHECKERBOARD    0x4
>> +#define TESTMODE_PN23_SEQ            0x5
>> +#define TESTMODE_PN9_SEQ             0x6
>> +#define TESTMODE_ONE_ZERO_TOGGLE     0x7
>> +
>> +/* ADC_REG_OUTPUT_MODE */
>> +#define OUTPUT_MODE_OFFSET_BINARY    0x0
>> +#define OUTPUT_MODE_TWOS_COMPLEMENT  0x1
>> +#define OUTPUT_MODE_GRAY_CODE                0x2
>> +
>> +/*
>> + * Analog Devices AD9467 16-Bit, 200/250 MSPS ADC
>> + */
>> +
>> +#define AD9467_DEF_OUTPUT_MODE               0x08
>> +#define AD9467_REG_VREF_MASK         0x0F
>> +#define CHIPID_AD9467                        0x50
>> +
>> +/*
>> + * Analog Devices AD9643 Dual 14-Bit, 170/210/250 MSPS ADC
>> + */
>> +
>> +#define CHIPID_AD9643                        0x82
>> +#define AD9643_REG_VREF_MASK         0x1F
>> +#define AD9643_DEF_OUTPUT_MODE               0x04
>> +
>> +enum {
>> +     ID_AD9467,
>> +     ID_AD9643,
>> +};
>> +
>> +struct aim_chip_info {
>> +     char                            name[8];
>> +     unsigned                        num_channels;
>> +     unsigned long                   available_scan_masks[2];
>> +     const int                       (*scale_table)[2];
>> +     int                             num_scales;
>> +     struct iio_chan_spec            channel[2];
>> +};
>> +
>> +struct aim_state {
>> +     struct spi_device               *spi;
>> +     struct mutex                    lock;
>> +     struct completion               dma_complete;
>> +     struct dma_chan                 *rx_chan;
>> +     const struct aim_chip_info      *chip_info;
>> +     void __iomem                    *regs;
>> +     void                            *buf_virt;
>> +     dma_addr_t                      buf_phys;
>> +     unsigned                        spi_ssel;
>> +     unsigned                        ring_lenght;
>> +     unsigned                        bytes_per_datum;
>> +     unsigned                        id;
>> +     /*
>> +      * DMA (thus cache coherency maintenance) requires the
>> +      * transfer buffers to live in their own cache lines.
>> +      */
> no blank line here and one above the comment perhaps?
>> +
>> +     unsigned char                   data[3] ____cacheline_aligned;
>> +};
>> +
>> +/*
>> + * IO accessors
>> + */
>> +
>> +static inline void aim_write(struct aim_state *st, unsigned reg, unsigned val)
>> +{
>> +     iowrite32(val, st->regs + reg);
>> +}
>> +
>> +static inline unsigned int aim_read(struct aim_state *st, unsigned reg)
>> +{
>> +     return ioread32(st->regs + reg);
>> +}
>> +
>> +void aim_register_ring_funcs(struct iio_dev *indio_dev);
>> +int aim_configure_ring(struct iio_dev *indio_dev);
>> +void aim_unconfigure_ring(struct iio_dev *indio_dev);
>> +
>> +#endif /* ADI_AIM_H_ */
>> diff --git a/drivers/staging/iio/adc/cf_ad9467_core.c b/drivers/staging/iio/adc/cf_ad9467_core.c
>> new file mode 100644
>> index 0000000..811a659
>> --- /dev/null
>> +++ b/drivers/staging/iio/adc/cf_ad9467_core.c
>> @@ -0,0 +1,585 @@
>> +/*
>> + * ADI-AIM ADI ADC Interface Module
>> + *
>> + * Copyright 2012 Analog Devices Inc.
>> + *
>> + * Licensed under the GPL-2.
>> + *
>> + * http://wiki.analog.com/resources/fpga/xilinx/fmc/ad9467
>> + */
>> +
>> +#include<linux/module.h>
>> +#include<linux/errno.h>
>> +#include<linux/slab.h>
>> +#include<linux/init.h>
>> +#include<linux/io.h>
>> +#include<linux/wait.h>
>> +#include<linux/spi/spi.h>
>> +#include<linux/dma-mapping.h>
>> +#include<linux/dmaengine.h>
>> +#include<linux/delay.h>
>> +#include<linux/debugfs.h>
>> +#include<linux/uaccess.h>
>> +
>> +#include<linux/of.h>
>> +#include<linux/of_device.h>
>> +#include<linux/of_platform.h>
>> +#include<linux/of_address.h>
>> +#include<linux/of_spi.h>
>> +
>> +#include "../iio.h"
>> +#include "../sysfs.h"
>> +#include "../buffer.h"
>> +
>> +#include "cf_ad9467.h"
>> +
>> +static int aim_spi_read(struct aim_state *st, unsigned reg)
>> +{
>> +     unsigned long timeout = jiffies + HZ / 4;
>> +     int ret;
>> +
>> +     if (st->spi) {
>> +             unsigned char *buf = st->data;
>> +             buf[0] = 0x80 | (reg>>  8);
>> +             buf[1] = reg&  0xFF;
>> +
>> +             ret = spi_write_then_read(st->spi,&buf[0], 2,&buf[2], 1);
>> +             if (ret<  0)
>> +                     return ret;
>> +
>> +             return buf[2];
>> +     }
>> +
>> +     aim_write(st, AD9467_PCORE_SPI_CTRL, 0);
>> +     aim_write(st, AD9467_PCORE_SPI_CTRL,
>> +             AD9647_SPI_START |
>> +             AD9647_SPI_READ |
>> +             AD9647_SPI_SEL(st->spi_ssel) |
>> +             AD9647_SPI_ADDR(reg));
>> +
>> +     while (!(aim_read(st, AD9467_PCORE_SPI_RDSTAT)&  AD9647_SPI_IDLE)) {
>> +             cpu_relax();
>> +             if (time_after(jiffies, timeout))
>> +                     return -EIO;
>> +     }
>> +
>> +     return AD9647_SPI_READVAL(aim_read(st, AD9467_PCORE_SPI_RDSTAT));
>> +}
>> +
>> +static int aim_spi_write(struct aim_state *st, unsigned reg, unsigned val)
>> +{
>> +     unsigned long timeout = jiffies + HZ / 4;
>> +
>> +     int ret;
>> +
>> +     if (st->spi) {
>> +             unsigned char *buf = st->data;
>> +             buf[0] = reg>>  8;
>> +             buf[1] = reg&  0xFF;
>> +             buf[2] = val;
>> +             ret = spi_write(st->spi, buf, 3);
>> +             if (ret<  0)
>> +                     return ret;
>> +
>> +             return 0;
>> +     }
>> +
> This is 'unusual'.  Perhaps it really should be a limited functionality
> spi driver?
>> +     aim_write(st, AD9467_PCORE_SPI_CTRL, 0);
>> +     aim_write(st, AD9467_PCORE_SPI_CTRL,
>> +             AD9647_SPI_START |
>> +             AD9647_SPI_WRITE |
>> +             AD9647_SPI_SEL(st->spi_ssel) |
>> +             AD9647_SPI_ADDR(reg) |
>> +             AD9647_SPI_DATA(val));
>> +
>> +
>> +     while (!(aim_read(st, AD9467_PCORE_SPI_RDSTAT)&  AD9647_SPI_IDLE)) {
>> +             cpu_relax();
>> +             if (time_after(jiffies, timeout))
>> +                     return -EIO;
>> +     }
>> +
>> +     return 0;
>> +}
>> +
>> +static int aim_debugfs_open(struct inode *inode, struct file *file)
>> +{
>> +     if (inode->i_private)
>> +             file->private_data = inode->i_private;
>> +
>> +     return 0;
>> +}
>> +
>> +static ssize_t aim_debugfs_pncheck_read(struct file *file, char __user *userbuf,
>> +                           size_t count, loff_t *ppos)
>> +{
>> +     struct iio_dev *indio_dev = file->private_data;
>> +     struct aim_state *st = iio_priv(indio_dev);
>> +     char buf[80], *p = buf;
>> +     unsigned stat;
>> +
>> +     stat = aim_read(st, AD9467_PCORE_VERSION);
>> +     p += sprintf(p, "%s %s\n", stat&  AD9467_PCORE_ADC_STAT_PN_OOS ?
>> +             "Out of Sync" : "", stat&  AD9467_PCORE_ADC_STAT_PN_ERR ?
>> +             "PN Error" : "");
>> +
>> +     return simple_read_from_buffer(userbuf, count, ppos, buf, p - buf);
>> +}
>> +
>> +static ssize_t aim_debugfs_pncheck_write(struct file *file,
>> +                  const char __user *userbuf, size_t count, loff_t *ppos)
>> +{
>> +     struct iio_dev *indio_dev = file->private_data;
>> +     struct aim_state *st = iio_priv(indio_dev);
>> +     unsigned mode;
>> +     char buf[80], *p = buf;
>> +
>> +     count = min_t(size_t, count, (sizeof(buf)-1));
>> +     if (copy_from_user(p, userbuf, count))
>> +             return -EFAULT;
>> +
>> +     if (sysfs_streq(p, "PN9"))
>> +             mode = TESTMODE_PN9_SEQ;
>> +     else if (sysfs_streq(p, "PN23"))
>> +             mode = TESTMODE_PN23_SEQ;
>> +     else
>> +             mode = TESTMODE_OFF;
>> +
>> +     aim_spi_write(st, ADC_REG_TEST_IO, mode);
>> +     aim_spi_write(st, ADC_REG_TRANSFER, TRANSFER_SYNC);
>> +
>> +     aim_write(st, AD9467_PCORE_PN_ERR_CTRL, mode == TESTMODE_PN23_SEQ ?
>> +               AD9467_PN23_EN : AD9467_PN9_EN);
>> +
>> +     mdelay(1);
> document why please.
>> +
>> +     aim_write(st, AD9467_PCORE_ADC_STAT,
>> +               AD9467_PCORE_ADC_STAT_PN_OOS |
>> +               AD9467_PCORE_ADC_STAT_PN_OOS |
>> +               AD9467_PCORE_ADC_STAT_OVR);
>> +
>> +     return count;
>> +}
>> +
>> +static const struct file_operations aim_debugfs_pncheck_fops = {
>> +     .open = aim_debugfs_open,
>> +     .read = aim_debugfs_pncheck_read,
>> +     .write = aim_debugfs_pncheck_write,
>> +};
>> +
>> +static int aim_reg_access(struct iio_dev *indio_dev,
>> +                           unsigned reg, unsigned writeval,
>> +                           unsigned *readval)
>> +{
>> +     struct aim_state *st = iio_priv(indio_dev);
>> +     int ret;
>> +
>> +     mutex_lock(&indio_dev->mlock);
>> +     if (readval == NULL) {
>> +             ret = aim_spi_write(st, reg, writeval);
>> +             aim_spi_write(st, ADC_REG_TRANSFER, TRANSFER_SYNC);
>> +     } else {
>> +             ret = aim_spi_read(st, reg);
>> +             if (ret<  0)
>> +                     return ret;
>> +             *readval = ret;
>> +             ret = 0;
>> +     }
> Seems like an excess blank line....
>> +
>> +     mutex_unlock(&indio_dev->mlock);
>> +
>> +     return ret;
>> +}
>> +
>> +static int aim_read_raw(struct iio_dev *indio_dev,
>> +                        struct iio_chan_spec const *chan,
>> +                        int *val,
>> +                        int *val2,
>> +                        long m)
>> +{
>> +     struct aim_state *st = iio_priv(indio_dev);
>> +     int i;
>> +     unsigned vref_val;
>> +
>> +     switch (m) {
>> +     case IIO_CHAN_INFO_SCALE:
>> +             vref_val = aim_spi_read(st, ADC_REG_VREF)&
>> +                     (st->id == CHIPID_AD9643 ? AD9643_REG_VREF_MASK :
>> +                     AD9467_REG_VREF_MASK);
>> +
>> +             for (i = 0; i<  st->chip_info->num_scales; i++)
>> +                     if (vref_val == st->chip_info->scale_table[i][1])
>> +                             break;
>> +
>> +             *val =  0;
>> +             *val2 = st->chip_info->scale_table[i][0];
>> +
>> +             return IIO_VAL_INT_PLUS_MICRO;
>> +     }
>> +     return -EINVAL;
>> +}
>> +
>> +
>> +static int aim_write_raw(struct iio_dev *indio_dev,
>> +                            struct iio_chan_spec const *chan,
>> +                            int val,
>> +                            int val2,
>> +                            long mask)
>> +{
>> +     struct aim_state *st = iio_priv(indio_dev);
>> +     int i;
>> +
>> +     switch (mask) {
>> +     case IIO_CHAN_INFO_SCALE:
>> +             if (val != 0)
>> +                     return -EINVAL;
>> +
>> +             for (i = 0; i<  st->chip_info->num_scales; i++)
>> +                     if (val2 == st->chip_info->scale_table[i][0])
>> +                             break;
> what if it isn't any of them?
good catch.

>> +
>> +             aim_spi_write(st, ADC_REG_VREF,
>> +                           st->chip_info->scale_table[i][1]);
>> +             aim_spi_write(st, ADC_REG_TRANSFER, TRANSFER_SYNC);
>> +
>> +             return 0;
>> +     default:
>> +             return -EINVAL;
>> +     }
>> +}
>> +
>> +static ssize_t aim_show_scale_available(struct device *dev,
>> +                     struct device_attribute *attr, char *buf)
>> +{
>> +     struct iio_dev *indio_dev = dev_get_drvdata(dev);
>> +     struct aim_state *st = iio_priv(indio_dev);
>> +     int i, len = 0;
>> +
>> +     for (i = 0; i<  st->chip_info->num_scales; i++)
>> +             len += sprintf(buf + len, "0.%06u ",
>> +                            st->chip_info->scale_table[i][0]);
>> +
>> +     len += sprintf(buf + len, "\n");
>> +
>> +     return len;
>> +}
>> +
>> +static IIO_DEVICE_ATTR(in_voltage_scale_available, S_IRUGO,
>> +                    aim_show_scale_available, NULL, 0);
>> +
>> +static struct attribute *aim_attributes[] = {
>> +&iio_dev_attr_in_voltage_scale_available.dev_attr.attr,
>> +     NULL,
>> +};
>> +
>> +static const struct attribute_group aim_attribute_group = {
>> +     .attrs = aim_attributes,
>> +};
>> +
>> +static const int ad9467_scale_table[][2] = {
>> +     {30517, 0}, {32043, 6}, {33569, 7},
>> +     {35095, 8}, {36621, 9}, {38146, 10},
>> +};
>> +
>> +static const int ad9643_scale_table[][2] = {
>> +     {31738, 0xF}, {31403, 0xE}, {31067, 0xD}, {30731, 0xC}, {30396, 0xB},
>> +     {30060, 0xA}, {29724, 0x9}, {29388, 0x8}, {29053, 0x7}, {28717, 0x6},
>> +     {28381, 0x5}, {28046, 0x4}, {27710, 0x3}, {27374, 0x2}, {27039, 0x1},
>> +     {26703, 0x0}, {26367, 0x1F}, {26031, 0x1E}, {25696, 0x1D},
>> +     {25360, 0x1C}, {25024, 0x1B}, {24689, 0x1A}, {24353, 0x19},
>> +     {24017, 0x18}, {23682, 0x17}, {23346, 0x16}, {23010, 0x15},
>> +     {22675, 0x14}, {22339, 0x13}, {22003, 0x12}, {21667, 0x11},
>> +     {21332, 0x10},
>> +};
>> +
>> +#define AIM_CHAN(_chan, _name, _si, _bits, _sign)                    \
>> +     { .type = IIO_VOLTAGE,                                          \
>> +       .indexed = 1,                                                 \
>> +       .extend_name = _name,                                         \
>> +       .channel = _chan,                                             \
>> +       .info_mask = IIO_CHAN_INFO_SCALE_SHARED_BIT,                  \
>> +       .scan_index = _si,                                            \
>> +       .scan_type =  IIO_ST(_sign, _bits, 16, 0)}
>> +
>> +static const struct aim_chip_info aim_chip_info_tbl[] = {
>> +     [ID_AD9467] = {
>> +             .name = "AD9467",
>> +             .scale_table = ad9467_scale_table,
>> +             .num_scales = ARRAY_SIZE(ad9467_scale_table),
>> +             .num_channels = 1,
>> +             .available_scan_masks[0] = BIT(0),
>> +             .channel[0] = AIM_CHAN(0, NULL, 0, 16, 's'),
>> +     },
>> +     [ID_AD9643] = {
>> +             .name = "AD9643",
>> +             .scale_table = ad9643_scale_table,
>> +             .num_scales = ARRAY_SIZE(ad9643_scale_table),
>> +             .num_channels = 2,
>> +             .available_scan_masks[0] = BIT(0) | BIT(1),
>> +             .channel[0] = AIM_CHAN(0, "I", 0, 14, 'u'),
>> +             .channel[1] = AIM_CHAN(1, "Q", 1, 14, 'u'),
>> +     },
> Why I and Q?  I clearly could be a quadrature pair of channels but
> is there anything in inherent in the device forcing this?  Also
> is this documented
Yeah this shouldn't be here - thought I removed it already.

>> +};
>> +
>> +static const struct iio_info aim_info = {
>> +     .driver_module = THIS_MODULE,
>> +     .read_raw =&aim_read_raw,
>> +     .write_raw =&aim_write_raw,
>> +     .attrs =&aim_attribute_group,
>> +     .debugfs_reg_access = aim_reg_access,
>> +};
>> +
>> +struct aim_dma_params {
>> +     struct device_node *of_node;
>> +     int chan_id;
>> +};
>> +
>> +static bool aim_dma_filter(struct dma_chan *chan, void *param)
>> +{
>> +     struct aim_dma_params *p = param;
>> +
>> +     return chan->device->dev->of_node == p->of_node&&
>> +             chan->chan_id == p->chan_id;
>> +}
>> +
>> +/**
>> + * aim_of_probe - probe method for the AIM device.
>> + * @of_dev:  pointer to OF device structure
>> + * @match:   pointer to the structure used for matching a device
>> + *
>> + * This function probes the AIM device in the device tree.
>> + * It initializes the driver data structure and the hardware.
>> + * It returns 0, if the driver is bound to the AIM device, or a negative
>> + * value if there is an error.
>> + */
>> +static int __devinit aim_of_probe(struct platform_device *op)
>> +{
>> +     struct iio_dev *indio_dev;
>> +     struct device *dev =&op->dev;
>> +     struct aim_state *st;
>> +     struct resource r_mem; /* IO mem resources */
>> +     struct spi_master *spi_master;
>> +     struct device_node *nspi;
>> +     struct aim_dma_params dma_params;
>> +     struct of_phandle_args dma_spec;
>> +     dma_cap_mask_t mask;
>> +     resource_size_t remap_size, phys_addr;
>> +     unsigned def_mode;
>> +     int ret;
>> +
>> +     dev_info(dev, "Device Tree Probing \'%s\'\n",
>> +              op->dev.of_node->name);
>> +
>> +     /* Get iospace for the device */
>> +     ret = of_address_to_resource(op->dev.of_node, 0,&r_mem);
>> +     if (ret) {
>> +             dev_err(dev, "invalid address\n");
>> +             return ret;
>> +     }
>> +
>> +     indio_dev = iio_allocate_device(sizeof(*st));
>> +     if (indio_dev == NULL)
>> +             return -ENOMEM;
>> +
>> +     st = iio_priv(indio_dev);
>> +
>> +     dev_set_drvdata(dev, indio_dev);
>> +     mutex_init(&st->lock);
>> +
>> +     phys_addr = r_mem.start;
>> +     remap_size = resource_size(&r_mem);
>> +     if (!request_mem_region(phys_addr, remap_size, KBUILD_MODNAME)) {
>> +             dev_err(dev, "Couldn't lock memory region at 0x%08llX\n",
>> +                     (unsigned long long)phys_addr);
>> +             ret = -EBUSY;
>> +             goto failed1;
>> +     }
>> +
>> +     st->regs = ioremap(phys_addr, remap_size);
>> +     if (st->regs == NULL) {
>> +             dev_err(dev, "Couldn't ioremap memory at 0x%08llX\n",
>> +                     (unsigned long long)phys_addr);
>> +             ret = -EFAULT;
>> +             goto failed2;
>> +     }
>> +     /* Get dma channel for the device */
>> +     ret = of_parse_phandle_with_args(op->dev.of_node, "dma-request",
>> +                                      "#dma-cells", 0,&dma_spec);
>> +     if (ret) {
>> +             dev_err(dev, "Couldn't parse dma-request\n");
>> +             goto failed2;
>> +     }
>> +
>> +     dma_params.of_node = dma_spec.np;
>> +     dma_params.chan_id = dma_spec.args[0];
>> +
>> +     dma_cap_zero(mask);
>> +     dma_cap_set(DMA_SLAVE | DMA_PRIVATE, mask);
>> +
>> +     st->rx_chan = dma_request_channel(mask, aim_dma_filter,&dma_params);
>> +     if (!st->rx_chan) {
>> +             dev_err(dev, "failed to find rx dma device\n");
>> +             goto failed2;
>> +     }
>> +     /*
>> +      * Get SPI configuration interface for the device
>> +      * We are platform_driver, so we need other means to get the
>> +      * associated control interface.
>> +      */
>> +     ret = of_property_read_u32(op->dev.of_node,
>> +                                "spibus-slaveselect-connected",
>> +&st->spi_ssel);
>> +     if (ret) {
>> +             dev_err(dev, "failed to get connected SPI slave select\n");
>> +             goto failed3;
>> +     }
>> +
>> +     nspi = of_parse_phandle(op->dev.of_node, "spibus-connected", 0);
> Could you explain decision to do registration like this.
> (platform device and manual handling of spi register?)
> Could do spi device with resources for platform pass into it for example.
> I'm not arguing one way or the other, just would like to see your
> thinking on this.
>
Well - either it's a platform driver, or spi bus driver, but it cannot 
be both.
Arguments why I think it's a platform driver see above.

The only way around is to split this into 3 drivers.
An adapter platform driver, where the SPI Bus Converter interface driver
registers with. For that approach the PCORE internal SPI interface also 
needs
to be a SPI Master driver.

For now I like to keep things simple, since most users are likley going 
with the
3-WIRE SPI (SDIO) PCORE/HDL implementation.


>> +     spi_master = spi_of_node_to_master(nspi);
>> +
>> +     if (spi_master != NULL) {
>> +             struct spi_board_info info = {
>> +                     .modalias = KBUILD_MODNAME,
>> +                     .max_speed_hz = 1000000,
>> +                     .chip_select = st->spi_ssel,
>> +                     .mode = SPI_MODE_0 | SPI_3WIRE,
>> +             };
>> +             st->spi = spi_new_device(spi_master,&info);
>> +             if (st->spi == NULL) {
>> +                     dev_err(dev, "failed to add spi device\n");
>> +                     goto failed3;
>> +             }
>> +     } else {
>> +             dev_dbg(dev, "could not find SPI master node,"
>> +                     "using pcore spi implementation\n");
>> +     }
>> +
>> +     /* Probe device */
>> +     st->id = aim_spi_read(st, ADC_REG_CHIP_ID);
>> +
>> +     switch (st->id) {
>> +     case CHIPID_AD9467:
>> +             st->chip_info =&aim_chip_info_tbl[ID_AD9467];
>> +             def_mode = AD9467_DEF_OUTPUT_MODE | OUTPUT_MODE_TWOS_COMPLEMENT;
>> +             break;
>> +     case CHIPID_AD9643:
>> +             st->chip_info =&aim_chip_info_tbl[ID_AD9643];
>> +             def_mode = AD9643_DEF_OUTPUT_MODE | OUTPUT_MODE_OFFSET_BINARY;
>> +             break;
>> +     default:
>> +             dev_err(dev, "Unrecognized CHIP_ID 0x%X\n", st->id);
>> +             ret = -ENODEV;
>> +             goto failed3;
>> +     }
>> +
>> +     indio_dev->dev.parent = dev;
>> +     indio_dev->name = op->dev.of_node->name;
>> +     indio_dev->channels = st->chip_info->channel;
>> +     indio_dev->modes = INDIO_DIRECT_MODE;
>> +     indio_dev->num_channels = st->chip_info->num_channels;
>> +     indio_dev->available_scan_masks = st->chip_info->available_scan_masks;
>> +     indio_dev->info =&aim_info;
>> +
>> +     init_completion(&st->dma_complete);
>> +
>> +     aim_spi_write(st, ADC_REG_OUTPUT_MODE, def_mode);
>> +     aim_spi_write(st, ADC_REG_TEST_IO, TESTMODE_OFF);
>> +     aim_spi_write(st, ADC_REG_TRANSFER, TRANSFER_SYNC);
>> +
>> +     aim_configure_ring(indio_dev);
>> +     aim_register_ring_funcs(indio_dev);
>> +     ret = iio_buffer_register(indio_dev,
>> +                               st->chip_info->channel,
>> +                               ARRAY_SIZE(st->chip_info->channel));
>> +     if (ret)
>> +             goto failed4;
>> +
>> +     ret = iio_device_register(indio_dev);
>> +     if (ret)
>> +             goto failed4;
>> +
>> +     dev_info(dev, "ADI AIM (0x%X) at 0x%08llX mapped to 0x%p,"
>> +              " DMA-%d probed ADC %s\n",
>> +              aim_read(st, AD9467_PCORE_VERSION),
>> +              (unsigned long long)phys_addr, st->regs,
>> +              st->rx_chan->chan_id, st->chip_info->name);
>> +
> documented anywhere?
Not yet.
Where do you like this to happen?
IIO per driver docs?

>> +     if (indio_dev->debugfs_dentry)
>> +             debugfs_create_file("pseudorandom_err_check", 0644,
>> +                                     indio_dev->debugfs_dentry,
>> +                                     indio_dev,&aim_debugfs_pncheck_fops);
>> +
>> +     return 0;
>> +
>> +failed4:
>> +     aim_unconfigure_ring(indio_dev);
>> +failed3:
>> +     dma_release_channel(st->rx_chan);
>> +failed2:
>> +     release_mem_region(phys_addr, remap_size);
>> +failed1:
>> +     iio_free_device(indio_dev);
>> +     dev_set_drvdata(dev, NULL);
>> +
>> +     return ret;
>> +}
>> +
>> +/**
>> + * aim_of_remove - unbinds the driver from the AIM device.
>> + * @of_dev:  pointer to OF device structure
>> + *
>> + * This function is called if a device is physically removed from the system or
>> + * if the driver module is being unloaded. It frees any resources allocated to
>> + * the device.
>> + */
>> +static int __devexit aim_of_remove(struct platform_device *op)
>> +{
>> +     struct device *dev =&op->dev;
>> +     struct resource r_mem; /* IO mem resources */
>> +     struct iio_dev *indio_dev = dev_get_drvdata(dev);
>> +     struct aim_state *st = iio_priv(indio_dev);
>> +
>> +     iio_device_unregister(indio_dev);
>> +     iio_buffer_unregister(indio_dev);
>> +     aim_unconfigure_ring(indio_dev);
>> +
>> +     dma_release_channel(st->rx_chan);
>> +
>> +     iounmap(st->regs);
>> +
>> +     /* Get iospace of the device */
>> +     if (of_address_to_resource(op->dev.of_node, 0,&r_mem))
>> +             dev_err(dev, "invalid address\n");
>> +     else
>> +             release_mem_region(r_mem.start, resource_size(&r_mem));
>> +
>> +     iio_free_device(indio_dev);
>> +
>> +     dev_set_drvdata(dev, NULL);
>> +
>> +     return 0;
>> +}
>> +
>> +/* Match table for of_platform binding */
>> +static const struct of_device_id aim_of_match[] __devinitconst = {
>> +     { .compatible = "xlnx,cf-ad9467-core-1.00.a", },
>> +     { .compatible = "xlnx,cf-ad9643-core-1.00.a", },
>> +     { /* end of list */ },
>> +};
>> +MODULE_DEVICE_TABLE(of, aim_of_match);
>> +
>> +static struct platform_driver aim_of_driver = {
>> +     .driver = {
>> +             .name = KBUILD_MODNAME,
>> +             .owner = THIS_MODULE,
>> +             .of_match_table = aim_of_match,
>> +     },
>> +     .probe          = aim_of_probe,
>> +     .remove         = __devexit_p(aim_of_remove),
>> +};
>> +
>> +module_platform_driver(aim_of_driver);
>> +
>> +MODULE_AUTHOR("Michael Hennerich<hennerich@blackfin.uclinux.org>");
>> +MODULE_DESCRIPTION("Analog Devices ADI-AIM");
>> +MODULE_LICENSE("GPL v2");
>> diff --git a/drivers/staging/iio/adc/cf_ad9467_ring.c b/drivers/staging/iio/adc/cf_ad9467_ring.c
>> new file mode 100644
>> index 0000000..c6b2230
>> --- /dev/null
>> +++ b/drivers/staging/iio/adc/cf_ad9467_ring.c
>> @@ -0,0 +1,214 @@
>> +/*
>> + * ADI-AIM ADC Interface Module
>> + *
>> + * Copyright 2012 Analog Devices Inc.
>> + *
>> + * Licensed under the GPL-2 or later.
>> + */
>> +
>> +#include<linux/slab.h>
>> +#include<linux/kernel.h>
>> +#include<linux/poll.h>
>> +#include<linux/io.h>
>> +#include<linux/dma-mapping.h>
>> +#include<linux/dmaengine.h>
>> +
>> +#include "../iio.h"
>> +#include "../sysfs.h"
>> +#include "../buffer.h"
>> +#include "../ring_hw.h"
>> +#include "cf_ad9467.h"
>> +
>> +static int aim_read_first_n_hw_rb(struct iio_buffer *r,
>> +                                   size_t count, char __user *buf)
>> +{
>> +     struct iio_hw_buffer *hw_ring = iio_to_hw_buf(r);
>> +     struct iio_dev *indio_dev = hw_ring->private;
>> +     struct aim_state *st = iio_priv(indio_dev);
>> +     struct dma_async_tx_descriptor *desc;
>> +     dma_cookie_t cookie;
>> +     unsigned rcount;
>> +     int ret;
>> +
>> +     mutex_lock(&st->lock);
>> +     if (count == 0) {
>> +             ret = -EINVAL;
>> +             goto error_ret;
>> +     }
>> +
>> +     if (count % 8)
>> +             rcount = (count + 8)&  0xFFFFFFF8;
>> +     else
>> +             rcount = count;
>> +
>> +     st->buf_virt = dma_alloc_coherent(indio_dev->dev.parent,
>> +                                       PAGE_ALIGN(rcount),&st->buf_phys,
>> +                                       GFP_KERNEL);
>> +     if (st->buf_virt == NULL) {
>> +             ret = -ENOMEM;
>> +             goto error_ret;
>> +     }
>> +
>> +     desc = dmaengine_prep_slave_single(st->rx_chan, st->buf_phys, rcount,
>> +                                        DMA_FROM_DEVICE, DMA_PREP_INTERRUPT);
>> +     if (!desc) {
>> +             dev_err(indio_dev->dev.parent,
>> +                     "Failed to allocate a dma descriptor\n");
>> +             ret = -ENOMEM;
>> +             goto error_free;
>> +     }
>> +
>> +     desc->callback = (dma_async_tx_callback) complete;
>> +     desc->callback_param =&st->dma_complete;
>> +
>> +     cookie = dmaengine_submit(desc);
>> +     if (cookie<  0) {
>> +             dev_err(indio_dev->dev.parent,
>> +                     "Failed to submit a dma transfer\n");
>> +             ret = cookie;
>> +             goto error_free;
>> +     }
>> +
>> +     dma_async_issue_pending(st->rx_chan);
>> +
>> +     aim_write(st, AD9467_PCORE_DMA_CTRL, 0);
>> +     aim_read(st, AD9467_PCORE_DMA_STAT);
>> +     aim_write(st, AD9467_PCORE_DMA_CTRL,
>> +               AD9647_DMA_CAP_EN | AD9647_DMA_CNT((rcount / 8) - 1));
>> +
>> +     ret = wait_for_completion_interruptible_timeout(&st->dma_complete,
>> +                                                     2 * HZ);
>> +     if (ret == 0) {
>> +             ret = -ETIMEDOUT;
>> +             goto error_free;
>> +     } else if (ret<  0) {
>> +             goto error_free;
>> +     }
>> +
>> +     if (copy_to_user(buf, st->buf_virt, count))
>> +             ret = -EFAULT;
>> +
>> +error_free:
>> +     dma_free_coherent(indio_dev->dev.parent, PAGE_ALIGN(rcount),
>> +                       st->buf_virt, st->buf_phys);
>> +     r->stufftoread = 0;
>> +error_ret:
>> +     mutex_unlock(&st->lock);
>> +
>> +     return ret ? ret : count;
>> +}
>> +
>> +static int aim_ring_get_length(struct iio_buffer *r)
>> +{
>> +     struct iio_hw_buffer *hw_ring = iio_to_hw_buf(r);
>> +     struct iio_dev *indio_dev = hw_ring->private;
>> +     struct aim_state *st = iio_priv(indio_dev);
>> +
>> +     return st->ring_lenght;
>> +}
>> +
>> +static int aim_ring_set_length(struct iio_buffer *r, int length)
>> +{
>> +     struct iio_hw_buffer *hw_ring = iio_to_hw_buf(r);
>> +     struct iio_dev *indio_dev = hw_ring->private;
>> +     struct aim_state *st = iio_priv(indio_dev);
> could squish last two lines together without it getting harder
> to follow.

struct aim_state *st = iio_priv(hw_ring->private); ?


>> +
>> +     st->ring_lenght = length;
> lenght?
>> +
>> +     return 0;
>> +}
>> +
>> +static int aim_ring_get_bytes_per_datum(struct iio_buffer *r)
>> +{
>> +     struct iio_hw_buffer *hw_ring = iio_to_hw_buf(r);
>> +     struct iio_dev *indio_dev = hw_ring->private;
>> +     struct aim_state *st = iio_priv(indio_dev);
>> +
>> +     return st->bytes_per_datum;
>> +}
>> +
>> +static IIO_BUFFER_ENABLE_ATTR;
>> +static IIO_BUFFER_LENGTH_ATTR;
>> +
>> +static struct attribute *aim_ring_attributes[] = {
>> +&dev_attr_length.attr,
>> +&dev_attr_enable.attr,
>> +     NULL,
>> +};
>> +
>> +static struct attribute_group aim_ring_attr = {
>> +     .attrs = aim_ring_attributes,
>> +     .name = "buffer",
>> +};
>> +
>> +static struct iio_buffer *aim_rb_allocate(struct iio_dev *indio_dev)
>> +{
>> +     struct iio_buffer *buf;
>> +     struct iio_hw_buffer *ring;
>> +
>> +     ring = kzalloc(sizeof *ring, GFP_KERNEL);
>> +     if (!ring)
>> +             return NULL;
>> +
>> +     ring->private = indio_dev;
>> +     buf =&ring->buf;
>> +     buf->stufftoread = 0;
> technically no need to zero as you just kzalloc'd this
>> +     buf->attrs =&aim_ring_attr;
>> +     iio_buffer_init(buf);
>> +
>> +     return buf;
>> +}
>> +
>> +static inline void aim_rb_free(struct iio_buffer *r)
>> +{
>> +     kfree(iio_to_hw_buf(r));
>> +}
>> +
>> +static const struct iio_buffer_access_funcs aim_ring_access_funcs = {
>> +     .read_first_n =&aim_read_first_n_hw_rb,
>> +     .get_length =&aim_ring_get_length,
>> +     .set_length =&aim_ring_set_length,
>> +     .get_bytes_per_datum =&aim_ring_get_bytes_per_datum,
>> +};
>> +
>> +int aim_configure_ring(struct iio_dev *indio_dev)
>> +{
>> +     indio_dev->buffer = aim_rb_allocate(indio_dev);
>> +     if (indio_dev->buffer == NULL)
>> +             return -ENOMEM;
>> +
>> +     indio_dev->modes |= INDIO_BUFFER_HARDWARE;
>> +     indio_dev->buffer->access =&aim_ring_access_funcs;
>> +
>> +     return 0;
>> +}
>> +
>> +void aim_unconfigure_ring(struct iio_dev *indio_dev)
>> +{
>> +     aim_rb_free(indio_dev->buffer);
>> +}
>> +
> These next three seem a little 'odd'.  Why have them at all?
> I don't think you are obliged to provide any of the callbacks...
>> +static inline int __aim_hw_ring_state_set(struct iio_dev *indio_dev, bool state)
>> +{
>> +     return 0;
>> +}
>> +
>> +static int aim_hw_ring_preenable(struct iio_dev *indio_dev)
>> +{
>> +     return __aim_hw_ring_state_set(indio_dev, 1);
>> +}
>> +
>> +static int aim_hw_ring_postdisable(struct iio_dev *indio_dev)
>> +{
>> +     return __aim_hw_ring_state_set(indio_dev, 0);
>> +}
>> +
>> +static const struct iio_buffer_setup_ops aim_ring_setup_ops = {
>> +     .preenable =&aim_hw_ring_preenable,
>> +     .postdisable =&aim_hw_ring_postdisable,
>> +};
>> +
>> +void aim_register_ring_funcs(struct iio_dev *indio_dev)
>> +{
>> +     indio_dev->setup_ops =&aim_ring_setup_ops;
>> +}
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>


-- 
Greetings,
Michael

--
Analog Devices GmbH      Wilhelm-Wagenfeld-Str. 6      80807 Muenchen
Sitz der Gesellschaft: Muenchen; Registergericht: Muenchen HRB 40368;
Geschaeftsfuehrer:Dr.Carsten Suckrow, Thomas Wessel, William A. Martin,
Margaret Seif

Re: [PATCH 4/4] iio: adc: New driver for AD9467 and AD9643 High-Speed LVDS ADCs

[Jonathan Cameron]

On 2/20/2012 11:41 AM, Michael Hennerich wrote:
> On 02/19/2012 05:09 PM, Jonathan Cameron wrote:
>> On 02/17/2012 12:46 PM, michael.hennerich@analog.com wrote:
>>> From: Michael Hennerich<michael.hennerich@analog.com>
>>>
>>> Suitable FPGA interface HDL is available here:
>>> http://wiki.analog.com/resources/fpga/xilinx/fmc/ad9467
>> I don't know my way around the dma stuff so would appreciate review
>> for someone who does (if any such person is reading!)
>>
>>
>> Mostly for this one I would like a little more explanation of some
>> of the design descisions.
>> 1) Registering as platform device rather than spi with extra bits.
>>      May well be cleaner this way around. If so please just briefly
>>      explain why.
>
> Hi Jonathan,
>
> thanks for the review.
>
> Well - this driver targets an 'on-chip' memory mapped peripheral,
> sitting on an AMBA AXI Bus Interface. Main purpose is to interface
> with high speed ADCs using an LVDS physical external interface.
> SPI is only used to query the connected converter and do some
> one time configuration setup.
>
> This approach is not different from other drivers in other subsystems,
> where the target device lives on an e.g. PCI bus and the driver
> also registers an I2C or SPI client.
> See drivers/drm, V4l drivers/media/video or also drivers/sound.
Fair enough.
>
>
>> 2) Effectively embedded spi implementation rather than a separate
>> spi driver for the aim to spi stuff.
>>      I can see it would be fairly clunky given this only does very
>>      very special forms of spi, but it would cleanup this end by
>>      removing the special casing.
> Not sure it really makes sense here. The PCORE/HDL internal
> SPI interface is only meant to interface with converters
> covered by  AN-877. It only supports one flavor of SPI register
> access. For example part of the transfer is fixed in hardware.
> Only 3-WIRE mode, without interrupt capabilities.
> A separate SPI master would only make sense if there were other
> potential users for it.
I'm not sure I agree on this point.  I think it might lead to simpler code
even with these restrictions.  If you have an spi controller driver
then there is no need to have the driver code care what spi bus
is talking to it.  Grant, what do you think?
>
>> 3) Separation (or lack there of) for the adc interface and the hardware
>> buffer implemented in the fpga.
>>      Probably tricky to pull off but a few lines saying where the 
>> problems
>>      lie would be informative.
> You probably noticed; to-date there is no real buffer involved.
> Given the sampling speed of 250MSPS every software manged buffer
> mechanism might overflow. Given the fact that the writer reader
> problem can't be solved in real-time. So right now, we do
> blocking reads. The AXI S2MM DMA reads the requested number of
> samples directly from the peripheral. Given the high sample rate,
> there wouldn't be any difference whether the data would come
> from an FPGA/peripheral internal buffer, or directly from the ADC.
Yes, that is true enough.  There might be ways of doing the buffering, but
it's certainly non trivial and probably easiest done with some hardware
in between..
>
>> A nice interest example and well written driver otherwise.  Couple
>> of things like documentation requests and small tidy ups inline.
>>
>>> Signed-off-by: Michael Hennerich<michael.hennerich@analog.com>
>>> ---
>>>   drivers/staging/iio/adc/Kconfig          |   13 +
>>>   drivers/staging/iio/adc/Makefile         |    3 +
>>>   drivers/staging/iio/adc/cf_ad9467.h      |  161 ++++++++
>>>   drivers/staging/iio/adc/cf_ad9467_core.c |  585 
>>> ++++++++++++++++++++++++++++++
>>>   drivers/staging/iio/adc/cf_ad9467_ring.c |  214 +++++++++++
>>>   5 files changed, 976 insertions(+), 0 deletions(-)
>>>   create mode 100644 drivers/staging/iio/adc/cf_ad9467.h
>>>   create mode 100644 drivers/staging/iio/adc/cf_ad9467_core.c
>>>   create mode 100644 drivers/staging/iio/adc/cf_ad9467_ring.c
>>>
>>> diff --git a/drivers/staging/iio/adc/Kconfig 
>>> b/drivers/staging/iio/adc/Kconfig
>>> index d9decea..266e807 100644
>>> --- a/drivers/staging/iio/adc/Kconfig
>>> +++ b/drivers/staging/iio/adc/Kconfig
>>> @@ -169,6 +169,19 @@ config AD7280
>>>          To compile this driver as a module, choose M here: the
>>>          module will be called ad7280a
>>>
>>> +config CF_AD9467
>>> +     tristate "Analog Devices AD9467 AD9643 High-Speed ADC driver"
>>> +     select IIO_BUFFER
>>> +     help
>>> +       Say yes here to build support for Analog Devices AD9467 and 
>>> AD9643,
>>> +       High-Speed LVDS analog to digital converters (ADC).
>>> +       FPGA interface HDL is available here:
>>> +       http://wiki.analog.com/resources/fpga/xilinx/fmc/ad9467
>>> +       If unsure, say N (but it's safe to say "Y").
>>> +
>>> +       To compile this driver as a module, choose M here: the
>>> +       module will be called cf_ad9467.
>>> +
>>>   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 ceee7f3..2f37d85 100644
>>> --- a/drivers/staging/iio/adc/Makefile
>>> +++ b/drivers/staging/iio/adc/Makefile
>>> @@ -29,6 +29,9 @@ ad7298-y := ad7298_core.o
>>>   ad7298-$(CONFIG_IIO_BUFFER) += ad7298_ring.o
>>>   obj-$(CONFIG_AD7298) += ad7298.o
>>>
>>> +cf_ad9467-y := cf_ad9467_core.o cf_ad9467_ring.o
>>> +obj-$(CONFIG_CF_AD9467) += cf_ad9467.o
>>> +
>>>   obj-$(CONFIG_AD7291) += ad7291.o
>>>   obj-$(CONFIG_AD7780) += ad7780.o
>>>   obj-$(CONFIG_AD7793) += ad7793.o
>>> diff --git a/drivers/staging/iio/adc/cf_ad9467.h 
>>> b/drivers/staging/iio/adc/cf_ad9467.h
>>> new file mode 100644
>>> index 0000000..1423e4c
>>> --- /dev/null
>>> +++ b/drivers/staging/iio/adc/cf_ad9467.h
>>> @@ -0,0 +1,161 @@
>>> +/*
>>> + * ADI-AIM ADI ADC Interface Module
>>> + *
>>> + * Copyright 2012 Analog Devices Inc.
>>> + *
>>> + * Licensed under the GPL-2.
>>> + *
>>> + * http://wiki.analog.com/resources/fpga/xilinx/fmc/ad9467
>>> + */
>>> +
>>> +#ifndef ADI_AIM_H_
>>> +#define ADI_AIM_H_
>>> +
>>> +/* PCORE CoreFPGA register map */
>>> +
>>> +#define AD9467_PCORE_VERSION         0x00
>>> +#define AD9467_PCORE_SPI_CTRL                0x04
>>> +#define AD9467_PCORE_SPI_RDSTAT              0x08
>>> +#define AD9467_PCORE_DMA_CTRL                0x0C
>>> +#define AD9467_PCORE_DMA_STAT                0x10
>>> +#define AD9467_PCORE_ADC_STAT                0x14
>>> +#define AD9467_PCORE_PN_ERR_CTRL     0x24
>>> +
>>> +/* AD9467_PCORE_SPI_CTRL */
>>> +#define AD9647_SPI_START             (1<<  25)
>>> +#define AD9647_SPI_SEL(x)            (((x)&  0x1)<<  24)
>>> +#define AD9647_SPI_READ                      (1<<  23)
>>> +#define AD9647_SPI_WRITE             (0<<  23)
>>> +#define AD9647_SPI_ADDR(x)           (((x)&  0x1FFF)<<  8)
>>> +#define AD9647_SPI_DATA(x)           (((x)&  0xFF)<<  0)
>>> +
>>> +/* AD9467_PCORE_SPI_RDSTAT */
>>> +#define AD9647_SPI_IDLE                      (1<<  8)
>>> +#define AD9647_SPI_READVAL(x)                ((x)&  0xFF)
>>> +
>>> +/* AD9467_PCORE_DMA_CTRL */
>>> +#define AD9647_DMA_CAP_EN            (1<<  16)
>>> +#define AD9647_DMA_CNT(x)            (((x)&  0xFFFF)<<  0)
>>> +
>>> +/* AD9467_PCORE_DMA_STAT */
>>> +#define AD9647_DMA_STAT_BUSY         (1<<  0)
>>> +#define AD9647_DMA_STAT_OVF          (1<<  1)
>>> +#define AD9647_DMA_STAT_UNF          (1<<  2)
>>> +
>>> +/* AD9467_PCORE_ADC_STAT */
>>> +#define AD9467_PCORE_ADC_STAT_OVR    (1<<  0)
>>> +#define AD9467_PCORE_ADC_STAT_PN_OOS (1<<  1) /* W1C */
>>> +#define AD9467_PCORE_ADC_STAT_PN_ERR (1<<  2) /* W1C */
>>> +
>>> +/* AD9467_PCORE_PN_ERR_CTRL */
>>> +#define AD9467_PN23_EN                       (1<<  0)
>>> +#define AD9467_PN9_EN                        (0<<  0)
>>> +
>>> +/*
>>> + * ADI High-Speed ADC common spi interface registers
>>> + * See Application-Note AN-877
>>> + */
>>> +
>>> +#define ADC_REG_CHIP_PORT_CONF               0x00
>>> +#define ADC_REG_CHIP_ID                      0x01
>>> +#define ADC_REG_CHIP_GRADE           0x02
>>> +#define ADC_REG_TRANSFER             0xFF
>>> +#define ADC_REG_MODES                        0x08
>>> +#define ADC_REG_TEST_IO                      0x0D
>>> +#define ADC_REG_ADC_INPUT            0x0F
>>> +#define ADC_REG_OFFSET                       0x10
>>> +#define ADC_REG_OUTPUT_MODE          0x14
>>> +#define ADC_REG_OUTPUT_ADJUST                0x15
>>> +#define ADC_REG_OUTPUT_PHASE         0x16
>>> +#define ADC_REG_OUTPUT_DELAY         0x17
>>> +#define ADC_REG_VREF                 0x18
>>> +#define ADC_REG_ANALOG_INPUT         0x2C
>>> +
>>> +/* ADC_REG_TRANSFER */
>>> +#define TRANSFER_SYNC                        0x1
>>> +
>>> +/* ADC_REG_TEST_IO */
>>> +#define TESTMODE_OFF                 0x0
>>> +#define TESTMODE_MIDSCALE_SHORT              0x1
>>> +#define TESTMODE_POS_FULLSCALE               0x2
>>> +#define TESTMODE_NEG_FULLSCALE               0x3
>>> +#define TESTMODE_ALT_CHECKERBOARD    0x4
>>> +#define TESTMODE_PN23_SEQ            0x5
>>> +#define TESTMODE_PN9_SEQ             0x6
>>> +#define TESTMODE_ONE_ZERO_TOGGLE     0x7
>>> +
>>> +/* ADC_REG_OUTPUT_MODE */
>>> +#define OUTPUT_MODE_OFFSET_BINARY    0x0
>>> +#define OUTPUT_MODE_TWOS_COMPLEMENT  0x1
>>> +#define OUTPUT_MODE_GRAY_CODE                0x2
>>> +
>>> +/*
>>> + * Analog Devices AD9467 16-Bit, 200/250 MSPS ADC
>>> + */
>>> +
>>> +#define AD9467_DEF_OUTPUT_MODE               0x08
>>> +#define AD9467_REG_VREF_MASK         0x0F
>>> +#define CHIPID_AD9467                        0x50
>>> +
>>> +/*
>>> + * Analog Devices AD9643 Dual 14-Bit, 170/210/250 MSPS ADC
>>> + */
>>> +
>>> +#define CHIPID_AD9643                        0x82
>>> +#define AD9643_REG_VREF_MASK         0x1F
>>> +#define AD9643_DEF_OUTPUT_MODE               0x04
>>> +
>>> +enum {
>>> +     ID_AD9467,
>>> +     ID_AD9643,
>>> +};
>>> +
>>> +struct aim_chip_info {
>>> +     char                            name[8];
>>> +     unsigned                        num_channels;
>>> +     unsigned long                   available_scan_masks[2];
>>> +     const int                       (*scale_table)[2];
>>> +     int                             num_scales;
>>> +     struct iio_chan_spec            channel[2];
>>> +};
>>> +
>>> +struct aim_state {
>>> +     struct spi_device               *spi;
>>> +     struct mutex                    lock;
>>> +     struct completion               dma_complete;
>>> +     struct dma_chan                 *rx_chan;
>>> +     const struct aim_chip_info      *chip_info;
>>> +     void __iomem                    *regs;
>>> +     void                            *buf_virt;
>>> +     dma_addr_t                      buf_phys;
>>> +     unsigned                        spi_ssel;
>>> +     unsigned                        ring_lenght;
>>> +     unsigned                        bytes_per_datum;
>>> +     unsigned                        id;
>>> +     /*
>>> +      * DMA (thus cache coherency maintenance) requires the
>>> +      * transfer buffers to live in their own cache lines.
>>> +      */
>> no blank line here and one above the comment perhaps?
>>> +
>>> +     unsigned char                   data[3] ____cacheline_aligned;
>>> +};
>>> +
>>> +/*
>>> + * IO accessors
>>> + */
>>> +
>>> +static inline void aim_write(struct aim_state *st, unsigned reg, 
>>> unsigned val)
>>> +{
>>> +     iowrite32(val, st->regs + reg);
>>> +}
>>> +
>>> +static inline unsigned int aim_read(struct aim_state *st, unsigned 
>>> reg)
>>> +{
>>> +     return ioread32(st->regs + reg);
>>> +}
>>> +
>>> +void aim_register_ring_funcs(struct iio_dev *indio_dev);
>>> +int aim_configure_ring(struct iio_dev *indio_dev);
>>> +void aim_unconfigure_ring(struct iio_dev *indio_dev);
>>> +
>>> +#endif /* ADI_AIM_H_ */
>>> diff --git a/drivers/staging/iio/adc/cf_ad9467_core.c 
>>> b/drivers/staging/iio/adc/cf_ad9467_core.c
>>> new file mode 100644
>>> index 0000000..811a659
>>> --- /dev/null
>>> +++ b/drivers/staging/iio/adc/cf_ad9467_core.c
>>> @@ -0,0 +1,585 @@
>>> +/*
>>> + * ADI-AIM ADI ADC Interface Module
>>> + *
>>> + * Copyright 2012 Analog Devices Inc.
>>> + *
>>> + * Licensed under the GPL-2.
>>> + *
>>> + * http://wiki.analog.com/resources/fpga/xilinx/fmc/ad9467
>>> + */
>>> +
>>> +#include<linux/module.h>
>>> +#include<linux/errno.h>
>>> +#include<linux/slab.h>
>>> +#include<linux/init.h>
>>> +#include<linux/io.h>
>>> +#include<linux/wait.h>
>>> +#include<linux/spi/spi.h>
>>> +#include<linux/dma-mapping.h>
>>> +#include<linux/dmaengine.h>
>>> +#include<linux/delay.h>
>>> +#include<linux/debugfs.h>
>>> +#include<linux/uaccess.h>
>>> +
>>> +#include<linux/of.h>
>>> +#include<linux/of_device.h>
>>> +#include<linux/of_platform.h>
>>> +#include<linux/of_address.h>
>>> +#include<linux/of_spi.h>
>>> +
>>> +#include "../iio.h"
>>> +#include "../sysfs.h"
>>> +#include "../buffer.h"
>>> +
>>> +#include "cf_ad9467.h"
>>> +
>>> +static int aim_spi_read(struct aim_state *st, unsigned reg)
>>> +{
>>> +     unsigned long timeout = jiffies + HZ / 4;
>>> +     int ret;
>>> +
>>> +     if (st->spi) {
>>> +             unsigned char *buf = st->data;
>>> +             buf[0] = 0x80 | (reg>>  8);
>>> +             buf[1] = reg&  0xFF;
>>> +
>>> +             ret = spi_write_then_read(st->spi,&buf[0], 2,&buf[2], 1);
>>> +             if (ret<  0)
>>> +                     return ret;
>>> +
>>> +             return buf[2];
>>> +     }
>>> +
>>> +     aim_write(st, AD9467_PCORE_SPI_CTRL, 0);
>>> +     aim_write(st, AD9467_PCORE_SPI_CTRL,
>>> +             AD9647_SPI_START |
>>> +             AD9647_SPI_READ |
>>> +             AD9647_SPI_SEL(st->spi_ssel) |
>>> +             AD9647_SPI_ADDR(reg));
>>> +
>>> +     while (!(aim_read(st, AD9467_PCORE_SPI_RDSTAT)&  
>>> AD9647_SPI_IDLE)) {
>>> +             cpu_relax();
>>> +             if (time_after(jiffies, timeout))
>>> +                     return -EIO;
>>> +     }
>>> +
>>> +     return AD9647_SPI_READVAL(aim_read(st, AD9467_PCORE_SPI_RDSTAT));
>>> +}
>>> +
>>> +static int aim_spi_write(struct aim_state *st, unsigned reg, 
>>> unsigned val)
>>> +{
>>> +     unsigned long timeout = jiffies + HZ / 4;
>>> +
>>> +     int ret;
>>> +
>>> +     if (st->spi) {
>>> +             unsigned char *buf = st->data;
>>> +             buf[0] = reg>>  8;
>>> +             buf[1] = reg&  0xFF;
>>> +             buf[2] = val;
>>> +             ret = spi_write(st->spi, buf, 3);
>>> +             if (ret<  0)
>>> +                     return ret;
>>> +
>>> +             return 0;
>>> +     }
>>> +
>> This is 'unusual'.  Perhaps it really should be a limited functionality
>> spi driver?
>>> +     aim_write(st, AD9467_PCORE_SPI_CTRL, 0);
>>> +     aim_write(st, AD9467_PCORE_SPI_CTRL,
>>> +             AD9647_SPI_START |
>>> +             AD9647_SPI_WRITE |
>>> +             AD9647_SPI_SEL(st->spi_ssel) |
>>> +             AD9647_SPI_ADDR(reg) |
>>> +             AD9647_SPI_DATA(val));
>>> +
>>> +
>>> +     while (!(aim_read(st, AD9467_PCORE_SPI_RDSTAT)&  
>>> AD9647_SPI_IDLE)) {
>>> +             cpu_relax();
>>> +             if (time_after(jiffies, timeout))
>>> +                     return -EIO;
>>> +     }
>>> +
>>> +     return 0;
>>> +}
>>> +
>>> +static int aim_debugfs_open(struct inode *inode, struct file *file)
>>> +{
>>> +     if (inode->i_private)
>>> +             file->private_data = inode->i_private;
>>> +
>>> +     return 0;
>>> +}
>>> +
>>> +static ssize_t aim_debugfs_pncheck_read(struct file *file, char 
>>> __user *userbuf,
>>> +                           size_t count, loff_t *ppos)
>>> +{
>>> +     struct iio_dev *indio_dev = file->private_data;
>>> +     struct aim_state *st = iio_priv(indio_dev);
>>> +     char buf[80], *p = buf;
>>> +     unsigned stat;
>>> +
>>> +     stat = aim_read(st, AD9467_PCORE_VERSION);
>>> +     p += sprintf(p, "%s %s\n", stat&  AD9467_PCORE_ADC_STAT_PN_OOS ?
>>> +             "Out of Sync" : "", stat&  AD9467_PCORE_ADC_STAT_PN_ERR ?
>>> +             "PN Error" : "");
>>> +
>>> +     return simple_read_from_buffer(userbuf, count, ppos, buf, p - 
>>> buf);
>>> +}
>>> +
>>> +static ssize_t aim_debugfs_pncheck_write(struct file *file,
>>> +                  const char __user *userbuf, size_t count, loff_t 
>>> *ppos)
>>> +{
>>> +     struct iio_dev *indio_dev = file->private_data;
>>> +     struct aim_state *st = iio_priv(indio_dev);
>>> +     unsigned mode;
>>> +     char buf[80], *p = buf;
>>> +
>>> +     count = min_t(size_t, count, (sizeof(buf)-1));
>>> +     if (copy_from_user(p, userbuf, count))
>>> +             return -EFAULT;
>>> +
>>> +     if (sysfs_streq(p, "PN9"))
>>> +             mode = TESTMODE_PN9_SEQ;
>>> +     else if (sysfs_streq(p, "PN23"))
>>> +             mode = TESTMODE_PN23_SEQ;
>>> +     else
>>> +             mode = TESTMODE_OFF;
>>> +
>>> +     aim_spi_write(st, ADC_REG_TEST_IO, mode);
>>> +     aim_spi_write(st, ADC_REG_TRANSFER, TRANSFER_SYNC);
>>> +
>>> +     aim_write(st, AD9467_PCORE_PN_ERR_CTRL, mode == 
>>> TESTMODE_PN23_SEQ ?
>>> +               AD9467_PN23_EN : AD9467_PN9_EN);
>>> +
>>> +     mdelay(1);
>> document why please.
>>> +
>>> +     aim_write(st, AD9467_PCORE_ADC_STAT,
>>> +               AD9467_PCORE_ADC_STAT_PN_OOS |
>>> +               AD9467_PCORE_ADC_STAT_PN_OOS |
>>> +               AD9467_PCORE_ADC_STAT_OVR);
>>> +
>>> +     return count;
>>> +}
>>> +
>>> +static const struct file_operations aim_debugfs_pncheck_fops = {
>>> +     .open = aim_debugfs_open,
>>> +     .read = aim_debugfs_pncheck_read,
>>> +     .write = aim_debugfs_pncheck_write,
>>> +};
>>> +
>>> +static int aim_reg_access(struct iio_dev *indio_dev,
>>> +                           unsigned reg, unsigned writeval,
>>> +                           unsigned *readval)
>>> +{
>>> +     struct aim_state *st = iio_priv(indio_dev);
>>> +     int ret;
>>> +
>>> +     mutex_lock(&indio_dev->mlock);
>>> +     if (readval == NULL) {
>>> +             ret = aim_spi_write(st, reg, writeval);
>>> +             aim_spi_write(st, ADC_REG_TRANSFER, TRANSFER_SYNC);
>>> +     } else {
>>> +             ret = aim_spi_read(st, reg);
>>> +             if (ret<  0)
>>> +                     return ret;
>>> +             *readval = ret;
>>> +             ret = 0;
>>> +     }
>> Seems like an excess blank line....
>>> +
>>> +     mutex_unlock(&indio_dev->mlock);
>>> +
>>> +     return ret;
>>> +}
>>> +
>>> +static int aim_read_raw(struct iio_dev *indio_dev,
>>> +                        struct iio_chan_spec const *chan,
>>> +                        int *val,
>>> +                        int *val2,
>>> +                        long m)
>>> +{
>>> +     struct aim_state *st = iio_priv(indio_dev);
>>> +     int i;
>>> +     unsigned vref_val;
>>> +
>>> +     switch (m) {
>>> +     case IIO_CHAN_INFO_SCALE:
>>> +             vref_val = aim_spi_read(st, ADC_REG_VREF)&
>>> +                     (st->id == CHIPID_AD9643 ? AD9643_REG_VREF_MASK :
>>> +                     AD9467_REG_VREF_MASK);
>>> +
>>> +             for (i = 0; i<  st->chip_info->num_scales; i++)
>>> +                     if (vref_val == st->chip_info->scale_table[i][1])
>>> +                             break;
>>> +
>>> +             *val =  0;
>>> +             *val2 = st->chip_info->scale_table[i][0];
>>> +
>>> +             return IIO_VAL_INT_PLUS_MICRO;
>>> +     }
>>> +     return -EINVAL;
>>> +}
>>> +
>>> +
>>> +static int aim_write_raw(struct iio_dev *indio_dev,
>>> +                            struct iio_chan_spec const *chan,
>>> +                            int val,
>>> +                            int val2,
>>> +                            long mask)
>>> +{
>>> +     struct aim_state *st = iio_priv(indio_dev);
>>> +     int i;
>>> +
>>> +     switch (mask) {
>>> +     case IIO_CHAN_INFO_SCALE:
>>> +             if (val != 0)
>>> +                     return -EINVAL;
>>> +
>>> +             for (i = 0; i<  st->chip_info->num_scales; i++)
>>> +                     if (val2 == st->chip_info->scale_table[i][0])
>>> +                             break;
>> what if it isn't any of them?
> good catch.
>
>>> +
>>> +             aim_spi_write(st, ADC_REG_VREF,
>>> +                           st->chip_info->scale_table[i][1]);
>>> +             aim_spi_write(st, ADC_REG_TRANSFER, TRANSFER_SYNC);
>>> +
>>> +             return 0;
>>> +     default:
>>> +             return -EINVAL;
>>> +     }
>>> +}
>>> +
>>> +static ssize_t aim_show_scale_available(struct device *dev,
>>> +                     struct device_attribute *attr, char *buf)
>>> +{
>>> +     struct iio_dev *indio_dev = dev_get_drvdata(dev);
>>> +     struct aim_state *st = iio_priv(indio_dev);
>>> +     int i, len = 0;
>>> +
>>> +     for (i = 0; i<  st->chip_info->num_scales; i++)
>>> +             len += sprintf(buf + len, "0.%06u ",
>>> +                            st->chip_info->scale_table[i][0]);
>>> +
>>> +     len += sprintf(buf + len, "\n");
>>> +
>>> +     return len;
>>> +}
>>> +
>>> +static IIO_DEVICE_ATTR(in_voltage_scale_available, S_IRUGO,
>>> +                    aim_show_scale_available, NULL, 0);
>>> +
>>> +static struct attribute *aim_attributes[] = {
>>> +&iio_dev_attr_in_voltage_scale_available.dev_attr.attr,
>>> +     NULL,
>>> +};
>>> +
>>> +static const struct attribute_group aim_attribute_group = {
>>> +     .attrs = aim_attributes,
>>> +};
>>> +
>>> +static const int ad9467_scale_table[][2] = {
>>> +     {30517, 0}, {32043, 6}, {33569, 7},
>>> +     {35095, 8}, {36621, 9}, {38146, 10},
>>> +};
>>> +
>>> +static const int ad9643_scale_table[][2] = {
>>> +     {31738, 0xF}, {31403, 0xE}, {31067, 0xD}, {30731, 0xC}, 
>>> {30396, 0xB},
>>> +     {30060, 0xA}, {29724, 0x9}, {29388, 0x8}, {29053, 0x7}, 
>>> {28717, 0x6},
>>> +     {28381, 0x5}, {28046, 0x4}, {27710, 0x3}, {27374, 0x2}, 
>>> {27039, 0x1},
>>> +     {26703, 0x0}, {26367, 0x1F}, {26031, 0x1E}, {25696, 0x1D},
>>> +     {25360, 0x1C}, {25024, 0x1B}, {24689, 0x1A}, {24353, 0x19},
>>> +     {24017, 0x18}, {23682, 0x17}, {23346, 0x16}, {23010, 0x15},
>>> +     {22675, 0x14}, {22339, 0x13}, {22003, 0x12}, {21667, 0x11},
>>> +     {21332, 0x10},
>>> +};
>>> +
>>> +#define AIM_CHAN(_chan, _name, _si, _bits, _sign)                    \
>>> +     { .type = IIO_VOLTAGE,                                          \
>>> +       .indexed = 1,                                                 \
>>> +       .extend_name = _name,                                         \
>>> +       .channel = _chan,                                             \
>>> +       .info_mask = IIO_CHAN_INFO_SCALE_SHARED_BIT,                  \
>>> +       .scan_index = _si,                                            \
>>> +       .scan_type =  IIO_ST(_sign, _bits, 16, 0)}
>>> +
>>> +static const struct aim_chip_info aim_chip_info_tbl[] = {
>>> +     [ID_AD9467] = {
>>> +             .name = "AD9467",
>>> +             .scale_table = ad9467_scale_table,
>>> +             .num_scales = ARRAY_SIZE(ad9467_scale_table),
>>> +             .num_channels = 1,
>>> +             .available_scan_masks[0] = BIT(0),
>>> +             .channel[0] = AIM_CHAN(0, NULL, 0, 16, 's'),
>>> +     },
>>> +     [ID_AD9643] = {
>>> +             .name = "AD9643",
>>> +             .scale_table = ad9643_scale_table,
>>> +             .num_scales = ARRAY_SIZE(ad9643_scale_table),
>>> +             .num_channels = 2,
>>> +             .available_scan_masks[0] = BIT(0) | BIT(1),
>>> +             .channel[0] = AIM_CHAN(0, "I", 0, 14, 'u'),
>>> +             .channel[1] = AIM_CHAN(1, "Q", 1, 14, 'u'),
>>> +     },
>> Why I and Q?  I clearly could be a quadrature pair of channels but
>> is there anything in inherent in the device forcing this?  Also
>> is this documented
> Yeah this shouldn't be here - thought I removed it already.
>
>>> +};
>>> +
>>> +static const struct iio_info aim_info = {
>>> +     .driver_module = THIS_MODULE,
>>> +     .read_raw =&aim_read_raw,
>>> +     .write_raw =&aim_write_raw,
>>> +     .attrs =&aim_attribute_group,
>>> +     .debugfs_reg_access = aim_reg_access,
>>> +};
>>> +
>>> +struct aim_dma_params {
>>> +     struct device_node *of_node;
>>> +     int chan_id;
>>> +};
>>> +
>>> +static bool aim_dma_filter(struct dma_chan *chan, void *param)
>>> +{
>>> +     struct aim_dma_params *p = param;
>>> +
>>> +     return chan->device->dev->of_node == p->of_node&&
>>> +             chan->chan_id == p->chan_id;
>>> +}
>>> +
>>> +/**
>>> + * aim_of_probe - probe method for the AIM device.
>>> + * @of_dev:  pointer to OF device structure
>>> + * @match:   pointer to the structure used for matching a device
>>> + *
>>> + * This function probes the AIM device in the device tree.
>>> + * It initializes the driver data structure and the hardware.
>>> + * It returns 0, if the driver is bound to the AIM device, or a 
>>> negative
>>> + * value if there is an error.
>>> + */
>>> +static int __devinit aim_of_probe(struct platform_device *op)
>>> +{
>>> +     struct iio_dev *indio_dev;
>>> +     struct device *dev =&op->dev;
>>> +     struct aim_state *st;
>>> +     struct resource r_mem; /* IO mem resources */
>>> +     struct spi_master *spi_master;
>>> +     struct device_node *nspi;
>>> +     struct aim_dma_params dma_params;
>>> +     struct of_phandle_args dma_spec;
>>> +     dma_cap_mask_t mask;
>>> +     resource_size_t remap_size, phys_addr;
>>> +     unsigned def_mode;
>>> +     int ret;
>>> +
>>> +     dev_info(dev, "Device Tree Probing \'%s\'\n",
>>> +              op->dev.of_node->name);
>>> +
>>> +     /* Get iospace for the device */
>>> +     ret = of_address_to_resource(op->dev.of_node, 0,&r_mem);
>>> +     if (ret) {
>>> +             dev_err(dev, "invalid address\n");
>>> +             return ret;
>>> +     }
>>> +
>>> +     indio_dev = iio_allocate_device(sizeof(*st));
>>> +     if (indio_dev == NULL)
>>> +             return -ENOMEM;
>>> +
>>> +     st = iio_priv(indio_dev);
>>> +
>>> +     dev_set_drvdata(dev, indio_dev);
>>> +     mutex_init(&st->lock);
>>> +
>>> +     phys_addr = r_mem.start;
>>> +     remap_size = resource_size(&r_mem);
>>> +     if (!request_mem_region(phys_addr, remap_size, KBUILD_MODNAME)) {
>>> +             dev_err(dev, "Couldn't lock memory region at 0x%08llX\n",
>>> +                     (unsigned long long)phys_addr);
>>> +             ret = -EBUSY;
>>> +             goto failed1;
>>> +     }
>>> +
>>> +     st->regs = ioremap(phys_addr, remap_size);
>>> +     if (st->regs == NULL) {
>>> +             dev_err(dev, "Couldn't ioremap memory at 0x%08llX\n",
>>> +                     (unsigned long long)phys_addr);
>>> +             ret = -EFAULT;
>>> +             goto failed2;
>>> +     }
>>> +     /* Get dma channel for the device */
>>> +     ret = of_parse_phandle_with_args(op->dev.of_node, "dma-request",
>>> +                                      "#dma-cells", 0,&dma_spec);
>>> +     if (ret) {
>>> +             dev_err(dev, "Couldn't parse dma-request\n");
>>> +             goto failed2;
>>> +     }
>>> +
>>> +     dma_params.of_node = dma_spec.np;
>>> +     dma_params.chan_id = dma_spec.args[0];
>>> +
>>> +     dma_cap_zero(mask);
>>> +     dma_cap_set(DMA_SLAVE | DMA_PRIVATE, mask);
>>> +
>>> +     st->rx_chan = dma_request_channel(mask, 
>>> aim_dma_filter,&dma_params);
>>> +     if (!st->rx_chan) {
>>> +             dev_err(dev, "failed to find rx dma device\n");
>>> +             goto failed2;
>>> +     }
>>> +     /*
>>> +      * Get SPI configuration interface for the device
>>> +      * We are platform_driver, so we need other means to get the
>>> +      * associated control interface.
>>> +      */
>>> +     ret = of_property_read_u32(op->dev.of_node,
>>> +                                "spibus-slaveselect-connected",
>>> +&st->spi_ssel);
>>> +     if (ret) {
>>> +             dev_err(dev, "failed to get connected SPI slave 
>>> select\n");
>>> +             goto failed3;
>>> +     }
>>> +
>>> +     nspi = of_parse_phandle(op->dev.of_node, "spibus-connected", 0);
>> Could you explain decision to do registration like this.
>> (platform device and manual handling of spi register?)
>> Could do spi device with resources for platform pass into it for 
>> example.
>> I'm not arguing one way or the other, just would like to see your
>> thinking on this.
>>
> Well - either it's a platform driver, or spi bus driver, but it cannot 
> be both.
> Arguments why I think it's a platform driver see above.
>
> The only way around is to split this into 3 drivers.
> An adapter platform driver, where the SPI Bus Converter interface driver
> registers with. For that approach the PCORE internal SPI interface 
> also needs
> to be a SPI Master driver.
>
> For now I like to keep things simple, since most users are likley 
> going with the
> 3-WIRE SPI (SDIO) PCORE/HDL implementation.
>
>
>>> +     spi_master = spi_of_node_to_master(nspi);
>>> +
>>> +     if (spi_master != NULL) {
>>> +             struct spi_board_info info = {
>>> +                     .modalias = KBUILD_MODNAME,
>>> +                     .max_speed_hz = 1000000,
>>> +                     .chip_select = st->spi_ssel,
>>> +                     .mode = SPI_MODE_0 | SPI_3WIRE,
>>> +             };
>>> +             st->spi = spi_new_device(spi_master,&info);
>>> +             if (st->spi == NULL) {
>>> +                     dev_err(dev, "failed to add spi device\n");
>>> +                     goto failed3;
>>> +             }
>>> +     } else {
>>> +             dev_dbg(dev, "could not find SPI master node,"
>>> +                     "using pcore spi implementation\n");
>>> +     }
>>> +
>>> +     /* Probe device */
>>> +     st->id = aim_spi_read(st, ADC_REG_CHIP_ID);
>>> +
>>> +     switch (st->id) {
>>> +     case CHIPID_AD9467:
>>> +             st->chip_info =&aim_chip_info_tbl[ID_AD9467];
>>> +             def_mode = AD9467_DEF_OUTPUT_MODE | 
>>> OUTPUT_MODE_TWOS_COMPLEMENT;
>>> +             break;
>>> +     case CHIPID_AD9643:
>>> +             st->chip_info =&aim_chip_info_tbl[ID_AD9643];
>>> +             def_mode = AD9643_DEF_OUTPUT_MODE | 
>>> OUTPUT_MODE_OFFSET_BINARY;
>>> +             break;
>>> +     default:
>>> +             dev_err(dev, "Unrecognized CHIP_ID 0x%X\n", st->id);
>>> +             ret = -ENODEV;
>>> +             goto failed3;
>>> +     }
>>> +
>>> +     indio_dev->dev.parent = dev;
>>> +     indio_dev->name = op->dev.of_node->name;
>>> +     indio_dev->channels = st->chip_info->channel;
>>> +     indio_dev->modes = INDIO_DIRECT_MODE;
>>> +     indio_dev->num_channels = st->chip_info->num_channels;
>>> +     indio_dev->available_scan_masks = 
>>> st->chip_info->available_scan_masks;
>>> +     indio_dev->info =&aim_info;
>>> +
>>> +     init_completion(&st->dma_complete);
>>> +
>>> +     aim_spi_write(st, ADC_REG_OUTPUT_MODE, def_mode);
>>> +     aim_spi_write(st, ADC_REG_TEST_IO, TESTMODE_OFF);
>>> +     aim_spi_write(st, ADC_REG_TRANSFER, TRANSFER_SYNC);
>>> +
>>> +     aim_configure_ring(indio_dev);
>>> +     aim_register_ring_funcs(indio_dev);
>>> +     ret = iio_buffer_register(indio_dev,
>>> +                               st->chip_info->channel,
>>> +                               ARRAY_SIZE(st->chip_info->channel));
>>> +     if (ret)
>>> +             goto failed4;
>>> +
>>> +     ret = iio_device_register(indio_dev);
>>> +     if (ret)
>>> +             goto failed4;
>>> +
>>> +     dev_info(dev, "ADI AIM (0x%X) at 0x%08llX mapped to 0x%p,"
>>> +              " DMA-%d probed ADC %s\n",
>>> +              aim_read(st, AD9467_PCORE_VERSION),
>>> +              (unsigned long long)phys_addr, st->regs,
>>> +              st->rx_chan->chan_id, st->chip_info->name);
>>> +
>> documented anywhere?
> Not yet.
> Where do you like this to happen?
> IIO per driver docs?
yes.  Don't think there is any standard place for debugfs docs.
>
>>> +     if (indio_dev->debugfs_dentry)
>>> +             debugfs_create_file("pseudorandom_err_check", 0644,
>>> +                                     indio_dev->debugfs_dentry,
>>> +                                     
>>> indio_dev,&aim_debugfs_pncheck_fops);
>>> +
>>> +     return 0;
>>> +
>>> +failed4:
>>> +     aim_unconfigure_ring(indio_dev);
>>> +failed3:
>>> +     dma_release_channel(st->rx_chan);
>>> +failed2:
>>> +     release_mem_region(phys_addr, remap_size);
>>> +failed1:
>>> +     iio_free_device(indio_dev);
>>> +     dev_set_drvdata(dev, NULL);
>>> +
>>> +     return ret;
>>> +}
>>> +
>>> +/**
>>> + * aim_of_remove - unbinds the driver from the AIM device.
>>> + * @of_dev:  pointer to OF device structure
>>> + *
>>> + * This function is called if a device is physically removed from 
>>> the system or
>>> + * if the driver module is being unloaded. It frees any resources 
>>> allocated to
>>> + * the device.
>>> + */
>>> +static int __devexit aim_of_remove(struct platform_device *op)
>>> +{
>>> +     struct device *dev =&op->dev;
>>> +     struct resource r_mem; /* IO mem resources */
>>> +     struct iio_dev *indio_dev = dev_get_drvdata(dev);
>>> +     struct aim_state *st = iio_priv(indio_dev);
>>> +
>>> +     iio_device_unregister(indio_dev);
>>> +     iio_buffer_unregister(indio_dev);
>>> +     aim_unconfigure_ring(indio_dev);
>>> +
>>> +     dma_release_channel(st->rx_chan);
>>> +
>>> +     iounmap(st->regs);
>>> +
>>> +     /* Get iospace of the device */
>>> +     if (of_address_to_resource(op->dev.of_node, 0,&r_mem))
>>> +             dev_err(dev, "invalid address\n");
>>> +     else
>>> +             release_mem_region(r_mem.start, resource_size(&r_mem));
>>> +
>>> +     iio_free_device(indio_dev);
>>> +
>>> +     dev_set_drvdata(dev, NULL);
>>> +
>>> +     return 0;
>>> +}
>>> +
>>> +/* Match table for of_platform binding */
>>> +static const struct of_device_id aim_of_match[] __devinitconst = {
>>> +     { .compatible = "xlnx,cf-ad9467-core-1.00.a", },
>>> +     { .compatible = "xlnx,cf-ad9643-core-1.00.a", },
>>> +     { /* end of list */ },
>>> +};
>>> +MODULE_DEVICE_TABLE(of, aim_of_match);
>>> +
>>> +static struct platform_driver aim_of_driver = {
>>> +     .driver = {
>>> +             .name = KBUILD_MODNAME,
>>> +             .owner = THIS_MODULE,
>>> +             .of_match_table = aim_of_match,
>>> +     },
>>> +     .probe          = aim_of_probe,
>>> +     .remove         = __devexit_p(aim_of_remove),
>>> +};
>>> +
>>> +module_platform_driver(aim_of_driver);
>>> +
>>> +MODULE_AUTHOR("Michael Hennerich<hennerich@blackfin.uclinux.org>");
>>> +MODULE_DESCRIPTION("Analog Devices ADI-AIM");
>>> +MODULE_LICENSE("GPL v2");
>>> diff --git a/drivers/staging/iio/adc/cf_ad9467_ring.c 
>>> b/drivers/staging/iio/adc/cf_ad9467_ring.c
>>> new file mode 100644
>>> index 0000000..c6b2230
>>> --- /dev/null
>>> +++ b/drivers/staging/iio/adc/cf_ad9467_ring.c
>>> @@ -0,0 +1,214 @@
>>> +/*
>>> + * ADI-AIM ADC Interface Module
>>> + *
>>> + * Copyright 2012 Analog Devices Inc.
>>> + *
>>> + * Licensed under the GPL-2 or later.
>>> + */
>>> +
>>> +#include<linux/slab.h>
>>> +#include<linux/kernel.h>
>>> +#include<linux/poll.h>
>>> +#include<linux/io.h>
>>> +#include<linux/dma-mapping.h>
>>> +#include<linux/dmaengine.h>
>>> +
>>> +#include "../iio.h"
>>> +#include "../sysfs.h"
>>> +#include "../buffer.h"
>>> +#include "../ring_hw.h"
>>> +#include "cf_ad9467.h"
>>> +
>>> +static int aim_read_first_n_hw_rb(struct iio_buffer *r,
>>> +                                   size_t count, char __user *buf)
>>> +{
>>> +     struct iio_hw_buffer *hw_ring = iio_to_hw_buf(r);
>>> +     struct iio_dev *indio_dev = hw_ring->private;
>>> +     struct aim_state *st = iio_priv(indio_dev);
>>> +     struct dma_async_tx_descriptor *desc;
>>> +     dma_cookie_t cookie;
>>> +     unsigned rcount;
>>> +     int ret;
>>> +
>>> +     mutex_lock(&st->lock);
>>> +     if (count == 0) {
>>> +             ret = -EINVAL;
>>> +             goto error_ret;
>>> +     }
>>> +
>>> +     if (count % 8)
>>> +             rcount = (count + 8)&  0xFFFFFFF8;
>>> +     else
>>> +             rcount = count;
>>> +
>>> +     st->buf_virt = dma_alloc_coherent(indio_dev->dev.parent,
>>> +                                       
>>> PAGE_ALIGN(rcount),&st->buf_phys,
>>> +                                       GFP_KERNEL);
>>> +     if (st->buf_virt == NULL) {
>>> +             ret = -ENOMEM;
>>> +             goto error_ret;
>>> +     }
>>> +
>>> +     desc = dmaengine_prep_slave_single(st->rx_chan, st->buf_phys, 
>>> rcount,
>>> +                                        DMA_FROM_DEVICE, 
>>> DMA_PREP_INTERRUPT);
>>> +     if (!desc) {
>>> +             dev_err(indio_dev->dev.parent,
>>> +                     "Failed to allocate a dma descriptor\n");
>>> +             ret = -ENOMEM;
>>> +             goto error_free;
>>> +     }
>>> +
>>> +     desc->callback = (dma_async_tx_callback) complete;
>>> +     desc->callback_param =&st->dma_complete;
>>> +
>>> +     cookie = dmaengine_submit(desc);
>>> +     if (cookie<  0) {
>>> +             dev_err(indio_dev->dev.parent,
>>> +                     "Failed to submit a dma transfer\n");
>>> +             ret = cookie;
>>> +             goto error_free;
>>> +     }
>>> +
>>> +     dma_async_issue_pending(st->rx_chan);
>>> +
>>> +     aim_write(st, AD9467_PCORE_DMA_CTRL, 0);
>>> +     aim_read(st, AD9467_PCORE_DMA_STAT);
>>> +     aim_write(st, AD9467_PCORE_DMA_CTRL,
>>> +               AD9647_DMA_CAP_EN | AD9647_DMA_CNT((rcount / 8) - 1));
>>> +
>>> +     ret = 
>>> wait_for_completion_interruptible_timeout(&st->dma_complete,
>>> +                                                     2 * HZ);
>>> +     if (ret == 0) {
>>> +             ret = -ETIMEDOUT;
>>> +             goto error_free;
>>> +     } else if (ret<  0) {
>>> +             goto error_free;
>>> +     }
>>> +
>>> +     if (copy_to_user(buf, st->buf_virt, count))
>>> +             ret = -EFAULT;
>>> +
>>> +error_free:
>>> +     dma_free_coherent(indio_dev->dev.parent, PAGE_ALIGN(rcount),
>>> +                       st->buf_virt, st->buf_phys);
>>> +     r->stufftoread = 0;
>>> +error_ret:
>>> +     mutex_unlock(&st->lock);
>>> +
>>> +     return ret ? ret : count;
>>> +}
>>> +
>>> +static int aim_ring_get_length(struct iio_buffer *r)
>>> +{
>>> +     struct iio_hw_buffer *hw_ring = iio_to_hw_buf(r);
>>> +     struct iio_dev *indio_dev = hw_ring->private;
>>> +     struct aim_state *st = iio_priv(indio_dev);
>>> +
>>> +     return st->ring_lenght;
>>> +}
>>> +
>>> +static int aim_ring_set_length(struct iio_buffer *r, int length)
>>> +{
>>> +     struct iio_hw_buffer *hw_ring = iio_to_hw_buf(r);
>>> +     struct iio_dev *indio_dev = hw_ring->private;
>>> +     struct aim_state *st = iio_priv(indio_dev);
>> could squish last two lines together without it getting harder
>> to follow.
>
> struct aim_state *st = iio_priv(hw_ring->private); ?
yup.
>
>
>>> +
>>> +     st->ring_lenght = length;
>> lenght?
>>> +
>>> +     return 0;
>>> +}
>>> +
>>> +static int aim_ring_get_bytes_per_datum(struct iio_buffer *r)
>>> +{
>>> +     struct iio_hw_buffer *hw_ring = iio_to_hw_buf(r);
>>> +     struct iio_dev *indio_dev = hw_ring->private;
>>> +     struct aim_state *st = iio_priv(indio_dev);
>>> +
>>> +     return st->bytes_per_datum;
>>> +}
>>> +
>>> +static IIO_BUFFER_ENABLE_ATTR;
>>> +static IIO_BUFFER_LENGTH_ATTR;
>>> +
>>> +static struct attribute *aim_ring_attributes[] = {
>>> +&dev_attr_length.attr,
>>> +&dev_attr_enable.attr,
>>> +     NULL,
>>> +};
>>> +
>>> +static struct attribute_group aim_ring_attr = {
>>> +     .attrs = aim_ring_attributes,
>>> +     .name = "buffer",
>>> +};
>>> +
>>> +static struct iio_buffer *aim_rb_allocate(struct iio_dev *indio_dev)
>>> +{
>>> +     struct iio_buffer *buf;
>>> +     struct iio_hw_buffer *ring;
>>> +
>>> +     ring = kzalloc(sizeof *ring, GFP_KERNEL);
>>> +     if (!ring)
>>> +             return NULL;
>>> +
>>> +     ring->private = indio_dev;
>>> +     buf =&ring->buf;
>>> +     buf->stufftoread = 0;
>> technically no need to zero as you just kzalloc'd this
>>> +     buf->attrs =&aim_ring_attr;
>>> +     iio_buffer_init(buf);
>>> +
>>> +     return buf;
>>> +}
>>> +
>>> +static inline void aim_rb_free(struct iio_buffer *r)
>>> +{
>>> +     kfree(iio_to_hw_buf(r));
>>> +}
>>> +
>>> +static const struct iio_buffer_access_funcs aim_ring_access_funcs = {
>>> +     .read_first_n =&aim_read_first_n_hw_rb,
>>> +     .get_length =&aim_ring_get_length,
>>> +     .set_length =&aim_ring_set_length,
>>> +     .get_bytes_per_datum =&aim_ring_get_bytes_per_datum,
>>> +};
>>> +
>>> +int aim_configure_ring(struct iio_dev *indio_dev)
>>> +{
>>> +     indio_dev->buffer = aim_rb_allocate(indio_dev);
>>> +     if (indio_dev->buffer == NULL)
>>> +             return -ENOMEM;
>>> +
>>> +     indio_dev->modes |= INDIO_BUFFER_HARDWARE;
>>> +     indio_dev->buffer->access =&aim_ring_access_funcs;
>>> +
>>> +     return 0;
>>> +}
>>> +
>>> +void aim_unconfigure_ring(struct iio_dev *indio_dev)
>>> +{
>>> +     aim_rb_free(indio_dev->buffer);
>>> +}
>>> +
>> These next three seem a little 'odd'.  Why have them at all?
>> I don't think you are obliged to provide any of the callbacks...
>>> +static inline int __aim_hw_ring_state_set(struct iio_dev 
>>> *indio_dev, bool state)
>>> +{
>>> +     return 0;
>>> +}
>>> +
>>> +static int aim_hw_ring_preenable(struct iio_dev *indio_dev)
>>> +{
>>> +     return __aim_hw_ring_state_set(indio_dev, 1);
>>> +}
>>> +
>>> +static int aim_hw_ring_postdisable(struct iio_dev *indio_dev)
>>> +{
>>> +     return __aim_hw_ring_state_set(indio_dev, 0);
>>> +}
>>> +
>>> +static const struct iio_buffer_setup_ops aim_ring_setup_ops = {
>>> +     .preenable =&aim_hw_ring_preenable,
>>> +     .postdisable =&aim_hw_ring_postdisable,
>>> +};
>>> +
>>> +void aim_register_ring_funcs(struct iio_dev *indio_dev)
>>> +{
>>> +     indio_dev->setup_ops =&aim_ring_setup_ops;
>>> +}
>> -- 
>> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>
>

Re: [PATCH 1/4] iio: core: constitfy available_scan_mask

[Jonathan Cameron]

On 2/17/2012 12:46 PM, michael.hennerich@analog.com wrote:
> From: Michael Hennerich<michael.hennerich@analog.com>
>
> The core must not modify available_scan_mask, because it causes problems
> with drivers where multiple instances of the driver share the same mask set.
> So make this explicit by marking available scan masks as const.
>
> The max1363 driver needs some minor adjustment to accommodate this change.
That's somewhat of a bigger hammer you've used to crack the nut.  
Nothing wrong with the
refactoring but might be worth saying you also pulled it out to a function.

Otherwise sensible change. Thanks.
>
> Signed-off-by: Michael Hennerich<michael.hennerich@analog.com>
Acked-by: Jonathan Cameron <jic23@kernel.org>
> ---
>   drivers/staging/iio/adc/max1363_core.c    |   36 +++++++++++++++++++---------
>   drivers/staging/iio/iio.h                 |    4 +-
>   drivers/staging/iio/industrialio-buffer.c |    6 ++--
>   3 files changed, 29 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/staging/iio/adc/max1363_core.c b/drivers/staging/iio/adc/max1363_core.c
> index 3a5bd20..7f39ca3 100644
> --- a/drivers/staging/iio/adc/max1363_core.c
> +++ b/drivers/staging/iio/adc/max1363_core.c
> @@ -1245,10 +1245,31 @@ static int max1363_initial_setup(struct max1363_state *st)
>   	return max1363_set_scan_mode(st);
>   }
>
> +static int __devinit max1363_alloc_scan_masks(struct iio_dev *indio_dev)
> +{
> +	struct max1363_state *st = iio_priv(indio_dev);
> +	unsigned long *masks;
> +	int i;
> +
> +	masks = kzalloc(BITS_TO_LONGS(MAX1363_MAX_CHANNELS)*sizeof(long)*
> +			  (st->chip_info->num_modes + 1), GFP_KERNEL);
> +	if (!masks)
> +		return -ENOMEM;
> +
> +	for (i = 0; i<  st->chip_info->num_modes; i++)
> +		bitmap_copy(masks + BITS_TO_LONGS(MAX1363_MAX_CHANNELS)*i,
> +			    max1363_mode_table[st->chip_info->mode_list[i]]
> +			    .modemask, MAX1363_MAX_CHANNELS);
> +
> +	indio_dev->available_scan_masks = masks;
> +
> +	return 0;
> +}
> +
>   static int __devinit max1363_probe(struct i2c_client *client,
>   				   const struct i2c_device_id *id)
>   {
> -	int ret, i;
> +	int ret;
>   	struct max1363_state *st;
>   	struct iio_dev *indio_dev;
>   	struct regulator *reg;
> @@ -1276,19 +1297,10 @@ static int __devinit max1363_probe(struct i2c_client *client,
>   	st->chip_info =&max1363_chip_info_tbl[id->driver_data];
>   	st->client = client;
>
> -	indio_dev->available_scan_masks
> -		= kzalloc(BITS_TO_LONGS(MAX1363_MAX_CHANNELS)*sizeof(long)*
> -			  (st->chip_info->num_modes + 1), GFP_KERNEL);
> -	if (!indio_dev->available_scan_masks) {
> -		ret = -ENOMEM;
> +	ret = max1363_alloc_scan_masks(indio_dev);
> +	if (ret)
>   		goto error_free_device;
> -	}
>
> -	for (i = 0; i<  st->chip_info->num_modes; i++)
> -		bitmap_copy(indio_dev->available_scan_masks +
> -			    BITS_TO_LONGS(MAX1363_MAX_CHANNELS)*i,
> -			    max1363_mode_table[st->chip_info->mode_list[i]]
> -			    .modemask, MAX1363_MAX_CHANNELS);
>   	/* Estabilish that the iio_dev is a child of the i2c device */
>   	indio_dev->dev.parent =&client->dev;
>   	indio_dev->name = id->name;
> diff --git a/drivers/staging/iio/iio.h b/drivers/staging/iio/iio.h
> index be6ced3..42a362c 100644
> --- a/drivers/staging/iio/iio.h
> +++ b/drivers/staging/iio/iio.h
> @@ -327,9 +327,9 @@ struct iio_dev {
>   	struct iio_buffer		*buffer;
>   	struct mutex			mlock;
>
> -	unsigned long			*available_scan_masks;
> +	const unsigned long		*available_scan_masks;
>   	unsigned			masklength;
> -	unsigned long			*active_scan_mask;
> +	const unsigned long		*active_scan_mask;
>   	struct iio_trigger		*trig;
>   	struct iio_poll_func		*pollfunc;
>
> diff --git a/drivers/staging/iio/industrialio-buffer.c b/drivers/staging/iio/industrialio-buffer.c
> index d7b1e9e..386ba76 100644
> --- a/drivers/staging/iio/industrialio-buffer.c
> +++ b/drivers/staging/iio/industrialio-buffer.c
> @@ -489,9 +489,9 @@ ssize_t iio_buffer_show_enable(struct device *dev,
>   EXPORT_SYMBOL(iio_buffer_show_enable);
>
>   /* note NULL used as error indicator as it doesn't make sense. */
> -static unsigned long *iio_scan_mask_match(unsigned long *av_masks,
> +static const unsigned long *iio_scan_mask_match(const unsigned long *av_masks,
>   					  unsigned int masklength,
> -					  unsigned long *mask)
> +					  const unsigned long *mask)
>   {
>   	if (bitmap_empty(mask, masklength))
>   		return NULL;
> @@ -554,7 +554,7 @@ EXPORT_SYMBOL(iio_sw_buffer_preenable);
>   int iio_scan_mask_set(struct iio_dev *indio_dev,
>   		      struct iio_buffer *buffer, int bit)
>   {
> -	unsigned long *mask;
> +	const unsigned long *mask;
>   	unsigned long *trialmask;
>
>   	trialmask = kmalloc(sizeof(*trialmask)*