AT91: Add a driver for the ADC

Cc Nicolas Ferre <[email protected]>

Cc Patrice Vilchez <[email protected]>

Signed-off-by Maxime Ripard <[email protected]>

---
 drivers/staging/iio/adc/Kconfig   |    6 +
 drivers/staging/iio/adc/Makefile  |    1 +
 drivers/staging/iio/adc/at91adc.c |  295 +++++++++++++++++++++++++++++++++++++
 3 files changed, 302 insertions(+), 0 deletions(-)
 create mode 100644 drivers/staging/iio/adc/at91adc.c

drivers/staging/iio/adc/Kconfig +6 -0

index 070efd3..98e8005 100644
@@ -206,6 +206,12 @@ config AD7280
 	  To compile this driver as a module, choose M here: the
 	  module will be called ad7280a
 
+config AT91ADC
+	tristate "Atmel Touchscreen ADC Controller"
+	depends on SYSFS && ARCH_AT91
+	help
+	  Say yes here to build support for Atmel TSADCC.
+
 config MAX1363
 	tristate "Maxim max1363 ADC driver"
 	depends on I2C

drivers/staging/iio/adc/Makefile +1 -0

index 8adf096..03e517a 100644
@@ -42,3 +42,4 @@ obj-$(CONFIG_ADT75) += adt75.o
 obj-$(CONFIG_ADT7310) += adt7310.o
 obj-$(CONFIG_ADT7410) += adt7410.o
 obj-$(CONFIG_AD7280) += ad7280a.o
+obj-$(CONFIG_AT91ADC) += at91adc.o

drivers/staging/iio/adc/at91adc.c new file +295 -0

