[PATCH 1/2] [RFC] Add per-channel oversampling count

[PATCH 1/2] [RFC] Add per-channel oversampling count

[Marek Vasut]

This allows each channel to configure it's oversampling count.

Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
Cc: Wolfgang Denk <wd@denx.de>
Cc: Stefano Babic <sbabic@denx.de>
Cc: Fabio Estevam <festevam@gmail.com>
---
 drivers/staging/iio/iio.h               |    5 +++++
 drivers/staging/iio/industrialio-core.c |    1 +
 2 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/drivers/staging/iio/iio.h b/drivers/staging/iio/iio.h
index b3a1740..0b626ae 100644
--- a/drivers/staging/iio/iio.h
+++ b/drivers/staging/iio/iio.h
@@ -36,6 +36,7 @@ enum iio_chan_info_enum {
 	IIO_CHAN_INFO_QUADRATURE_CORRECTION_RAW,
 	IIO_CHAN_INFO_AVERAGE_RAW,
 	IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY,
+	IIO_CHAN_INFO_OVERSAMPLE_COUNT,
 };
 
 #define IIO_CHAN_INFO_SHARED_BIT(type) BIT(type*2)
@@ -81,6 +82,10 @@ enum iio_chan_info_enum {
 #define IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY_SEPARATE_BIT \
 	IIO_CHAN_INFO_SEPARATE_BIT(			       \
 		IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY)
+#define IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT		\
+	IIO_CHAN_INFO_SEPARATE_BIT(IIO_CHAN_INFO_OVERSAMPLE_COUNT)
+#define IIO_CHAN_INFO_OVERSAMPLE_COUNT_SHARED_BIT		\
+	IIO_CHAN_INFO_SHARED_BIT(IIO_CHAN_INFO_OVERSAMPLE_COUNT)
 
 enum iio_endian {
 	IIO_CPU,
diff --git a/drivers/staging/iio/industrialio-core.c b/drivers/staging/iio/industrialio-core.c
index 9c41c83..05b6fdd 100644
--- a/drivers/staging/iio/industrialio-core.c
+++ b/drivers/staging/iio/industrialio-core.c
@@ -87,6 +87,7 @@ static const char * const iio_chan_info_postfix[] = {
 	[IIO_CHAN_INFO_AVERAGE_RAW] = "mean_raw",
 	[IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY]
 	= "filter_low_pass_3db_frequency",
+	[IIO_CHAN_INFO_OVERSAMPLE_COUNT] = "oversample_count",
 };
 
 const struct iio_chan_spec
-- 
1.7.8.3

[PATCH 2/2] [RFC] Basic support for MX28 LRADC IIO interface

[Marek Vasut]

Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
Cc: Wolfgang Denk <wd@denx.de>
Cc: Stefano Babic <sbabic@denx.de>
Cc: Fabio Estevam <festevam@gmail.com>
---
 arch/arm/mach-mxs/mach-m28evk.c        |   35 ++-
 drivers/staging/iio/adc/Kconfig        |   10 +
 drivers/staging/iio/adc/Makefile       |    1 +
 drivers/staging/iio/adc/mxs-lradc.c    |  666 ++++++++++++++++++++++++++++++++
 4 files changed, 700 insertions(+), 4 deletions(-)
 create mode 100644 drivers/staging/iio/adc/mxs-lradc.c

NOTE: The quality of code is still really crap here, I'm more interested in
knowing whether I'm going in the right direction about this.

Thanks!

diff --git a/arch/arm/mach-mxs/mach-m28evk.c b/arch/arm/mach-mxs/mach-m28evk.c
index 2f27582..7a37d29 100644
--- a/arch/arm/mach-mxs/mach-m28evk.c
+++ b/arch/arm/mach-mxs/mach-m28evk.c
@@ -320,6 +320,31 @@ static struct mxs_mmc_platform_data m28evk_mmc_pdata[] __initdata = {
 	},
 };
 
+#define	RES_IRQ(id, res)	{ .name = id, .start = (res), .end = (res), .flags = IORESOURCE_IRQ }
+
+static struct resource mxs_lradc_rsrc[] = {
+	[0] = {
+		.start	= 0x80050000,
+		.end	= 0x80050000 + SZ_8K - 1,
+		.flags	= IORESOURCE_MEM,
+	},
+	RES_IRQ("LRADC CH0", MX28_INT_LRADC_CH0),
+	RES_IRQ("LRADC CH1", MX28_INT_LRADC_CH1),
+	RES_IRQ("LRADC CH2", MX28_INT_LRADC_CH2),
+	RES_IRQ("LRADC CH3", MX28_INT_LRADC_CH3),
+	RES_IRQ("LRADC CH4", MX28_INT_LRADC_CH4),
+	RES_IRQ("LRADC CH5", MX28_INT_LRADC_CH5),
+	RES_IRQ("LRADC CH6", MX28_INT_LRADC_CH6),
+	RES_IRQ("LRADC CH7", MX28_INT_LRADC_CH7),
+};
+
+static struct platform_device mxs_lradc = {
+	.name		= "mxs-lradc",
+	.id		= -1,
+	.resource	= mxs_lradc_rsrc,
+	.num_resources	= ARRAY_SIZE(mxs_lradc_rsrc),
+};
+
 static void __init m28evk_init(void)
 {
 	mxs_iomux_setup_multiple_pads(m28evk_pads, ARRAY_SIZE(m28evk_pads));
@@ -347,6 +372,8 @@ static void __init m28evk_init(void)
 	mx28_add_mxs_i2c(0);
 	i2c_register_board_info(0, m28_stk5v3_i2c_boardinfo,
 			ARRAY_SIZE(m28_stk5v3_i2c_boardinfo));
+
+	platform_device_register(&mxs_lradc);
 }
 
 static void __init m28evk_timer_init(void)
diff --git a/drivers/staging/iio/adc/Kconfig b/drivers/staging/iio/adc/Kconfig
index d9decea..7f33032 100644
--- a/drivers/staging/iio/adc/Kconfig
+++ b/drivers/staging/iio/adc/Kconfig
@@ -193,4 +193,14 @@ config MAX1363_RING_BUFFER
 	  Say yes here to include ring buffer support in the MAX1363
 	  ADC driver.
 
+config MXS_LRADC
+	tristate "Freescale i.MX23/i.MX28 LRADC"
+	depends on ARCH_MXS
+	help
+	  Say yes here to build support for i.MX23/i.MX28 LRADC convertor
+	  built into these chips.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called mxs-lradc.
+
 endmenu
diff --git a/drivers/staging/iio/adc/Makefile b/drivers/staging/iio/adc/Makefile
index ceee7f3..2d764ec 100644
--- a/drivers/staging/iio/adc/Makefile
+++ b/drivers/staging/iio/adc/Makefile
@@ -37,3 +37,4 @@ obj-$(CONFIG_AD7192) += ad7192.o
 obj-$(CONFIG_ADT7310) += adt7310.o
 obj-$(CONFIG_ADT7410) += adt7410.o
 obj-$(CONFIG_AD7280) += ad7280a.o
+obj-$(CONFIG_MXS_LRADC) += mxs-lradc.o
diff --git a/drivers/staging/iio/adc/mxs-lradc.c b/drivers/staging/iio/adc/mxs-lradc.c
new file mode 100644
index 0000000..da50088
--- /dev/null
+++ b/drivers/staging/iio/adc/mxs-lradc.c
@@ -0,0 +1,666 @@
+/*
+ * ADT7410 digital temperature sensor driver supporting ADT7410
+ *
+ * Copyright 2010 Analog Devices Inc.
+ *
+ * Licensed under the GPL-2 or later.
+ */
+
+#include <linux/interrupt.h>
+#include <linux/device.h>
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/sysfs.h>
+#include <linux/list.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/spinlock.h>
+#include <linux/wait.h>
+#include <linux/sched.h>
+
+#include <mach/mxs.h>
+#include <mach/common.h>
+
+#include "../iio.h"
+#include "../sysfs.h"
+
+#define	MAPPING_USED		(1 << 7)
+#define	MAPPING_XXX		(1 << 6)
+#define	MAPPING_SET_SLOT(m)	(((m) & 0x3) << 4)
+#define	MAPPING_GET_SLOT(m)	(((m) >> 4) & 0x3)
+#define	MAPPING_CHAN(m)		((m) & 0xf)
+
+struct mxs_lradc_mapped_channel {
+	wait_queue_head_t		wq;
+	bool				wq_done;
+	uint16_t			chan_data;
+	uint16_t			mapping;
+	uint32_t			users;
+};
+
+struct mxs_lradc_drv_data {
+	struct iio_dev			*iio[4];
+	void __iomem			*mmio_base;
+
+	spinlock_t			lock;
+
+	uint16_t			claimed;
+
+	struct mxs_lradc_mapped_channel	ch[8];
+	uint8_t				ch_oversample[16];
+};
+
+struct mxs_lradc_data {
+	struct mxs_lradc_drv_data	*drv_data;
+	int				id;
+	spinlock_t			lock;
+
+	uint16_t			claimed;
+
+	uint32_t			loop_interval;
+};
+
+
+#define	LRADC_CH(n)				(0x50 + (0x10 * (n)))
+#define	LRADC_CH_ACCUMULATE			(1 << 29)
+#define	LRADC_CH_NUM_SAMPLES_MASK		(0x1f << 24)
+#define	LRADC_CH_NUM_SAMPLES_OFFSET		24
+#define	LRADC_CH_VALUE_MASK			0x3ffff
+#define	LRADC_CH_VALUE_OFFSET			0
+
+#define	LRADC_DELAY(n)				(0xd0 + (0x10 * (n)))
+#define	LRADC_DELAY_TRIGGER_LRADCS_MASK		(0xff << 24)
+#define	LRADC_DELAY_TRIGGER_LRADCS_OFFSET	24
+#define	LRADC_DELAY_KICK			(1 << 20)
+#define	LRADC_DELAY_TRIGGER_DELAYS_MASK		(0xf << 16)
+#define	LRADC_DELAY_TRIGGER_DELAYS_OFFSET	16
+#define	LRADC_DELAY_LOOP_COUNT_MASK		(0x1f << 11)
+#define	LRADC_DELAY_LOOP_COUNT_OFFSET		11
+#define	LRADC_DELAY_DELAY_MASK			0x7ff
+#define	LRADC_DELAY_DELAY_OFFSET		0
+
+#define	LRADC_CTRL0				0x00
+
+#define	LRADC_CTRL1				0x10
+#define	LRADC_CTRL1_LRADC_IRQ(n)		(1 << (n))
+#define	LRADC_CTRL1_LRADC_IRQ_EN(n)		(1 << ((n) + 16))
+
+#define	LRADC_CTRL2				0x20
+#define	LRADC_CTRL2_TEMPSENSE_PWD		(1 << 15)
+
+#define	LRADC_CTRL3				0x30
+
+#define	LRADC_CTRL4				0x140
+#define	LRADC_CTRL4_LRADCSELECT_MASK(n)		(0xf << ((n) * 4))
+#define	LRADC_CTRL4_LRADCSELECT_OFFSET(n)	((n) * 4)
+
+/*
+ * Global IIO attributes
+ */
+static ssize_t mxs_lradc_show_sampling_rate(struct device *dev,
+			struct device_attribute *attr, char *buf)
+{
+	struct iio_dev *iio_dev = dev_get_drvdata(dev);
+	struct mxs_lradc_data *data = iio_priv(iio_dev);
+
+	return sprintf(buf, "%u\n", data->loop_interval);
+}
+
+static ssize_t mxs_lradc_store_sampling_rate(struct device *dev,
+		struct device_attribute *attr, const char *buf, size_t count)
+{
+	struct iio_dev *iio_dev = dev_get_drvdata(dev);
+	struct mxs_lradc_data *data = iio_priv(iio_dev);
+	struct mxs_lradc_drv_data *drv_data = data->drv_data;
+	uint32_t reg;
+	unsigned long lval;
+	unsigned long lflags;
+
+	if (strict_strtoul(buf, 10, &lval))
+		return -EINVAL;
+
+	/* Check if the value is in requested range */
+	if (lval >= 2048)
+		return -EINVAL;
+
+	/*
+	 * Noone is accessing our delay trigger in the LRADC reg space so we
+	 * don't need to claim top-level spinlock.
+	 */
+	spin_lock_irqsave(&data->lock, lflags);
+
+	if (data->loop_interval != lval) {
+		data->loop_interval = lval;
+
+		/* Update the delay channel */
+		reg = readl(drv_data->mmio_base + LRADC_DELAY(data->id));
+		reg &= ~LRADC_DELAY_DELAY_MASK;
+		reg |= data->loop_interval;
+		writel(reg, drv_data->mmio_base + LRADC_DELAY(data->id));
+	}
+
+	spin_unlock_irqrestore(&data->lock, lflags);
+
+	return count;
+}
+
+static IIO_DEVICE_ATTR(sampling_rate, S_IRUGO | S_IWUSR,
+		       mxs_lradc_show_sampling_rate,
+		       mxs_lradc_store_sampling_rate, 0);
+static IIO_CONST_ATTR(sampling_rate_available,
+			"0...2047 (in 1/2000 second steps)");
+
+static struct attribute *mxs_lradc_attributes[] = {
+	&iio_dev_attr_sampling_rate.dev_attr.attr,
+	&iio_const_attr_sampling_rate_available.dev_attr.attr,
+	NULL,
+};
+
+static int mxs_lradc_can_claim_channel(struct iio_dev *iio_dev,
+			const struct iio_chan_spec *chan)
+{
+	struct mxs_lradc_data *data = iio_priv(iio_dev);
+	struct mxs_lradc_drv_data *drv_data = data->drv_data;
+	int i, count = 0;
+
+	/* The channel is already claimed by us. */
+	if (data->claimed & (1 << chan->address))
+		return 0;
+
+	/* Check if someone else didn't claim this */
+	if (drv_data->claimed & (1 << chan->address))
+		return -EINVAL;
+
+	for (i = 0; i < 16; i++)
+		if (drv_data->claimed & (1 << i))
+			count++;
+
+	/* Too many channels claimed */
+	if (count == 8)
+		return -EINVAL;
+
+	return 0;
+}
+
+static int mxs_lradc_claim_channel(struct iio_dev *iio_dev,
+			const struct iio_chan_spec *chan)
+{
+	struct mxs_lradc_data *data = iio_priv(iio_dev);
+	struct mxs_lradc_drv_data *drv_data = data->drv_data;
+	int i, ret;
+	unsigned long gflags;
+	uint32_t overspl = 0;
+
+	spin_lock_irqsave(&drv_data->lock, gflags);
+
+	ret = mxs_lradc_can_claim_channel(iio_dev, chan);
+	if (ret)
+		goto err;
+
+	/* Claim the channel */
+	drv_data->claimed |= 1 << chan->address;
+	data->claimed |= 1 << chan->address;
+
+	/* Map the channel */
+	for (i = 0; i < 8; i++)
+		if (!(drv_data->ch[i].mapping & MAPPING_USED))
+			break;
+
+	drv_data->ch[i].mapping = MAPPING_USED |
+				MAPPING_SET_SLOT(data->id) |
+				MAPPING_CHAN(chan->address);
+
+	/* Setup the mapping */
+	__mxs_clrl(LRADC_CTRL4_LRADCSELECT_MASK(i),
+		drv_data->mmio_base + LRADC_CTRL4);
+	__mxs_setl(chan->address << LRADC_CTRL4_LRADCSELECT_OFFSET(i),
+		drv_data->mmio_base + LRADC_CTRL4);
+
+	spin_unlock_irqrestore(&drv_data->lock, gflags);
+
+	overspl =
+		((drv_data->ch_oversample[chan->address] ? 1 : 0)
+			* LRADC_CH_ACCUMULATE) |
+		(drv_data->ch_oversample[chan->address]
+			<< LRADC_CH_NUM_SAMPLES_OFFSET);
+	writel(overspl, drv_data->mmio_base  + LRADC_CH(i));
+
+	/* Enable IRQ on the channel */
+	__mxs_clrl(LRADC_CTRL1_LRADC_IRQ(i),
+		drv_data->mmio_base + LRADC_CTRL1);
+	__mxs_setl(LRADC_CTRL1_LRADC_IRQ_EN(i),
+		drv_data->mmio_base + LRADC_CTRL1);
+
+	/* Set out channel to be triggers by this delay queue */
+	__mxs_setl(1 << (LRADC_DELAY_TRIGGER_LRADCS_OFFSET + i),
+		drv_data->mmio_base + LRADC_DELAY(data->id));
+
+	return 0;
+
+err:
+	spin_unlock_irqrestore(&drv_data->lock, gflags);
+	return -EINVAL;
+}
+
+static void mxs_lradc_relinquish_channel(struct iio_dev *iio_dev,
+			const struct iio_chan_spec *chan, int chanidx)
+{
+	struct mxs_lradc_data *data = iio_priv(iio_dev);
+	struct mxs_lradc_drv_data *drv_data = data->drv_data;
+	unsigned long gflags;
+	int i;
+
+	drv_data->ch[chanidx].users--;
+	if (drv_data->ch[chanidx].users == 0) {
+		/* No more users for this channel, stop generating interrupts */
+		__mxs_setl(LRADC_CTRL1_LRADC_IRQ(i) |
+			LRADC_CTRL1_LRADC_IRQ_EN(i),
+			drv_data->mmio_base + LRADC_CTRL1);
+
+		/* Don't trigger this channel */
+		__mxs_clrl(1 << (LRADC_DELAY_TRIGGER_LRADCS_OFFSET + i),
+			drv_data->mmio_base + LRADC_DELAY(data->id));
+
+		spin_lock_irqsave(&drv_data->lock, gflags);
+
+		/* Relinquish this channel */
+		drv_data->claimed &= ~(1 << chan->address);
+		data->claimed &= ~(1 << chan->address);
+		drv_data->ch[chanidx].mapping = 0;
+
+		spin_unlock_irqrestore(&drv_data->lock, gflags);
+	}
+}
+
+/*
+ * I/O operations
+ */
+static int mxs_lradc_read_raw(struct iio_dev *iio_dev,
+			const struct iio_chan_spec *chan,
+			int *val, int *val2, long m)
+{
+	struct mxs_lradc_data *data = iio_priv(iio_dev);
+	struct mxs_lradc_drv_data *drv_data = data->drv_data;
+	unsigned long lflags;
+	int i, ret;
+
+	switch (m) {
+	case 0:
+		spin_lock_irqsave(&data->lock, lflags);
+
+		ret = mxs_lradc_claim_channel(iio_dev, chan);
+		if (ret) {
+			spin_unlock_irqrestore(&data->lock, lflags);
+			return ret;
+		}
+
+		/* 
+		 * Once we are here, the channel is mapped by us already.
+		 * Find the mapping.
+		 */
+		for (i = 0; i < 8; i++) {
+			if (!(drv_data->ch[i].mapping & MAPPING_USED))
+				continue;
+
+			if (MAPPING_CHAN(drv_data->ch[i].mapping) ==
+				chan->address)
+				break;
+		}
+
+		/* Wait until sampling is done */
+		drv_data->ch[i].wq_done = false;
+
+		drv_data->ch[i].users++;
+
+		spin_unlock_irqrestore(&data->lock, lflags);
+
+		ret = wait_event_interruptible(drv_data->ch[i].wq,
+					drv_data->ch[i].wq_done);
+		if (ret)
+			ret = -EINTR;
+		else {
+			*val = readl(drv_data->mmio_base + LRADC_CH(i)) &
+					LRADC_CH_VALUE_MASK;
+			*val /= (drv_data->ch_oversample[chan->address] + 1);
+			ret = IIO_VAL_INT;
+		}
+
+		spin_lock_irqsave(&data->lock, lflags);
+
+		mxs_lradc_relinquish_channel(iio_dev, chan, i);
+
+		spin_unlock_irqrestore(&data->lock, lflags);
+
+		return ret;
+
+	case IIO_CHAN_INFO_OVERSAMPLE_COUNT:
+		*val = drv_data->ch_oversample[chan->address];
+		return IIO_VAL_INT;
+	}
+
+	return -EINVAL;
+}
+
+static int mxs_lradc_write_raw(struct iio_dev *iio_dev,
+			const struct iio_chan_spec *chan,
+			int val, int val2, long m)
+{
+	struct mxs_lradc_data *data = iio_priv(iio_dev);
+	struct mxs_lradc_drv_data *drv_data = data->drv_data;
+
+	switch (m) {
+	case IIO_CHAN_INFO_OVERSAMPLE_COUNT:
+		if ((val <= 0) || (val >= 32))
+			return -EINVAL;
+
+		drv_data->ch_oversample[chan->address] = val - 1;
+		return 0;
+	}
+
+	return -EINVAL;
+}
+
+static irqreturn_t mxs_lradc_handle_irq(int irq, void *data)
+{
+	struct mxs_lradc_drv_data *drv_data = data;
+	uint32_t reg = readl(drv_data->mmio_base + LRADC_CTRL1);
+	int i;
+
+	for (i = 0; i < 8; i++)
+		if (reg & LRADC_CTRL1_LRADC_IRQ(i)) {
+			drv_data->ch[i].wq_done = true;
+			wake_up_interruptible(&drv_data->ch[i].wq);
+		}
+
+	__mxs_clrl(0xffff, drv_data->mmio_base + LRADC_CTRL1);
+
+	return IRQ_HANDLED;
+}
+
+static const struct iio_info mxs_lradc_iio_info = {
+	.driver_module		= THIS_MODULE,
+	.read_raw		= mxs_lradc_read_raw,
+	.write_raw		= mxs_lradc_write_raw,
+};
+
+static const struct iio_chan_spec mxs_lradc_chan_spec[] = {
+	[0] = IIO_CHAN(IIO_VOLTAGE, IIO_NO_MOD, 1, IIO_RAW, NULL, 0, 0,
+		IIO_CHAN_INFO_SCALE_SEPARATE_BIT |
+		IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT,
+		0, 0, IIO_ST('u', 18, 32, 0), 0),
+	[1] = IIO_CHAN(IIO_VOLTAGE, IIO_NO_MOD, 1, IIO_RAW, NULL, 1, 0,
+		IIO_CHAN_INFO_SCALE_SEPARATE_BIT |
+		IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT,
+		1, 1, IIO_ST('u', 18, 32, 0), 0),
+	[2] = IIO_CHAN(IIO_VOLTAGE, IIO_NO_MOD, 1, IIO_RAW, NULL, 2, 0,
+		IIO_CHAN_INFO_SCALE_SEPARATE_BIT |
+		IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT,
+		2, 2, IIO_ST('u', 18, 32, 0), 0),
+	[3] = IIO_CHAN(IIO_VOLTAGE, IIO_NO_MOD, 1, IIO_RAW, NULL, 3, 0,
+		IIO_CHAN_INFO_SCALE_SEPARATE_BIT |
+		IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT,
+		3, 3, IIO_ST('u', 18, 32, 0), 0),
+	[4] = IIO_CHAN(IIO_VOLTAGE, IIO_NO_MOD, 1, IIO_RAW, NULL, 4, 0,
+		IIO_CHAN_INFO_SCALE_SEPARATE_BIT |
+		IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT,
+		4, 4, IIO_ST('u', 18, 32, 0), 0),
+	[5] = IIO_CHAN(IIO_VOLTAGE, IIO_NO_MOD, 1, IIO_RAW, NULL, 5, 0,
+		IIO_CHAN_INFO_SCALE_SEPARATE_BIT |
+		IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT,
+		5, 5, IIO_ST('u', 18, 32, 0), 0),
+	[6] = IIO_CHAN(IIO_VOLTAGE, IIO_NO_MOD, 1, IIO_RAW, NULL, 6, 0,
+		IIO_CHAN_INFO_SCALE_SEPARATE_BIT |
+		IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT,
+		6, 6, IIO_ST('u', 18, 32, 0), 0),
+	/* VBATT */
+	[7] = IIO_CHAN(IIO_VOLTAGE, IIO_NO_MOD, 1, IIO_RAW, NULL, 7, 0,
+		IIO_CHAN_INFO_SCALE_SEPARATE_BIT |
+		IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT,
+		7, 7, IIO_ST('u', 18, 32, 0), 0),
+	/* Temp sense 0 */
+	[8] = IIO_CHAN(IIO_TEMP, IIO_NO_MOD, 1, IIO_RAW, NULL, 8, 0,
+		IIO_CHAN_INFO_SCALE_SEPARATE_BIT |
+		IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT,
+		8, 8, IIO_ST('u', 18, 32, 0), 0),
+	/* Temp sense 1 */
+	[9] = IIO_CHAN(IIO_TEMP, IIO_NO_MOD, 1, IIO_RAW, NULL, 9, 0,
+		IIO_CHAN_INFO_SCALE_SEPARATE_BIT |
+		IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT,
+		9, 9, IIO_ST('u', 18, 32, 0), 0),
+	/* VDDIO */
+	[10] = IIO_CHAN(IIO_VOLTAGE, IIO_NO_MOD, 1, IIO_RAW, NULL, 10, 0,
+		IIO_CHAN_INFO_SCALE_SEPARATE_BIT |
+		IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT,
+		10, 10, IIO_ST('u', 18, 32, 0), 0),
+	/* VTH */
+	[11] = IIO_CHAN(IIO_VOLTAGE, IIO_NO_MOD, 1, IIO_RAW, NULL, 11, 0,
+		IIO_CHAN_INFO_SCALE_SEPARATE_BIT |
+		IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT,
+		11, 11, IIO_ST('u', 18, 32, 0), 0),
+	/* VDDA */
+	[12] = IIO_CHAN(IIO_VOLTAGE, IIO_NO_MOD, 1, IIO_RAW, NULL, 12, 0,
+		IIO_CHAN_INFO_SCALE_SEPARATE_BIT |
+		IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT,
+		12, 12, IIO_ST('u', 18, 32, 0), 0),
+	/* VDDD */
+	[13] = IIO_CHAN(IIO_VOLTAGE, IIO_NO_MOD, 1, IIO_RAW, NULL, 13, 0,
+		IIO_CHAN_INFO_SCALE_SEPARATE_BIT |
+		IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT,
+		13, 13, IIO_ST('u', 18, 32, 0), 0),
+	/* VBG */
+	[14] = IIO_CHAN(IIO_VOLTAGE, IIO_NO_MOD, 1, IIO_RAW, NULL, 14, 0,
+		IIO_CHAN_INFO_SCALE_SEPARATE_BIT |
+		IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT,
+		14, 14, IIO_ST('u', 18, 32, 0), 0),
+	/* VDD5V */
+	[15] = IIO_CHAN(IIO_VOLTAGE, IIO_NO_MOD, 1, IIO_RAW, NULL, 15, 0,
+		IIO_CHAN_INFO_SCALE_SEPARATE_BIT |
+		IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT,
+		15, 15, IIO_ST('u', 18, 32, 0), 0),
+};
+
+static void mxs_lradc_config(struct mxs_lradc_drv_data *drv_data)
+{
+	/* FIXME */
+	int freq = 0x3;	/* 6MHz */
+	int onchip_ground_ref = 0;
+
+	int i;
+
+	mxs_reset_block(drv_data->mmio_base + LRADC_CTRL0);
+
+	if (onchip_ground_ref)
+		__mxs_setl(1 << 26, drv_data->mmio_base + LRADC_CTRL0);
+	else
+		__mxs_clrl(1 << 26, drv_data->mmio_base + LRADC_CTRL0);
+
+	__mxs_clrl(0x3 << 8, drv_data->mmio_base + LRADC_CTRL3);
+	__mxs_setl(freq, drv_data->mmio_base + LRADC_CTRL3);
+
+	/* The delay channels constantly retrigger themself */
+	for (i = 0; i < 4; i++)
+		__mxs_setl(LRADC_DELAY_KICK |
+			(1 << (LRADC_DELAY_TRIGGER_DELAYS_OFFSET + i)) |
+			0x7ff,	/* FIXME */
+			drv_data->mmio_base + LRADC_DELAY(i));
+
+	/* Start temperature sensing */
+	writel(0, drv_data->mmio_base + LRADC_CTRL2);
+}
+
+/*static void mxs_lradc_config(struct mxs_lradc_pdata *pdata)
+{
+
+}
+*/
+static int __devinit mxs_lradc_probe(struct platform_device *pdev)
+{
+	struct mxs_lradc_data *data[4];
+	struct mxs_lradc_drv_data *drv_data;
+	struct iio_dev *iio;
+	struct resource *r;
+	int ret = 0;
+	int irq;
+	int i;
+
+	/*
+	 * DEVM management
+	 */
+	if (!devres_open_group(&pdev->dev, mxs_lradc_probe, GFP_KERNEL)) {
+		dev_err(&pdev->dev, "Can't open resource group\n");
+		goto err0;
+	}
+
+	drv_data = devm_kzalloc(&pdev->dev, sizeof(*drv_data), GFP_KERNEL);
+	if (!drv_data) {
+		dev_err(&pdev->dev, "Failed to allocate driver data\n");
+		ret = -ENOMEM;
+		goto err0;
+	}
+
+	spin_lock_init(&drv_data->lock);
+
+	/* 
+	 * IIO ops
+	 */
+	for (i = 0; i < 4; i++) {
+		iio = iio_allocate_device(sizeof(*data));
+		if (!iio) {
+			dev_err(&pdev->dev,
+				"Failed to allocate IIO device %i\n", i);
+			ret = -ENOMEM;
+			goto err1;
+		}
+
+		iio->name = pdev->name;
+		iio->dev.parent = &pdev->dev;
+		iio->info = &mxs_lradc_iio_info;
+		iio->modes = INDIO_DIRECT_MODE;
+		/* Channels */
+		iio->channels = mxs_lradc_chan_spec;
+		iio->num_channels = ARRAY_SIZE(mxs_lradc_chan_spec);
+		iio->attrs = mxs_lradc_attributes;
+
+		data[i] = iio_priv(iio);
+		data[i]->drv_data = drv_data;
+		data[i]->id = i;
+
+		spin_lock_init(&data[i]->lock);
+
+		drv_data->iio[i] = iio;
+	}
+
+	for (i = 0; i < 8; i++)
+		init_waitqueue_head(&drv_data->ch[i].wq);
+
+	dev_set_drvdata(&pdev->dev, drv_data);
+
+	/*
+	 * Allocate address space
+	 */
+	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (r == NULL) {
+		dev_err(&pdev->dev, "No I/O memory resource defined\n");
+		ret = -ENODEV;
+		goto err1;
+	}
+
+	r = devm_request_mem_region(&pdev->dev, r->start,
+				resource_size(r), pdev->name);
+	if (r == NULL) {
+		dev_err(&pdev->dev, "Failed to request I/O memory\n");
+		ret = -EBUSY;
+		goto err1;
+	}
+
+	drv_data->mmio_base = devm_ioremap(&pdev->dev, r->start, resource_size(r));
+	if (!drv_data->mmio_base) {
+		dev_err(&pdev->dev, "Failed to map I/O memory\n");
+		ret = -EBUSY;
+		goto err1;
+	}
+
+	/*
+	 * Allocate IRQ
+	 */
+	for (irq = 0; irq < 8; irq++) {
+		r = platform_get_resource(pdev, IORESOURCE_IRQ, irq);
+		if (r == NULL) {
+			dev_err(&pdev->dev, "No IRQ resource defined\n");
+			ret = -ENODEV;
+			goto err1;
+		}
+
+		ret = request_irq(r->start, mxs_lradc_handle_irq, 0, r->name, drv_data);
+		if (ret) {
+			dev_err(&pdev->dev, "request_irq %i failed: %d\n", irq, ret);
+			ret = -EBUSY;
+			goto err1;
+		}
+	}
+
+	/*
+	 * Register IIO device
+	 */
+	for (i = 0; i < 4; i++) {
+		ret = iio_device_register(drv_data->iio[i]);
+		if (ret) {
+			dev_err(&pdev->dev, "Failed to register IIO device\n");
+			ret = -EBUSY;
+			goto err2;
+		}
+	}
+
+	devres_remove_group(&pdev->dev, mxs_lradc_probe);
+
+	mxs_lradc_config(drv_data);
+
+	return 0;
+
+err2:
+	while (--i >= 0)
+		iio_device_unregister(drv_data->iio[i]);
+err1:
+	for (i = 0; i < 4; i++)
+		if (drv_data->iio[i])
+			iio_free_device(drv_data->iio[i]);
+err0:
+	devres_release_group(&pdev->dev, mxs_lradc_probe);
+	return ret;
+}
+
+static int __devexit mxs_lradc_remove(struct platform_device *pdev)
+{
+	struct mxs_lradc_drv_data *drv_data = dev_get_drvdata(&pdev->dev);
+	struct resource *r;
+	int i, irq;
+
+	for (i = 0; i < 4; i++)
+		iio_device_unregister(drv_data->iio[i]);
+
+	for (irq = 0; irq < 8; irq++) {
+		r = platform_get_resource(pdev, IORESOURCE_IRQ, irq);
+		if (r != NULL)
+			free_irq(r->start, drv_data);
+	}
+
+	for (i = 0; i < 4; i++)
+		iio_free_device(drv_data->iio[i]);
+
+	return 0;
+}
+
+static struct platform_driver mxs_lradc_driver = {
+	.driver = {
+		.name = "mxs-lradc",
+	},
+	.probe = mxs_lradc_probe,
+	.remove = __devexit_p(mxs_lradc_remove),
+};
+
+module_platform_driver(mxs_lradc_driver);
+
+MODULE_AUTHOR("Marek Vasut <marek.vasut@gmail.com>");
+MODULE_DESCRIPTION("Freescale i.MX23/i.MX28 LRADC driver");
+MODULE_LICENSE("GPL v2");
-- 
1.7.8.3

Re: [PATCH 2/2] [RFC] Basic support for MX28 LRADC IIO interface

[Jonathan Cameron]

On 2/10/2012 5:52 AM, Marek Vasut wrote:
> Signed-off-by: Marek Vasut<marek.vasut@gmail.com>
> Cc: Wolfgang Denk<wd@denx.de>
> Cc: Stefano Babic<sbabic@denx.de>
> Cc: Fabio Estevam<festevam@gmail.com>
> ---
>   arch/arm/mach-mxs/mach-m28evk.c        |   35 ++-
>   drivers/staging/iio/adc/Kconfig        |   10 +
>   drivers/staging/iio/adc/Makefile       |    1 +
>   drivers/staging/iio/adc/mxs-lradc.c    |  666 ++++++++++++++++++++++++++++++++
>   4 files changed, 700 insertions(+), 4 deletions(-)
>   create mode 100644 drivers/staging/iio/adc/mxs-lradc.c
>
> NOTE: The quality of code is still really crap here, I'm more interested in
> knowing whether I'm going in the right direction about this.
I may well point out stuff you'd fix anyway, but its easier for me that 
way ;)