new file mode 100644
index 0000000..2737261
@@ -0,0 +1,295 @@
+/*
+ * Driver for the TouchScreen ADC Controller present in the Atmel AT91
+ * evaluation boards.
+ *
+ * Copyright 2011 Free Electrons
+ *
+ * Licensed under the GPLv2 or later.
+ */
+
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/interrupt.h>
+#include <linux/jiffies.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/wait.h>
+
+#include "../iio.h"
+
+#include <mach/at91_adc.h>
+#include <mach/board.h>
+
+#define at91adc_reg_read(base, reg)        __raw_readl((base) + (reg))
+#define at91adc_reg_write(base, reg, val)  __raw_writel((val), (base) + (reg))
+
+struct at91adc_state {
+	struct clk *clk;
+	bool done;
+	struct mutex lock;
+	struct iio_chan_spec *channels;
+	int nb_chan;
+	int irq;
+	wait_queue_head_t wq_data_avail;
+	u16 lcdr;
+	void __iomem *reg_base;
+
+};
+
+static irqreturn_t at91adc_eoc_trigger(int irq, void *private)
+{
+	int chan;
+	struct at91adc_state *st = private;
+	struct iio_dev *idev = iio_priv_to_dev(st);
+	unsigned int status = at91adc_reg_read(st->reg_base, AT91_ADC_SR);
+
+	if (!(status & AT91_ADC_DRDY))
+		return IRQ_HANDLED;
+
+	for (chan = 0; chan < st->nb_chan; chan++) {
+		if (status & AT91_ADC_EOC(chan)) {
+			st->done = true;
+			st->lcdr = at91adc_reg_read(st->reg_base,
+						    AT91_ADC_CHR(chan));
+		}
+	}
+	wake_up_interruptible(&st->wq_data_avail);
+
+	return IRQ_HANDLED;
+}
+
+static int at91adc_channel_init(struct at91adc_state *st)
+{
+	int ret = 0, i;
+	st->channels = kzalloc(sizeof(struct iio_chan_spec) * st->nb_chan,
+			       GFP_KERNEL);
+	if (st->channels == NULL)
+		return -ENOMEM;
+
+	for (i = 0; i < st->nb_chan; i++) {
+		struct iio_chan_spec *chan = st->channels + i;
+		chan->type = IIO_VOLTAGE;
+		chan->indexed = 1;
+		chan->channel = i;
+		++ret;
+	}
+
+	return ret;
+}
+
+static int at91adc_read_raw(struct iio_dev *idev,
+			    struct iio_chan_spec const *chan,
+			    int *val, int *val2, long mask)
+{
+	struct at91adc_state *st = iio_priv(idev);
+
+	switch (mask) {
+	case 0:
+		mutex_lock(&st->lock);
+
+		at91adc_reg_write(st->reg_base, AT91_ADC_CHER,
+				  AT91_ADC_CH(chan->channel));
+		at91adc_reg_write(st->reg_base, AT91_ADC_IER,
+				  AT91_ADC_EOC(chan->channel));
+		at91adc_reg_write(st->reg_base, AT91_ADC_CR, AT91_ADC_START);
+
+		wait_event_interruptible(st->wq_data_avail, st->done);
+		*val = st->lcdr;
+
+		at91adc_reg_write(st->reg_base, AT91_ADC_CHDR,
+				  AT91_ADC_CH(chan->channel));
+		at91adc_reg_write(st->reg_base, AT91_ADC_IDR,
+				  AT91_ADC_EOC(chan->channel));
+
+		st->lcdr = 0;
+		st->done = false;
+		mutex_unlock(&st->lock);
+		return IIO_VAL_INT;
+	default:
+		break;
+	}
+	return -EINVAL;
+}
+
+static const struct iio_info at91adc_info = {
+	.driver_module = THIS_MODULE,
+	.read_raw = &at91adc_read_raw,
+};
+
+static int __devinit at91adc_probe(struct platform_device *pdev)
+{
+	unsigned int prsc, mstrclk, ticks;
+	int ret;
+	struct iio_dev *idev;
+	struct at91adc_state *st;
+	struct resource *res;
+	struct at91_adc_data *pdata = pdev->dev.platform_data;
+
+	dev_dbg(&pdev->dev, "AT91ADC probed\n");
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		dev_err(&pdev->dev, "No resource defined\n");
+		ret = -ENXIO;
+		goto error_ret;
+	}
+
+	idev = iio_allocate_device(sizeof(*st));
+	if (idev == NULL) {
+		dev_err(&pdev->dev, "Failed to allocate memory.\n");
+		ret = -ENOMEM;
+		goto error_ret;
+	}
+	platform_set_drvdata(pdev, idev);
+
+	idev->dev.parent = &pdev->dev;
+	idev->name = platform_get_device_id(pdev)->name;
+	idev->modes = INDIO_DIRECT_MODE;
+	idev->info = &at91adc_info;
+
+	st = iio_priv(idev);
+	st->irq = platform_get_irq(pdev, 0);
+	if (st->irq < 0) {
+		dev_err(&pdev->dev, "No IRQ ID is designated\n");
+		ret = -ENODEV;
+		goto error_free_device;
+	}
+
+	if (!request_mem_region(res->start, resource_size(res),
+				"AT91 adc registers")) {
+		dev_err(&pdev->dev, "Resources are unavailable.\n");
+		ret = -EBUSY;
+		goto error_free_device;
+	}
+
+	st->reg_base = ioremap(res->start, resource_size(res));
+	if (!st->reg_base) {
+		dev_err(&pdev->dev, "Failed to map registers.\n");
+		ret = -ENOMEM;
+		goto error_release_mem;
+	}
+
+	/*
+	 * Disable all IRQs before setting up the handler
+	 */
+	at91adc_reg_write(st->reg_base, AT91_ADC_CR, AT91_ADC_SWRST);
+	at91adc_reg_write(st->reg_base, AT91_ADC_IDR, 0xFFFFFFFF);
+	ret = request_irq(st->irq,
+			  at91adc_eoc_trigger, 0, pdev->dev.driver->name, st);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to allocate IRQ.\n");
+		goto error_unmap_reg;
+	}
+
+	st->clk = clk_get(&pdev->dev, "adc_clk");
+	if (IS_ERR(st->clk)) {
+		dev_err(&pdev->dev, "Failed to get the clock.\n");
+		ret = PTR_ERR(st->clk);
+		goto error_free_irq;
+	}
+
+	clk_enable(st->clk);
+	mstrclk = clk_get_rate(st->clk);
+
+	if (!pdata) {
+		dev_err(&pdev->dev, "No platform data available.\n");
+		ret = -EINVAL;
+		goto error_free_clk;
+	}
+
+	if (!pdata->adc_clock) {
+		dev_err(&pdev->dev, "No ADCClock available.\n");
+		ret = -EINVAL;
+		goto error_free_clk;
+	}
+
+	prsc = (mstrclk / (2 * pdata->adc_clock)) - 1;
+
+	if (!pdata->startup_time) {
+		dev_err(&pdev->dev, "No startup time available.\n");
+		ret = -EINVAL;
+		goto error_free_clk;
+	}
+
+	ticks = round_up((pdata->startup_time * pdata->adc_clock /
+			  1000000) - 1, 8) / 8;
+	at91adc_reg_write(st->reg_base, AT91_ADC_MR,
+			  (AT91_ADC_PRESCAL_(prsc) & AT91_ADC_PRESCAL) |
+			  (AT91_ADC_STARTUP_(ticks) & AT91_ADC_STARTUP));
+
+	/* Setup the ADC channels available on the board */
+	st->nb_chan = pdata->channels;
+	ret = at91adc_channel_init(st);
+	if (ret < 0) {
+		goto error_free_clk;
+	}
+
+	idev->channels = st->channels;
+	idev->num_channels = st->nb_chan;
+
+	init_waitqueue_head(&st->wq_data_avail);
+	mutex_init(&st->lock);
+
+	ret = iio_device_register(idev);
+	if (ret < 0) {
+		goto error_free_clk;
+	}
+
+	return 0;
+
+error_free_clk:
+	clk_disable(st->clk);
+	clk_put(st->clk);
+error_free_irq:
+	free_irq(st->irq, st);
+error_unmap_reg:
+	iounmap(st->reg_base);
+error_release_mem:
+	release_mem_region(res->start, resource_size(res));
+error_free_device:
+	iio_free_device(idev);
+error_ret:
+	return ret;
+}
+
+static int __devexit at91adc_remove(struct platform_device *pdev)
+{
+	struct iio_dev *idev = platform_get_drvdata(pdev);
+	struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	struct at91adc_state *st = iio_priv(idev);
+
+	free_irq(st->irq, st);
+	iounmap(st->reg_base);
+	release_mem_region(res->start, resource_size(res));
+	iio_device_unregister(idev);
+
+	return 0;
+}
+
+static struct platform_driver at91adc_driver = {
+	.probe = at91adc_probe,
+	.remove = __devexit_p(at91adc_remove),
+	.driver = {
+		   .name = "at91adc",
+		   },
+};
+
+static int __init at91adc_init(void)
+{
+	return platform_driver_register(&at91adc_driver);
+}
+
+static void __exit at91adc_exit(void)
+{
+	platform_driver_unregister(&at91adc_driver);
+}
+
+module_init(at91adc_init);
+module_exit(at91adc_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Atmel AT91 ADC Driver");
+MODULE_AUTHOR("Maxime Ripard <[email protected]>");

Afraid I'm out of time for today, but just thought I'd ask one quick question...

Why isn't this touchscreen ADC in input?

Quoted 327 lines

Cc Nicolas Ferre <[email protected]>

Cc Patrice Vilchez <[email protected]>

Signed-off-by Maxime Ripard <[email protected]>

---
 drivers/staging/iio/adc/Kconfig   |    6 +
 drivers/staging/iio/adc/Makefile  |    1 +
 drivers/staging/iio/adc/at91adc.c |  295 +++++++++++++++++++++++++++++++++++++
 3 files changed, 302 insertions(+), 0 deletions(-)
 create mode 100644 drivers/staging/iio/adc/at91adc.c

drivers/staging/iio/adc/Kconfig +6 -0

index 070efd3..98e8005 100644
@@ -206,6 +206,12 @@ config AD7280
 	  To compile this driver as a module, choose M here: the
 	  module will be called ad7280a
 
+config AT91ADC
+	tristate "Atmel Touchscreen ADC Controller"
+	depends on SYSFS && ARCH_AT91
+	help
+	  Say yes here to build support for Atmel TSADCC.
+
 config MAX1363
 	tristate "Maxim max1363 ADC driver"
 	depends on I2C

drivers/staging/iio/adc/Makefile +1 -0

index 8adf096..03e517a 100644
@@ -42,3 +42,4 @@ obj-$(CONFIG_ADT75) += adt75.o
 obj-$(CONFIG_ADT7310) += adt7310.o
 obj-$(CONFIG_ADT7410) += adt7410.o
 obj-$(CONFIG_AD7280) += ad7280a.o
+obj-$(CONFIG_AT91ADC) += at91adc.o

drivers/staging/iio/adc/at91adc.c new file +285 -0

new file mode 100644
index 0000000..2737261
@@ -0,0 +1,295 @@
+/*
+ * Driver for the TouchScreen ADC Controller present in the Atmel AT91
+ * evaluation boards.
+ *
+ * Copyright 2011 Free Electrons
+ *
+ * Licensed under the GPLv2 or later.
+ */
+
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/interrupt.h>
+#include <linux/jiffies.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/wait.h>
+
+#include "../iio.h"
+
+#include <mach/at91_adc.h>
+#include <mach/board.h>
+
+#define at91adc_reg_read(base, reg)        __raw_readl((base) + (reg))
+#define at91adc_reg_write(base, reg, val)  __raw_writel((val), (base) + (reg))
+
+struct at91adc_state {
+	struct clk *clk;
+	bool done;
+	struct mutex lock;
+	struct iio_chan_spec *channels;
+	int nb_chan;
+	int irq;
+	wait_queue_head_t wq_data_avail;
+	u16 lcdr;
+	void __iomem *reg_base;
+
+};
+
+static irqreturn_t at91adc_eoc_trigger(int irq, void *private)
+{
+	int chan;
+	struct at91adc_state *st = private;
+	struct iio_dev *idev = iio_priv_to_dev(st);
+	unsigned int status = at91adc_reg_read(st->reg_base, AT91_ADC_SR);
+
+	if (!(status & AT91_ADC_DRDY))
+		return IRQ_HANDLED;
+
+	for (chan = 0; chan < st->nb_chan; chan++) {
+		if (status & AT91_ADC_EOC(chan)) {
+			st->done = true;
+			st->lcdr = at91adc_reg_read(st->reg_base,
+						    AT91_ADC_CHR(chan));
+		}
+	}
+	wake_up_interruptible(&st->wq_data_avail);
+
+	return IRQ_HANDLED;
+}
+
+static int at91adc_channel_init(struct at91adc_state *st)
+{
+	int ret = 0, i;
+	st->channels = kzalloc(sizeof(struct iio_chan_spec) * st->nb_chan,
+			       GFP_KERNEL);
+	if (st->channels == NULL)
+		return -ENOMEM;
+
+	for (i = 0; i < st->nb_chan; i++) {
+		struct iio_chan_spec *chan = st->channels + i;
+		chan->type = IIO_VOLTAGE;
+		chan->indexed = 1;
+		chan->channel = i;
+		++ret;
+	}
+
+	return ret;
+}
+
+static int at91adc_read_raw(struct iio_dev *idev,
+			    struct iio_chan_spec const *chan,
+			    int *val, int *val2, long mask)
+{
+	struct at91adc_state *st = iio_priv(idev);
+
+	switch (mask) {
+	case 0:
+		mutex_lock(&st->lock);
+
+		at91adc_reg_write(st->reg_base, AT91_ADC_CHER,
+				  AT91_ADC_CH(chan->channel));
+		at91adc_reg_write(st->reg_base, AT91_ADC_IER,
+				  AT91_ADC_EOC(chan->channel));
+		at91adc_reg_write(st->reg_base, AT91_ADC_CR, AT91_ADC_START);
+
+		wait_event_interruptible(st->wq_data_avail, st->done);
+		*val = st->lcdr;
+
+		at91adc_reg_write(st->reg_base, AT91_ADC_CHDR,
+				  AT91_ADC_CH(chan->channel));
+		at91adc_reg_write(st->reg_base, AT91_ADC_IDR,
+				  AT91_ADC_EOC(chan->channel));
+
+		st->lcdr = 0;
+		st->done = false;
+		mutex_unlock(&st->lock);
+		return IIO_VAL_INT;
+	default:
+		break;
+	}
+	return -EINVAL;
+}
+
+static const struct iio_info at91adc_info = {
+	.driver_module = THIS_MODULE,
+	.read_raw = &at91adc_read_raw,
+};
+
+static int __devinit at91adc_probe(struct platform_device *pdev)
+{
+	unsigned int prsc, mstrclk, ticks;
+	int ret;
+	struct iio_dev *idev;
+	struct at91adc_state *st;
+	struct resource *res;
+	struct at91_adc_data *pdata = pdev->dev.platform_data;
+
+	dev_dbg(&pdev->dev, "AT91ADC probed\n");
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		dev_err(&pdev->dev, "No resource defined\n");
+		ret = -ENXIO;
+		goto error_ret;
+	}
+
+	idev = iio_allocate_device(sizeof(*st));
+	if (idev == NULL) {
+		dev_err(&pdev->dev, "Failed to allocate memory.\n");
+		ret = -ENOMEM;
+		goto error_ret;
+	}
+	platform_set_drvdata(pdev, idev);
+
+	idev->dev.parent = &pdev->dev;
+	idev->name = platform_get_device_id(pdev)->name;
+	idev->modes = INDIO_DIRECT_MODE;
+	idev->info = &at91adc_info;
+
+	st = iio_priv(idev);
+	st->irq = platform_get_irq(pdev, 0);
+	if (st->irq < 0) {
+		dev_err(&pdev->dev, "No IRQ ID is designated\n");
+		ret = -ENODEV;
+		goto error_free_device;
+	}
+
+	if (!request_mem_region(res->start, resource_size(res),
+				"AT91 adc registers")) {
+		dev_err(&pdev->dev, "Resources are unavailable.\n");
+		ret = -EBUSY;
+		goto error_free_device;
+	}
+
+	st->reg_base = ioremap(res->start, resource_size(res));
+	if (!st->reg_base) {
+		dev_err(&pdev->dev, "Failed to map registers.\n");
+		ret = -ENOMEM;
+		goto error_release_mem;
+	}
+
+	/*
+	 * Disable all IRQs before setting up the handler
+	 */
+	at91adc_reg_write(st->reg_base, AT91_ADC_CR, AT91_ADC_SWRST);
+	at91adc_reg_write(st->reg_base, AT91_ADC_IDR, 0xFFFFFFFF);
+	ret = request_irq(st->irq,
+			  at91adc_eoc_trigger, 0, pdev->dev.driver->name, st);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to allocate IRQ.\n");
+		goto error_unmap_reg;
+	}
+
+	st->clk = clk_get(&pdev->dev, "adc_clk");
+	if (IS_ERR(st->clk)) {
+		dev_err(&pdev->dev, "Failed to get the clock.\n");
+		ret = PTR_ERR(st->clk);
+		goto error_free_irq;
+	}
+
+	clk_enable(st->clk);
+	mstrclk = clk_get_rate(st->clk);
+
+	if (!pdata) {
+		dev_err(&pdev->dev, "No platform data available.\n");
+		ret = -EINVAL;
+		goto error_free_clk;
+	}
+
+	if (!pdata->adc_clock) {
+		dev_err(&pdev->dev, "No ADCClock available.\n");
+		ret = -EINVAL;
+		goto error_free_clk;
+	}
+
+	prsc = (mstrclk / (2 * pdata->adc_clock)) - 1;
+
+	if (!pdata->startup_time) {
+		dev_err(&pdev->dev, "No startup time available.\n");
+		ret = -EINVAL;
+		goto error_free_clk;
+	}
+
+	ticks = round_up((pdata->startup_time * pdata->adc_clock /
+			  1000000) - 1, 8) / 8;
+	at91adc_reg_write(st->reg_base, AT91_ADC_MR,
+			  (AT91_ADC_PRESCAL_(prsc) & AT91_ADC_PRESCAL) |
+			  (AT91_ADC_STARTUP_(ticks) & AT91_ADC_STARTUP));
+
+	/* Setup the ADC channels available on the board */
+	st->nb_chan = pdata->channels;
+	ret = at91adc_channel_init(st);
+	if (ret < 0) {
+		goto error_free_clk;
+	}
+
+	idev->channels = st->channels;
+	idev->num_channels = st->nb_chan;
+
+	init_waitqueue_head(&st->wq_data_avail);
+	mutex_init(&st->lock);
+
+	ret = iio_device_register(idev);
+	if (ret < 0) {
+		goto error_free_clk;
+	}
+
+	return 0;
+
+error_free_clk:
+	clk_disable(st->clk);
+	clk_put(st->clk);
+error_free_irq:
+	free_irq(st->irq, st);
+error_unmap_reg:
+	iounmap(st->reg_base);
+error_release_mem:
+	release_mem_region(res->start, resource_size(res));
+error_free_device:
+	iio_free_device(idev);
+error_ret:
+	return ret;
+}
+
+static int __devexit at91adc_remove(struct platform_device *pdev)
+{
+	struct iio_dev *idev = platform_get_drvdata(pdev);
+	struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	struct at91adc_state *st = iio_priv(idev);
+
+	free_irq(st->irq, st);
+	iounmap(st->reg_base);
+	release_mem_region(res->start, resource_size(res));
+	iio_device_unregister(idev);
+
+	return 0;
+}
+
+static struct platform_driver at91adc_driver = {
+	.probe = at91adc_probe,
+	.remove = __devexit_p(at91adc_remove),
+	.driver = {
+		   .name = "at91adc",
+		   },
+};
+
+static int __init at91adc_init(void)
+{
+	return platform_driver_register(&at91adc_driver);
+}
+
+static void __exit at91adc_exit(void)

drivers/staging/iio/adc/at91adc.c line 295

Quoted 10 lines

drivers/staging/iio/adc/at91adc.c new file +10 -0

+{
+	platform_driver_unregister(&at91adc_driver);
+}
+
+module_init(at91adc_init);
+module_exit(at91adc_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Atmel AT91 ADC Driver");
+MODULE_AUTHOR("Maxime Ripard <[email protected]>");

--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 3 lines

Afraid I'm out of time for today, but just thought I'd ask one quick question...

Why isn't this touchscreen ADC in input?

Because what I say in the Kconfig is mostly wrong... Sorry about that.

Actually, like I was saying, this ADC is present in a lot of AT91 SoC, and while it is always an ADC, sometimes (like on the G45), it can be used as a touchscreen controller, or even as both, with channels for the touchscreen and channels as ADC. On the G20 however, it is just an ADC.

For now, the plan is only to support the ADC mode, so I put it in adc.

Hello,
=20
Le Wed, 19 Oct 2011 20:23:50 +0200,
Maxime Ripard <[email protected]> a =C3=A9crit :
=20

Quoted 2 lines · Jonathan Cameron

Quoted 1 line

Afraid I'm out of time for today, but just thought I'd ask one quic=

Quoted 4 lines

Quoted 2 lines

Why isn't this touchscreen ADC in input?

Because what I say in the Kconfig is mostly wrong... Sorry about tha=

Quoted 2 lines

Actually, like I was saying, this ADC is present in a lot of AT91 So=

Quoted 1 line

and while it is always an ADC, sometimes (like on the G45), it can b=

Quoted 1 line

used as a touchscreen controller, or even as both, with channels for=

Quoted 1 line

touchscreen and channels as ADC. On the G20 however, it is just an A=

Quoted 2 lines

For now, the plan is only to support the ADC mode, so I put it in ad=

=20
Just to expand a bit on this: there is already in the kernel an input driver for the AT91 touchscreen which uses some ADC channels (that ha=

additional touchscreen-related features: on the G45, you have 8 ADCs channels, 4 can optionally be used for touchscreen, the 4 other are just raw ADCs).
=20
The ultimate goal is of course to make it possible to use both the II=

AT91 ADC driver and the AT91 touchscreen driver work simultaneously (with of course non-conflicting ADC channels usage). However, as show=

by recent discussions on this list, it is not yet entirely clearly ho=

this should be done:
=20
* Should we have some low-level ADC driver in arch/arm/mach-at91/ th=

allows to request/release/access the ADC channels, this driver providing an internal kernel API used by the IIO ADC driver and th=

touchscreen driver ?

=20
sysfs access /dev/input/...
to ADC channels access to touchscreen
|| ||
\/ \/
+------------------+ +------------------+
| AT91 IIO | | AT91 touchscreen |
+------------------+ +------------------+
| |
\_______________________________/
|
+---------------------+
| Some low-level ADC |
| code to request, |
| release and access |
| ADC channels |
+---------------------+
=20
* Should IIO provide some internal kernel API (as you suggested some time ago) so that kernel drivers can use ADC channels ?
=20
=20

            +-------------------+
            | AT91 touchscreen  | -> /dev/input/... access to touchsc=

            +-------------------+
                     ||
                     \/
            +-------------------+
            |   AT91 IIO        | -> sysfs access to ADC channels
            +-------------------+

=20
Regards,
=20
Thomas

Quoted 4 lines

* Should we have some low-level ADC driver in arch/arm/mach-at91/ that allows to request/release/access the ADC channels, this driver providing an internal kernel API used by the IIO ADC driver and the touchscreen driver ?

That's definitely not going to go down well against moves to move everything that looks like a driver out of the arch directories. An equivalent somewhere else might work though.

Agreed. This is currently up in the air. The current state of those IIO hooks is that they do pull only. Push based capture is tricky as for IIO we want to control the triggering of the push at a far finer scale than makes sense for input. I'm not yet clear how these two will play together.

If we sit something underneath the IIO and input drivers (or use the lower portions of IIO to do this) then the intent would be to have a fully generic input touchscreen driver on top. I'm not sure that's possible.

For example are the switches on the G45's adc (from datasheet) something that all/many similar touch screen controllers have?

Le Thu, 20 Oct 2011 09:33:29 +0100,
Jonathan Cameron <[email protected]> a =C3=A9crit :
=20

Quoted 1 line

Quoted 1 line

* Should we have some low-level ADC driver in arch/arm/mach-at91/ =

Quoted 2 lines

Quoted 2 lines

allows to request/release/access the ADC channels, this driver providing an internal kernel API used by the IIO ADC driver and =

Quoted 2 lines

Quoted 1 line

touchscreen driver ?

That's definitely not going to go down well against moves to move ev=

Quoted 1 line

that looks like a driver out of the arch directories. An equivalent =

Quoted 1 line

else might work though.

=20
Yes, of course if this needs to be implemented, it should be in some other location than arch/arm, but not sure where (which is basically the discussion that was started some time ago).
=20

Quoted 1 line

Agreed. This is currently up in the air. The current state of those=

Quoted 1 line

hooks is that they do pull only. Push based capture is tricky as fo=

Quoted 1 line

we want to control the triggering of the push at a far finer scale t=

Quoted 1 line

makes sense for input. I'm not yet clear how these two will play tog=

=20
My IIO knowledge is still too limited to understand what's the proble=

with exposing an API for push-based capture.

=20

Quoted 1 line

If we sit something underneath the IIO and input drivers (or use the=

Quoted 1 line

portions of IIO to do this) then the intent would be to have a fully=

Quoted 1 line

input touchscreen driver on top. I'm not sure that's possible.

=20
I am not sure it's possible to make the touchscreen driver generic. O=

the G45, the ADC IP has some generic ADC registers, but also some touchscreen-specific registers. So the touchscreen driver probably cannot be completely generic and some cooperation between the AT91 AD=

driver and the AT91 touchscreen driver might be needed. I'd have to look into more details on how the AT91 touchscreen thing works to provide some more details here.
=20

Quoted 1 line

For example are the switches on the G45's adc (from datasheet) somet=

Quoted 1 line

all/many similar touch screen controllers have?

=20
I have no idea.
=20
However, I have also seen a platform (SuperH 2A) where ADC channels a=

used for keypad handling. 4 ADCs channels, each connected to 4 keys, each having a different resistor. Depending on the voltage measured a=

the ADC, you're capable of knowing which key of the 4x4 keypad is pressed. This is a different situation where the ADC values need to b=

used by another in-kernel driver.

=20
Regards,
=20
Thomas

Agreed. This is currently up in the air. The current state of those IIO hooks is that they do pull only. Push based capture is tricky as for IIO we want to control the triggering of the push at a far finer scale than makes sense for input. I'm not yet clear how these two will play together.

If we sit something underneath the IIO and input drivers (or use the lower portions of IIO to do this) then the intent would be to have a fully generic input touchscreen driver on top. I'm not sure that's possible. For

drivers/staging/iio/adc/at91adc.c new file +31 -0

+
+struct at91adc_state {
+	struct clk *clk;
+	bool done;
+	struct mutex lock;
+	struct iio_chan_spec *channels;
+	int nb_chan;
+	int irq;
+	wait_queue_head_t wq_data_avail;
+	u16 lcdr;
+	void __iomem *reg_base;
+
+};
+
+static irqreturn_t at91adc_eoc_trigger(int irq, void *private)
+{
+	int chan;
+	struct at91adc_state *st = private;
+	struct iio_dev *idev = iio_priv_to_dev(st);
+	unsigned int status = at91adc_reg_read(st->reg_base, AT91_ADC_SR);
+
+	if (!(status & AT91_ADC_DRDY))
+		return IRQ_HANDLED;
+
+	for (chan = 0; chan < st->nb_chan; chan++) {
+		if (status & AT91_ADC_EOC(chan)) {
+			st->done = true;
+			st->lcdr = at91adc_reg_read(st->reg_base,
+						    AT91_ADC_CHR(chan));
+		}
+	}

drivers/staging/iio/adc/at91adc.c new file +9 -0

+}
+
+static int at91adc_read_raw(struct iio_dev *idev,
+			    struct iio_chan_spec const *chan,
+			    int *val, int *val2, long mask)
+{
+	struct at91adc_state *st = iio_priv(idev);
+
+	switch (mask) {

drivers/staging/iio/adc/at91adc.c new file +40 -0

+		*val = st->lcdr;
+
+		at91adc_reg_write(st->reg_base, AT91_ADC_CHDR,
+				  AT91_ADC_CH(chan->channel));
+		at91adc_reg_write(st->reg_base, AT91_ADC_IDR,
+				  AT91_ADC_EOC(chan->channel));
+
+		st->lcdr = 0;
+		st->done = false;
+		mutex_unlock(&st->lock);
+		return IIO_VAL_INT;
+	default:
+		break;
+	}
+	return -EINVAL;
+}
+
+static const struct iio_info at91adc_info = {
+	.driver_module = THIS_MODULE,
+	.read_raw = &at91adc_read_raw,
+};
+
+static int __devinit at91adc_probe(struct platform_device *pdev)
+{
+	unsigned int prsc, mstrclk, ticks;
+	int ret;
+	struct iio_dev *idev;
+	struct at91adc_state *st;
+	struct resource *res;
+	struct at91_adc_data *pdata = pdev->dev.platform_data;
+
+	dev_dbg(&pdev->dev, "AT91ADC probed\n");
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		dev_err(&pdev->dev, "No resource defined\n");
+		ret = -ENXIO;
+		goto error_ret;
+	}
+

drivers/staging/iio/adc/at91adc.c new file +7 -0

+	idev->modes = INDIO_DIRECT_MODE;
+	idev->info = &at91adc_info;
+
+[...]
+}
+
+static int __devexit at91adc_remove(struct platform_device *pdev)

drivers/staging/iio/adc/at91adc.c new file +3 -0

+	return 0;
+}
+
[...]

drivers/staging/iio/adc/at91adc.c line 27

Quoted 8 lines · Maxime Ripard

Cc Nicolas Ferre <[email protected]>

Cc Patrice Vilchez <[email protected]>

Signed-off-by Maxime Ripard <[email protected]>

---
[...]

drivers/staging/iio/adc/at91adc.c new file +3 -0