Anyhow, having taken a look I like the general approach.  The 'claim' 
logic is nice and clean.
One nasty bit I hadn't realized is that you get separate interrupts per 
channel. This is going
to make the data aggregation and demuxing used with buffers interesting 
if you add that
support.

So thinking ahead, lets say you have 4 channels with oversampling as 
ax2,bx1,cx4,dx8
It will interrupt on each channel as
xxxxxxxx
   a a a a a
bbbbbbb
     c    c
         d
Easiest option will be to build up this as a single described 'scan' and 
push that to the demux.
Fiddly and if there is a big oversampling value, everything gets 
delayed.  Maybe we send scans
with a flag indicating no data for some fields and demux accordingly.  
Probably reasonable to
restrict a consumer driver to having only one oversampling so as far as 
they know it will come
through at that lower rate.

Still all that stuff is for another day.  Your driver is fine in as far 
as it goes!
>
> Thanks!
>
> diff --git a/arch/arm/mach-mxs/mach-m28evk.c b/arch/arm/mach-mxs/mach-m28evk.c
> index 2f27582..7a37d29 100644
> --- a/arch/arm/mach-mxs/mach-m28evk.c
> +++ b/arch/arm/mach-mxs/mach-m28evk.c
> @@ -320,6 +320,31 @@ static struct mxs_mmc_platform_data m28evk_mmc_pdata[] __initdata = {
>   	},
>   };
>
> +#define	RES_IRQ(id, res)	{ .name = id, .start = (res), .end = (res), .flags = IORESOURCE_IRQ }
> +
> +static struct resource mxs_lradc_rsrc[] = {
> +	[0] = {
> +		.start	= 0x80050000,
> +		.end	= 0x80050000 + SZ_8K - 1,
> +		.flags	= IORESOURCE_MEM,
> +	},
> +	RES_IRQ("LRADC CH0", MX28_INT_LRADC_CH0),
> +	RES_IRQ("LRADC CH1", MX28_INT_LRADC_CH1),
> +	RES_IRQ("LRADC CH2", MX28_INT_LRADC_CH2),
> +	RES_IRQ("LRADC CH3", MX28_INT_LRADC_CH3),
> +	RES_IRQ("LRADC CH4", MX28_INT_LRADC_CH4),
> +	RES_IRQ("LRADC CH5", MX28_INT_LRADC_CH5),
> +	RES_IRQ("LRADC CH6", MX28_INT_LRADC_CH6),
> +	RES_IRQ("LRADC CH7", MX28_INT_LRADC_CH7),
> +};
> +
> +static struct platform_device mxs_lradc = {
> +	.name		= "mxs-lradc",
> +	.id		= -1,
> +	.resource	= mxs_lradc_rsrc,
> +	.num_resources	= ARRAY_SIZE(mxs_lradc_rsrc),
> +};
> +
>   static void __init m28evk_init(void)
>   {
>   	mxs_iomux_setup_multiple_pads(m28evk_pads, ARRAY_SIZE(m28evk_pads));
> @@ -347,6 +372,8 @@ static void __init m28evk_init(void)
>   	mx28_add_mxs_i2c(0);
>   	i2c_register_board_info(0, m28_stk5v3_i2c_boardinfo,
>   			ARRAY_SIZE(m28_stk5v3_i2c_boardinfo));
> +
> +	platform_device_register(&mxs_lradc);
>   }
>
>   static void __init m28evk_timer_init(void)
> diff --git a/drivers/staging/iio/adc/Kconfig b/drivers/staging/iio/adc/Kconfig
> index d9decea..7f33032 100644
> --- a/drivers/staging/iio/adc/Kconfig
> +++ b/drivers/staging/iio/adc/Kconfig
> @@ -193,4 +193,14 @@ config MAX1363_RING_BUFFER
>   	  Say yes here to include ring buffer support in the MAX1363
>   	  ADC driver.
>
> +config MXS_LRADC
> +	tristate "Freescale i.MX23/i.MX28 LRADC"
> +	depends on ARCH_MXS
> +	help
> +	  Say yes here to build support for i.MX23/i.MX28 LRADC convertor
> +	  built into these chips.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called mxs-lradc.
> +
>   endmenu
> diff --git a/drivers/staging/iio/adc/Makefile b/drivers/staging/iio/adc/Makefile
> index ceee7f3..2d764ec 100644
> --- a/drivers/staging/iio/adc/Makefile
> +++ b/drivers/staging/iio/adc/Makefile
> @@ -37,3 +37,4 @@ obj-$(CONFIG_AD7192) += ad7192.o
>   obj-$(CONFIG_ADT7310) += adt7310.o
>   obj-$(CONFIG_ADT7410) += adt7410.o
>   obj-$(CONFIG_AD7280) += ad7280a.o
> +obj-$(CONFIG_MXS_LRADC) += mxs-lradc.o
> diff --git a/drivers/staging/iio/adc/mxs-lradc.c b/drivers/staging/iio/adc/mxs-lradc.c
> new file mode 100644
> index 0000000..da50088
> --- /dev/null
> +++ b/drivers/staging/iio/adc/mxs-lradc.c
> @@ -0,0 +1,666 @@
> +/*
> + * ADT7410 digital temperature sensor driver supporting ADT7410
> + *
> + * Copyright 2010 Analog Devices Inc.
Obvious bit to fix here ;)  Doesn't bode that well that you have the 
copyright
notice for a driver I've been trying to get the heck out of IIO. 
*fingers crossed*
> + *
> + * Licensed under the GPL-2 or later.
> + */
> +
> +#include<linux/interrupt.h>
> +#include<linux/device.h>
> +#include<linux/kernel.h>
> +#include<linux/slab.h>
> +#include<linux/sysfs.h>
> +#include<linux/list.h>
> +#include<linux/io.h>
> +#include<linux/module.h>
> +#include<linux/platform_device.h>
> +#include<linux/spinlock.h>
> +#include<linux/wait.h>
> +#include<linux/sched.h>
> +
> +#include<mach/mxs.h>
> +#include<mach/common.h>
> +
> +#include "../iio.h"
> +#include "../sysfs.h"
> +
> +#define	MAPPING_USED		(1<<  7)
> +#define	MAPPING_XXX		(1<<  6)
> +#define	MAPPING_SET_SLOT(m)	(((m)&  0x3)<<  4)
> +#define	MAPPING_GET_SLOT(m)	(((m)>>  4)&  0x3)
> +#define	MAPPING_CHAN(m)		((m)&  0xf)
> +
> +struct mxs_lradc_mapped_channel {
> +	wait_queue_head_t		wq;
> +	bool				wq_done;
> +	uint16_t			chan_data;
> +	uint16_t			mapping;
> +	uint32_t			users;
> +};
> +
> +struct mxs_lradc_drv_data {
> +	struct iio_dev			*iio[4];
> +	void __iomem			*mmio_base;
> +
> +	spinlock_t			lock;
> +
> +	uint16_t			claimed;
> +
> +	struct mxs_lradc_mapped_channel	ch[8];
yikes. Individual oversample controls. I was assuming these were per 
sample trigger source.
That's going to make for some fiddly data management when you add 
buffers.  We'll
have to define the ordering of data very carefully and the demux unit 
will need adapting to
handle this.  Fun fun fun.  (note the demux unit becomes very important 
for in kernel consumers
of iio buffered data).  I really need to get that code out there in a 
more up to date form.
  Now I think Greg has been talked round to taking the 'pull interface' 
set we can hopefully make
real progress on that front.

I'm never sure if I like 'interesting' hardware or if it just gives me a 
headache!
> +	uint8_t				ch_oversample[16];
> +};
> +
> +struct mxs_lradc_data {
> +	struct mxs_lradc_drv_data	*drv_data;
> +	int				id;
> +	spinlock_t			lock;
> +
> +	uint16_t			claimed;
> +
> +	uint32_t			loop_interval;
> +};
> +
> +
> +#define	LRADC_CH(n)				(0x50 + (0x10 * (n)))
> +#define	LRADC_CH_ACCUMULATE			(1<<  29)
> +#define	LRADC_CH_NUM_SAMPLES_MASK		(0x1f<<  24)
> +#define	LRADC_CH_NUM_SAMPLES_OFFSET		24
> +#define	LRADC_CH_VALUE_MASK			0x3ffff
> +#define	LRADC_CH_VALUE_OFFSET			0
> +
> +#define	LRADC_DELAY(n)				(0xd0 + (0x10 * (n)))
> +#define	LRADC_DELAY_TRIGGER_LRADCS_MASK		(0xff<<  24)
> +#define	LRADC_DELAY_TRIGGER_LRADCS_OFFSET	24
> +#define	LRADC_DELAY_KICK			(1<<  20)
> +#define	LRADC_DELAY_TRIGGER_DELAYS_MASK		(0xf<<  16)
> +#define	LRADC_DELAY_TRIGGER_DELAYS_OFFSET	16
> +#define	LRADC_DELAY_LOOP_COUNT_MASK		(0x1f<<  11)
> +#define	LRADC_DELAY_LOOP_COUNT_OFFSET		11
> +#define	LRADC_DELAY_DELAY_MASK			0x7ff
> +#define	LRADC_DELAY_DELAY_OFFSET		0
> +
> +#define	LRADC_CTRL0				0x00
> +
> +#define	LRADC_CTRL1				0x10
> +#define	LRADC_CTRL1_LRADC_IRQ(n)		(1<<  (n))
> +#define	LRADC_CTRL1_LRADC_IRQ_EN(n)		(1<<  ((n) + 16))
> +
> +#define	LRADC_CTRL2				0x20
> +#define	LRADC_CTRL2_TEMPSENSE_PWD		(1<<  15)
> +
> +#define	LRADC_CTRL3				0x30
> +
> +#define	LRADC_CTRL4				0x140
> +#define	LRADC_CTRL4_LRADCSELECT_MASK(n)		(0xf<<  ((n) * 4))
> +#define	LRADC_CTRL4_LRADCSELECT_OFFSET(n)	((n) * 4)
> +
> +/*
> + * Global IIO attributes
> + */
> +static ssize_t mxs_lradc_show_sampling_rate(struct device *dev,
> +			struct device_attribute *attr, char *buf)
> +{
> +	struct iio_dev *iio_dev = dev_get_drvdata(dev);
> +	struct mxs_lradc_data *data = iio_priv(iio_dev);
> +
> +	return sprintf(buf, "%u\n", data->loop_interval);
> +}
> +
> +static ssize_t mxs_lradc_store_sampling_rate(struct device *dev,
> +		struct device_attribute *attr, const char *buf, size_t count)
> +{
> +	struct iio_dev *iio_dev = dev_get_drvdata(dev);
> +	struct mxs_lradc_data *data = iio_priv(iio_dev);
> +	struct mxs_lradc_drv_data *drv_data = data->drv_data;
> +	uint32_t reg;
> +	unsigned long lval;
> +	unsigned long lflags;
> +
> +	if (strict_strtoul(buf, 10,&lval))
> +		return -EINVAL;
> +
> +	/* Check if the value is in requested range */
> +	if (lval>= 2048)
> +		return -EINVAL;
> +
> +	/*
> +	 * Noone is accessing our delay trigger in the LRADC reg space so we
> +	 * don't need to claim top-level spinlock.
> +	 */
> +	spin_lock_irqsave(&data->lock, lflags);
> +
> +	if (data->loop_interval != lval) {
> +		data->loop_interval = lval;
> +
> +		/* Update the delay channel */
> +		reg = readl(drv_data->mmio_base + LRADC_DELAY(data->id));
> +		reg&= ~LRADC_DELAY_DELAY_MASK;
> +		reg |= data->loop_interval;
> +		writel(reg, drv_data->mmio_base + LRADC_DELAY(data->id));
> +	}
> +
> +	spin_unlock_irqrestore(&data->lock, lflags);
> +
> +	return count;
> +}
> +
> +static IIO_DEVICE_ATTR(sampling_rate, S_IRUGO | S_IWUSR,
> +		       mxs_lradc_show_sampling_rate,
> +		       mxs_lradc_store_sampling_rate, 0);