+
+#define at91adc_reg_read(base, reg)        __raw_readl((base) + (reg))
+#define at91adc_reg_write(base, reg, val)  __raw_writel((val), (base) + (reg))

Shouldn't this rather be {readl,writel}_relaxed?

Quoted 35 lines

drivers/staging/iio/adc/at91adc.c new file +35 -0

+
+struct at91adc_state {
+	struct clk *clk;
+	bool done;
+	struct mutex lock;
+	struct iio_chan_spec *channels;
+	int nb_chan;
+	int irq;
+	wait_queue_head_t wq_data_avail;
+	u16 lcdr;
+	void __iomem *reg_base;
+
+};
+
+static irqreturn_t at91adc_eoc_trigger(int irq, void *private)
+{
+	int chan;
+	struct at91adc_state *st = private;
+	struct iio_dev *idev = iio_priv_to_dev(st);
+	unsigned int status = at91adc_reg_read(st->reg_base, AT91_ADC_SR);
+
+	if (!(status & AT91_ADC_DRDY))
+		return IRQ_HANDLED;
+
+	for (chan = 0; chan < st->nb_chan; chan++) {
+		if (status & AT91_ADC_EOC(chan)) {
+			st->done = true;
+			st->lcdr = at91adc_reg_read(st->reg_base,
+						    AT91_ADC_CHR(chan));
+		}
+	}
+	wake_up_interruptible(&st->wq_data_avail);
+
+	return IRQ_HANDLED;
+}

drivers/staging/iio/adc/at91adc.c line 68

Quoted 6 lines

drivers/staging/iio/adc/at91adc.c new file +6 -0

+
+static int at91adc_channel_init(struct at91adc_state *st)
+{
+	int ret = 0, i;
+	st->channels = kzalloc(sizeof(struct iio_chan_spec) * st->nb_chan,
+			       GFP_KERNEL);

kcalloc

drivers/staging/iio/adc/at91adc.c line 76

Quoted 8 lines

drivers/staging/iio/adc/at91adc.c new file +8 -0

+	if (st->channels == NULL)
+		return -ENOMEM;
+
+	for (i = 0; i < st->nb_chan; i++) {
+		struct iio_chan_spec *chan = st->channels + i;
+		chan->type = IIO_VOLTAGE;
+		chan->indexed = 1;
+		chan->channel = i;

It probably makes sense to initialize scan_type as well.

drivers/staging/iio/adc/at91adc.c line 80

Quoted 4 lines

drivers/staging/iio/adc/at91adc.c new file +4 -0

+		++ret;
+	}
+
+	return ret;

Why do you need ret? Shouldn't this be just st->nb_chan?

Quoted 14 lines

drivers/staging/iio/adc/at91adc.c new file +14 -0

+}
+
+static int at91adc_read_raw(struct iio_dev *idev,
+			    struct iio_chan_spec const *chan,
+			    int *val, int *val2, long mask)
+{
+	struct at91adc_state *st = iio_priv(idev);
+
+	switch (mask) {
+	case 0:
+		mutex_lock(&st->lock);
+
+		at91adc_reg_write(st->reg_base, AT91_ADC_CHER,
+				  AT91_ADC_CH(chan->channel));

drivers/staging/iio/adc/at91adc.c line 99

Quoted 5 lines

drivers/staging/iio/adc/at91adc.c new file +5 -0

+		at91adc_reg_write(st->reg_base, AT91_ADC_IER,
+				  AT91_ADC_EOC(chan->channel));
+		at91adc_reg_write(st->reg_base, AT91_ADC_CR, AT91_ADC_START);
+
+		wait_event_interruptible(st->wq_data_avail, st->done);

Its probably a good idea to add a timeout in case there is a problem with the hardware.

Quoted 45 lines

drivers/staging/iio/adc/at91adc.c new file +45 -0

+		*val = st->lcdr;
+
+		at91adc_reg_write(st->reg_base, AT91_ADC_CHDR,
+				  AT91_ADC_CH(chan->channel));
+		at91adc_reg_write(st->reg_base, AT91_ADC_IDR,
+				  AT91_ADC_EOC(chan->channel));
+
+		st->lcdr = 0;
+		st->done = false;
+		mutex_unlock(&st->lock);
+		return IIO_VAL_INT;
+	default:
+		break;
+	}
+	return -EINVAL;
+}
+
+static const struct iio_info at91adc_info = {
+	.driver_module = THIS_MODULE,
+	.read_raw = &at91adc_read_raw,
+};
+
+static int __devinit at91adc_probe(struct platform_device *pdev)
+{
+	unsigned int prsc, mstrclk, ticks;
+	int ret;
+	struct iio_dev *idev;
+	struct at91adc_state *st;
+	struct resource *res;
+	struct at91_adc_data *pdata = pdev->dev.platform_data;
+
+	dev_dbg(&pdev->dev, "AT91ADC probed\n");
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		dev_err(&pdev->dev, "No resource defined\n");
+		ret = -ENXIO;
+		goto error_ret;
+	}
+
+	idev = iio_allocate_device(sizeof(*st));
+	if (idev == NULL) {
+		dev_err(&pdev->dev, "Failed to allocate memory.\n");
+		ret = -ENOMEM;
+		goto error_ret;

drivers/staging/iio/adc/at91adc.c line 149

Quoted 5 lines

drivers/staging/iio/adc/at91adc.c new file +5 -0

+	}
+	platform_set_drvdata(pdev, idev);
+
+	idev->dev.parent = &pdev->dev;
+	idev->name = platform_get_device_id(pdev)->name;

You don't use device ids. This should cause a NULL pointer deref? Maybe use dev_name(&pdev->dev) instead.

Quoted 13 lines

drivers/staging/iio/adc/at91adc.c new file +13 -0

+	idev->modes = INDIO_DIRECT_MODE;
+	idev->info = &at91adc_info;
+
+[...]
+}
+
+static int __devexit at91adc_remove(struct platform_device *pdev)
+{
+	struct iio_dev *idev = platform_get_drvdata(pdev);
+	struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	struct at91adc_state *st = iio_priv(idev);
+
+	free_irq(st->irq, st);

drivers/staging/iio/adc/at91adc.c line 268

Quoted 4 lines

drivers/staging/iio/adc/at91adc.c new file +4 -0

+	iounmap(st->reg_base);
+	release_mem_region(res->start, resource_size(res));
+	iio_device_unregister(idev);
+

The iio API changed a bit recently and uses a two stage unregistration now. You should call iio_device_unregister before freeing resources and iio_device_free afterwards.

[...]

Quoted 7 lines

Quoted 7 lines

drivers/staging/iio/adc/at91adc.c new file +7 -0

+static int __devexit at91adc_remove(struct platform_device *pdev)
+{
+	struct iio_dev *idev = platform_get_drvdata(pdev);
+	struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	struct at91adc_state *st = iio_priv(idev);
+
+	free_irq(st->irq, st);

Quoted 8 lines

drivers/staging/iio/adc/at91adc.c line 268

Quoted 4 lines

drivers/staging/iio/adc/at91adc.c new file +4 -0

+	iounmap(st->reg_base);
+	release_mem_region(res->start, resource_size(res));
+	iio_device_unregister(idev);
+

The iio API changed a bit recently and uses a two stage unregistration now. You should call iio_device_unregister before freeing resources and iio_device_free afterwards.

Hmmm, I don't see the iio_device_free in Jonathan's git. Is it iio_free_device, or should I use another git repo/branch as base ?

Thanks,

Cc Nicolas Ferre <[email protected]>

Cc Patrice Vilchez <[email protected]>

Signed-off-by Maxime Ripard <[email protected]>

drivers/staging/iio/adc/at91adc.c new file +57 -0

+
+static int at91adc_channel_init(struct at91adc_state *st)
+{
+	int ret = 0, i;
+	st->channels = kzalloc(sizeof(struct iio_chan_spec) * st->nb_chan,
+			       GFP_KERNEL);
+	if (st->channels == NULL)
+		return -ENOMEM;
+
+	for (i = 0; i < st->nb_chan; i++) {
+		struct iio_chan_spec *chan = st->channels + i;
+		chan->type = IIO_VOLTAGE;
+		chan->indexed = 1;
+		chan->channel = i;
+		++ret;
+	}
+
+	return ret;
+}
+
+static int at91adc_read_raw(struct iio_dev *idev,
+			    struct iio_chan_spec const *chan,
+			    int *val, int *val2, long mask)
+{
+	struct at91adc_state *st = iio_priv(idev);
+
+	switch (mask) {
+	case 0:
+		mutex_lock(&st->lock);
+
+		at91adc_reg_write(st->reg_base, AT91_ADC_CHER,
+				  AT91_ADC_CH(chan->channel));
+		at91adc_reg_write(st->reg_base, AT91_ADC_IER,
+				  AT91_ADC_EOC(chan->channel));
+		at91adc_reg_write(st->reg_base, AT91_ADC_CR, AT91_ADC_START);
+
+		wait_event_interruptible(st->wq_data_avail, st->done);
+		*val = st->lcdr;
+
+		at91adc_reg_write(st->reg_base, AT91_ADC_CHDR,
+				  AT91_ADC_CH(chan->channel));
+		at91adc_reg_write(st->reg_base, AT91_ADC_IDR,
+				  AT91_ADC_EOC(chan->channel));
+
+		st->lcdr = 0;
+		st->done = false;
+		mutex_unlock(&st->lock);
+		return IIO_VAL_INT;
+	default:
+		break;
+	}
+	return -EINVAL;
+}
+
+static const struct iio_info at91adc_info = {
+	.driver_module = THIS_MODULE,
+	.read_raw = &at91adc_read_raw,

drivers/staging/iio/adc/at91adc.c new file +68 -0

+
+	dev_dbg(&pdev->dev, "AT91ADC probed\n");
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		dev_err(&pdev->dev, "No resource defined\n");
+		ret = -ENXIO;
+		goto error_ret;
+	}
+
+	idev = iio_allocate_device(sizeof(*st));
+	if (idev == NULL) {
+		dev_err(&pdev->dev, "Failed to allocate memory.\n");
+		ret = -ENOMEM;
+		goto error_ret;
+	}
+	platform_set_drvdata(pdev, idev);
+
+	idev->dev.parent = &pdev->dev;
+	idev->name = platform_get_device_id(pdev)->name;
+	idev->modes = INDIO_DIRECT_MODE;
+	idev->info = &at91adc_info;
+
+	st = iio_priv(idev);
+	st->irq = platform_get_irq(pdev, 0);
+	if (st->irq < 0) {
+		dev_err(&pdev->dev, "No IRQ ID is designated\n");
+		ret = -ENODEV;
+		goto error_free_device;
+	}
+
+	if (!request_mem_region(res->start, resource_size(res),
+				"AT91 adc registers")) {
+		dev_err(&pdev->dev, "Resources are unavailable.\n");
+		ret = -EBUSY;
+		goto error_free_device;
+	}
+
+	st->reg_base = ioremap(res->start, resource_size(res));
+	if (!st->reg_base) {
+		dev_err(&pdev->dev, "Failed to map registers.\n");
+		ret = -ENOMEM;
+		goto error_release_mem;
+	}
+
+	/*
+	 * Disable all IRQs before setting up the handler
+	 */
+	at91adc_reg_write(st->reg_base, AT91_ADC_CR, AT91_ADC_SWRST);
+	at91adc_reg_write(st->reg_base, AT91_ADC_IDR, 0xFFFFFFFF);
+	ret = request_irq(st->irq,
+			  at91adc_eoc_trigger, 0, pdev->dev.driver->name, st);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to allocate IRQ.\n");
+		goto error_unmap_reg;
+	}
+
+	st->clk = clk_get(&pdev->dev, "adc_clk");
+	if (IS_ERR(st->clk)) {
+		dev_err(&pdev->dev, "Failed to get the clock.\n");
+		ret = PTR_ERR(st->clk);
+		goto error_free_irq;
+	}
+
+	clk_enable(st->clk);
+	mstrclk = clk_get_rate(st->clk);
+
+	if (!pdata) {

Quoted 3 lines · Maxime Ripard

Cc Nicolas Ferre <[email protected]>

Cc Patrice Vilchez <[email protected]>

Signed-off-by Maxime Ripard <[email protected]>

please keep me in CC

Quoted 60 lines

drivers/staging/iio/adc/at91adc.c new file +60 -0

+
+static int at91adc_channel_init(struct at91adc_state *st)
+{
+	int ret = 0, i;
+	st->channels = kzalloc(sizeof(struct iio_chan_spec) * st->nb_chan,
+			       GFP_KERNEL);
+	if (st->channels == NULL)
+		return -ENOMEM;
+
+	for (i = 0; i < st->nb_chan; i++) {
+		struct iio_chan_spec *chan = st->channels + i;
+		chan->type = IIO_VOLTAGE;
+		chan->indexed = 1;
+		chan->channel = i;
+		++ret;
+	}
+
+	return ret;
+}
+
+static int at91adc_read_raw(struct iio_dev *idev,
+			    struct iio_chan_spec const *chan,
+			    int *val, int *val2, long mask)
+{
+	struct at91adc_state *st = iio_priv(idev);
+
+	switch (mask) {
+	case 0:
+		mutex_lock(&st->lock);
+
+		at91adc_reg_write(st->reg_base, AT91_ADC_CHER,
+				  AT91_ADC_CH(chan->channel));
+		at91adc_reg_write(st->reg_base, AT91_ADC_IER,
+				  AT91_ADC_EOC(chan->channel));
+		at91adc_reg_write(st->reg_base, AT91_ADC_CR, AT91_ADC_START);
+
+		wait_event_interruptible(st->wq_data_avail, st->done);
+		*val = st->lcdr;
+
+		at91adc_reg_write(st->reg_base, AT91_ADC_CHDR,
+				  AT91_ADC_CH(chan->channel));
+		at91adc_reg_write(st->reg_base, AT91_ADC_IDR,
+				  AT91_ADC_EOC(chan->channel));
+
+		st->lcdr = 0;
+		st->done = false;
+		mutex_unlock(&st->lock);
+		return IIO_VAL_INT;
+	default:
+		break;
+	}
+	return -EINVAL;
+}
+
+static const struct iio_info at91adc_info = {
+	.driver_module = THIS_MODULE,
+	.read_raw = &at91adc_read_raw,
+};
+
+static int __devinit at91adc_probe(struct platform_device *pdev)

drivers/staging/iio/adc/at91adc.c line 129

Quoted 7 lines

drivers/staging/iio/adc/at91adc.c new file +7 -0

+{
+	unsigned int prsc, mstrclk, ticks;
+	int ret;
+	struct iio_dev *idev;
+	struct at91adc_state *st;
+	struct resource *res;
+	struct at91_adc_data *pdata = pdev->dev.platform_data;

do not refence it copy need for the DT

Quoted 71 lines

drivers/staging/iio/adc/at91adc.c new file +71 -0

+
+	dev_dbg(&pdev->dev, "AT91ADC probed\n");
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		dev_err(&pdev->dev, "No resource defined\n");
+		ret = -ENXIO;
+		goto error_ret;
+	}
+
+	idev = iio_allocate_device(sizeof(*st));
+	if (idev == NULL) {
+		dev_err(&pdev->dev, "Failed to allocate memory.\n");
+		ret = -ENOMEM;
+		goto error_ret;
+	}
+	platform_set_drvdata(pdev, idev);
+
+	idev->dev.parent = &pdev->dev;
+	idev->name = platform_get_device_id(pdev)->name;
+	idev->modes = INDIO_DIRECT_MODE;
+	idev->info = &at91adc_info;
+
+	st = iio_priv(idev);
+	st->irq = platform_get_irq(pdev, 0);
+	if (st->irq < 0) {
+		dev_err(&pdev->dev, "No IRQ ID is designated\n");
+		ret = -ENODEV;
+		goto error_free_device;
+	}
+
+	if (!request_mem_region(res->start, resource_size(res),
+				"AT91 adc registers")) {
+		dev_err(&pdev->dev, "Resources are unavailable.\n");
+		ret = -EBUSY;
+		goto error_free_device;
+	}
+
+	st->reg_base = ioremap(res->start, resource_size(res));
+	if (!st->reg_base) {
+		dev_err(&pdev->dev, "Failed to map registers.\n");
+		ret = -ENOMEM;
+		goto error_release_mem;
+	}
+
+	/*
+	 * Disable all IRQs before setting up the handler
+	 */
+	at91adc_reg_write(st->reg_base, AT91_ADC_CR, AT91_ADC_SWRST);
+	at91adc_reg_write(st->reg_base, AT91_ADC_IDR, 0xFFFFFFFF);
+	ret = request_irq(st->irq,
+			  at91adc_eoc_trigger, 0, pdev->dev.driver->name, st);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to allocate IRQ.\n");
+		goto error_unmap_reg;
+	}
+
+	st->clk = clk_get(&pdev->dev, "adc_clk");
+	if (IS_ERR(st->clk)) {
+		dev_err(&pdev->dev, "Failed to get the clock.\n");
+		ret = PTR_ERR(st->clk);
+		goto error_free_irq;
+	}
+
+	clk_enable(st->clk);
+	mstrclk = clk_get_rate(st->clk);
+
+	if (!pdata) {
+		dev_err(&pdev->dev, "No platform data available.\n");
+		ret = -EINVAL;
+		goto error_free_clk;

drivers/staging/iio/adc/at91adc.c line 207

Quoted 7 lines

drivers/staging/iio/adc/at91adc.c new file +7 -0

+	}
+
+	if (!pdata->adc_clock) {
+		dev_err(&pdev->dev, "No ADCClock available.\n");
+		ret = -EINVAL;
+		goto error_free_clk;
+	}

where is the platform data struct?

(quoted diff) +0 -0

b/arch/arm/mach-at91/at91sam9260_devices.c index 39f81f4..4f5e0f9 100644

arch/arm/mach-at91/at91sam9260_devices.c +15 -0

[...]
+static struct platform_device at91_adc_device = {
+	.name		= "at91adc",
+	.id		= -1,
+	.dev		= {
+				.platform_data		= &adc_data,
+	},
+	.resource	= adc_resources,
+	.num_resources	= ARRAY_SIZE(adc_resources),
+};
+
+void __init at91_add_device_adc(struct at91_adc_data *data)
+{
+	if (!data)
+		return;
+

Quoted 25 lines · On Wednesday 19 October 2011 18:18:54 Maxime Ripard wrote:

(quoted diff) +0 -0

b/arch/arm/mach-at91/at91sam9260_devices.c index 39f81f4..4f5e0f9 100644

arch/arm/mach-at91/at91sam9260_devices.c +20 -0

[...]
+static struct platform_device at91_adc_device = {
+	.name		= "at91adc",
+	.id		= -1,
+	.dev		= {
+				.platform_data		= &adc_data,
+	},
+	.resource	= adc_resources,
+	.num_resources	= ARRAY_SIZE(adc_resources),
+};
+
+void __init at91_add_device_adc(struct at91_adc_data *data)
+{
+	if (!data)
+		return;
+
+	at91_set_A_periph(AT91_PIN_PC0, 0);
+	at91_set_A_periph(AT91_PIN_PC1, 0);
+	at91_set_A_periph(AT91_PIN_PC2, 0);
+	at91_set_A_periph(AT91_PIN_PC3, 0);
+

arch/arm/mach-at91/at91sam9260_devices.c void __init at91_add_device_cf(struct at91_cf_data *data) · line 1358

Quoted 5 lines

arch/arm/mach-at91/at91sam9260_devices.c +5 -0

+	adc_data = *data;
+	platform_device_register(&at91_adc_device);
+}
+
+

This assumes that all 4 channels are used every time. How about only using an AD channel on PC2 or all but the one on PC1?

Cc Nicolas Ferre <[email protected]>

Cc Patrice Vilchez <[email protected]>

Signed-off-by Maxime Ripard <[email protected]>

---
?arch/arm/mach-at91/include/mach/board.h | ? 22 ++++++++++++++++++++++ ?1 files changed, 22 insertions(+), 0 deletions(-)

2011/11/3 Maxime Ripard <[email protected]>:

Quoted 6 lines

Cc Nicolas Ferre <[email protected]>

Cc Patrice Vilchez <[email protected]>

Signed-off-by Maxime Ripard <[email protected]>

---
 arch/arm/mach-at91/include/mach/board.h |   22 ++++++++++++++++++++++
 1 files changed, 22 insertions(+), 0 deletions(-)

We're not supposed to have platform data dependent to stuff in staging under arch/arm or anyplace else in the main kernel tree.

Please move this to
drivers/staging/iio/adc/at91adc-board.h
or so.

As for calling the at91_add_device_adc() function (which I guess you want to do at some point) the pattern I followed for other drivers is to declare a dummy function in arch/arm/mach-* with __weak and let the staging driver override that. This way the staging driver can go away without any compilation trouble happening.

[Me]

Quoted 6 lines

We're not supposed to have platform data dependent to stuff in staging under arch/arm or anyplace else in the main kernel tree.

Please move this to
drivers/staging/iio/adc/at91adc-board.h
or so.

Won't moving this part to staging prevent from using this structure in board files ? If so, how will I be able to declare a new board that is using this ADC (or add the support for the ADC to a new one) ?

Quoted 6 lines

As for calling the at91_add_device_adc() function (which I guess you want to do at some point) the pattern I followed for other drivers is to declare a dummy function in arch/arm/mach-* with __weak and let the staging driver override that. This way the staging driver can go away without any compilation trouble happening.

I don't really see why my changes will break the compilation if the driver is no longer present in staging. At worst, the structure will be filled but used by no one, right ?

Quoted 11 lines · Maxime Ripard

[Me]

Quoted 6 lines

We're not supposed to have platform data dependent to stuff in staging under arch/arm or anyplace else in the main kernel tree.

Please move this to
drivers/staging/iio/adc/at91adc-board.h
or so.

Won't moving this part to staging prevent from using this structure in board files ? If so, how will I be able to declare a new board that is using this ADC (or add the support for the ADC to a new one) ?

Put this into a separate board file living under staging/iio/adc

Compare:
drivers/staging/ste_rmi4/board-mop500-u8500uib-rmi4.c

Quoted 7 lines

Quoted 6 lines

As for calling the at91_add_device_adc() function (which I guess you want to do at some point) the pattern I followed for other drivers is to declare a dummy function in arch/arm/mach-* with __weak and let the staging driver override that. This way the staging driver can go away without any compilation trouble happening.

Quoted 3 lines

I don't really see why my changes will break the compilation if the driver is no longer present in staging. At worst, the structure will be filled but used by no one, right ?

You're right.

But still, we cannot add that header file for a driver that is in the staging tree. Header files go into the staging dir too.

Cc Nicolas Ferre <[email protected]>

Cc Patrice Vilchez <[email protected]>

Signed-off-by Maxime Ripard <[email protected]>

---
 arch/arm/mach-at91/include/mach/board.h |   22 ++++++++++++++++++++++
 1 files changed, 22 insertions(+), 0 deletions(-)

arch/arm/mach-at91/include/mach/board.h +22 -0

index ed544a0..b837da8 100644
@@ -207,4 +207,26 @@ extern void __init at91_pwm_leds(struct gpio_led *leds, int nr);
 /* FIXME: this needs a better location, but gets stuff building again */
 extern int at91_suspend_entering_slow_clock(void);
 
+/* ADC */
+#define AT91_ADC_MAX_CHANNELS   16
+
+struct at91_adc_data {
+	/* ADC Clock as specified by the datasheet, in Hz. */
+	unsigned int adc_clock;
+	/*
+	 * Global number of channels available (to specify which channels are
+	 * indeed used on the board, see the channels_used array).
+	 */
+	u8 num_channels;
+	/* Channels in use on the board */
+	u8 channels_used[AT91_ADC_MAX_CHANNELS];
+	/* Number of channels in use */
+	u8 num_channels_used;
+	/* Startup time of the ADC, in microseconds. */
+	u8 startup_time;
+	/* Reference voltage for the ADC in millivolts */
+	unsigned short vref;
+};
+extern void __init at91_add_device_adc(struct at91_adc_data *data);
+
 #endif

Quoted 30 lines · Maxime Ripard

Cc Nicolas Ferre <[email protected]>

Cc Patrice Vilchez <[email protected]>

Signed-off-by Maxime Ripard <[email protected]>

---
 arch/arm/mach-at91/include/mach/board.h |   22 ++++++++++++++++++++++
 1 files changed, 22 insertions(+), 0 deletions(-)

arch/arm/mach-at91/include/mach/board.h +15 -0

index ed544a0..b837da8 100644
@@ -207,4 +207,26 @@ extern void __init at91_pwm_leds(struct gpio_led *leds, int nr);
 /* FIXME: this needs a better location, but gets stuff building again */
 extern int at91_suspend_entering_slow_clock(void);
 
+/* ADC */
+#define AT91_ADC_MAX_CHANNELS   16
+
+struct at91_adc_data {
+	/* ADC Clock as specified by the datasheet, in Hz. */
+	unsigned int adc_clock;
+	/*
+	 * Global number of channels available (to specify which channels are
+	 * indeed used on the board, see the channels_used array).
+	 */
+	u8 num_channels;
+	/* Channels in use on the board */
+	u8 channels_used[AT91_ADC_MAX_CHANNELS];
+	/* Number of channels in use */
+	u8 num_channels_used;

arch/arm/mach-at91/include/mach/board.h extern void __init at91_pwm_leds(struct gpio_led *leds, int nr); · line 232

Quoted 8 lines

arch/arm/mach-at91/include/mach/board.h +7 -0

+	/* Startup time of the ADC, in microseconds. */
+	u8 startup_time;
+	/* Reference voltage for the ADC in millivolts */
+	unsigned short vref;
+};
+extern void __init at91_add_device_adc(struct at91_adc_data *data);
+
 #endif

Cc Nicolas Ferre <[email protected]>

Cc Patrice Vilchez <[email protected]>

Signed-off-by Maxime Ripard <[email protected]>

---
 arch/arm/mach-at91/include/mach/board.h |   22 ++++++++++++++++++++++
 1 files changed, 22 insertions(+), 0 deletions(-)

arch/arm/mach-at91/include/mach/board.h +2 -0

index ed544a0..b837da8 100644
@@ -207,4 +207,26 @@ extern void __init at91_pwm_leds(struct gpio_led *leds, int nr);
 /* FIXME: this needs a better location, but gets stuff building again */
 extern int at91_suspend_entering_slow_clock(void);
 
+/* ADC */
+#define AT91_ADC_MAX_CHANNELS   16

arch/arm/mach-at91/include/mach/board.h +8 -0

+	u8 num_channels_used;
+	/* Startup time of the ADC, in microseconds. */
+	u8 startup_time;
+	/* Reference voltage for the ADC in millivolts */
+	unsigned short vref;
+};
+extern void __init at91_add_device_adc(struct at91_adc_data *data);
+
 #endif

arch/arm/mach-at91/include/mach/board.h extern void __init at91_pwm_leds(struct gpio_led *leds, int nr); · line 221

Quoted 12 lines · Maxime Ripard

arch/arm/mach-at91/include/mach/board.h +12 -0

+/* ADC */
+#define AT91_ADC_MAX_CHANNELS   16
+
+struct at91_adc_data {
+	/* ADC Clock as specified by the datasheet, in Hz. */
+	unsigned int adc_clock;
+	/*
+	 * Global number of channels available (to specify which channels are
+	 * indeed used on the board, see the channels_used array).
+	 */
+	u8 num_channels;
+	/* Channels in use on the board */

put it in a unsigned long and use the bitmap functions.

arch/arm/mach-at91/include/mach/board.h extern void __init at91_pwm_leds(struct gpio_led *leds, int nr); · line 223

Quoted 2 lines

arch/arm/mach-at91/include/mach/board.h +2 -0

+	u8 channels_used[AT91_ADC_MAX_CHANNELS];
+	/* Number of channels in use */

just use bitmap_weight with the long above and
length set to num_channels.

arch/arm/mach-at91/include/mach/board.h extern void __init at91_pwm_leds(struct gpio_led *leds, int nr); · line 224

Quoted 1 line

arch/arm/mach-at91/include/mach/board.h +1 -0

+	u8 num_channels_used;

Hi Jonathan,

Quoted 14 lines · Jonathan Cameron

arch/arm/mach-at91/include/mach/board.h extern void __init at91_pwm_leds(struct gpio_led *leds, int nr); · line 221

Quoted 12 lines · Maxime Ripard

arch/arm/mach-at91/include/mach/board.h +12 -0

+/* ADC */
+#define AT91_ADC_MAX_CHANNELS   16
+
+struct at91_adc_data {
+	/* ADC Clock as specified by the datasheet, in Hz. */
+	unsigned int adc_clock;
+	/*
+	 * Global number of channels available (to specify which channels are
+	 * indeed used on the board, see the channels_used array).
+	 */
+	u8 num_channels;
+	/* Channels in use on the board */

put it in a unsigned long and use the bitmap functions.

Quoted 5 lines

arch/arm/mach-at91/include/mach/board.h extern void __init at91_pwm_leds(struct gpio_led *leds, int nr); · line 223

Quoted 2 lines

arch/arm/mach-at91/include/mach/board.h +2 -0

+	u8 channels_used[AT91_ADC_MAX_CHANNELS];
+	/* Number of channels in use */

just use bitmap_weight with the long above and
length set to num_channels.

arch/arm/mach-at91/include/mach/board.h extern void __init at91_pwm_leds(struct gpio_led *leds, int nr); · line 224

Quoted 1 line

arch/arm/mach-at91/include/mach/board.h +1 -0

+	u8 num_channels_used;

I didn't know bitmap functions before you mentioned it, but if I read it correctly, while I agree with your point, I will lose the ability to define a combination of enabled and disabled channel in a board specific manner.

In the third patch, I defined channels_used as an array of one. But let's say that instead I want all channels except the second one. With what's in the driver for now, I initialise it to {1, 0, 1, 1}. I don't think it's possible to do so with bitmap functions, or am I missing something ?

Or put the calls to bitmap_set in the board_init function ?

Cc Nicolas Ferre <[email protected]>

Cc Patrice Vilchez <[email protected]>

Cc Jean-Christophe Plagniol-Villard <[email protected]>

Signed-off-by Maxime Ripard <[email protected]>

---
 drivers/staging/iio/adc/Kconfig   |    6 +
 drivers/staging/iio/adc/Makefile  |    1 +
 drivers/staging/iio/adc/at91adc.c |  310 +++++++++++++++++++++++++++++++++++++
 3 files changed, 317 insertions(+), 0 deletions(-)
 create mode 100644 drivers/staging/iio/adc/at91adc.c

drivers/staging/iio/adc/Kconfig +6 -0

index d9decea..c384894 100644
@@ -169,6 +169,12 @@ config AD7280
 	  To compile this driver as a module, choose M here: the
 	  module will be called ad7280a
 
+config AT91ADC
+	tristate "Atmel AT91 ADC"
+	depends on SYSFS && ARCH_AT91
+	help
+	  Say yes here to build support for Atmel AT91 ADC.
+
 config MAX1363
 	tristate "Maxim max1363 ADC driver"
 	depends on I2C

drivers/staging/iio/adc/Makefile +1 -0

index ceee7f3..d250d06 100644
@@ -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_AT91ADC) += at91adc.o

drivers/staging/iio/adc/at91adc.c new file +0 -0

new file mode 100644
index 0000000..acf656d

drivers/staging/iio/adc/at91adc.c new file +12 -0

+ *
+ * Copyright 2011 Free Electrons
+ *
+ * Licensed under the GPLv2 or later.
+ */
+
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/interrupt.h>
+#include <linux/jiffies.h>
+#include <linux/kernel.h>

drivers/staging/iio/adc/at91adc.c new file +4 -0

+	u16 lcdr;
+	void __iomem *reg_base;
+	unsigned int vref_mv;
+};

drivers/staging/iio/adc/at91adc.c new file +11 -0

+	for (chan = 0; chan < st->nb_chan; chan++) {
+		if (status & AT91_ADC_EOC(chan)) {
+			st->done = true;
+			st->lcdr = at91adc_reg_read(st->reg_base,
+						    AT91_ADC_CHR(chan));
+		}
+	}
+	wake_up_interruptible(&st->wq_data_avail);
+
+	return IRQ_HANDLED;
+}