Hmm..  Not abi compliant, we probably do need to come up with a format 
for specifying
such range restrictions. Also note sampling rate is always in hz.  This 
will require
some fixed point fiddling but the interface has to be predictable.
> +static IIO_CONST_ATTR(sampling_rate_available,
> +			"0...2047 (in 1/2000 second steps)");
> +
> +static struct attribute *mxs_lradc_attributes[] = {
> +	&iio_dev_attr_sampling_rate.dev_attr.attr,
> +	&iio_const_attr_sampling_rate_available.dev_attr.attr,
> +	NULL,
> +};
> +
> +static int mxs_lradc_can_claim_channel(struct iio_dev *iio_dev,
> +			const struct iio_chan_spec *chan)
> +{
> +	struct mxs_lradc_data *data = iio_priv(iio_dev);
> +	struct mxs_lradc_drv_data *drv_data = data->drv_data;
> +	int i, count = 0;
> +
> +	/* The channel is already claimed by us. */
> +	if (data->claimed&  (1<<  chan->address))
> +		return 0;
> +
> +	/* Check if someone else didn't claim this */
> +	if (drv_data->claimed&  (1<<  chan->address))
> +		return -EINVAL;
> +
> +	for (i = 0; i<  16; i++)
> +		if (drv_data->claimed&  (1<<  i))
> +			count++;
> +
> +	/* Too many channels claimed */
> +	if (count == 8)
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +
> +static int mxs_lradc_claim_channel(struct iio_dev *iio_dev,
> +			const struct iio_chan_spec *chan)
> +{
> +	struct mxs_lradc_data *data = iio_priv(iio_dev);
> +	struct mxs_lradc_drv_data *drv_data = data->drv_data;
> +	int i, ret;
> +	unsigned long gflags;
> +	uint32_t overspl = 0;
> +
> +	spin_lock_irqsave(&drv_data->lock, gflags);
> +
> +	ret = mxs_lradc_can_claim_channel(iio_dev, chan);
> +	if (ret)
> +		goto err;
> +
> +	/* Claim the channel */
> +	drv_data->claimed |= 1<<  chan->address;
> +	data->claimed |= 1<<  chan->address;
> +
> +	/* Map the channel */
> +	for (i = 0; i<  8; i++)
> +		if (!(drv_data->ch[i].mapping&  MAPPING_USED))
> +			break;
> +
> +	drv_data->ch[i].mapping = MAPPING_USED |
> +				MAPPING_SET_SLOT(data->id) |
> +				MAPPING_CHAN(chan->address);
> +
> +	/* Setup the mapping */
> +	__mxs_clrl(LRADC_CTRL4_LRADCSELECT_MASK(i),
> +		drv_data->mmio_base + LRADC_CTRL4);
> +	__mxs_setl(chan->address<<  LRADC_CTRL4_LRADCSELECT_OFFSET(i),
> +		drv_data->mmio_base + LRADC_CTRL4);
> +
> +	spin_unlock_irqrestore(&drv_data->lock, gflags);
> +
> +	overspl =
> +		((drv_data->ch_oversample[chan->address] ? 1 : 0)
> +			* LRADC_CH_ACCUMULATE) |
> +		(drv_data->ch_oversample[chan->address]
> +			<<  LRADC_CH_NUM_SAMPLES_OFFSET);
> +	writel(overspl, drv_data->mmio_base  + LRADC_CH(i));
> +
> +	/* Enable IRQ on the channel */
> +	__mxs_clrl(LRADC_CTRL1_LRADC_IRQ(i),
> +		drv_data->mmio_base + LRADC_CTRL1);
> +	__mxs_setl(LRADC_CTRL1_LRADC_IRQ_EN(i),
> +		drv_data->mmio_base + LRADC_CTRL1);
> +
> +	/* Set out channel to be triggers by this delay queue */
> +	__mxs_setl(1<<  (LRADC_DELAY_TRIGGER_LRADCS_OFFSET + i),
> +		drv_data->mmio_base + LRADC_DELAY(data->id));
> +
> +	return 0;
> +
> +err:
> +	spin_unlock_irqrestore(&drv_data->lock, gflags);
> +	return -EINVAL;
> +}
> +
> +static void mxs_lradc_relinquish_channel(struct iio_dev *iio_dev,
> +			const struct iio_chan_spec *chan, int chanidx)
> +{
> +	struct mxs_lradc_data *data = iio_priv(iio_dev);
> +	struct mxs_lradc_drv_data *drv_data = data->drv_data;
> +	unsigned long gflags;
> +	int i;
> +
> +	drv_data->ch[chanidx].users--;
> +	if (drv_data->ch[chanidx].users == 0) {
> +		/* No more users for this channel, stop generating interrupts */
> +		__mxs_setl(LRADC_CTRL1_LRADC_IRQ(i) |
> +			LRADC_CTRL1_LRADC_IRQ_EN(i),
> +			drv_data->mmio_base + LRADC_CTRL1);
> +
> +		/* Don't trigger this channel */
> +		__mxs_clrl(1<<  (LRADC_DELAY_TRIGGER_LRADCS_OFFSET + i),
> +			drv_data->mmio_base + LRADC_DELAY(data->id));
> +
> +		spin_lock_irqsave(&drv_data->lock, gflags);
> +
> +		/* Relinquish this channel */
> +		drv_data->claimed&= ~(1<<  chan->address);
> +		data->claimed&= ~(1<<  chan->address);
> +		drv_data->ch[chanidx].mapping = 0;
> +
> +		spin_unlock_irqrestore(&drv_data->lock, gflags);
> +	}
> +}
> +
> +/*
> + * I/O operations
> + */
> +static int mxs_lradc_read_raw(struct iio_dev *iio_dev,
> +			const struct iio_chan_spec *chan,
> +			int *val, int *val2, long m)
> +{
> +	struct mxs_lradc_data *data = iio_priv(iio_dev);
> +	struct mxs_lradc_drv_data *drv_data = data->drv_data;
> +	unsigned long lflags;
> +	int i, ret;
> +
> +	switch (m) {
> +	case 0:
> +		spin_lock_irqsave(&data->lock, lflags);
> +
So we claim the channel for this IIO dev (which is one of the 4 timed 
triggers).
Then wait for the next sample.  Guess that does the job, though this is 
all faking
a kind of polled device whereas this will fit much better as a interrupt 
driven
device outputting via buffers (probably the next step once this is 
fine).  Note
that we typically disable this sort of access if the device is doing 
interrupt driven
reading.  I have been thinking about adding a buffer implementation that 
would
provide just the last value and hence allowing this sort of single 
channel read, but
that requires some infrastructure that hasn't merged yet.
> +		ret = mxs_lradc_claim_channel(iio_dev, chan);
> +		if (ret) {
> +			spin_unlock_irqrestore(&data->lock, lflags);
> +			return ret;
> +		}
> +
> +		/*
> +		 * Once we are here, the channel is mapped by us already.
> +		 * Find the mapping.
> +		 */
> +		for (i = 0; i<  8; i++) {
> +			if (!(drv_data->ch[i].mapping&  MAPPING_USED))
> +				continue;
> +
> +			if (MAPPING_CHAN(drv_data->ch[i].mapping) ==
> +				chan->address)
> +				break;
> +		}
> +
> +		/* Wait until sampling is done */
> +		drv_data->ch[i].wq_done = false;
> +
> +		drv_data->ch[i].users++;
> +
> +		spin_unlock_irqrestore(&data->lock, lflags);
> +
> +		ret = wait_event_interruptible(drv_data->ch[i].wq,
> +					drv_data->ch[i].wq_done);
> +		if (ret)
> +			ret = -EINTR;
> +		else {
> +			*val = readl(drv_data->mmio_base + LRADC_CH(i))&
> +					LRADC_CH_VALUE_MASK;
> +			*val /= (drv_data->ch_oversample[chan->address] + 1);
> +			ret = IIO_VAL_INT;
> +		}
> +
> +		spin_lock_irqsave(&data->lock, lflags);
> +
> +		mxs_lradc_relinquish_channel(iio_dev, chan, i);
> +
> +		spin_unlock_irqrestore(&data->lock, lflags);
> +
> +		return ret;
> +
> +	case IIO_CHAN_INFO_OVERSAMPLE_COUNT:
> +		*val = drv_data->ch_oversample[chan->address];
> +		return IIO_VAL_INT;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static int mxs_lradc_write_raw(struct iio_dev *iio_dev,
> +			const struct iio_chan_spec *chan,
> +			int val, int val2, long m)
> +{
> +	struct mxs_lradc_data *data = iio_priv(iio_dev);
> +	struct mxs_lradc_drv_data *drv_data = data->drv_data;
> +
> +	switch (m) {
> +	case IIO_CHAN_INFO_OVERSAMPLE_COUNT:
> +		if ((val<= 0) || (val>= 32))
> +			return -EINVAL;
> +
> +		drv_data->ch_oversample[chan->address] = val - 1;
> +		return 0;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static irqreturn_t mxs_lradc_handle_irq(int irq, void *data)
> +{
> +	struct mxs_lradc_drv_data *drv_data = data;
> +	uint32_t reg = readl(drv_data->mmio_base + LRADC_CTRL1);
> +	int i;
> +
> +	for (i = 0; i<  8; i++)
> +		if (reg&  LRADC_CTRL1_LRADC_IRQ(i)) {
> +			drv_data->ch[i].wq_done = true;
> +			wake_up_interruptible(&drv_data->ch[i].wq);
> +		}
> +
> +	__mxs_clrl(0xffff, drv_data->mmio_base + LRADC_CTRL1);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static const struct iio_info mxs_lradc_iio_info = {
> +	.driver_module		= THIS_MODULE,
> +	.read_raw		= mxs_lradc_read_raw,
> +	.write_raw		= mxs_lradc_write_raw,
> +};
> +
These all look good...  However, one thing you weren't to know is that 
we are trying
to deprecate the IIO_CHAN macro.  It's become a bit of a maintenance 
nightmare
given the underlying structure keeps getting more complex as new 
requirements
turn up. Arnd warned me about this when I originally put it in, but it 
took me a
while to realise how right he was!

Either define a local macro filling just those fields that matter here or
fill the structure directly.  Also you could leave out the IIO_ST stuff 
for now as it
only means anything in a driver providing buffered access via the chrdev.
Same is true of scan_index.
> +static const struct iio_chan_spec mxs_lradc_chan_spec[] = {
> +	[0] = IIO_CHAN(IIO_VOLTAGE, IIO_NO_MOD, 1, IIO_RAW, NULL, 0, 0,
> +		IIO_CHAN_INFO_SCALE_SEPARATE_BIT |
> +		IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT,
> +		0, 0, IIO_ST('u', 18, 32, 0), 0),
> +	[1] = IIO_CHAN(IIO_VOLTAGE, IIO_NO_MOD, 1, IIO_RAW, NULL, 1, 0,
> +		IIO_CHAN_INFO_SCALE_SEPARATE_BIT |
> +		IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT,
> +		1, 1, IIO_ST('u', 18, 32, 0), 0),
> +	[2] = IIO_CHAN(IIO_VOLTAGE, IIO_NO_MOD, 1, IIO_RAW, NULL, 2, 0,
> +		IIO_CHAN_INFO_SCALE_SEPARATE_BIT |
> +		IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT,
> +		2, 2, IIO_ST('u', 18, 32, 0), 0),
> +	[3] = IIO_CHAN(IIO_VOLTAGE, IIO_NO_MOD, 1, IIO_RAW, NULL, 3, 0,
> +		IIO_CHAN_INFO_SCALE_SEPARATE_BIT |
> +		IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT,
> +		3, 3, IIO_ST('u', 18, 32, 0), 0),
> +	[4] = IIO_CHAN(IIO_VOLTAGE, IIO_NO_MOD, 1, IIO_RAW, NULL, 4, 0,
> +		IIO_CHAN_INFO_SCALE_SEPARATE_BIT |
> +		IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT,
> +		4, 4, IIO_ST('u', 18, 32, 0), 0),
> +	[5] = IIO_CHAN(IIO_VOLTAGE, IIO_NO_MOD, 1, IIO_RAW, NULL, 5, 0,
> +		IIO_CHAN_INFO_SCALE_SEPARATE_BIT |
> +		IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT,
> +		5, 5, IIO_ST('u', 18, 32, 0), 0),
> +	[6] = IIO_CHAN(IIO_VOLTAGE, IIO_NO_MOD, 1, IIO_RAW, NULL, 6, 0,
> +		IIO_CHAN_INFO_SCALE_SEPARATE_BIT |
> +		IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT,
> +		6, 6, IIO_ST('u', 18, 32, 0), 0),
> +	/* VBATT */
> +	[7] = IIO_CHAN(IIO_VOLTAGE, IIO_NO_MOD, 1, IIO_RAW, NULL, 7, 0,
> +		IIO_CHAN_INFO_SCALE_SEPARATE_BIT |
> +		IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT,
> +		7, 7, IIO_ST('u', 18, 32, 0), 0),
> +	/* Temp sense 0 */
> +	[8] = IIO_CHAN(IIO_TEMP, IIO_NO_MOD, 1, IIO_RAW, NULL, 8, 0,
> +		IIO_CHAN_INFO_SCALE_SEPARATE_BIT |
> +		IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT,
> +		8, 8, IIO_ST('u', 18, 32, 0), 0),
> +	/* Temp sense 1 */
> +	[9] = IIO_CHAN(IIO_TEMP, IIO_NO_MOD, 1, IIO_RAW, NULL, 9, 0,
> +		IIO_CHAN_INFO_SCALE_SEPARATE_BIT |
> +		IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT,
> +		9, 9, IIO_ST('u', 18, 32, 0), 0),
> +	/* VDDIO */
> +	[10] = IIO_CHAN(IIO_VOLTAGE, IIO_NO_MOD, 1, IIO_RAW, NULL, 10, 0,
> +		IIO_CHAN_INFO_SCALE_SEPARATE_BIT |
> +		IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT,
> +		10, 10, IIO_ST('u', 18, 32, 0), 0),
> +	/* VTH */
> +	[11] = IIO_CHAN(IIO_VOLTAGE, IIO_NO_MOD, 1, IIO_RAW, NULL, 11, 0,
> +		IIO_CHAN_INFO_SCALE_SEPARATE_BIT |
> +		IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT,
> +		11, 11, IIO_ST('u', 18, 32, 0), 0),
> +	/* VDDA */
> +	[12] = IIO_CHAN(IIO_VOLTAGE, IIO_NO_MOD, 1, IIO_RAW, NULL, 12, 0,
> +		IIO_CHAN_INFO_SCALE_SEPARATE_BIT |
> +		IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT,
> +		12, 12, IIO_ST('u', 18, 32, 0), 0),
> +	/* VDDD */
> +	[13] = IIO_CHAN(IIO_VOLTAGE, IIO_NO_MOD, 1, IIO_RAW, NULL, 13, 0,
> +		IIO_CHAN_INFO_SCALE_SEPARATE_BIT |
> +		IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT,
> +		13, 13, IIO_ST('u', 18, 32, 0), 0),
> +	/* VBG */
> +	[14] = IIO_CHAN(IIO_VOLTAGE, IIO_NO_MOD, 1, IIO_RAW, NULL, 14, 0,
> +		IIO_CHAN_INFO_SCALE_SEPARATE_BIT |
> +		IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT,
> +		14, 14, IIO_ST('u', 18, 32, 0), 0),
> +	/* VDD5V */
> +	[15] = IIO_CHAN(IIO_VOLTAGE, IIO_NO_MOD, 1, IIO_RAW, NULL, 15, 0,
> +		IIO_CHAN_INFO_SCALE_SEPARATE_BIT |
> +		IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT,
> +		15, 15, IIO_ST('u', 18, 32, 0), 0),
> +};
> +
> +static void mxs_lradc_config(struct mxs_lradc_drv_data *drv_data)
> +{
> +	/* FIXME */
> +	int freq = 0x3;	/* 6MHz */
> +	int onchip_ground_ref = 0;
> +
> +	int i;
> +
> +	mxs_reset_block(drv_data->mmio_base + LRADC_CTRL0);
> +
> +	if (onchip_ground_ref)
> +		__mxs_setl(1<<  26, drv_data->mmio_base + LRADC_CTRL0);
> +	else
> +		__mxs_clrl(1<<  26, drv_data->mmio_base + LRADC_CTRL0);
> +
> +	__mxs_clrl(0x3<<  8, drv_data->mmio_base + LRADC_CTRL3);
> +	__mxs_setl(freq, drv_data->mmio_base + LRADC_CTRL3);
> +
> +	/* The delay channels constantly retrigger themself */
> +	for (i = 0; i<  4; i++)
> +		__mxs_setl(LRADC_DELAY_KICK |
> +			(1<<  (LRADC_DELAY_TRIGGER_DELAYS_OFFSET + i)) |
> +			0x7ff,	/* FIXME */
> +			drv_data->mmio_base + LRADC_DELAY(i));
> +
> +	/* Start temperature sensing */
> +	writel(0, drv_data->mmio_base + LRADC_CTRL2);
> +}
> +
> +/*static void mxs_lradc_config(struct mxs_lradc_pdata *pdata)
> +{
> +
> +}
> +*/
> +static int __devinit mxs_lradc_probe(struct platform_device *pdev)
> +{
> +	struct mxs_lradc_data *data[4];
> +	struct mxs_lradc_drv_data *drv_data;
> +	struct iio_dev *iio;
> +	struct resource *r;
> +	int ret = 0;
> +	int irq;
> +	int i;
> +
> +	/*
> +	 * DEVM management
> +	 */
> +	if (!devres_open_group(&pdev->dev, mxs_lradc_probe, GFP_KERNEL)) {
> +		dev_err(&pdev->dev, "Can't open resource group\n");
> +		goto err0;
> +	}
> +
> +	drv_data = devm_kzalloc(&pdev->dev, sizeof(*drv_data), GFP_KERNEL);
> +	if (!drv_data) {
> +		dev_err(&pdev->dev, "Failed to allocate driver data\n");
> +		ret = -ENOMEM;
> +		goto err0;
> +	}
> +
> +	spin_lock_init(&drv_data->lock);
> +
> +	/*
> +	 * IIO ops
> +	 */
> +	for (i = 0; i<  4; i++) {
> +		iio = iio_allocate_device(sizeof(*data));
> +		if (!iio) {
> +			dev_err(&pdev->dev,
> +				"Failed to allocate IIO device %i\n", i);
> +			ret = -ENOMEM;
> +			goto err1;
> +		}
> +
> +		iio->name = pdev->name;
You will probably want to add an id to that name to distinguish between 
the difference instances.
Even better would be to have a text description of each one.
> +		iio->dev.parent =&pdev->dev;
> +		iio->info =&mxs_lradc_iio_info;
> +		iio->modes = INDIO_DIRECT_MODE;
> +		/* Channels */
> +		iio->channels = mxs_lradc_chan_spec;
> +		iio->num_channels = ARRAY_SIZE(mxs_lradc_chan_spec);
> +		iio->attrs = mxs_lradc_attributes;
> +
Given data[i] is only used in here, why not just have one of them?
> +		data[i] = iio_priv(iio);
> +		data[i]->drv_data = drv_data;
> +		data[i]->id = i;
> +
> +		spin_lock_init(&data[i]->lock);
> +
> +		drv_data->iio[i] = iio;
> +	}
> +
> +	for (i = 0; i<  8; i++)
> +		init_waitqueue_head(&drv_data->ch[i].wq);
> +
> +	dev_set_drvdata(&pdev->dev, drv_data);
> +
> +	/*
> +	 * Allocate address space
> +	 */
> +	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (r == NULL) {
> +		dev_err(&pdev->dev, "No I/O memory resource defined\n");
> +		ret = -ENODEV;
> +		goto err1;
> +	}
> +
> +	r = devm_request_mem_region(&pdev->dev, r->start,
> +				resource_size(r), pdev->name);
> +	if (r == NULL) {
> +		dev_err(&pdev->dev, "Failed to request I/O memory\n");
> +		ret = -EBUSY;
> +		goto err1;
> +	}
> +
> +	drv_data->mmio_base = devm_ioremap(&pdev->dev, r->start, resource_size(r));
> +	if (!drv_data->mmio_base) {
> +		dev_err(&pdev->dev, "Failed to map I/O memory\n");
> +		ret = -EBUSY;
> +		goto err1;
> +	}
> +
> +	/*
> +	 * Allocate IRQ
> +	 */
> +	for (irq = 0; irq<  8; irq++) {
> +		r = platform_get_resource(pdev, IORESOURCE_IRQ, irq);
> +		if (r == NULL) {
> +			dev_err(&pdev->dev, "No IRQ resource defined\n");
> +			ret = -ENODEV;
> +			goto err1;
> +		}
> +
> +		ret = request_irq(r->start, mxs_lradc_handle_irq, 0, r->name, drv_data);
> +		if (ret) {
> +			dev_err(&pdev->dev, "request_irq %i failed: %d\n", irq, ret);
> +			ret = -EBUSY;
> +			goto err1;
> +		}
> +	}
> +
> +	/*
> +	 * Register IIO device
> +	 */
> +	for (i = 0; i<  4; i++) {
> +		ret = iio_device_register(drv_data->iio[i]);
> +		if (ret) {
> +			dev_err(&pdev->dev, "Failed to register IIO device\n");
> +			ret = -EBUSY;
> +			goto err2;
> +		}
> +	}
> +
> +	devres_remove_group(&pdev->dev, mxs_lradc_probe);
> +
> +	mxs_lradc_config(drv_data);
> +
> +	return 0;
> +
> +err2:
> +	while (--i>= 0)
> +
Clearing out irqs?
> 		iio_device_unregister(drv_data->iio[i]);
> +err1:
> +	for (i = 0; i<  4; i++)
> +		if (drv_data->iio[i])
> +			iio_free_device(drv_data->iio[i]);
> +err0:
> +	devres_release_group(&pdev->dev, mxs_lradc_probe);
> +	return ret;
> +}
> +
> +static int __devexit mxs_lradc_remove(struct platform_device *pdev)
> +{
> +	struct mxs_lradc_drv_data *drv_data = dev_get_drvdata(&pdev->dev);
> +	struct resource *r;
> +	int i, irq;
> +
     Obvious but use ARRAY_SIZE(drv_data->iio) instead of 4.
> +	for (i = 0; i<  4; i++)
> +		iio_device_unregister(drv_data->iio[i]);
> +
> +	for (irq = 0; irq<  8; irq++) {
> +		r = platform_get_resource(pdev, IORESOURCE_IRQ, irq);
> +		if (r != NULL)
> +			free_irq(r->start, drv_data);
> +	}
> +
> +	for (i = 0; i<  4; i++)
> +		iio_free_device(drv_data->iio[i]);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver mxs_lradc_driver = {
> +	.driver = {
> +		.name = "mxs-lradc",
> +	},
> +	.probe = mxs_lradc_probe,
> +	.remove = __devexit_p(mxs_lradc_remove),
> +};
> +
> +module_platform_driver(mxs_lradc_driver);
> +
> +MODULE_AUTHOR("Marek Vasut<marek.vasut@gmail.com>");
> +MODULE_DESCRIPTION("Freescale i.MX23/i.MX28 LRADC driver");
> +MODULE_LICENSE("GPL v2");

Re: [PATCH 2/2] [RFC] Basic support for MX28 LRADC IIO interface

[Marek Vasut]

> On 2/10/2012 5:52 AM, Marek Vasut wrote:
> > Signed-off-by: Marek Vasut<marek.vasut@gmail.com>
> > Cc: Wolfgang Denk<wd@denx.de>
> > Cc: Stefano Babic<sbabic@denx.de>
> > Cc: Fabio Estevam<festevam@gmail.com>
> > ---
> > 
> >   arch/arm/mach-mxs/mach-m28evk.c        |   35 ++-
> >   drivers/staging/iio/adc/Kconfig        |   10 +
> >   drivers/staging/iio/adc/Makefile       |    1 +
> >   drivers/staging/iio/adc/mxs-lradc.c    |  666
> >   ++++++++++++++++++++++++++++++++ 4 files changed, 700 insertions(+), 4
> >   deletions(-)
> >   create mode 100644 drivers/staging/iio/adc/mxs-lradc.c
> > 
> > NOTE: The quality of code is still really crap here, I'm more interested
> > in knowing whether I'm going in the right direction about this.
> 
> I may well point out stuff you'd fix anyway, but its easier for me that
> way ;)
> 
> Anyhow, having taken a look I like the general approach.  The 'claim'
> logic is nice and clean.

Really? Or what that irony ? ;-)

> One nasty bit I hadn't realized is that you get separate interrupts per
> channel. This is going
> to make the data aggregation and demuxing used with buffers interesting
> if you add that
> support.
> 
> So thinking ahead, lets say you have 4 channels with oversampling as
> ax2,bx1,cx4,dx8

But the oversampling happens in hardware. And I get a single interrupt at the 
end of the sampling.

> It will interrupt on each channel as
> xxxxxxxx
>    a a a a a
> bbbbbbb
>      c    c
>          d
> Easiest option will be to build up this as a single described 'scan' and
> push that to the demux.
> Fiddly and if there is a big oversampling value, everything gets
> delayed.  Maybe we send scans
> with a flag indicating no data for some fields and demux accordingly.
> Probably reasonable to
> restrict a consumer driver to having only one oversampling so as far as
> they know it will come
> through at that lower rate.
> 
> Still all that stuff is for another day.  Your driver is fine in as far
> as it goes!

Great, now I can continue to adding touchscreen, temp etc.

Thanks!

> 
> > Thanks!
> > 
> > diff --git a/arch/arm/mach-mxs/mach-m28evk.c
> > b/arch/arm/mach-mxs/mach-m28evk.c index 2f27582..7a37d29 100644
> > --- a/arch/arm/mach-mxs/mach-m28evk.c
> > +++ b/arch/arm/mach-mxs/mach-m28evk.c
> > @@ -320,6 +320,31 @@ static struct mxs_mmc_platform_data
> > m28evk_mmc_pdata[] __initdata = {
> > 
> >   	},
> >   
> >   };
> > 
> > +#define	RES_IRQ(id, res)	{ .name = id, .start = (res), .end = 
(res),
> > .flags = IORESOURCE_IRQ } +
> > +static struct resource mxs_lradc_rsrc[] = {
> > +	[0] = {
> > +		.start	= 0x80050000,
> > +		.end	= 0x80050000 + SZ_8K - 1,
> > +		.flags	= IORESOURCE_MEM,
> > +	},
> > +	RES_IRQ("LRADC CH0", MX28_INT_LRADC_CH0),
> > +	RES_IRQ("LRADC CH1", MX28_INT_LRADC_CH1),
> > +	RES_IRQ("LRADC CH2", MX28_INT_LRADC_CH2),
> > +	RES_IRQ("LRADC CH3", MX28_INT_LRADC_CH3),
> > +	RES_IRQ("LRADC CH4", MX28_INT_LRADC_CH4),
> > +	RES_IRQ("LRADC CH5", MX28_INT_LRADC_CH5),
> > +	RES_IRQ("LRADC CH6", MX28_INT_LRADC_CH6),
> > +	RES_IRQ("LRADC CH7", MX28_INT_LRADC_CH7),
> > +};
> > +
> > +static struct platform_device mxs_lradc = {
> > +	.name		= "mxs-lradc",
> > +	.id		= -1,
> > +	.resource	= mxs_lradc_rsrc,
> > +	.num_resources	= ARRAY_SIZE(mxs_lradc_rsrc),
> > +};
> > +
> > 
> >   static void __init m28evk_init(void)
> >   {
> >   
> >   	mxs_iomux_setup_multiple_pads(m28evk_pads, ARRAY_SIZE(m28evk_pads));
> > 
> > @@ -347,6 +372,8 @@ static void __init m28evk_init(void)
> > 
> >   	mx28_add_mxs_i2c(0);
> >   	i2c_register_board_info(0, m28_stk5v3_i2c_boardinfo,
> >   	
> >   			ARRAY_SIZE(m28_stk5v3_i2c_boardinfo));
> > 
> > +
> > +	platform_device_register(&mxs_lradc);
> > 
> >   }
> >   
> >   static void __init m28evk_timer_init(void)
> > 
> > diff --git a/drivers/staging/iio/adc/Kconfig
> > b/drivers/staging/iio/adc/Kconfig index d9decea..7f33032 100644
> > --- a/drivers/staging/iio/adc/Kconfig
> > +++ b/drivers/staging/iio/adc/Kconfig
> > @@ -193,4 +193,14 @@ config MAX1363_RING_BUFFER
> > 
> >   	  Say yes here to include ring buffer support in the MAX1363
> >   	  ADC driver.
> > 
> > +config MXS_LRADC
> > +	tristate "Freescale i.MX23/i.MX28 LRADC"
> > +	depends on ARCH_MXS
> > +	help
> > +	  Say yes here to build support for i.MX23/i.MX28 LRADC convertor
> > +	  built into these chips.
> > +
> > +	  To compile this driver as a module, choose M here: the
> > +	  module will be called mxs-lradc.
> > +
> > 
> >   endmenu
> > 
> > diff --git a/drivers/staging/iio/adc/Makefile
> > b/drivers/staging/iio/adc/Makefile index ceee7f3..2d764ec 100644
> > --- a/drivers/staging/iio/adc/Makefile
> > +++ b/drivers/staging/iio/adc/Makefile
> > @@ -37,3 +37,4 @@ obj-$(CONFIG_AD7192) += ad7192.o
> > 
> >   obj-$(CONFIG_ADT7310) += adt7310.o
> >   obj-$(CONFIG_ADT7410) += adt7410.o
> >   obj-$(CONFIG_AD7280) += ad7280a.o
> > 
> > +obj-$(CONFIG_MXS_LRADC) += mxs-lradc.o
> > diff --git a/drivers/staging/iio/adc/mxs-lradc.c
> > b/drivers/staging/iio/adc/mxs-lradc.c new file mode 100644
> > index 0000000..da50088
> > --- /dev/null
> > +++ b/drivers/staging/iio/adc/mxs-lradc.c
> > @@ -0,0 +1,666 @@
> > +/*
> > + * ADT7410 digital temperature sensor driver supporting ADT7410
> > + *
> > + * Copyright 2010 Analog Devices Inc.
> 
> Obvious bit to fix here ;)  Doesn't bode that well that you have the
> copyright
> notice for a driver I've been trying to get the heck out of IIO.
> *fingers crossed*

Heh, thanks for pointing this out.

> 
> > + *
> > + * Licensed under the GPL-2 or later.
> > + */
> > +
> > +#include<linux/interrupt.h>
> > +#include<linux/device.h>
> > +#include<linux/kernel.h>
> > +#include<linux/slab.h>
> > +#include<linux/sysfs.h>
> > +#include<linux/list.h>
> > +#include<linux/io.h>
> > +#include<linux/module.h>
> > +#include<linux/platform_device.h>
> > +#include<linux/spinlock.h>
> > +#include<linux/wait.h>
> > +#include<linux/sched.h>
> > +
> > +#include<mach/mxs.h>
> > +#include<mach/common.h>
> > +
> > +#include "../iio.h"
> > +#include "../sysfs.h"
> > +
> > +#define	MAPPING_USED		(1<<  7)
> > +#define	MAPPING_XXX		(1<<  6)
> > +#define	MAPPING_SET_SLOT(m)	(((m)&  0x3)<<  4)
> > +#define	MAPPING_GET_SLOT(m)	(((m)>>  4)&  0x3)
> > +#define	MAPPING_CHAN(m)		((m)&  0xf)
> > +
> > +struct mxs_lradc_mapped_channel {
> > +	wait_queue_head_t		wq;
> > +	bool				wq_done;
> > +	uint16_t			chan_data;
> > +	uint16_t			mapping;
> > +	uint32_t			users;
> > +};
> > +
> > +struct mxs_lradc_drv_data {
> > +	struct iio_dev			*iio[4];
> > +	void __iomem			*mmio_base;
> > +
> > +	spinlock_t			lock;
> > +
> > +	uint16_t			claimed;
> > +
> > +	struct mxs_lradc_mapped_channel	ch[8];
> 
> yikes. Individual oversample controls. I was assuming these were per
> sample trigger source.
> That's going to make for some fiddly data management when you add
> buffers.  We'll
> have to define the ordering of data very carefully and the demux unit
> will need adapting to
> handle this.  Fun fun fun.  (note the demux unit becomes very important
> for in kernel consumers
> of iio buffered data).  I really need to get that code out there in a
> more up to date form.
>   Now I think Greg has been talked round to taking the 'pull interface'
> set we can hopefully make
> real progress on that front.
> 
> I'm never sure if I like 'interesting' hardware or if it just gives me a
> headache!

Right!
> 
> > +	uint8_t				ch_oversample[16];
> > +};
> > +
> > +struct mxs_lradc_data {
> > +	struct mxs_lradc_drv_data	*drv_data;
> > +	int				id;
> > +	spinlock_t			lock;
> > +
> > +	uint16_t			claimed;
> > +
> > +	uint32_t			loop_interval;
> > +};
> > +
> > +
> > +#define	LRADC_CH(n)				(0x50 + (0x10 * (n)))
> > +#define	LRADC_CH_ACCUMULATE			(1<<  29)
> > +#define	LRADC_CH_NUM_SAMPLES_MASK		(0x1f<<  24)
> > +#define	LRADC_CH_NUM_SAMPLES_OFFSET		24
> > +#define	LRADC_CH_VALUE_MASK			0x3ffff
> > +#define	LRADC_CH_VALUE_OFFSET			0
> > +
> > +#define	LRADC_DELAY(n)				(0xd0 + (0x10 * (n)))
> > +#define	LRADC_DELAY_TRIGGER_LRADCS_MASK		(0xff<<  24)
> > +#define	LRADC_DELAY_TRIGGER_LRADCS_OFFSET	24
> > +#define	LRADC_DELAY_KICK			(1<<  20)
> > +#define	LRADC_DELAY_TRIGGER_DELAYS_MASK		(0xf<<  16)
> > +#define	LRADC_DELAY_TRIGGER_DELAYS_OFFSET	16
> > +#define	LRADC_DELAY_LOOP_COUNT_MASK		(0x1f<<  11)
> > +#define	LRADC_DELAY_LOOP_COUNT_OFFSET		11
> > +#define	LRADC_DELAY_DELAY_MASK			0x7ff
> > +#define	LRADC_DELAY_DELAY_OFFSET		0
> > +
> > +#define	LRADC_CTRL0				0x00
> > +
> > +#define	LRADC_CTRL1				0x10
> > +#define	LRADC_CTRL1_LRADC_IRQ(n)		(1<<  (n))
> > +#define	LRADC_CTRL1_LRADC_IRQ_EN(n)		(1<<  ((n) + 16))
> > +
> > +#define	LRADC_CTRL2				0x20
> > +#define	LRADC_CTRL2_TEMPSENSE_PWD		(1<<  15)
> > +
> > +#define	LRADC_CTRL3				0x30
> > +
> > +#define	LRADC_CTRL4				0x140
> > +#define	LRADC_CTRL4_LRADCSELECT_MASK(n)		(0xf<<  ((n) * 4))
> > +#define	LRADC_CTRL4_LRADCSELECT_OFFSET(n)	((n) * 4)

The mess above will go into separate include.

> > +
> > +/*
> > + * Global IIO attributes
> > + */
> > +static ssize_t mxs_lradc_show_sampling_rate(struct device *dev,
> > +			struct device_attribute *attr, char *buf)
> > +{
> > +	struct iio_dev *iio_dev = dev_get_drvdata(dev);
> > +	struct mxs_lradc_data *data = iio_priv(iio_dev);
> > +
> > +	return sprintf(buf, "%u\n", data->loop_interval);
> > +}
> > +
> > +static ssize_t mxs_lradc_store_sampling_rate(struct device *dev,
> > +		struct device_attribute *attr, const char *buf, size_t count)
> > +{
> > +	struct iio_dev *iio_dev = dev_get_drvdata(dev);
> > +	struct mxs_lradc_data *data = iio_priv(iio_dev);
> > +	struct mxs_lradc_drv_data *drv_data = data->drv_data;
> > +	uint32_t reg;
> > +	unsigned long lval;
> > +	unsigned long lflags;
> > +
> > +	if (strict_strtoul(buf, 10,&lval))
> > +		return -EINVAL;
> > +
> > +	/* Check if the value is in requested range */
> > +	if (lval>= 2048)
> > +		return -EINVAL;
> > +
> > +	/*
> > +	 * Noone is accessing our delay trigger in the LRADC reg space so we
> > +	 * don't need to claim top-level spinlock.
> > +	 */
> > +	spin_lock_irqsave(&data->lock, lflags);
> > +
> > +	if (data->loop_interval != lval) {
> > +		data->loop_interval = lval;
> > +
> > +		/* Update the delay channel */
> > +		reg = readl(drv_data->mmio_base + LRADC_DELAY(data->id));
> > +		reg&= ~LRADC_DELAY_DELAY_MASK;
> > +		reg |= data->loop_interval;
> > +		writel(reg, drv_data->mmio_base + LRADC_DELAY(data->id));
> > +	}
> > +
> > +	spin_unlock_irqrestore(&data->lock, lflags);
> > +
> > +	return count;
> > +}
> > +
> > +static IIO_DEVICE_ATTR(sampling_rate, S_IRUGO | S_IWUSR,
> > +		       mxs_lradc_show_sampling_rate,
> > +		       mxs_lradc_store_sampling_rate, 0);
> 
> Hmm..  Not abi compliant, we probably do need to come up with a format
> for specifying
> such range restrictions. Also note sampling rate is always in hz.  This
> will require
> some fixed point fiddling but the interface has to be predictable.

Yep, I didn't know how to actually handle this. Since this configuration is per-
iio device, unlike the channel oversampling. Any pointers welcome.

> 
> > +static IIO_CONST_ATTR(sampling_rate_available,
> > +			"0...2047 (in 1/2000 second steps)");
> > +
> > +static struct attribute *mxs_lradc_attributes[] = {
> > +	&iio_dev_attr_sampling_rate.dev_attr.attr,
> > +	&iio_const_attr_sampling_rate_available.dev_attr.attr,
> > +	NULL,
> > +};
> > +
> > +static int mxs_lradc_can_claim_channel(struct iio_dev *iio_dev,
> > +			const struct iio_chan_spec *chan)
> > +{
> > +	struct mxs_lradc_data *data = iio_priv(iio_dev);
> > +	struct mxs_lradc_drv_data *drv_data = data->drv_data;
> > +	int i, count = 0;
> > +
> > +	/* The channel is already claimed by us. */
> > +	if (data->claimed&  (1<<  chan->address))
> > +		return 0;
> > +
> > +	/* Check if someone else didn't claim this */
> > +	if (drv_data->claimed&  (1<<  chan->address))
> > +		return -EINVAL;
> > +
> > +	for (i = 0; i<  16; i++)
> > +		if (drv_data->claimed&  (1<<  i))
> > +			count++;
> > +
> > +	/* Too many channels claimed */
> > +	if (count == 8)
> > +		return -EINVAL;
> > +
> > +	return 0;
> > +}
> > +
> > +static int mxs_lradc_claim_channel(struct iio_dev *iio_dev,
> > +			const struct iio_chan_spec *chan)
> > +{
> > +	struct mxs_lradc_data *data = iio_priv(iio_dev);
> > +	struct mxs_lradc_drv_data *drv_data = data->drv_data;
> > +	int i, ret;
> > +	unsigned long gflags;
> > +	uint32_t overspl = 0;
> > +
> > +	spin_lock_irqsave(&drv_data->lock, gflags);
> > +
> > +	ret = mxs_lradc_can_claim_channel(iio_dev, chan);
> > +	if (ret)
> > +		goto err;
> > +
> > +	/* Claim the channel */
> > +	drv_data->claimed |= 1<<  chan->address;
> > +	data->claimed |= 1<<  chan->address;
> > +
> > +	/* Map the channel */
> > +	for (i = 0; i<  8; i++)
> > +		if (!(drv_data->ch[i].mapping&  MAPPING_USED))
> > +			break;
> > +
> > +	drv_data->ch[i].mapping = MAPPING_USED |
> > +				MAPPING_SET_SLOT(data->id) |
> > +				MAPPING_CHAN(chan->address);
> > +
> > +	/* Setup the mapping */
> > +	__mxs_clrl(LRADC_CTRL4_LRADCSELECT_MASK(i),
> > +		drv_data->mmio_base + LRADC_CTRL4);
> > +	__mxs_setl(chan->address<<  LRADC_CTRL4_LRADCSELECT_OFFSET(i),
> > +		drv_data->mmio_base + LRADC_CTRL4);
> > +
> > +	spin_unlock_irqrestore(&drv_data->lock, gflags);
> > +
> > +	overspl =
> > +		((drv_data->ch_oversample[chan->address] ? 1 : 0)
> > +			* LRADC_CH_ACCUMULATE) |
> > +		(drv_data->ch_oversample[chan->address]
> > +			<<  LRADC_CH_NUM_SAMPLES_OFFSET);
> > +	writel(overspl, drv_data->mmio_base  + LRADC_CH(i));
> > +
> > +	/* Enable IRQ on the channel */
> > +	__mxs_clrl(LRADC_CTRL1_LRADC_IRQ(i),
> > +		drv_data->mmio_base + LRADC_CTRL1);
> > +	__mxs_setl(LRADC_CTRL1_LRADC_IRQ_EN(i),
> > +		drv_data->mmio_base + LRADC_CTRL1);
> > +
> > +	/* Set out channel to be triggers by this delay queue */
> > +	__mxs_setl(1<<  (LRADC_DELAY_TRIGGER_LRADCS_OFFSET + i),
> > +		drv_data->mmio_base + LRADC_DELAY(data->id));
> > +
> > +	return 0;
> > +
> > +err:
> > +	spin_unlock_irqrestore(&drv_data->lock, gflags);
> > +	return -EINVAL;
> > +}
> > +
> > +static void mxs_lradc_relinquish_channel(struct iio_dev *iio_dev,
> > +			const struct iio_chan_spec *chan, int chanidx)
> > +{
> > +	struct mxs_lradc_data *data = iio_priv(iio_dev);
> > +	struct mxs_lradc_drv_data *drv_data = data->drv_data;
> > +	unsigned long gflags;
> > +	int i;
> > +
> > +	drv_data->ch[chanidx].users--;
> > +	if (drv_data->ch[chanidx].users == 0) {
> > +		/* No more users for this channel, stop generating interrupts */
> > +		__mxs_setl(LRADC_CTRL1_LRADC_IRQ(i) |
> > +			LRADC_CTRL1_LRADC_IRQ_EN(i),
> > +			drv_data->mmio_base + LRADC_CTRL1);
> > +
> > +		/* Don't trigger this channel */
> > +		__mxs_clrl(1<<  (LRADC_DELAY_TRIGGER_LRADCS_OFFSET + i),
> > +			drv_data->mmio_base + LRADC_DELAY(data->id));
> > +
> > +		spin_lock_irqsave(&drv_data->lock, gflags);
> > +
> > +		/* Relinquish this channel */
> > +		drv_data->claimed&= ~(1<<  chan->address);
> > +		data->claimed&= ~(1<<  chan->address);
> > +		drv_data->ch[chanidx].mapping = 0;
> > +
> > +		spin_unlock_irqrestore(&drv_data->lock, gflags);
> > +	}
> > +}
> > +
> > +/*
> > + * I/O operations
> > + */
> > +static int mxs_lradc_read_raw(struct iio_dev *iio_dev,
> > +			const struct iio_chan_spec *chan,
> > +			int *val, int *val2, long m)
> > +{
> > +	struct mxs_lradc_data *data = iio_priv(iio_dev);
> > +	struct mxs_lradc_drv_data *drv_data = data->drv_data;
> > +	unsigned long lflags;
> > +	int i, ret;
> > +
> > +	switch (m) {
> > +	case 0:
> > +		spin_lock_irqsave(&data->lock, lflags);
> > +
> 
> So we claim the channel for this IIO dev (which is one of the 4 timed
> triggers).
> Then wait for the next sample.

Correct

> Guess that does the job, though this is
> all faking
> a kind of polled device whereas this will fit much better as a interrupt
> driven
> device outputting via buffers (probably the next step once this is
> fine).

This is actually a good idea since the sampling is constantly running anyway and 
it'd allow for the data to be readily available without the delay.

> Note
> that we typically disable this sort of access if the device is doing
> interrupt driven
> reading.  I have been thinking about adding a buffer implementation that
> would
> provide just the last value and hence allowing this sort of single
> channel read, but
> that requires some infrastructure that hasn't merged yet.

Heh. Maybe I should add buffer support into the driver and simply push the stuff 
into the buffers. I'd need to study this.

> 
> > +		ret = mxs_lradc_claim_channel(iio_dev, chan);
> > +		if (ret) {
> > +			spin_unlock_irqrestore(&data->lock, lflags);
> > +			return ret;
> > +		}
> > +
> > +		/*
> > +		 * Once we are here, the channel is mapped by us already.
> > +		 * Find the mapping.
> > +		 */
> > +		for (i = 0; i<  8; i++) {
> > +			if (!(drv_data->ch[i].mapping&  MAPPING_USED))
> > +				continue;
> > +
> > +			if (MAPPING_CHAN(drv_data->ch[i].mapping) ==
> > +				chan->address)
> > +				break;
> > +		}
> > +
> > +		/* Wait until sampling is done */
> > +		drv_data->ch[i].wq_done = false;
> > +
> > +		drv_data->ch[i].users++;
> > +
> > +		spin_unlock_irqrestore(&data->lock, lflags);
> > +
> > +		ret = wait_event_interruptible(drv_data->ch[i].wq,
> > +					drv_data->ch[i].wq_done);
> > +		if (ret)
> > +			ret = -EINTR;
> > +		else {
> > +			*val = readl(drv_data->mmio_base + LRADC_CH(i))&
> > +					LRADC_CH_VALUE_MASK;
> > +			*val /= (drv_data->ch_oversample[chan->address] + 1);
> > +			ret = IIO_VAL_INT;
> > +		}
> > +
> > +		spin_lock_irqsave(&data->lock, lflags);
> > +
> > +		mxs_lradc_relinquish_channel(iio_dev, chan, i);
> > +
> > +		spin_unlock_irqrestore(&data->lock, lflags);
> > +
> > +		return ret;
> > +
> > +	case IIO_CHAN_INFO_OVERSAMPLE_COUNT:
> > +		*val = drv_data->ch_oversample[chan->address];
> > +		return IIO_VAL_INT;
> > +	}
> > +
> > +	return -EINVAL;
> > +}
> > +
> > +static int mxs_lradc_write_raw(struct iio_dev *iio_dev,
> > +			const struct iio_chan_spec *chan,
> > +			int val, int val2, long m)
> > +{
> > +	struct mxs_lradc_data *data = iio_priv(iio_dev);
> > +	struct mxs_lradc_drv_data *drv_data = data->drv_data;
> > +
> > +	switch (m) {
> > +	case IIO_CHAN_INFO_OVERSAMPLE_COUNT:
> > +		if ((val<= 0) || (val>= 32))
> > +			return -EINVAL;
> > +
> > +		drv_data->ch_oversample[chan->address] = val - 1;
> > +		return 0;
> > +	}
> > +
> > +	return -EINVAL;
> > +}
> > +
> > +static irqreturn_t mxs_lradc_handle_irq(int irq, void *data)
> > +{
> > +	struct mxs_lradc_drv_data *drv_data = data;
> > +	uint32_t reg = readl(drv_data->mmio_base + LRADC_CTRL1);
> > +	int i;
> > +
> > +	for (i = 0; i<  8; i++)
> > +		if (reg&  LRADC_CTRL1_LRADC_IRQ(i)) {
> > +			drv_data->ch[i].wq_done = true;
> > +			wake_up_interruptible(&drv_data->ch[i].wq);
> > +		}
> > +
> > +	__mxs_clrl(0xffff, drv_data->mmio_base + LRADC_CTRL1);
> > +
> > +	return IRQ_HANDLED;
> > +}
> > +
> > +static const struct iio_info mxs_lradc_iio_info = {
> > +	.driver_module		= THIS_MODULE,
> > +	.read_raw		= mxs_lradc_read_raw,
> > +	.write_raw		= mxs_lradc_write_raw,
> > +};
> > +
> 
> These all look good...  However, one thing you weren't to know is that
> we are trying
> to deprecate the IIO_CHAN macro.  It's become a bit of a maintenance
> nightmare
> given the underlying structure keeps getting more complex as new
> requirements
> turn up. Arnd warned me about this when I originally put it in, but it
> took me a
> while to realise how right he was!

Roger that, I can even add a much simpler one.
> 
> Either define a local macro filling just those fields that matter here or
> fill the structure directly.  Also you could leave out the IIO_ST stuff
> for now as it
> only means anything in a driver providing buffered access via the chrdev.
> Same is true of scan_index.

This is something I still don't clearly understand (and I know I'm asking for 
the third time or so).

How do I do continuous sampling (aka. opening a file and reading stuff from it)? 
Since if I do sysfs read, this triggers raw_read() which opens the device and 
closes it. So is there some /dev/ representation of the iio device or something?
> 
> > +static const struct iio_chan_spec mxs_lradc_chan_spec[] = {
> > +	[0] = IIO_CHAN(IIO_VOLTAGE, IIO_NO_MOD, 1, IIO_RAW, NULL, 0, 0,
> > +		IIO_CHAN_INFO_SCALE_SEPARATE_BIT |
> > +		IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT,
> > +		0, 0, IIO_ST('u', 18, 32, 0), 0),
> > +	[1] = IIO_CHAN(IIO_VOLTAGE, IIO_NO_MOD, 1, IIO_RAW, NULL, 1, 0,
> > +		IIO_CHAN_INFO_SCALE_SEPARATE_BIT |
> > +		IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT,
> > +		1, 1, IIO_ST('u', 18, 32, 0), 0),
> > +	[2] = IIO_CHAN(IIO_VOLTAGE, IIO_NO_MOD, 1, IIO_RAW, NULL, 2, 0,
> > +		IIO_CHAN_INFO_SCALE_SEPARATE_BIT |
> > +		IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT,
> > +		2, 2, IIO_ST('u', 18, 32, 0), 0),
> > +	[3] = IIO_CHAN(IIO_VOLTAGE, IIO_NO_MOD, 1, IIO_RAW, NULL, 3, 0,
> > +		IIO_CHAN_INFO_SCALE_SEPARATE_BIT |
> > +		IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT,
> > +		3, 3, IIO_ST('u', 18, 32, 0), 0),
> > +	[4] = IIO_CHAN(IIO_VOLTAGE, IIO_NO_MOD, 1, IIO_RAW, NULL, 4, 0,
> > +		IIO_CHAN_INFO_SCALE_SEPARATE_BIT |
> > +		IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT,
> > +		4, 4, IIO_ST('u', 18, 32, 0), 0),
> > +	[5] = IIO_CHAN(IIO_VOLTAGE, IIO_NO_MOD, 1, IIO_RAW, NULL, 5, 0,
> > +		IIO_CHAN_INFO_SCALE_SEPARATE_BIT |
> > +		IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT,
> > +		5, 5, IIO_ST('u', 18, 32, 0), 0),
> > +	[6] = IIO_CHAN(IIO_VOLTAGE, IIO_NO_MOD, 1, IIO_RAW, NULL, 6, 0,
> > +		IIO_CHAN_INFO_SCALE_SEPARATE_BIT |
> > +		IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT,
> > +		6, 6, IIO_ST('u', 18, 32, 0), 0),
> > +	/* VBATT */
> > +	[7] = IIO_CHAN(IIO_VOLTAGE, IIO_NO_MOD, 1, IIO_RAW, NULL, 7, 0,
> > +		IIO_CHAN_INFO_SCALE_SEPARATE_BIT |
> > +		IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT,
> > +		7, 7, IIO_ST('u', 18, 32, 0), 0),
> > +	/* Temp sense 0 */
> > +	[8] = IIO_CHAN(IIO_TEMP, IIO_NO_MOD, 1, IIO_RAW, NULL, 8, 0,
> > +		IIO_CHAN_INFO_SCALE_SEPARATE_BIT |
> > +		IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT,
> > +		8, 8, IIO_ST('u', 18, 32, 0), 0),
> > +	/* Temp sense 1 */
> > +	[9] = IIO_CHAN(IIO_TEMP, IIO_NO_MOD, 1, IIO_RAW, NULL, 9, 0,
> > +		IIO_CHAN_INFO_SCALE_SEPARATE_BIT |
> > +		IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT,
> > +		9, 9, IIO_ST('u', 18, 32, 0), 0),
> > +	/* VDDIO */
> > +	[10] = IIO_CHAN(IIO_VOLTAGE, IIO_NO_MOD, 1, IIO_RAW, NULL, 10, 0,
> > +		IIO_CHAN_INFO_SCALE_SEPARATE_BIT |
> > +		IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT,
> > +		10, 10, IIO_ST('u', 18, 32, 0), 0),
> > +	/* VTH */
> > +	[11] = IIO_CHAN(IIO_VOLTAGE, IIO_NO_MOD, 1, IIO_RAW, NULL, 11, 0,
> > +		IIO_CHAN_INFO_SCALE_SEPARATE_BIT |
> > +		IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT,
> > +		11, 11, IIO_ST('u', 18, 32, 0), 0),
> > +	/* VDDA */
> > +	[12] = IIO_CHAN(IIO_VOLTAGE, IIO_NO_MOD, 1, IIO_RAW, NULL, 12, 0,
> > +		IIO_CHAN_INFO_SCALE_SEPARATE_BIT |
> > +		IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT,
> > +		12, 12, IIO_ST('u', 18, 32, 0), 0),
> > +	/* VDDD */
> > +	[13] = IIO_CHAN(IIO_VOLTAGE, IIO_NO_MOD, 1, IIO_RAW, NULL, 13, 0,
> > +		IIO_CHAN_INFO_SCALE_SEPARATE_BIT |
> > +		IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT,
> > +		13, 13, IIO_ST('u', 18, 32, 0), 0),
> > +	/* VBG */
> > +	[14] = IIO_CHAN(IIO_VOLTAGE, IIO_NO_MOD, 1, IIO_RAW, NULL, 14, 0,
> > +		IIO_CHAN_INFO_SCALE_SEPARATE_BIT |
> > +		IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT,
> > +		14, 14, IIO_ST('u', 18, 32, 0), 0),
> > +	/* VDD5V */
> > +	[15] = IIO_CHAN(IIO_VOLTAGE, IIO_NO_MOD, 1, IIO_RAW, NULL, 15, 0,
> > +		IIO_CHAN_INFO_SCALE_SEPARATE_BIT |
> > +		IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT,
> > +		15, 15, IIO_ST('u', 18, 32, 0), 0),
> > +};
> > +
> > +static void mxs_lradc_config(struct mxs_lradc_drv_data *drv_data)
> > +{
> > +	/* FIXME */
> > +	int freq = 0x3;	/* 6MHz */
> > +	int onchip_ground_ref = 0;
> > +
> > +	int i;
> > +
> > +	mxs_reset_block(drv_data->mmio_base + LRADC_CTRL0);
> > +
> > +	if (onchip_ground_ref)
> > +		__mxs_setl(1<<  26, drv_data->mmio_base + LRADC_CTRL0);
> > +	else
> > +		__mxs_clrl(1<<  26, drv_data->mmio_base + LRADC_CTRL0);
> > +
> > +	__mxs_clrl(0x3<<  8, drv_data->mmio_base + LRADC_CTRL3);
> > +	__mxs_setl(freq, drv_data->mmio_base + LRADC_CTRL3);
> > +
> > +	/* The delay channels constantly retrigger themself */
> > +	for (i = 0; i<  4; i++)
> > +		__mxs_setl(LRADC_DELAY_KICK |
> > +			(1<<  (LRADC_DELAY_TRIGGER_DELAYS_OFFSET + i)) |
> > +			0x7ff,	/* FIXME */
> > +			drv_data->mmio_base + LRADC_DELAY(i));
> > +
> > +	/* Start temperature sensing */
> > +	writel(0, drv_data->mmio_base + LRADC_CTRL2);
> > +}
> > +
> > +/*static void mxs_lradc_config(struct mxs_lradc_pdata *pdata)
> > +{
> > +
> > +}
> > +*/
> > +static int __devinit mxs_lradc_probe(struct platform_device *pdev)
> > +{
> > +	struct mxs_lradc_data *data[4];
> > +	struct mxs_lradc_drv_data *drv_data;
> > +	struct iio_dev *iio;
> > +	struct resource *r;
> > +	int ret = 0;
> > +	int irq;
> > +	int i;
> > +
> > +	/*
> > +	 * DEVM management
> > +	 */
> > +	if (!devres_open_group(&pdev->dev, mxs_lradc_probe, GFP_KERNEL)) {
> > +		dev_err(&pdev->dev, "Can't open resource group\n");
> > +		goto err0;
> > +	}
> > +
> > +	drv_data = devm_kzalloc(&pdev->dev, sizeof(*drv_data), GFP_KERNEL);
> > +	if (!drv_data) {
> > +		dev_err(&pdev->dev, "Failed to allocate driver data\n");
> > +		ret = -ENOMEM;
> > +		goto err0;
> > +	}
> > +
> > +	spin_lock_init(&drv_data->lock);
> > +
> > +	/*
> > +	 * IIO ops
> > +	 */
> > +	for (i = 0; i<  4; i++) {
> > +		iio = iio_allocate_device(sizeof(*data));
> > +		if (!iio) {
> > +			dev_err(&pdev->dev,
> > +				"Failed to allocate IIO device %i\n", i);
> > +			ret = -ENOMEM;
> > +			goto err1;
> > +		}
> > +
> > +		iio->name = pdev->name;
> 
> You will probably want to add an id to that name to distinguish between
> the difference instances.
> Even better would be to have a text description of each one.
> 
> > +		iio->dev.parent =&pdev->dev;
> > +		iio->info =&mxs_lradc_iio_info;
> > +		iio->modes = INDIO_DIRECT_MODE;
> > +		/* Channels */
> > +		iio->channels = mxs_lradc_chan_spec;
> > +		iio->num_channels = ARRAY_SIZE(mxs_lradc_chan_spec);
> > +		iio->attrs = mxs_lradc_attributes;
> > +
> 
> Given data[i] is only used in here, why not just have one of them?

These are actually four distinct private data for four distinct iio-devs.

> 
> > +		data[i] = iio_priv(iio);
> > +		data[i]->drv_data = drv_data;
> > +		data[i]->id = i;
> > +
> > +		spin_lock_init(&data[i]->lock);
> > +
> > +		drv_data->iio[i] = iio;
> > +	}
> > +
> > +	for (i = 0; i<  8; i++)
> > +		init_waitqueue_head(&drv_data->ch[i].wq);
> > +
> > +	dev_set_drvdata(&pdev->dev, drv_data);
> > +
> > +	/*
> > +	 * Allocate address space
> > +	 */
> > +	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +	if (r == NULL) {
> > +		dev_err(&pdev->dev, "No I/O memory resource defined\n");
> > +		ret = -ENODEV;
> > +		goto err1;
> > +	}
> > +
> > +	r = devm_request_mem_region(&pdev->dev, r->start,
> > +				resource_size(r), pdev->name);
> > +	if (r == NULL) {
> > +		dev_err(&pdev->dev, "Failed to request I/O memory\n");
> > +		ret = -EBUSY;
> > +		goto err1;
> > +	}
> > +
> > +	drv_data->mmio_base = devm_ioremap(&pdev->dev, r->start,
> > resource_size(r)); +	if (!drv_data->mmio_base) {
> > +		dev_err(&pdev->dev, "Failed to map I/O memory\n");
> > +		ret = -EBUSY;
> > +		goto err1;
> > +	}
> > +
> > +	/*
> > +	 * Allocate IRQ
> > +	 */
> > +	for (irq = 0; irq<  8; irq++) {
> > +		r = platform_get_resource(pdev, IORESOURCE_IRQ, irq);
> > +		if (r == NULL) {
> > +			dev_err(&pdev->dev, "No IRQ resource defined\n");
> > +			ret = -ENODEV;
> > +			goto err1;
> > +		}
> > +
> > +		ret = request_irq(r->start, mxs_lradc_handle_irq, 0, r->name,
> > drv_data); +		if (ret) {
> > +			dev_err(&pdev->dev, "request_irq %i failed: %d\n", irq, 
ret);
> > +			ret = -EBUSY;
> > +			goto err1;
> > +		}
> > +	}
> > +
> > +	/*
> > +	 * Register IIO device
> > +	 */
> > +	for (i = 0; i<  4; i++) {
> > +		ret = iio_device_register(drv_data->iio[i]);
> > +		if (ret) {
> > +			dev_err(&pdev->dev, "Failed to register IIO device\n");
> > +			ret = -EBUSY;
> > +			goto err2;
> > +		}
> > +	}
> > +
> > +	devres_remove_group(&pdev->dev, mxs_lradc_probe);
> > +
> > +	mxs_lradc_config(drv_data);
> > +
> > +	return 0;
> > +
> > +err2:
> > +	while (--i>= 0)
> > +
> 
> Clearing out irqs?

Missed that one with all the devm stuff, thanks :)

> 
> > 		iio_device_unregister(drv_data->iio[i]);
> > 
> > +err1:
> > +	for (i = 0; i<  4; i++)
> > +		if (drv_data->iio[i])
> > +			iio_free_device(drv_data->iio[i]);
> > +err0:
> > +	devres_release_group(&pdev->dev, mxs_lradc_probe);
> > +	return ret;
> > +}
> > +
> > +static int __devexit mxs_lradc_remove(struct platform_device *pdev)
> > +{
> > +	struct mxs_lradc_drv_data *drv_data = dev_get_drvdata(&pdev->dev);
> > +	struct resource *r;
> > +	int i, irq;
> > +
> 
>      Obvious but use ARRAY_SIZE(drv_data->iio) instead of 4.

This stupidity is not present only here. The driver is still full of arbitrary 
constants and crud.

Thanks for the guidance!

M

> 
> > +	for (i = 0; i<  4; i++)
> > +		iio_device_unregister(drv_data->iio[i]);
> > +
> > +	for (irq = 0; irq<  8; irq++) {
> > +		r = platform_get_resource(pdev, IORESOURCE_IRQ, irq);
> > +		if (r != NULL)
> > +			free_irq(r->start, drv_data);
> > +	}
> > +
> > +	for (i = 0; i<  4; i++)
> > +		iio_free_device(drv_data->iio[i]);
> > +
> > +	return 0;
> > +}
> > +
> > +static struct platform_driver mxs_lradc_driver = {
> > +	.driver = {
> > +		.name = "mxs-lradc",
> > +	},
> > +	.probe = mxs_lradc_probe,
> > +	.remove = __devexit_p(mxs_lradc_remove),
> > +};
> > +
> > +module_platform_driver(mxs_lradc_driver);
> > +
> > +MODULE_AUTHOR("Marek Vasut<marek.vasut@gmail.com>");
> > +MODULE_DESCRIPTION("Freescale i.MX23/i.MX28 LRADC driver");
> > +MODULE_LICENSE("GPL v2");

Re: [PATCH 2/2] [RFC] Basic support for MX28 LRADC IIO interface

[Jonathan Cameron]

On 2/10/2012 11:17 AM, Marek Vasut wrote:
>> On 2/10/2012 5:52 AM, Marek Vasut wrote:
>>> Signed-off-by: Marek Vasut<marek.vasut@gmail.com>
>>> Cc: Wolfgang Denk<wd@denx.de>
>>> Cc: Stefano Babic<sbabic@denx.de>
>>> Cc: Fabio Estevam<festevam@gmail.com>
>>> ---
>>>
>>>    arch/arm/mach-mxs/mach-m28evk.c        |   35 ++-
>>>    drivers/staging/iio/adc/Kconfig        |   10 +
>>>    drivers/staging/iio/adc/Makefile       |    1 +
>>>    drivers/staging/iio/adc/mxs-lradc.c    |  666
>>>    ++++++++++++++++++++++++++++++++ 4 files changed, 700 insertions(+), 4
>>>    deletions(-)
>>>    create mode 100644 drivers/staging/iio/adc/mxs-lradc.c
>>>
>>> NOTE: The quality of code is still really crap here, I'm more interested
>>> in knowing whether I'm going in the right direction about this.
>> I may well point out stuff you'd fix anyway, but its easier for me that
>> way ;)
>>
>> Anyhow, having taken a look I like the general approach.  The 'claim'
>> logic is nice and clean.
> Really? Or what that irony ? ;-)
Logic is clean enough.  Never said anything about the implementation and 
the complexity of doing it ;)
>
>> One nasty bit I hadn't realized is that you get separate interrupts per
>> channel. This is going
>> to make the data aggregation and demuxing used with buffers interesting
>> if you add that
>> support.
>>
>> So thinking ahead, lets say you have 4 channels with oversampling as
>> ax2,bx1,cx4,dx8
> But the oversampling happens in hardware. And I get a single interrupt at the
> end of the sampling.
The problem with our buffered route is that it is done as one buffer per 
iio device.  This makes sense
from the point of view of most devices which have quick means of 
dumpling a whole 'scan' across the channels.
The snag here is you can get different numbers of readings from 
different channels even when driven by the
same underlying trigger signal.
>
>> It will interrupt on each channel as
>> xxxxxxxx
>>     a a a a a
>> bbbbbbb
>>       c    c
>>           d
>> Easiest option will be to build up this as a single described 'scan' and
>> push that to the demux.
>> Fiddly and if there is a big oversampling value, everything gets
>> delayed.  Maybe we send scans
>> with a flag indicating no data for some fields and demux accordingly.
>> Probably reasonable to
>> restrict a consumer driver to having only one oversampling so as far as
>> they know it will come
>> through at that lower rate.
>>
>> Still all that stuff is for another day.  Your driver is fine in as far
>> as it goes!
> Great, now I can continue to adding touchscreen, temp etc.
Yes. This would make an interesting test case for our various in kernel 
interfaces though I appreciate you may
not want to take that path as there will be a few corners requiring 
changes in the IIO core.
>
> Thanks!
>
>>> Thanks!
>>>
>>> diff --git a/arch/arm/mach-mxs/mach-m28evk.c
>>> b/arch/arm/mach-mxs/mach-m28evk.c index 2f27582..7a37d29 100644
>>> --- a/arch/arm/mach-mxs/mach-m28evk.c
>>> +++ b/arch/arm/mach-mxs/mach-m28evk.c
>>> @@ -320,6 +320,31 @@ static struct mxs_mmc_platform_data
>>> m28evk_mmc_pdata[] __initdata = {
>>>
>>>    	},
>>>
>>>    };
>>>
>>> +#define	RES_IRQ(id, res)	{ .name = id, .start = (res), .end =
> (res),
>>> .flags = IORESOURCE_IRQ } +
>>> +static struct resource mxs_lradc_rsrc[] = {
>>> +	[0] = {
>>> +		.start	= 0x80050000,
>>> +		.end	= 0x80050000 + SZ_8K - 1,
>>> +		.flags	= IORESOURCE_MEM,
>>> +	},
>>> +	RES_IRQ("LRADC CH0", MX28_INT_LRADC_CH0),
>>> +	RES_IRQ("LRADC CH1", MX28_INT_LRADC_CH1),
>>> +	RES_IRQ("LRADC CH2", MX28_INT_LRADC_CH2),
>>> +	RES_IRQ("LRADC CH3", MX28_INT_LRADC_CH3),
>>> +	RES_IRQ("LRADC CH4", MX28_INT_LRADC_CH4),
>>> +	RES_IRQ("LRADC CH5", MX28_INT_LRADC_CH5),
>>> +	RES_IRQ("LRADC CH6", MX28_INT_LRADC_CH6),
>>> +	RES_IRQ("LRADC CH7", MX28_INT_LRADC_CH7),
>>> +};
>>> +
>>> +static struct platform_device mxs_lradc = {
>>> +	.name		= "mxs-lradc",
>>> +	.id		= -1,
>>> +	.resource	= mxs_lradc_rsrc,
>>> +	.num_resources	= ARRAY_SIZE(mxs_lradc_rsrc),
>>> +};
>>> +
>>>
>>>    static void __init m28evk_init(void)
>>>    {
>>>
>>>    	mxs_iomux_setup_multiple_pads(m28evk_pads, ARRAY_SIZE(m28evk_pads));
>>>
>>> @@ -347,6 +372,8 @@ static void __init m28evk_init(void)
>>>
>>>    	mx28_add_mxs_i2c(0);
>>>    	i2c_register_board_info(0, m28_stk5v3_i2c_boardinfo,
>>>    	
>>>    			ARRAY_SIZE(m28_stk5v3_i2c_boardinfo));
>>>
>>> +
>>> +	platform_device_register(&mxs_lradc);
>>>
>>>    }
>>>
>>>    static void __init m28evk_timer_init(void)
>>>
>>> diff --git a/drivers/staging/iio/adc/Kconfig
>>> b/drivers/staging/iio/adc/Kconfig index d9decea..7f33032 100644
>>> --- a/drivers/staging/iio/adc/Kconfig
>>> +++ b/drivers/staging/iio/adc/Kconfig
>>> @@ -193,4 +193,14 @@ config MAX1363_RING_BUFFER
>>>
>>>    	  Say yes here to include ring buffer support in the MAX1363
>>>    	  ADC driver.
>>>
>>> +config MXS_LRADC
>>> +	tristate "Freescale i.MX23/i.MX28 LRADC"
>>> +	depends on ARCH_MXS
>>> +	help
>>> +	  Say yes here to build support for i.MX23/i.MX28 LRADC convertor
>>> +	  built into these chips.
>>> +
>>> +	  To compile this driver as a module, choose M here: the
>>> +	  module will be called mxs-lradc.
>>> +
>>>
>>>    endmenu
>>>
>>> diff --git a/drivers/staging/iio/adc/Makefile
>>> b/drivers/staging/iio/adc/Makefile index ceee7f3..2d764ec 100644
>>> --- a/drivers/staging/iio/adc/Makefile
>>> +++ b/drivers/staging/iio/adc/Makefile
>>> @@ -37,3 +37,4 @@ obj-$(CONFIG_AD7192) += ad7192.o
>>>
>>>    obj-$(CONFIG_ADT7310) += adt7310.o
>>>    obj-$(CONFIG_ADT7410) += adt7410.o
>>>    obj-$(CONFIG_AD7280) += ad7280a.o
>>>
>>> +obj-$(CONFIG_MXS_LRADC) += mxs-lradc.o
>>> diff --git a/drivers/staging/iio/adc/mxs-lradc.c
>>> b/drivers/staging/iio/adc/mxs-lradc.c new file mode 100644
>>> index 0000000..da50088
>>> --- /dev/null
>>> +++ b/drivers/staging/iio/adc/mxs-lradc.c
>>> @@ -0,0 +1,666 @@
>>> +/*
>>> + * ADT7410 digital temperature sensor driver supporting ADT7410
>>> + *
>>> + * Copyright 2010 Analog Devices Inc.
>> Obvious bit to fix here ;)  Doesn't bode that well that you have the
>> copyright
>> notice for a driver I've been trying to get the heck out of IIO.
>> *fingers crossed*
> Heh, thanks for pointing this out.
>
>>> + *
>>> + * Licensed under the GPL-2 or later.
>>> + */
>>> +
>>> +#include<linux/interrupt.h>
>>> +#include<linux/device.h>
>>> +#include<linux/kernel.h>
>>> +#include<linux/slab.h>
>>> +#include<linux/sysfs.h>
>>> +#include<linux/list.h>
>>> +#include<linux/io.h>
>>> +#include<linux/module.h>
>>> +#include<linux/platform_device.h>
>>> +#include<linux/spinlock.h>
>>> +#include<linux/wait.h>
>>> +#include<linux/sched.h>
>>> +
>>> +#include<mach/mxs.h>
>>> +#include<mach/common.h>
>>> +
>>> +#include "../iio.h"
>>> +#include "../sysfs.h"
>>> +
>>> +#define	MAPPING_USED		(1<<   7)
>>> +#define	MAPPING_XXX		(1<<   6)
>>> +#define	MAPPING_SET_SLOT(m)	(((m)&   0x3)<<   4)
>>> +#define	MAPPING_GET_SLOT(m)	(((m)>>   4)&   0x3)
>>> +#define	MAPPING_CHAN(m)		((m)&   0xf)
>>> +
>>> +struct mxs_lradc_mapped_channel {
>>> +	wait_queue_head_t		wq;
>>> +	bool				wq_done;
>>> +	uint16_t			chan_data;
>>> +	uint16_t			mapping;
>>> +	uint32_t			users;
>>> +};
>>> +
>>> +struct mxs_lradc_drv_data {
>>> +	struct iio_dev			*iio[4];
>>> +	void __iomem			*mmio_base;
>>> +
>>> +	spinlock_t			lock;
>>> +
>>> +	uint16_t			claimed;
>>> +
>>> +	struct mxs_lradc_mapped_channel	ch[8];
>> yikes. Individual oversample controls. I was assuming these were per
>> sample trigger source.
>> That's going to make for some fiddly data management when you add
>> buffers.  We'll
>> have to define the ordering of data very carefully and the demux unit
>> will need adapting to
>> handle this.  Fun fun fun.  (note the demux unit becomes very important
>> for in kernel consumers
>> of iio buffered data).  I really need to get that code out there in a
>> more up to date form.
>>    Now I think Greg has been talked round to taking the 'pull interface'
>> set we can hopefully make
>> real progress on that front.
>>
>> I'm never sure if I like 'interesting' hardware or if it just gives me a
>> headache!
> Right!
>>> +	uint8_t				ch_oversample[16];
>>> +};
>>> +
>>> +struct mxs_lradc_data {
>>> +	struct mxs_lradc_drv_data	*drv_data;
>>> +	int				id;
>>> +	spinlock_t			lock;
>>> +
>>> +	uint16_t			claimed;
>>> +
>>> +	uint32_t			loop_interval;
>>> +};
>>> +
>>> +
>>> +#define	LRADC_CH(n)				(0x50 + (0x10 * (n)))
>>> +#define	LRADC_CH_ACCUMULATE			(1<<   29)
>>> +#define	LRADC_CH_NUM_SAMPLES_MASK		(0x1f<<   24)
>>> +#define	LRADC_CH_NUM_SAMPLES_OFFSET		24
>>> +#define	LRADC_CH_VALUE_MASK			0x3ffff
>>> +#define	LRADC_CH_VALUE_OFFSET			0
>>> +
>>> +#define	LRADC_DELAY(n)				(0xd0 + (0x10 * (n)))
>>> +#define	LRADC_DELAY_TRIGGER_LRADCS_MASK		(0xff<<   24)
>>> +#define	LRADC_DELAY_TRIGGER_LRADCS_OFFSET	24
>>> +#define	LRADC_DELAY_KICK			(1<<   20)
>>> +#define	LRADC_DELAY_TRIGGER_DELAYS_MASK		(0xf<<   16)
>>> +#define	LRADC_DELAY_TRIGGER_DELAYS_OFFSET	16
>>> +#define	LRADC_DELAY_LOOP_COUNT_MASK		(0x1f<<   11)
>>> +#define	LRADC_DELAY_LOOP_COUNT_OFFSET		11
>>> +#define	LRADC_DELAY_DELAY_MASK			0x7ff
>>> +#define	LRADC_DELAY_DELAY_OFFSET		0
>>> +
>>> +#define	LRADC_CTRL0				0x00
>>> +
>>> +#define	LRADC_CTRL1				0x10
>>> +#define	LRADC_CTRL1_LRADC_IRQ(n)		(1<<   (n))
>>> +#define	LRADC_CTRL1_LRADC_IRQ_EN(n)		(1<<   ((n) + 16))
>>> +
>>> +#define	LRADC_CTRL2				0x20
>>> +#define	LRADC_CTRL2_TEMPSENSE_PWD		(1<<   15)
>>> +
>>> +#define	LRADC_CTRL3				0x30
>>> +
>>> +#define	LRADC_CTRL4				0x140
>>> +#define	LRADC_CTRL4_LRADCSELECT_MASK(n)		(0xf<<   ((n) * 4))
>>> +#define	LRADC_CTRL4_LRADCSELECT_OFFSET(n)	((n) * 4)
> The mess above will go into separate include.
Only do that it if it is needed elsewhere, otherwise it should just stay 
in here even if it looks a little messy.
>
>>> +
>>> +/*
>>> + * Global IIO attributes
>>> + */
>>> +static ssize_t mxs_lradc_show_sampling_rate(struct device *dev,
>>> +			struct device_attribute *attr, char *buf)
>>> +{
>>> +	struct iio_dev *iio_dev = dev_get_drvdata(dev);
>>> +	struct mxs_lradc_data *data = iio_priv(iio_dev);
>>> +
>>> +	return sprintf(buf, "%u\n", data->loop_interval);
>>> +}
>>> +
>>> +static ssize_t mxs_lradc_store_sampling_rate(struct device *dev,
>>> +		struct device_attribute *attr, const char *buf, size_t count)
>>> +{
>>> +	struct iio_dev *iio_dev = dev_get_drvdata(dev);
>>> +	struct mxs_lradc_data *data = iio_priv(iio_dev);
>>> +	struct mxs_lradc_drv_data *drv_data = data->drv_data;
>>> +	uint32_t reg;
>>> +	unsigned long lval;
>>> +	unsigned long lflags;
>>> +
>>> +	if (strict_strtoul(buf, 10,&lval))
>>> +		return -EINVAL;
>>> +
>>> +	/* Check if the value is in requested range */
>>> +	if (lval>= 2048)
>>> +		return -EINVAL;
>>> +
>>> +	/*
>>> +	 * Noone is accessing our delay trigger in the LRADC reg space so we
>>> +	 * don't need to claim top-level spinlock.
>>> +	 */
>>> +	spin_lock_irqsave(&data->lock, lflags);
>>> +
>>> +	if (data->loop_interval != lval) {
>>> +		data->loop_interval = lval;
>>> +
>>> +		/* Update the delay channel */
>>> +		reg = readl(drv_data->mmio_base + LRADC_DELAY(data->id));
>>> +		reg&= ~LRADC_DELAY_DELAY_MASK;
>>> +		reg |= data->loop_interval;
>>> +		writel(reg, drv_data->mmio_base + LRADC_DELAY(data->id));
>>> +	}
>>> +
>>> +	spin_unlock_irqrestore(&data->lock, lflags);
>>> +
>>> +	return count;
>>> +}
>>> +
>>> +static IIO_DEVICE_ATTR(sampling_rate, S_IRUGO | S_IWUSR,
>>> +		       mxs_lradc_show_sampling_rate,
>>> +		       mxs_lradc_store_sampling_rate, 0);
>> Hmm..  Not abi compliant, we probably do need to come up with a format
>> for specifying
>> such range restrictions. Also note sampling rate is always in hz.  This
>> will require
>> some fixed point fiddling but the interface has to be predictable.
> Yep, I didn't know how to actually handle this. Since this configuration is per-
> iio device, unlike the channel oversampling. Any pointers welcome.
Pretty much as you have done, just with stuff in hz.  Getting frequency 
control to a point where we can get to
it from client drivers is on the todo list, but right now what you have 
is the only way of doing it.
>
>>> +static IIO_CONST_ATTR(sampling_rate_available,
>>> +			"0...2047 (in 1/2000 second steps)");
>>> +
>>> +static struct attribute *mxs_lradc_attributes[] = {
>>> +	&iio_dev_attr_sampling_rate.dev_attr.attr,
>>> +	&iio_const_attr_sampling_rate_available.dev_attr.attr,
>>> +	NULL,
>>> +};
>>> +
>>> +static int mxs_lradc_can_claim_channel(struct iio_dev *iio_dev,
>>> +			const struct iio_chan_spec *chan)
>>> +{
>>> +	struct mxs_lradc_data *data = iio_priv(iio_dev);
>>> +	struct mxs_lradc_drv_data *drv_data = data->drv_data;
>>> +	int i, count = 0;
>>> +
>>> +	/* The channel is already claimed by us. */
>>> +	if (data->claimed&   (1<<   chan->address))
>>> +		return 0;
>>> +
>>> +	/* Check if someone else didn't claim this */
>>> +	if (drv_data->claimed&   (1<<   chan->address))
>>> +		return -EINVAL;
>>> +
>>> +	for (i = 0; i<   16; i++)
>>> +		if (drv_data->claimed&   (1<<   i))
>>> +			count++;
>>> +
>>> +	/* Too many channels claimed */
>>> +	if (count == 8)
>>> +		return -EINVAL;
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static int mxs_lradc_claim_channel(struct iio_dev *iio_dev,
>>> +			const struct iio_chan_spec *chan)
>>> +{
>>> +	struct mxs_lradc_data *data = iio_priv(iio_dev);
>>> +	struct mxs_lradc_drv_data *drv_data = data->drv_data;
>>> +	int i, ret;
>>> +	unsigned long gflags;
>>> +	uint32_t overspl = 0;
>>> +
>>> +	spin_lock_irqsave(&drv_data->lock, gflags);
>>> +
>>> +	ret = mxs_lradc_can_claim_channel(iio_dev, chan);
>>> +	if (ret)
>>> +		goto err;
>>> +
>>> +	/* Claim the channel */
>>> +	drv_data->claimed |= 1<<   chan->address;
>>> +	data->claimed |= 1<<   chan->address;
>>> +
>>> +	/* Map the channel */
>>> +	for (i = 0; i<   8; i++)
>>> +		if (!(drv_data->ch[i].mapping&   MAPPING_USED))
>>> +			break;
>>> +
>>> +	drv_data->ch[i].mapping = MAPPING_USED |
>>> +				MAPPING_SET_SLOT(data->id) |
>>> +				MAPPING_CHAN(chan->address);
>>> +
>>> +	/* Setup the mapping */
>>> +	__mxs_clrl(LRADC_CTRL4_LRADCSELECT_MASK(i),
>>> +		drv_data->mmio_base + LRADC_CTRL4);
>>> +	__mxs_setl(chan->address<<   LRADC_CTRL4_LRADCSELECT_OFFSET(i),
>>> +		drv_data->mmio_base + LRADC_CTRL4);
>>> +
>>> +	spin_unlock_irqrestore(&drv_data->lock, gflags);
>>> +
>>> +	overspl =
>>> +		((drv_data->ch_oversample[chan->address] ? 1 : 0)
>>> +			* LRADC_CH_ACCUMULATE) |
>>> +		(drv_data->ch_oversample[chan->address]
>>> +			<<   LRADC_CH_NUM_SAMPLES_OFFSET);
>>> +	writel(overspl, drv_data->mmio_base  + LRADC_CH(i));
>>> +
>>> +	/* Enable IRQ on the channel */
>>> +	__mxs_clrl(LRADC_CTRL1_LRADC_IRQ(i),
>>> +		drv_data->mmio_base + LRADC_CTRL1);
>>> +	__mxs_setl(LRADC_CTRL1_LRADC_IRQ_EN(i),
>>> +		drv_data->mmio_base + LRADC_CTRL1);
>>> +
>>> +	/* Set out channel to be triggers by this delay queue */
>>> +	__mxs_setl(1<<   (LRADC_DELAY_TRIGGER_LRADCS_OFFSET + i),
>>> +		drv_data->mmio_base + LRADC_DELAY(data->id));
>>> +
>>> +	return 0;
>>> +
>>> +err:
>>> +	spin_unlock_irqrestore(&drv_data->lock, gflags);
>>> +	return -EINVAL;
>>> +}
>>> +
>>> +static void mxs_lradc_relinquish_channel(struct iio_dev *iio_dev,
>>> +			const struct iio_chan_spec *chan, int chanidx)
>>> +{
>>> +	struct mxs_lradc_data *data = iio_priv(iio_dev);
>>> +	struct mxs_lradc_drv_data *drv_data = data->drv_data;
>>> +	unsigned long gflags;
>>> +	int i;
>>> +
>>> +	drv_data->ch[chanidx].users--;
>>> +	if (drv_data->ch[chanidx].users == 0) {
>>> +		/* No more users for this channel, stop generating interrupts */
>>> +		__mxs_setl(LRADC_CTRL1_LRADC_IRQ(i) |
>>> +			LRADC_CTRL1_LRADC_IRQ_EN(i),
>>> +			drv_data->mmio_base + LRADC_CTRL1);
>>> +
>>> +		/* Don't trigger this channel */
>>> +		__mxs_clrl(1<<   (LRADC_DELAY_TRIGGER_LRADCS_OFFSET + i),
>>> +			drv_data->mmio_base + LRADC_DELAY(data->id));
>>> +
>>> +		spin_lock_irqsave(&drv_data->lock, gflags);
>>> +
>>> +		/* Relinquish this channel */
>>> +		drv_data->claimed&= ~(1<<   chan->address);
>>> +		data->claimed&= ~(1<<   chan->address);
>>> +		drv_data->ch[chanidx].mapping = 0;
>>> +
>>> +		spin_unlock_irqrestore(&drv_data->lock, gflags);
>>> +	}
>>> +}
>>> +
>>> +/*
>>> + * I/O operations
>>> + */
>>> +static int mxs_lradc_read_raw(struct iio_dev *iio_dev,
>>> +			const struct iio_chan_spec *chan,
>>> +			int *val, int *val2, long m)
>>> +{
>>> +	struct mxs_lradc_data *data = iio_priv(iio_dev);
>>> +	struct mxs_lradc_drv_data *drv_data = data->drv_data;
>>> +	unsigned long lflags;
>>> +	int i, ret;
>>> +
>>> +	switch (m) {
>>> +	case 0:
>>> +		spin_lock_irqsave(&data->lock, lflags);
>>> +
>> So we claim the channel for this IIO dev (which is one of the 4 timed
>> triggers).
>> Then wait for the next sample.
> Correct
>
>> Guess that does the job, though this is
>> all faking
>> a kind of polled device whereas this will fit much better as a interrupt
>> driven
>> device outputting via buffers (probably the next step once this is
>> fine).
> This is actually a good idea since the sampling is constantly running anyway and
> it'd allow for the data to be readily available without the delay.
>
>> Note
>> that we typically disable this sort of access if the device is doing
>> interrupt driven
>> reading.  I have been thinking about adding a buffer implementation that
>> would
>> provide just the last value and hence allowing this sort of single
>> channel read, but
>> that requires some infrastructure that hasn't merged yet.
> Heh. Maybe I should add buffer support into the driver and simply push the stuff
> into the buffers. I'd need to study this.
As mentioned above, that would be easy without those pesky oversampling 
controls.
>
>>> +		ret = mxs_lradc_claim_channel(iio_dev, chan);
>>> +		if (ret) {
>>> +			spin_unlock_irqrestore(&data->lock, lflags);
>>> +			return ret;
>>> +		}
>>> +
>>> +		/*
>>> +		 * Once we are here, the channel is mapped by us already.
>>> +		 * Find the mapping.
>>> +		 */
>>> +		for (i = 0; i<   8; i++) {
>>> +			if (!(drv_data->ch[i].mapping&   MAPPING_USED))
>>> +				continue;
>>> +
>>> +			if (MAPPING_CHAN(drv_data->ch[i].mapping) ==
>>> +				chan->address)
>>> +				break;
>>> +		}
>>> +
>>> +		/* Wait until sampling is done */
>>> +		drv_data->ch[i].wq_done = false;
>>> +
>>> +		drv_data->ch[i].users++;
>>> +
>>> +		spin_unlock_irqrestore(&data->lock, lflags);
>>> +
>>> +		ret = wait_event_interruptible(drv_data->ch[i].wq,
>>> +					drv_data->ch[i].wq_done);
>>> +		if (ret)
>>> +			ret = -EINTR;
>>> +		else {
>>> +			*val = readl(drv_data->mmio_base + LRADC_CH(i))&
>>> +					LRADC_CH_VALUE_MASK;
>>> +			*val /= (drv_data->ch_oversample[chan->address] + 1);
>>> +			ret = IIO_VAL_INT;
>>> +		}
>>> +
>>> +		spin_lock_irqsave(&data->lock, lflags);
>>> +
>>> +		mxs_lradc_relinquish_channel(iio_dev, chan, i);
>>> +
>>> +		spin_unlock_irqrestore(&data->lock, lflags);
>>> +
>>> +		return ret;
>>> +
>>> +	case IIO_CHAN_INFO_OVERSAMPLE_COUNT:
>>> +		*val = drv_data->ch_oversample[chan->address];
>>> +		return IIO_VAL_INT;
>>> +	}
>>> +
>>> +	return -EINVAL;
>>> +}
>>> +
>>> +static int mxs_lradc_write_raw(struct iio_dev *iio_dev,
>>> +			const struct iio_chan_spec *chan,
>>> +			int val, int val2, long m)
>>> +{
>>> +	struct mxs_lradc_data *data = iio_priv(iio_dev);
>>> +	struct mxs_lradc_drv_data *drv_data = data->drv_data;
>>> +
>>> +	switch (m) {
>>> +	case IIO_CHAN_INFO_OVERSAMPLE_COUNT:
>>> +		if ((val<= 0) || (val>= 32))
>>> +			return -EINVAL;
>>> +
>>> +		drv_data->ch_oversample[chan->address] = val - 1;
>>> +		return 0;
>>> +	}
>>> +
>>> +	return -EINVAL;
>>> +}
>>> +
>>> +static irqreturn_t mxs_lradc_handle_irq(int irq, void *data)
>>> +{
>>> +	struct mxs_lradc_drv_data *drv_data = data;
>>> +	uint32_t reg = readl(drv_data->mmio_base + LRADC_CTRL1);
>>> +	int i;
>>> +
>>> +	for (i = 0; i<   8; i++)
>>> +		if (reg&   LRADC_CTRL1_LRADC_IRQ(i)) {
>>> +			drv_data->ch[i].wq_done = true;
>>> +			wake_up_interruptible(&drv_data->ch[i].wq);
>>> +		}
>>> +
>>> +	__mxs_clrl(0xffff, drv_data->mmio_base + LRADC_CTRL1);
>>> +
>>> +	return IRQ_HANDLED;
>>> +}
>>> +
>>> +static const struct iio_info mxs_lradc_iio_info = {
>>> +	.driver_module		= THIS_MODULE,
>>> +	.read_raw		= mxs_lradc_read_raw,
>>> +	.write_raw		= mxs_lradc_write_raw,
>>> +};
>>> +
>> These all look good...  However, one thing you weren't to know is that
>> we are trying
>> to deprecate the IIO_CHAN macro.  It's become a bit of a maintenance
>> nightmare
>> given the underlying structure keeps getting more complex as new
>> requirements
>> turn up. Arnd warned me about this when I originally put it in, but it
>> took me a
>> while to realise how right he was!
> Roger that, I can even add a much simpler one.
>> Either define a local macro filling just those fields that matter here or
>> fill the structure directly.  Also you could leave out the IIO_ST stuff
>> for now as it
>> only means anything in a driver providing buffered access via the chrdev.
>> Same is true of scan_index.
> This is something I still don't clearly understand (and I know I'm asking for
> the third time or so).
>
> How do I do continuous sampling (aka. opening a file and reading stuff from it)?
> Since if I do sysfs read, this triggers raw_read() which opens the device and
> closes it. So is there some /dev/ representation of the iio device or something?
Yes. If you register buffers you get such a chrdev.  But note that the 
data coming out is raw scans
across the channels.  All the IIO_ST stuff in the chan_spec and 
scan_index values etc are about
describing to userspace what it is getting.   Take a look at the example 
userspace code in
the documentation directory. That is probably the best way of 
understanding the outputs.
>>> +static const struct iio_chan_spec mxs_lradc_chan_spec[] = {
>>> +	[0] = IIO_CHAN(IIO_VOLTAGE, IIO_NO_MOD, 1, IIO_RAW, NULL, 0, 0,
>>> +		IIO_CHAN_INFO_SCALE_SEPARATE_BIT |
>>> +		IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT,
>>> +		0, 0, IIO_ST('u', 18, 32, 0), 0),
>>> +	[1] = IIO_CHAN(IIO_VOLTAGE, IIO_NO_MOD, 1, IIO_RAW, NULL, 1, 0,
>>> +		IIO_CHAN_INFO_SCALE_SEPARATE_BIT |
>>> +		IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT,
>>> +		1, 1, IIO_ST('u', 18, 32, 0), 0),
>>> +	[2] = IIO_CHAN(IIO_VOLTAGE, IIO_NO_MOD, 1, IIO_RAW, NULL, 2, 0,
>>> +		IIO_CHAN_INFO_SCALE_SEPARATE_BIT |
>>> +		IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT,
>>> +		2, 2, IIO_ST('u', 18, 32, 0), 0),
>>> +	[3] = IIO_CHAN(IIO_VOLTAGE, IIO_NO_MOD, 1, IIO_RAW, NULL, 3, 0,
>>> +		IIO_CHAN_INFO_SCALE_SEPARATE_BIT |
>>> +		IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT,
>>> +		3, 3, IIO_ST('u', 18, 32, 0), 0),
>>> +	[4] = IIO_CHAN(IIO_VOLTAGE, IIO_NO_MOD, 1, IIO_RAW, NULL, 4, 0,
>>> +		IIO_CHAN_INFO_SCALE_SEPARATE_BIT |
>>> +		IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT,
>>> +		4, 4, IIO_ST('u', 18, 32, 0), 0),
>>> +	[5] = IIO_CHAN(IIO_VOLTAGE, IIO_NO_MOD, 1, IIO_RAW, NULL, 5, 0,
>>> +		IIO_CHAN_INFO_SCALE_SEPARATE_BIT |
>>> +		IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT,
>>> +		5, 5, IIO_ST('u', 18, 32, 0), 0),
>>> +	[6] = IIO_CHAN(IIO_VOLTAGE, IIO_NO_MOD, 1, IIO_RAW, NULL, 6, 0,
>>> +		IIO_CHAN_INFO_SCALE_SEPARATE_BIT |
>>> +		IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT,
>>> +		6, 6, IIO_ST('u', 18, 32, 0), 0),
>>> +	/* VBATT */
>>> +	[7] = IIO_CHAN(IIO_VOLTAGE, IIO_NO_MOD, 1, IIO_RAW, NULL, 7, 0,
>>> +		IIO_CHAN_INFO_SCALE_SEPARATE_BIT |
>>> +		IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT,
>>> +		7, 7, IIO_ST('u', 18, 32, 0), 0),
>>> +	/* Temp sense 0 */
>>> +	[8] = IIO_CHAN(IIO_TEMP, IIO_NO_MOD, 1, IIO_RAW, NULL, 8, 0,
>>> +		IIO_CHAN_INFO_SCALE_SEPARATE_BIT |
>>> +		IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT,
>>> +		8, 8, IIO_ST('u', 18, 32, 0), 0),
>>> +	/* Temp sense 1 */
>>> +	[9] = IIO_CHAN(IIO_TEMP, IIO_NO_MOD, 1, IIO_RAW, NULL, 9, 0,
>>> +		IIO_CHAN_INFO_SCALE_SEPARATE_BIT |
>>> +		IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT,
>>> +		9, 9, IIO_ST('u', 18, 32, 0), 0),
>>> +	/* VDDIO */
>>> +	[10] = IIO_CHAN(IIO_VOLTAGE, IIO_NO_MOD, 1, IIO_RAW, NULL, 10, 0,
>>> +		IIO_CHAN_INFO_SCALE_SEPARATE_BIT |
>>> +		IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT,
>>> +		10, 10, IIO_ST('u', 18, 32, 0), 0),
>>> +	/* VTH */
>>> +	[11] = IIO_CHAN(IIO_VOLTAGE, IIO_NO_MOD, 1, IIO_RAW, NULL, 11, 0,
>>> +		IIO_CHAN_INFO_SCALE_SEPARATE_BIT |
>>> +		IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT,
>>> +		11, 11, IIO_ST('u', 18, 32, 0), 0),
>>> +	/* VDDA */
>>> +	[12] = IIO_CHAN(IIO_VOLTAGE, IIO_NO_MOD, 1, IIO_RAW, NULL, 12, 0,
>>> +		IIO_CHAN_INFO_SCALE_SEPARATE_BIT |
>>> +		IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT,
>>> +		12, 12, IIO_ST('u', 18, 32, 0), 0),
>>> +	/* VDDD */
>>> +	[13] = IIO_CHAN(IIO_VOLTAGE, IIO_NO_MOD, 1, IIO_RAW, NULL, 13, 0,
>>> +		IIO_CHAN_INFO_SCALE_SEPARATE_BIT |
>>> +		IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT,
>>> +		13, 13, IIO_ST('u', 18, 32, 0), 0),
>>> +	/* VBG */
>>> +	[14] = IIO_CHAN(IIO_VOLTAGE, IIO_NO_MOD, 1, IIO_RAW, NULL, 14, 0,
>>> +		IIO_CHAN_INFO_SCALE_SEPARATE_BIT |
>>> +		IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT,
>>> +		14, 14, IIO_ST('u', 18, 32, 0), 0),
>>> +	/* VDD5V */
>>> +	[15] = IIO_CHAN(IIO_VOLTAGE, IIO_NO_MOD, 1, IIO_RAW, NULL, 15, 0,
>>> +		IIO_CHAN_INFO_SCALE_SEPARATE_BIT |
>>> +		IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT,
>>> +		15, 15, IIO_ST('u', 18, 32, 0), 0),
>>> +};
>>> +
>>> +static void mxs_lradc_config(struct mxs_lradc_drv_data *drv_data)
>>> +{
>>> +	/* FIXME */
>>> +	int freq = 0x3;	/* 6MHz */
>>> +	int onchip_ground_ref = 0;
>>> +
>>> +	int i;
>>> +
>>> +	mxs_reset_block(drv_data->mmio_base + LRADC_CTRL0);
>>> +
>>> +	if (onchip_ground_ref)
>>> +		__mxs_setl(1<<   26, drv_data->mmio_base + LRADC_CTRL0);
>>> +	else
>>> +		__mxs_clrl(1<<   26, drv_data->mmio_base + LRADC_CTRL0);
>>> +
>>> +	__mxs_clrl(0x3<<   8, drv_data->mmio_base + LRADC_CTRL3);
>>> +	__mxs_setl(freq, drv_data->mmio_base + LRADC_CTRL3);
>>> +
>>> +	/* The delay channels constantly retrigger themself */
>>> +	for (i = 0; i<   4; i++)
>>> +		__mxs_setl(LRADC_DELAY_KICK |
>>> +			(1<<   (LRADC_DELAY_TRIGGER_DELAYS_OFFSET + i)) |
>>> +			0x7ff,	/* FIXME */
>>> +			drv_data->mmio_base + LRADC_DELAY(i));
>>> +
>>> +	/* Start temperature sensing */
>>> +	writel(0, drv_data->mmio_base + LRADC_CTRL2);
>>> +}
>>> +
>>> +/*static void mxs_lradc_config(struct mxs_lradc_pdata *pdata)
>>> +{
>>> +
>>> +}
>>> +*/
>>> +static int __devinit mxs_lradc_probe(struct platform_device *pdev)
>>> +{
>>> +	struct mxs_lradc_data *data[4];
>>> +	struct mxs_lradc_drv_data *drv_data;
>>> +	struct iio_dev *iio;
>>> +	struct resource *r;
>>> +	int ret = 0;
>>> +	int irq;
>>> +	int i;
>>> +
>>> +	/*
>>> +	 * DEVM management
>>> +	 */
>>> +	if (!devres_open_group(&pdev->dev, mxs_lradc_probe, GFP_KERNEL)) {
>>> +		dev_err(&pdev->dev, "Can't open resource group\n");
>>> +		goto err0;
>>> +	}
>>> +
>>> +	drv_data = devm_kzalloc(&pdev->dev, sizeof(*drv_data), GFP_KERNEL);
>>> +	if (!drv_data) {
>>> +		dev_err(&pdev->dev, "Failed to allocate driver data\n");
>>> +		ret = -ENOMEM;
>>> +		goto err0;
>>> +	}
>>> +
>>> +	spin_lock_init(&drv_data->lock);
>>> +
>>> +	/*
>>> +	 * IIO ops
>>> +	 */
>>> +	for (i = 0; i<   4; i++) {
>>> +		iio = iio_allocate_device(sizeof(*data));
>>> +		if (!iio) {
>>> +			dev_err(&pdev->dev,
>>> +				"Failed to allocate IIO device %i\n", i);
>>> +			ret = -ENOMEM;
>>> +			goto err1;
>>> +		}
>>> +
>>> +		iio->name = pdev->name;
>> You will probably want to add an id to that name to distinguish between
>> the difference instances.
>> Even better would be to have a text description of each one.
>>
>>> +		iio->dev.parent =&pdev->dev;
>>> +		iio->info =&mxs_lradc_iio_info;
>>> +		iio->modes = INDIO_DIRECT_MODE;
>>> +		/* Channels */
>>> +		iio->channels = mxs_lradc_chan_spec;
>>> +		iio->num_channels = ARRAY_SIZE(mxs_lradc_chan_spec);
>>> +		iio->attrs = mxs_lradc_attributes;
>>> +
>> Given data[i] is only used in here, why not just have one of them?
> These are actually four distinct private data for four distinct iio-devs.
Agreed. I was just observing that the local copy of iio_priv is only 
used here so you could
copy each one in turn into the same local variable rather than using 
different elements of
a local array of pointers.
>
>>> +		data[i] = iio_priv(iio);
>>> +		data[i]->drv_data = drv_data;
>>> +		data[i]->id = i;
>>> +
>>> +		spin_lock_init(&data[i]->lock);
>>> +
>>> +		drv_data->iio[i] = iio;
>>> +	}
>>> +
>>> +	for (i = 0; i<   8; i++)
>>> +		init_waitqueue_head(&drv_data->ch[i].wq);
>>> +
>>> +	dev_set_drvdata(&pdev->dev, drv_data);
>>> +
>>> +	/*
>>> +	 * Allocate address space
>>> +	 */
>>> +	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>> +	if (r == NULL) {
>>> +		dev_err(&pdev->dev, "No I/O memory resource defined\n");
>>> +		ret = -ENODEV;
>>> +		goto err1;
>>> +	}
>>> +
>>> +	r = devm_request_mem_region(&pdev->dev, r->start,
>>> +				resource_size(r), pdev->name);
>>> +	if (r == NULL) {
>>> +		dev_err(&pdev->dev, "Failed to request I/O memory\n");
>>> +		ret = -EBUSY;
>>> +		goto err1;
>>> +	}
>>> +
>>> +	drv_data->mmio_base = devm_ioremap(&pdev->dev, r->start,
>>> resource_size(r)); +	if (!drv_data->mmio_base) {
>>> +		dev_err(&pdev->dev, "Failed to map I/O memory\n");
>>> +		ret = -EBUSY;
>>> +		goto err1;
>>> +	}
>>> +
>>> +	/*
>>> +	 * Allocate IRQ
>>> +	 */
>>> +	for (irq = 0; irq<   8; irq++) {
>>> +		r = platform_get_resource(pdev, IORESOURCE_IRQ, irq);
>>> +		if (r == NULL) {
>>> +			dev_err(&pdev->dev, "No IRQ resource defined\n");
>>> +			ret = -ENODEV;
>>> +			goto err1;
>>> +		}
>>> +
>>> +		ret = request_irq(r->start, mxs_lradc_handle_irq, 0, r->name,
>>> drv_data); +		if (ret) {
>>> +			dev_err(&pdev->dev, "request_irq %i failed: %d\n", irq,
> ret);
>>> +			ret = -EBUSY;
>>> +			goto err1;
>>> +		}
>>> +	}
>>> +
>>> +	/*
>>> +	 * Register IIO device
>>> +	 */
>>> +	for (i = 0; i<   4; i++) {
>>> +		ret = iio_device_register(drv_data->iio[i]);
>>> +		if (ret) {
>>> +			dev_err(&pdev->dev, "Failed to register IIO device\n");
>>> +			ret = -EBUSY;
>>> +			goto err2;
>>> +		}
>>> +	}
>>> +
>>> +	devres_remove_group(&pdev->dev, mxs_lradc_probe);
>>> +
>>> +	mxs_lradc_config(drv_data);
>>> +
>>> +	return 0;
>>> +
>>> +err2:
>>> +	while (--i>= 0)
>>> +
>> Clearing out irqs?
> Missed that one with all the devm stuff, thanks :)
>
>>> 		iio_device_unregister(drv_data->iio[i]);
>>>
>>> +err1:
>>> +	for (i = 0; i<   4; i++)
>>> +		if (drv_data->iio[i])
>>> +			iio_free_device(drv_data->iio[i]);
>>> +err0:
>>> +	devres_release_group(&pdev->dev, mxs_lradc_probe);
>>> +	return ret;
>>> +}
>>> +
>>> +static int __devexit mxs_lradc_remove(struct platform_device *pdev)
>>> +{
>>> +	struct mxs_lradc_drv_data *drv_data = dev_get_drvdata(&pdev->dev);
>>> +	struct resource *r;
>>> +	int i, irq;
>>> +
>>       Obvious but use ARRAY_SIZE(drv_data->iio) instead of 4.
> This stupidity is not present only here. The driver is still full of arbitrary
> constants and crud.
>
> Thanks for the guidance!
>
> M
>
>>> +	for (i = 0; i<   4; i++)
>>> +		iio_device_unregister(drv_data->iio[i]);
>>> +
>>> +	for (irq = 0; irq<   8; irq++) {
>>> +		r = platform_get_resource(pdev, IORESOURCE_IRQ, irq);
>>> +		if (r != NULL)
>>> +			free_irq(r->start, drv_data);
>>> +	}
>>> +
>>> +	for (i = 0; i<   4; i++)
>>> +		iio_free_device(drv_data->iio[i]);
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static struct platform_driver mxs_lradc_driver = {
>>> +	.driver = {
>>> +		.name = "mxs-lradc",
>>> +	},
>>> +	.probe = mxs_lradc_probe,
>>> +	.remove = __devexit_p(mxs_lradc_remove),
>>> +};
>>> +
>>> +module_platform_driver(mxs_lradc_driver);
>>> +
>>> +MODULE_AUTHOR("Marek Vasut<marek.vasut@gmail.com>");
>>> +MODULE_DESCRIPTION("Freescale i.MX23/i.MX28 LRADC driver");
>>> +MODULE_LICENSE("GPL v2");

Re: [PATCH 1/2] [RFC] Add per-channel oversampling count

[Jonathan Cameron]

On 2/10/2012 5:52 AM, Marek Vasut wrote:
> This allows each channel to configure it's oversampling count.
Fine, but please also add documention in 
staging/iio/Documentation/sysfs-bus-iio

> Signed-off-by: Marek Vasut<marek.vasut@gmail.com>
> Cc: Wolfgang Denk<wd@denx.de>
> Cc: Stefano Babic<sbabic@denx.de>
> Cc: Fabio Estevam<festevam@gmail.com>
> ---
>   drivers/staging/iio/iio.h               |    5 +++++
>   drivers/staging/iio/industrialio-core.c |    1 +
>   2 files changed, 6 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/staging/iio/iio.h b/drivers/staging/iio/iio.h
> index b3a1740..0b626ae 100644
> --- a/drivers/staging/iio/iio.h
> +++ b/drivers/staging/iio/iio.h
> @@ -36,6 +36,7 @@ enum iio_chan_info_enum {
>   	IIO_CHAN_INFO_QUADRATURE_CORRECTION_RAW,
>   	IIO_CHAN_INFO_AVERAGE_RAW,
>   	IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY,
> +	IIO_CHAN_INFO_OVERSAMPLE_COUNT,
>   };
>
>   #define IIO_CHAN_INFO_SHARED_BIT(type) BIT(type*2)
> @@ -81,6 +82,10 @@ enum iio_chan_info_enum {
>   #define IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY_SEPARATE_BIT \
>   	IIO_CHAN_INFO_SEPARATE_BIT(			       \
>   		IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY)
> +#define IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT		\
> +	IIO_CHAN_INFO_SEPARATE_BIT(IIO_CHAN_INFO_OVERSAMPLE_COUNT)
> +#define IIO_CHAN_INFO_OVERSAMPLE_COUNT_SHARED_BIT		\
> +	IIO_CHAN_INFO_SHARED_BIT(IIO_CHAN_INFO_OVERSAMPLE_COUNT)
>
>   enum iio_endian {
>   	IIO_CPU,
> diff --git a/drivers/staging/iio/industrialio-core.c b/drivers/staging/iio/industrialio-core.c
> index 9c41c83..05b6fdd 100644
> --- a/drivers/staging/iio/industrialio-core.c
> +++ b/drivers/staging/iio/industrialio-core.c
> @@ -87,6 +87,7 @@ static const char * const iio_chan_info_postfix[] = {
>   	[IIO_CHAN_INFO_AVERAGE_RAW] = "mean_raw",
>   	[IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY]
>   	= "filter_low_pass_3db_frequency",
> +	[IIO_CHAN_INFO_OVERSAMPLE_COUNT] = "oversample_count",
>   };
>
>   const struct iio_chan_spec

Re: [PATCH 1/2] [RFC] Add per-channel oversampling count

[Marek Vasut]

> On 2/10/2012 5:52 AM, Marek Vasut wrote:
> > This allows each channel to configure it's oversampling count.
> 
> Fine, but please also add documention in
> staging/iio/Documentation/sysfs-bus-iio

Understood. Is doing it this way OK with you?

M

> 
> > Signed-off-by: Marek Vasut<marek.vasut@gmail.com>
> > Cc: Wolfgang Denk<wd@denx.de>
> > Cc: Stefano Babic<sbabic@denx.de>
> > Cc: Fabio Estevam<festevam@gmail.com>
> > ---
> > 
> >   drivers/staging/iio/iio.h               |    5 +++++
> >   drivers/staging/iio/industrialio-core.c |    1 +
> >   2 files changed, 6 insertions(+), 0 deletions(-)
> > 
> > diff --git a/drivers/staging/iio/iio.h b/drivers/staging/iio/iio.h
> > index b3a1740..0b626ae 100644
> > --- a/drivers/staging/iio/iio.h
> > +++ b/drivers/staging/iio/iio.h
> > @@ -36,6 +36,7 @@ enum iio_chan_info_enum {
> > 
> >   	IIO_CHAN_INFO_QUADRATURE_CORRECTION_RAW,
> >   	IIO_CHAN_INFO_AVERAGE_RAW,
> >   	IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY,
> > 
> > +	IIO_CHAN_INFO_OVERSAMPLE_COUNT,
> > 
> >   };
> >   
> >   #define IIO_CHAN_INFO_SHARED_BIT(type) BIT(type*2)
> > 
> > @@ -81,6 +82,10 @@ enum iio_chan_info_enum {
> > 
> >   #define IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY_SEPARATE_BIT \
> >   
> >   	IIO_CHAN_INFO_SEPARATE_BIT(			       \
> >   	
> >   		IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY)
> > 
> > +#define IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT		\
> > +	IIO_CHAN_INFO_SEPARATE_BIT(IIO_CHAN_INFO_OVERSAMPLE_COUNT)
> > +#define IIO_CHAN_INFO_OVERSAMPLE_COUNT_SHARED_BIT		\
> > +	IIO_CHAN_INFO_SHARED_BIT(IIO_CHAN_INFO_OVERSAMPLE_COUNT)
> > 
> >   enum iio_endian {
> >   
> >   	IIO_CPU,
> > 
> > diff --git a/drivers/staging/iio/industrialio-core.c
> > b/drivers/staging/iio/industrialio-core.c index 9c41c83..05b6fdd 100644
> > --- a/drivers/staging/iio/industrialio-core.c
> > +++ b/drivers/staging/iio/industrialio-core.c
> > @@ -87,6 +87,7 @@ static const char * const iio_chan_info_postfix[] = {
> > 
> >   	[IIO_CHAN_INFO_AVERAGE_RAW] = "mean_raw",
> >   	[IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY]
> >   	= "filter_low_pass_3db_frequency",
> > 
> > +	[IIO_CHAN_INFO_OVERSAMPLE_COUNT] = "oversample_count",
> > 
> >   };
> >   
> >   const struct iio_chan_spec

Re: [PATCH 1/2] [RFC] Add per-channel oversampling count

[Jonathan Cameron]

On 2/10/2012 10:44 AM, Marek Vasut wrote:
>> On 2/10/2012 5:52 AM, Marek Vasut wrote:
>>> This allows each channel to configure it's oversampling count.
>> Fine, but please also add documention in
>> staging/iio/Documentation/sysfs-bus-iio
> Understood. Is doing it this way OK with you?
Yup. Though as the other email states, it is going to be 'interesting' 
handling this through the demux
that sends the right data to individual consumer drivers.
>
> M
>
>>> Signed-off-by: Marek Vasut<marek.vasut@gmail.com>
>>> Cc: Wolfgang Denk<wd@denx.de>
>>> Cc: Stefano Babic<sbabic@denx.de>
>>> Cc: Fabio Estevam<festevam@gmail.com>
>>> ---
>>>
>>>    drivers/staging/iio/iio.h               |    5 +++++
>>>    drivers/staging/iio/industrialio-core.c |    1 +
>>>    2 files changed, 6 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/drivers/staging/iio/iio.h b/drivers/staging/iio/iio.h
>>> index b3a1740..0b626ae 100644
>>> --- a/drivers/staging/iio/iio.h
>>> +++ b/drivers/staging/iio/iio.h
>>> @@ -36,6 +36,7 @@ enum iio_chan_info_enum {
>>>
>>>    	IIO_CHAN_INFO_QUADRATURE_CORRECTION_RAW,
>>>    	IIO_CHAN_INFO_AVERAGE_RAW,
>>>    	IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY,
>>>
>>> +	IIO_CHAN_INFO_OVERSAMPLE_COUNT,
>>>
>>>    };
>>>
>>>    #define IIO_CHAN_INFO_SHARED_BIT(type) BIT(type*2)
>>>
>>> @@ -81,6 +82,10 @@ enum iio_chan_info_enum {
>>>
>>>    #define IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY_SEPARATE_BIT \
>>>
>>>    	IIO_CHAN_INFO_SEPARATE_BIT(			       \
>>>    	
>>>    		IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY)
>>>
>>> +#define IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT		\
>>> +	IIO_CHAN_INFO_SEPARATE_BIT(IIO_CHAN_INFO_OVERSAMPLE_COUNT)
>>> +#define IIO_CHAN_INFO_OVERSAMPLE_COUNT_SHARED_BIT		\
>>> +	IIO_CHAN_INFO_SHARED_BIT(IIO_CHAN_INFO_OVERSAMPLE_COUNT)
>>>
>>>    enum iio_endian {
>>>
>>>    	IIO_CPU,
>>>
>>> diff --git a/drivers/staging/iio/industrialio-core.c
>>> b/drivers/staging/iio/industrialio-core.c index 9c41c83..05b6fdd 100644
>>> --- a/drivers/staging/iio/industrialio-core.c
>>> +++ b/drivers/staging/iio/industrialio-core.c
>>> @@ -87,6 +87,7 @@ static const char * const iio_chan_info_postfix[] = {
>>>
>>>    	[IIO_CHAN_INFO_AVERAGE_RAW] = "mean_raw",
>>>    	[IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY]
>>>    	= "filter_low_pass_3db_frequency",
>>>
>>> +	[IIO_CHAN_INFO_OVERSAMPLE_COUNT] = "oversample_count",
>>>
>>>    };
>>>
>>>    const struct iio_chan_spec

Re: [PATCH 1/2] [RFC] Add per-channel oversampling count

[Marek Vasut]

> On 2/10/2012 10:44 AM, Marek Vasut wrote:
> >> On 2/10/2012 5:52 AM, Marek Vasut wrote:
> >>> This allows each channel to configure it's oversampling count.
> >> 
> >> Fine, but please also add documention in
> >> staging/iio/Documentation/sysfs-bus-iio
> > 
> > Understood. Is doing it this way OK with you?
> 
> Yup. Though as the other email states, it is going to be 'interesting'
> handling this through the demux
> that sends the right data to individual consumer drivers.

The only consumer're gonna be temp and touchscreen I believe. And since the 
touchscreen stuff is quite tightly integrated with the LRADC, I might even end 
up exporting a few functions from the iio driver to bind the touchscreen.

Though I'll need to thing about it.

M

> > 
> >>> Signed-off-by: Marek Vasut<marek.vasut@gmail.com>
> >>> Cc: Wolfgang Denk<wd@denx.de>
> >>> Cc: Stefano Babic<sbabic@denx.de>
> >>> Cc: Fabio Estevam<festevam@gmail.com>
> >>> ---
> >>> 
> >>>    drivers/staging/iio/iio.h               |    5 +++++
> >>>    drivers/staging/iio/industrialio-core.c |    1 +
> >>>    2 files changed, 6 insertions(+), 0 deletions(-)
> >>> 
> >>> diff --git a/drivers/staging/iio/iio.h b/drivers/staging/iio/iio.h
> >>> index b3a1740..0b626ae 100644
> >>> --- a/drivers/staging/iio/iio.h
> >>> +++ b/drivers/staging/iio/iio.h
> >>> @@ -36,6 +36,7 @@ enum iio_chan_info_enum {
> >>> 
> >>>    	IIO_CHAN_INFO_QUADRATURE_CORRECTION_RAW,
> >>>    	IIO_CHAN_INFO_AVERAGE_RAW,
> >>>    	IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY,
> >>> 
> >>> +	IIO_CHAN_INFO_OVERSAMPLE_COUNT,
> >>> 
> >>>    };
> >>>    
> >>>    #define IIO_CHAN_INFO_SHARED_BIT(type) BIT(type*2)
> >>> 
> >>> @@ -81,6 +82,10 @@ enum iio_chan_info_enum {
> >>> 
> >>>    #define IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY_SEPARATE_BIT \
> >>>    
> >>>    	IIO_CHAN_INFO_SEPARATE_BIT(			       \
> >>>    	
> >>>    		IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY)
> >>> 
> >>> +#define IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT		\
> >>> +	IIO_CHAN_INFO_SEPARATE_BIT(IIO_CHAN_INFO_OVERSAMPLE_COUNT)
> >>> +#define IIO_CHAN_INFO_OVERSAMPLE_COUNT_SHARED_BIT		\
> >>> +	IIO_CHAN_INFO_SHARED_BIT(IIO_CHAN_INFO_OVERSAMPLE_COUNT)
> >>> 
> >>>    enum iio_endian {
> >>>    
> >>>    	IIO_CPU,
> >>> 
> >>> diff --git a/drivers/staging/iio/industrialio-core.c
> >>> b/drivers/staging/iio/industrialio-core.c index 9c41c83..05b6fdd 100644
> >>> --- a/drivers/staging/iio/industrialio-core.c
> >>> +++ b/drivers/staging/iio/industrialio-core.c
> >>> @@ -87,6 +87,7 @@ static const char * const iio_chan_info_postfix[] = {
> >>> 
> >>>    	[IIO_CHAN_INFO_AVERAGE_RAW] = "mean_raw",
> >>>    	[IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY]
> >>>    	= "filter_low_pass_3db_frequency",
> >>> 
> >>> +	[IIO_CHAN_INFO_OVERSAMPLE_COUNT] = "oversample_count",
> >>> 
> >>>    };
> >>>    
> >>>    const struct iio_chan_spec

Re: [PATCH 1/2] [RFC] Add per-channel oversampling count

[Jonathan Cameron]

On 2/10/2012 11:19 AM, Marek Vasut wrote:
>> On 2/10/2012 10:44 AM, Marek Vasut wrote:
>>>> On 2/10/2012 5:52 AM, Marek Vasut wrote:
>>>>> This allows each channel to configure it's oversampling count.
>>>> Fine, but please also add documention in
>>>> staging/iio/Documentation/sysfs-bus-iio
>>> Understood. Is doing it this way OK with you?
>> Yup. Though as the other email states, it is going to be 'interesting'
>> handling this through the demux
>> that sends the right data to individual consumer drivers.
> The only consumer're gonna be temp and touchscreen I believe. And since the
> touchscreen stuff is quite tightly integrated with the LRADC, I might even end
> up exporting a few functions from the iio driver to bind the touchscreen.
We end up discussing how to do touchscreen drivers every time.   It 
always comes back to whether
we can generalize across enough different implementations to make it 
worth adding control hooks
to the IIO core, or as you say just add the support directly into the 
IIO device driver (basically
have it register an input device as well - or export enough stuff to 
allow that elsewhere.)

hwmon is easy, though I need to push out latest version of the code for 
that. Personally stalled on
testing whilst trying to nail a reason all the mfp config on my test 
board is throwing up errors right now.
The previous version I posted is find for development though as all 
changes are internal to the IIO core.
>
> Though I'll need to thing about it.
>
> M
>
>>>>> Signed-off-by: Marek Vasut<marek.vasut@gmail.com>
>>>>> Cc: Wolfgang Denk<wd@denx.de>
>>>>> Cc: Stefano Babic<sbabic@denx.de>
>>>>> Cc: Fabio Estevam<festevam@gmail.com>
>>>>> ---
>>>>>
>>>>>     drivers/staging/iio/iio.h               |    5 +++++
>>>>>     drivers/staging/iio/industrialio-core.c |    1 +
>>>>>     2 files changed, 6 insertions(+), 0 deletions(-)
>>>>>
>>>>> diff --git a/drivers/staging/iio/iio.h b/drivers/staging/iio/iio.h
>>>>> index b3a1740..0b626ae 100644
>>>>> --- a/drivers/staging/iio/iio.h
>>>>> +++ b/drivers/staging/iio/iio.h
>>>>> @@ -36,6 +36,7 @@ enum iio_chan_info_enum {
>>>>>
>>>>>     	IIO_CHAN_INFO_QUADRATURE_CORRECTION_RAW,
>>>>>     	IIO_CHAN_INFO_AVERAGE_RAW,
>>>>>     	IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY,
>>>>>
>>>>> +	IIO_CHAN_INFO_OVERSAMPLE_COUNT,
>>>>>
>>>>>     };
>>>>>
>>>>>     #define IIO_CHAN_INFO_SHARED_BIT(type) BIT(type*2)
>>>>>
>>>>> @@ -81,6 +82,10 @@ enum iio_chan_info_enum {
>>>>>
>>>>>     #define IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY_SEPARATE_BIT \
>>>>>
>>>>>     	IIO_CHAN_INFO_SEPARATE_BIT(			       \
>>>>>     	
>>>>>     		IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY)
>>>>>
>>>>> +#define IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT		\
>>>>> +	IIO_CHAN_INFO_SEPARATE_BIT(IIO_CHAN_INFO_OVERSAMPLE_COUNT)
>>>>> +#define IIO_CHAN_INFO_OVERSAMPLE_COUNT_SHARED_BIT		\
>>>>> +	IIO_CHAN_INFO_SHARED_BIT(IIO_CHAN_INFO_OVERSAMPLE_COUNT)
>>>>>
>>>>>     enum iio_endian {
>>>>>
>>>>>     	IIO_CPU,
>>>>>
>>>>> diff --git a/drivers/staging/iio/industrialio-core.c
>>>>> b/drivers/staging/iio/industrialio-core.c index 9c41c83..05b6fdd 100644
>>>>> --- a/drivers/staging/iio/industrialio-core.c
>>>>> +++ b/drivers/staging/iio/industrialio-core.c
>>>>> @@ -87,6 +87,7 @@ static const char * const iio_chan_info_postfix[] = {
>>>>>
>>>>>     	[IIO_CHAN_INFO_AVERAGE_RAW] = "mean_raw",
>>>>>     	[IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY]
>>>>>     	= "filter_low_pass_3db_frequency",
>>>>>
>>>>> +	[IIO_CHAN_INFO_OVERSAMPLE_COUNT] = "oversample_count",
>>>>>
>>>>>     };
>>>>>
>>>>>     const struct iio_chan_spec