drivers/staging/iio/adc/at91adc.c new file +61 -0

+			chan->info_mask = IIO_CHAN_INFO_SCALE_SHARED_BIT;
+			++idx;
+		}
+	}
+
+	return st->nb_chan;
+}
+
+static int at91adc_read_raw(struct iio_dev *idev,
+			    struct iio_chan_spec const *chan,
+			    int *val, int *val2, long mask)
+{
+	struct at91adc_state *st = iio_priv(idev);
+	unsigned int scale_uv;
+
+	switch (mask) {
+	case 0:
+		mutex_lock(&st->lock);
+
+		at91adc_reg_write(st->reg_base, AT91_ADC_CHER,
+				  AT91_ADC_CH(chan->channel));
+		at91adc_reg_write(st->reg_base, AT91_ADC_IER,
+				  AT91_ADC_EOC(chan->channel));
+		at91adc_reg_write(st->reg_base, AT91_ADC_CR, AT91_ADC_START);
+
+		wait_event_interruptible_timeout(st->wq_data_avail, st->done,
+						 msecs_to_jiffies(10 * 1000));
+		*val = st->lcdr;
+
+		at91adc_reg_write(st->reg_base, AT91_ADC_CHDR,
+				  AT91_ADC_CH(chan->channel));
+		at91adc_reg_write(st->reg_base, AT91_ADC_IDR,
+				  AT91_ADC_EOC(chan->channel));
+
+		st->lcdr = 0;
+		st->done = false;
+		mutex_unlock(&st->lock);
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_SCALE:
+		scale_uv = (st->vref_mv * 1000) >> chan->scan_type.realbits;
+		*val = scale_uv / 1000;
+		*val2 = (scale_uv % 1000) * 1000;
+		return IIO_VAL_INT_PLUS_MICRO;
+	default:
+		break;
+	}
+	return -EINVAL;
+}
+
+static const struct iio_info at91adc_info = {
+	.driver_module = THIS_MODULE,
+	.read_raw = &at91adc_read_raw,
+};
+
+static int __devinit at91adc_probe(struct platform_device *pdev)
+{
+	unsigned int prsc, mstrclk, ticks;
+	int ret;
+	struct iio_dev *idev;
+	struct at91adc_state *st;
+	struct resource *res;

drivers/staging/iio/adc/at91adc.c new file +75 -0

+		dev_err(&pdev->dev, "Failed to allocate memory.\n");
+		ret = -ENOMEM;
+		goto error_ret;
+	}
+	platform_set_drvdata(pdev, idev);
+
+	idev->dev.parent = &pdev->dev;
+	idev->name = dev_name(&pdev->dev);
+	idev->modes = INDIO_DIRECT_MODE;
+	idev->info = &at91adc_info;
+
+	st = iio_priv(idev);
+	st->irq = platform_get_irq(pdev, 0);
+	if (st->irq < 0) {
+		dev_err(&pdev->dev, "No IRQ ID is designated\n");
+		ret = -ENODEV;
+		goto error_free_device;
+	}
+
+	if (!request_mem_region(res->start, resource_size(res),
+				"AT91 adc registers")) {
+		dev_err(&pdev->dev, "Resources are unavailable.\n");
+		ret = -EBUSY;
+		goto error_free_device;
+	}
+
+	st->reg_base = ioremap(res->start, resource_size(res));
+	if (!st->reg_base) {
+		dev_err(&pdev->dev, "Failed to map registers.\n");
+		ret = -ENOMEM;
+		goto error_release_mem;
+	}
+
+	/*
+	 * Disable all IRQs before setting up the handler
+	 */
+	at91adc_reg_write(st->reg_base, AT91_ADC_CR, AT91_ADC_SWRST);
+	at91adc_reg_write(st->reg_base, AT91_ADC_IDR, 0xFFFFFFFF);
+	ret = request_irq(st->irq,
+			  at91adc_eoc_trigger, 0, pdev->dev.driver->name, st);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to allocate IRQ.\n");
+		goto error_unmap_reg;
+	}
+
+	st->clk = clk_get(&pdev->dev, "adc_clk");
+	if (IS_ERR(st->clk)) {
+		dev_err(&pdev->dev, "Failed to get the clock.\n");
+		ret = PTR_ERR(st->clk);
+		goto error_free_irq;
+	}
+
+	clk_enable(st->clk);
+	mstrclk = clk_get_rate(st->clk);
+
+	if (!pdata) {
+		dev_err(&pdev->dev, "No platform data available.\n");
+		ret = -EINVAL;
+		goto error_free_clk;
+	}
+
+	if (!pdata->adc_clock) {
+		dev_err(&pdev->dev, "No ADCClock available.\n");
+		ret = -EINVAL;
+		goto error_free_clk;
+	}
+
+	prsc = (mstrclk / (2 * pdata->adc_clock)) - 1;
+
+	if (!pdata->startup_time) {
+		dev_err(&pdev->dev, "No startup time available.\n");
+		ret = -EINVAL;
+		goto error_free_clk;
+	}
+

drivers/staging/iio/adc/at91adc.c new file +17 -0

+		goto error_free_clk;
+	}
+
+	return 0;
+
+error_free_clk:
+	clk_disable(st->clk);
+	clk_put(st->clk);
+error_free_irq:
+	free_irq(st->irq, st);
+error_unmap_reg:
+	iounmap(st->reg_base);
+error_release_mem:
+	release_mem_region(res->start, resource_size(res));
+error_free_device:
+	iio_free_device(idev);
+error_ret:

drivers/staging/iio/adc/at91adc.c new file +16 -0

+	free_irq(st->irq, st);
+	iounmap(st->reg_base);
+	release_mem_region(res->start, resource_size(res));
+	iio_free_device(idev);
+
+	return 0;
+}
+
+static struct platform_driver at91adc_driver = {
+	.probe = at91adc_probe,
+	.remove = __devexit_p(at91adc_remove),
+	.driver = {
+		   .name = "at91adc",
+		   },
+};
+

drivers/staging/iio/adc/at91adc.c new file +6 -0

+module_init(at91adc_init);
+module_exit(at91adc_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Atmel AT91 ADC Driver");
+MODULE_AUTHOR("Maxime Ripard <[email protected]>");

drivers/staging/iio/adc/at91adc.c line 3

Quoted 9 lines · Maxime Ripard

drivers/staging/iio/adc/at91adc.c new file +3 -0

new file mode 100644
index 0000000..acf656d
@@ -0,0 +1,310 @@
+/*
+ * Driver for the TouchScreen ADC Controller present in the Atmel AT91
+ * evaluation boards.

I'd add something here to make it clear that it isn't being used for touchscreen input in this driver.

Quoted 16 lines

drivers/staging/iio/adc/at91adc.c new file +16 -0

+ *
+ * Copyright 2011 Free Electrons
+ *
+ * Licensed under the GPLv2 or later.
+ */
+
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/interrupt.h>
+#include <linux/jiffies.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/wait.h>

drivers/staging/iio/adc/at91adc.c line 25

Quoted 6 lines

drivers/staging/iio/adc/at91adc.c new file +6 -0

+
+#include "../iio.h"
+
+#include <mach/at91_adc.h>
+#include <mach/board.h>
+

I'd marginally prefer to see these as inlines rather than macros. Far from critical though!

drivers/staging/iio/adc/at91adc.c line 32

Quoted 7 lines

drivers/staging/iio/adc/at91adc.c new file +7 -0

+#define at91adc_reg_read(base, reg)        readl_relaxed((base) + (reg))
+#define at91adc_reg_write(base, reg, val)  writel_relaxed((val), (base) + (reg))
+
+struct at91adc_state {
+	struct clk *clk;
+	bool done;
+	struct mutex lock;

why hold a local pointer? Just put these
directly into iio_dev->channels.

drivers/staging/iio/adc/at91adc.c line 33

Quoted 1 line

drivers/staging/iio/adc/at91adc.c new file +1 -0

+	struct iio_chan_spec *channels;

likewise, we already have iio_dev->num_channels.

drivers/staging/iio/adc/at91adc.c line 34

Quoted 1 line

drivers/staging/iio/adc/at91adc.c new file +1 -0

+	int nb_chan;

drivers/staging/iio/adc/at91adc.c line 36

Quoted 2 lines

drivers/staging/iio/adc/at91adc.c new file +2 -0

+	int irq;
+	wait_queue_head_t wq_data_avail;

cryptic element. Nice to have everything documented, but this one is most important as it isn't self explanatory.

Quoted 4 lines

drivers/staging/iio/adc/at91adc.c new file +4 -0

+	u16 lcdr;
+	void __iomem *reg_base;
+	unsigned int vref_mv;
+};

drivers/staging/iio/adc/at91adc.c line 50

Quoted 10 lines

drivers/staging/iio/adc/at91adc.c new file +10 -0

+
+static irqreturn_t at91adc_eoc_trigger(int irq, void *private)
+{
+	int chan;
+	struct at91adc_state *st = private;
+	unsigned int status = at91adc_reg_read(st->reg_base, AT91_ADC_SR);
+
+	if (!(status & AT91_ADC_DRDY))
+		return IRQ_HANDLED;
+

extra unneeded brackets for the for loop.

Quoted 11 lines

drivers/staging/iio/adc/at91adc.c new file +11 -0

+	for (chan = 0; chan < st->nb_chan; chan++) {
+		if (status & AT91_ADC_EOC(chan)) {
+			st->done = true;
+			st->lcdr = at91adc_reg_read(st->reg_base,
+						    AT91_ADC_CHR(chan));
+		}
+	}
+	wake_up_interruptible(&st->wq_data_avail);
+
+	return IRQ_HANDLED;
+}

drivers/staging/iio/adc/at91adc.c line 71

Quoted 10 lines

drivers/staging/iio/adc/at91adc.c new file +10 -0

+
+static int at91adc_channel_init(struct at91adc_state *st,
+				struct at91_adc_data *pdata)
+{
+	int i, idx = 0;
+	st->channels = kcalloc(pdata->num_channels_used,
+			       sizeof(struct iio_chan_spec), GFP_KERNEL);
+	if (st->channels == NULL)
+		return -ENOMEM;
+

Again, extra brackets. Also, some wrong spacing for the if. can you run checkpatch.pl over this again and make sure you have fixed everything.

drivers/staging/iio/adc/at91adc.c line 72

Quoted 1 line

drivers/staging/iio/adc/at91adc.c new file +1 -0

+	for (i = 0; i < st->nb_chan; i++) {

as stated for the previous patch, this should be a bitmap rather
than array of u8's. You can then use for_each_bit_set
to clean this code up nicely ;)

drivers/staging/iio/adc/at91adc.c line 80

Quoted 8 lines

drivers/staging/iio/adc/at91adc.c new file +8 -0

+		if(pdata->channels_used[i]) {
+			struct iio_chan_spec *chan = st->channels + idx;
+			chan->type = IIO_VOLTAGE;
+			chan->indexed = 1;
+			chan->channel = i;
+			chan->scan_type.sign = 's';
+			chan->scan_type.realbits = 10;
+			chan->scan_type.storagebits = 32;

That is pretty inefficient storage! If we do implement buffering on this, given mass reads don't look to be quicker, we'll just munge it down to 16 bits.

drivers/staging/iio/adc/at91adc.c line 81

Quoted 2 lines

drivers/staging/iio/adc/at91adc.c new file +1 -0

+			chan->scan_type.shift = 0;

don't bother initialising this value as it is zero (and that's an obvious default).

Quoted 64 lines

drivers/staging/iio/adc/at91adc.c new file +64 -0

+			chan->info_mask = IIO_CHAN_INFO_SCALE_SHARED_BIT;
+			++idx;
+		}
+	}
+
+	return st->nb_chan;
+}
+
+static int at91adc_read_raw(struct iio_dev *idev,
+			    struct iio_chan_spec const *chan,
+			    int *val, int *val2, long mask)
+{
+	struct at91adc_state *st = iio_priv(idev);
+	unsigned int scale_uv;
+
+	switch (mask) {
+	case 0:
+		mutex_lock(&st->lock);
+
+		at91adc_reg_write(st->reg_base, AT91_ADC_CHER,
+				  AT91_ADC_CH(chan->channel));
+		at91adc_reg_write(st->reg_base, AT91_ADC_IER,
+				  AT91_ADC_EOC(chan->channel));
+		at91adc_reg_write(st->reg_base, AT91_ADC_CR, AT91_ADC_START);
+
+		wait_event_interruptible_timeout(st->wq_data_avail, st->done,
+						 msecs_to_jiffies(10 * 1000));
+		*val = st->lcdr;
+
+		at91adc_reg_write(st->reg_base, AT91_ADC_CHDR,
+				  AT91_ADC_CH(chan->channel));
+		at91adc_reg_write(st->reg_base, AT91_ADC_IDR,
+				  AT91_ADC_EOC(chan->channel));
+
+		st->lcdr = 0;
+		st->done = false;
+		mutex_unlock(&st->lock);
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_SCALE:
+		scale_uv = (st->vref_mv * 1000) >> chan->scan_type.realbits;
+		*val = scale_uv / 1000;
+		*val2 = (scale_uv % 1000) * 1000;
+		return IIO_VAL_INT_PLUS_MICRO;
+	default:
+		break;
+	}
+	return -EINVAL;
+}
+
+static const struct iio_info at91adc_info = {
+	.driver_module = THIS_MODULE,
+	.read_raw = &at91adc_read_raw,
+};
+
+static int __devinit at91adc_probe(struct platform_device *pdev)
+{
+	unsigned int prsc, mstrclk, ticks;
+	int ret;
+	struct iio_dev *idev;
+	struct at91adc_state *st;
+	struct resource *res;
+	struct at91_adc_data *pdata = pdev->dev.platform_data;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);

drivers/staging/iio/adc/at91adc.c line 152

Quoted 7 lines

drivers/staging/iio/adc/at91adc.c new file +7 -0

+	if (!res) {
+		dev_err(&pdev->dev, "No resource defined\n");
+		ret = -ENXIO;
+		goto error_ret;
+	}
+
+	idev = iio_allocate_device(sizeof(struct at91adc_state));

sizeof(*st) is a little neater?

drivers/staging/iio/adc/at91adc.c line 153

Quoted 1 line

drivers/staging/iio/adc/at91adc.c new file +1 -0

+	if (idev == NULL) {

I'm not sure this message adds anything or is even necessarily correct as that function doesn't just allocate memory!

Quoted 75 lines

drivers/staging/iio/adc/at91adc.c new file +75 -0

+		dev_err(&pdev->dev, "Failed to allocate memory.\n");
+		ret = -ENOMEM;
+		goto error_ret;
+	}
+	platform_set_drvdata(pdev, idev);
+
+	idev->dev.parent = &pdev->dev;
+	idev->name = dev_name(&pdev->dev);
+	idev->modes = INDIO_DIRECT_MODE;
+	idev->info = &at91adc_info;
+
+	st = iio_priv(idev);
+	st->irq = platform_get_irq(pdev, 0);
+	if (st->irq < 0) {
+		dev_err(&pdev->dev, "No IRQ ID is designated\n");
+		ret = -ENODEV;
+		goto error_free_device;
+	}
+
+	if (!request_mem_region(res->start, resource_size(res),
+				"AT91 adc registers")) {
+		dev_err(&pdev->dev, "Resources are unavailable.\n");
+		ret = -EBUSY;
+		goto error_free_device;
+	}
+
+	st->reg_base = ioremap(res->start, resource_size(res));
+	if (!st->reg_base) {
+		dev_err(&pdev->dev, "Failed to map registers.\n");
+		ret = -ENOMEM;
+		goto error_release_mem;
+	}
+
+	/*
+	 * Disable all IRQs before setting up the handler
+	 */
+	at91adc_reg_write(st->reg_base, AT91_ADC_CR, AT91_ADC_SWRST);
+	at91adc_reg_write(st->reg_base, AT91_ADC_IDR, 0xFFFFFFFF);
+	ret = request_irq(st->irq,
+			  at91adc_eoc_trigger, 0, pdev->dev.driver->name, st);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to allocate IRQ.\n");
+		goto error_unmap_reg;
+	}
+
+	st->clk = clk_get(&pdev->dev, "adc_clk");
+	if (IS_ERR(st->clk)) {
+		dev_err(&pdev->dev, "Failed to get the clock.\n");
+		ret = PTR_ERR(st->clk);
+		goto error_free_irq;
+	}
+
+	clk_enable(st->clk);
+	mstrclk = clk_get_rate(st->clk);
+
+	if (!pdata) {
+		dev_err(&pdev->dev, "No platform data available.\n");
+		ret = -EINVAL;
+		goto error_free_clk;
+	}
+
+	if (!pdata->adc_clock) {
+		dev_err(&pdev->dev, "No ADCClock available.\n");
+		ret = -EINVAL;
+		goto error_free_clk;
+	}
+
+	prsc = (mstrclk / (2 * pdata->adc_clock)) - 1;
+
+	if (!pdata->startup_time) {
+		dev_err(&pdev->dev, "No startup time available.\n");
+		ret = -EINVAL;
+		goto error_free_clk;
+	}
+

drivers/staging/iio/adc/at91adc.c line 238

Quoted 10 lines

drivers/staging/iio/adc/at91adc.c new file +10 -0

+	ticks = round_up((pdata->startup_time * pdata->adc_clock /
+			  1000000) - 1, 8) / 8;
+	at91adc_reg_write(st->reg_base, AT91_ADC_MR,
+			  (AT91_ADC_PRESCAL_(prsc) & AT91_ADC_PRESCAL) |
+			  (AT91_ADC_STARTUP_(ticks) & AT91_ADC_STARTUP));
+
+	/* Setup the ADC channels available on the board */
+	st->nb_chan = pdata->num_channels;
+	ret = at91adc_channel_init(st, pdata);
+	if (ret < 0) {

no brackets please. Again, checkpatch would probably have shouted about this.

drivers/staging/iio/adc/at91adc.c line 214

Quoted 3 lines

drivers/staging/iio/adc/at91adc.c new file +3 -0

+		goto error_free_clk;
+	}
+

This is what I meant when I asked why you have two copies of the pointer and count?

drivers/staging/iio/adc/at91adc.c line 250

Quoted 9 lines

drivers/staging/iio/adc/at91adc.c new file +9 -0

+	idev->channels = st->channels;
+	idev->num_channels = pdata->num_channels_used;
+
+	init_waitqueue_head(&st->wq_data_avail);
+	mutex_init(&st->lock);
+
+	st->vref_mv = pdata->vref;
+
+	ret = iio_device_register(idev);

again incorrect brackets for the code style.

drivers/staging/iio/adc/at91adc.c line 251

Quoted 1 line

drivers/staging/iio/adc/at91adc.c new file +1 -0

+	if (ret < 0) {

unwind the channel init?

Quoted 17 lines

drivers/staging/iio/adc/at91adc.c new file +17 -0

+		goto error_free_clk;
+	}
+
+	return 0;
+
+error_free_clk:
+	clk_disable(st->clk);
+	clk_put(st->clk);
+error_free_irq:
+	free_irq(st->irq, st);
+error_unmap_reg:
+	iounmap(st->reg_base);
+error_release_mem:
+	release_mem_region(res->start, resource_size(res));
+error_free_device:
+	iio_free_device(idev);
+error_ret:

drivers/staging/iio/adc/at91adc.c line 278

Quoted 10 lines

drivers/staging/iio/adc/at91adc.c new file +10 -0

+	return ret;
+}
+
+static int __devexit at91adc_remove(struct platform_device *pdev)
+{
+	struct iio_dev *idev = platform_get_drvdata(pdev);
+	struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	struct at91adc_state *st = iio_priv(idev);
+
+	iio_device_unregister(idev);

Free channels array?

Quoted 16 lines

drivers/staging/iio/adc/at91adc.c new file +16 -0

+	free_irq(st->irq, st);
+	iounmap(st->reg_base);
+	release_mem_region(res->start, resource_size(res));
+	iio_free_device(idev);
+
+	return 0;
+}
+
+static struct platform_driver at91adc_driver = {
+	.probe = at91adc_probe,
+	.remove = __devexit_p(at91adc_remove),
+	.driver = {
+		   .name = "at91adc",
+		   },
+};
+

drivers/staging/iio/adc/at91adc.c line 304

Quoted 10 lines

drivers/staging/iio/adc/at91adc.c new file +10 -0

+static int __init at91adc_init(void)
+{
+	return platform_driver_register(&at91adc_driver);
+}
+
+static void __exit at91adc_exit(void)
+{
+	platform_driver_unregister(&at91adc_driver);
+}
+

Some neat boiler plate removal stuff was just merged for platform drivers. See module_platform_driver in include/linux/platform.h

Definitely want to use that! I'm hoping the spi and i2c versions turn up soon as it will save us a lot of silly cut and paste code.