From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ppsw-41.csi.cam.ac.uk ([131.111.8.141]:41388 "EHLO ppsw-41.csi.cam.ac.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1759421Ab2EIOZs (ORCPT ); Wed, 9 May 2012 10:25:48 -0400 Message-ID: <4FAA7E69.5050200@cam.ac.uk> Date: Wed, 09 May 2012 15:25:45 +0100 From: Jonathan Cameron MIME-Version: 1.0 To: Maxime Ripard CC: linux-iio@vger.kernel.org, linux-arm-kernel@lists.infradead.org, thomas@free-electrons.com, patrice.vilchez@atmel.com, plagnioj@jcrosoft.com, grant.likely@secretlab.ca Subject: Re: [PATCH 2/9] ARM: AT91: IIO: Add AT91 ADC driver. References: <1336568528-29877-1-git-send-email-maxime.ripard@free-electrons.com> <1336568528-29877-3-git-send-email-maxime.ripard@free-electrons.com> In-Reply-To: <1336568528-29877-3-git-send-email-maxime.ripard@free-electrons.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Sender: linux-iio-owner@vger.kernel.org List-Id: linux-iio@vger.kernel.org On 5/9/2012 2:02 PM, Maxime Ripard wrote: > Add the ADC driver for Atmel's AT91SAM9G20-EK, AT91SAM9M10G45-EK > and AT91SAM9X5 family boards. > > It has support for both software and hardware triggers. Totally trivial, but please add a newline where it says below. Otherwise, looks fine to me. Couple of other passing comments inline, but nothing else to change unless you particularly want to. > Signed-off-by: Maxime Ripard > Acked-by: Nicolas Ferre Acked-by: Jonathan Cameron > --- > drivers/iio/Kconfig | 2 + > drivers/iio/Makefile | 2 + > drivers/iio/adc/Kconfig | 16 ++ > drivers/iio/adc/Makefile | 5 + > drivers/iio/adc/at91_adc.c | 673 ++++++++++++++++++++++++++++++++++++++++++++ > 5 files changed, 698 insertions(+) > create mode 100644 drivers/iio/adc/Kconfig > create mode 100644 drivers/iio/adc/Makefile > create mode 100644 drivers/iio/adc/at91_adc.c > > diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig > index 3ab7d48..47e0920 100644 > --- a/drivers/iio/Kconfig > +++ b/drivers/iio/Kconfig > @@ -48,4 +48,6 @@ config IIO_CONSUMERS_PER_TRIGGER > This value controls the maximum number of consumers that a > given trigger may handle. Default is 2. > > +source "drivers/iio/adc/Kconfig" > + > endif # IIO > diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile > index d5fc57d..eb926a2 100644 > --- a/drivers/iio/Makefile > +++ b/drivers/iio/Makefile > @@ -8,3 +8,5 @@ industrialio-$(CONFIG_IIO_BUFFER) += industrialio-buffer.o > industrialio-$(CONFIG_IIO_TRIGGER) += industrialio-trigger.o > > obj-$(CONFIG_IIO_KFIFO_BUF) += kfifo_buf.o > + > +obj-y += adc/ > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig > new file mode 100644 > index 0000000..9a0df81 > --- /dev/null > +++ b/drivers/iio/adc/Kconfig > @@ -0,0 +1,16 @@ > +# > +# ADC drivers > +# > +menu "Analog to digital converters" > + > +config AT91_ADC > + tristate "Atmel AT91 ADC" > + depends on ARCH_AT91 > + select IIO_BUFFER > + select IIO_KFIFO_BUF > + select IIO_TRIGGER > + select SYSFS > + help > + Say yes here to build support for Atmel AT91 ADC. > + > +endmenu > diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile > new file mode 100644 > index 0000000..b62d488 > --- /dev/null > +++ b/drivers/iio/adc/Makefile > @@ -0,0 +1,5 @@ > +# > +# Makefile for IIO ADC drivers > +# > + > +obj-$(CONFIG_AT91_ADC) += at91_adc.o Please add a newline here. > \ No newline at end of file > diff --git a/drivers/iio/adc/at91_adc.c b/drivers/iio/adc/at91_adc.c > new file mode 100644 > index 0000000..e5d73b1 > --- /dev/null > +++ b/drivers/iio/adc/at91_adc.c > @@ -0,0 +1,673 @@ > +/* > + * Driver for the ADC present in the Atmel AT91 evaluation boards. > + * > + * Copyright 2011 Free Electrons > + * > + * Licensed under the GPLv2 or later. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > + > +#include > +#include > +#include > +#include > +#include > + > +#include > + > +#define AT91_ADC_CHAN(st, ch) \ > + (st->registers->channel_base + (ch * 4)) > +#define at91_adc_readl(st, reg) \ > + (readl_relaxed(st->reg_base + reg)) > +#define at91_adc_writel(st, reg, val) \ > + (writel_relaxed(val, st->reg_base + reg)) > + > +struct at91_adc_state { > + struct clk *adc_clk; > + u16 *buffer; > + unsigned long channels_mask; > + struct clk *clk; > + bool done; > + int irq; > + bool irq_enabled; > + u16 last_value; > + struct mutex lock; > + u8 num_channels; > + void __iomem *reg_base; > + struct at91_adc_reg_desc *registers; > + u8 startup_time; > + struct iio_trigger **trig; > + struct at91_adc_trigger *trigger_list; > + u32 trigger_number; > + bool use_external; > + u32 vref_mv; > + wait_queue_head_t wq_data_avail; > +}; > + > +static irqreturn_t at91_adc_trigger_handler(int irq, void *p) > +{ > + struct iio_poll_func *pf = p; > + struct iio_dev *idev = pf->indio_dev; > + struct at91_adc_state *st = iio_priv(idev); > + struct iio_buffer *buffer = idev->buffer; > + int i, j = 0; > + > + for (i = 0; i< idev->masklength; i++) { > + if (!test_bit(i, idev->active_scan_mask)) > + continue; > + st->buffer[j] = at91_adc_readl(st, AT91_ADC_CHAN(st, i)); > + j++; > + } > + > + if (idev->scan_timestamp) { > + s64 *timestamp = (s64 *)((u8 *)st->buffer + > + ALIGN(j, sizeof(s64))); > + *timestamp = pf->timestamp; > + } > + > + buffer->access->store_to(buffer, (u8 *)st->buffer, pf->timestamp); > + > + iio_trigger_notify_done(idev->trig); > + st->irq_enabled = true; > + > + /* Needed to ACK the DRDY interruption */ > + at91_adc_readl(st, AT91_ADC_LCDR); > + > + enable_irq(st->irq); > + > + return IRQ_HANDLED; > +} > + > +static irqreturn_t at91_adc_eoc_trigger(int irq, void *private) > +{ > + struct iio_dev *idev = private; > + struct at91_adc_state *st = iio_priv(idev); > + u32 status = at91_adc_readl(st, st->registers->status_register); > + > + if (!(status& st->registers->drdy_mask)) > + return IRQ_HANDLED; > + > + if (iio_buffer_enabled(idev)) { > + disable_irq_nosync(irq); > + st->irq_enabled = false; > + iio_trigger_poll(idev->trig, iio_get_time_ns()); > + } else { > + st->last_value = at91_adc_readl(st, AT91_ADC_LCDR); > + st->done = true; > + wake_up_interruptible(&st->wq_data_avail); > + } > + > + return IRQ_HANDLED; > +} > + > +static int at91_adc_channel_init(struct iio_dev *idev) > +{ > + struct at91_adc_state *st = iio_priv(idev); > + struct iio_chan_spec *chan_array, *timestamp; > + int bit, idx = 0; > + > + idev->num_channels = bitmap_weight(&st->channels_mask, > + st->num_channels) + 1; > + > + chan_array = devm_kzalloc(&idev->dev, > + ((idev->num_channels + 1) * > + sizeof(struct iio_chan_spec)), > + GFP_KERNEL); > + > + if (!chan_array) > + return -ENOMEM; > + > + for_each_set_bit(bit,&st->channels_mask, st->num_channels) { > + struct iio_chan_spec *chan = chan_array + idx; > + > + chan->type = IIO_VOLTAGE; > + chan->indexed = 1; > + chan->channel = bit; > + chan->scan_index = idx; > + chan->scan_type.sign = 'u'; > + chan->scan_type.realbits = 10; > + chan->scan_type.storagebits = 16; > + chan->info_mask = IIO_CHAN_INFO_SCALE_SHARED_BIT | > + IIO_CHAN_INFO_RAW_SEPARATE_BIT; > + idx++; > + } > + timestamp = chan_array + idx; > + > + timestamp->type = IIO_TIMESTAMP; > + timestamp->channel = -1; > + timestamp->scan_index = idx; > + timestamp->scan_type.sign = 's'; > + timestamp->scan_type.realbits = 64; > + timestamp->scan_type.storagebits = 64; > + > + idev->channels = chan_array; > + return idev->num_channels; > +} > + > +static u8 at91_adc_get_trigger_value_by_name(struct iio_dev *idev, > + struct at91_adc_trigger *triggers, > + const char *trigger_name) > +{ > + struct at91_adc_state *st = iio_priv(idev); > + u8 value = 0; > + int i; > + > + for (i = 0; i< st->trigger_number; i++) { > + char *name = kasprintf(GFP_KERNEL, > + "%s-dev%d-%s", > + idev->name, > + idev->id, > + triggers[i].name); > + if (!name) > + return -ENOMEM; > + > + if (strcmp(trigger_name, name) == 0) { > + value = triggers[i].value; > + kfree(name); > + break; > + } > + > + kfree(name); > + } > + > + return value; > +} > + > +static int at91_adc_configure_trigger(struct iio_trigger *trig, bool state) > +{ > + struct iio_dev *idev = trig->private_data; > + struct at91_adc_state *st = iio_priv(idev); > + struct iio_buffer *buffer = idev->buffer; > + struct at91_adc_reg_desc *reg = st->registers; > + u32 status = at91_adc_readl(st, reg->trigger_register); > + u8 value; > + u8 bit; > + > + value = at91_adc_get_trigger_value_by_name(idev, > + st->trigger_list, > + idev->trig->name); > + if (value == 0) > + return -EINVAL; > + > + if (state) { > + st->buffer = kmalloc(idev->scan_bytes, GFP_KERNEL); > + if (st->buffer == NULL) > + return -ENOMEM; > + > + at91_adc_writel(st, reg->trigger_register, > + status | value); > + > + for_each_set_bit(bit, buffer->scan_mask, > + st->num_channels) { > + struct iio_chan_spec const *chan = idev->channels + bit; > + at91_adc_writel(st, AT91_ADC_CHER, > + AT91_ADC_CH(chan->channel)); > + } > + > + at91_adc_writel(st, AT91_ADC_IER, reg->drdy_mask); > + > + } else { > + at91_adc_writel(st, AT91_ADC_IDR, reg->drdy_mask); > + > + at91_adc_writel(st, reg->trigger_register, > + status& ~value); > + > + for_each_set_bit(bit, buffer->scan_mask, > + st->num_channels) { > + struct iio_chan_spec const *chan = idev->channels + bit; > + at91_adc_writel(st, AT91_ADC_CHDR, > + AT91_ADC_CH(chan->channel)); > + } > + kfree(st->buffer); > + } > + > + return 0; > +} > + > +static const struct iio_trigger_ops at91_adc_trigger_ops = { > + .owner = THIS_MODULE, > + .set_trigger_state =&at91_adc_configure_trigger, > +}; > + > +static struct iio_trigger *at91_adc_allocate_trigger(struct iio_dev *idev, > + struct at91_adc_trigger *trigger) > +{ > + struct iio_trigger *trig; > + int ret; > + > + trig = iio_trigger_alloc("%s-dev%d-%s", idev->name, > + idev->id, trigger->name); > + if (trig == NULL) > + return NULL; > + > + trig->dev.parent = idev->dev.parent; > + trig->private_data = idev; > + trig->ops =&at91_adc_trigger_ops; > + > + ret = iio_trigger_register(trig); > + if (ret) > + return NULL; > + > + return trig; > +} > + > +static int at91_adc_trigger_init(struct iio_dev *idev) > +{ > + struct at91_adc_state *st = iio_priv(idev); > + int i, ret; > + > + st->trig = devm_kzalloc(&idev->dev, > + st->trigger_number * sizeof(st->trig), > + GFP_KERNEL); > + > + if (st->trig == NULL) { > + ret = -ENOMEM; > + goto error_ret; > + } > + > + for (i = 0; i< st->trigger_number; i++) { > + if (st->trigger_list[i].is_external&& !(st->use_external)) > + continue; > + > + st->trig[i] = at91_adc_allocate_trigger(idev, > + st->trigger_list + i); > + if (st->trig[i] == NULL) { > + dev_err(&idev->dev, > + "Could not allocate trigger %d\n", i); > + ret = -ENOMEM; > + goto error_trigger; > + } > + } > + > + return 0; > + > +error_trigger: > + for (i--; i>= 0; i--) { > + iio_trigger_unregister(st->trig[i]); > + iio_trigger_free(st->trig[i]); > + } > +error_ret: > + return ret; > +} > + > +static void at91_adc_trigger_remove(struct iio_dev *idev) > +{ > + struct at91_adc_state *st = iio_priv(idev); > + int i; > + > + for (i = 0; i< st->trigger_number; i++) { > + iio_trigger_unregister(st->trig[i]); > + iio_trigger_free(st->trig[i]); > + } > +} > + > +static const struct iio_buffer_setup_ops at91_adc_buffer_ops = { > + .preenable =&iio_sw_buffer_preenable, > + .postenable =&iio_triggered_buffer_postenable, > + .predisable =&iio_triggered_buffer_predisable, > +}; > + > +static int at91_adc_buffer_init(struct iio_dev *idev) > +{ > + int ret; > + > + idev->buffer = iio_kfifo_allocate(idev); > + if (!idev->buffer) { > + ret = -ENOMEM; > + goto error_ret; > + } > + > + idev->pollfunc = iio_alloc_pollfunc(&iio_pollfunc_store_time, > + &at91_adc_trigger_handler, > + IRQF_ONESHOT, > + idev, > + "%s-consumer%d", > + idev->name, > + idev->id); > + if (idev->pollfunc == NULL) { > + ret = -ENOMEM; > + goto error_pollfunc; > + } > + > + idev->setup_ops =&at91_adc_buffer_ops; > + idev->modes |= INDIO_BUFFER_TRIGGERED; > + > + ret = iio_buffer_register(idev, > + idev->channels, > + idev->num_channels); > + if (ret) > + goto error_register; > + > + return 0; > + > +error_register: > + iio_dealloc_pollfunc(idev->pollfunc); > +error_pollfunc: > + iio_kfifo_free(idev->buffer); > +error_ret: > + return ret; > +} > + > +static void at91_adc_buffer_remove(struct iio_dev *idev) > +{ > + iio_buffer_unregister(idev); > + iio_dealloc_pollfunc(idev->pollfunc); > + iio_kfifo_free(idev->buffer); > +} > + > +static int at91_adc_read_raw(struct iio_dev *idev, > + struct iio_chan_spec const *chan, > + int *val, int *val2, long mask) > +{ > + struct at91_adc_state *st = iio_priv(idev); > + int ret; > + > + switch (mask) { > + case IIO_CHAN_INFO_RAW: > + mutex_lock(&st->lock); > + > + at91_adc_writel(st, AT91_ADC_CHER, > + AT91_ADC_CH(chan->channel)); > + at91_adc_writel(st, AT91_ADC_IER, st->registers->drdy_mask); > + at91_adc_writel(st, AT91_ADC_CR, AT91_ADC_START); > + > + ret = wait_event_interruptible_timeout(st->wq_data_avail, > + st->done, > + msecs_to_jiffies(1000)); > + if (ret == 0) > + return -ETIMEDOUT; > + else if (ret< 0) > + return ret; > + > + *val = st->last_value; > + > + at91_adc_writel(st, AT91_ADC_CHDR, > + AT91_ADC_CH(chan->channel)); > + at91_adc_writel(st, AT91_ADC_IDR, st->registers->drdy_mask); > + > + st->last_value = 0; > + st->done = false; > + mutex_unlock(&st->lock); > + return IIO_VAL_INT; > + > + case IIO_CHAN_INFO_SCALE: > + *val = (st->vref_mv * 1000)>> chan->scan_type.realbits; > + *val2 = 0; > + return IIO_VAL_INT_PLUS_MICRO; > + default: > + break; > + } > + return -EINVAL; > +} > + > +static int at91_adc_probe_pdata(struct at91_adc_state *st, > + struct platform_device *pdev) > +{ > + struct at91_adc_data *pdata = pdev->dev.platform_data; > + > + if (!pdata) > + return -EINVAL; > + This does look rather like it would be better to just keep a copy of pdata around... Still, not critical at all. > + st->use_external = pdata->use_external_triggers; > + st->vref_mv = pdata->vref; > + st->channels_mask = pdata->channels_used; > + st->num_channels = pdata->num_channels; > + st->startup_time = pdata->startup_time; > + st->trigger_number = pdata->trigger_number; > + st->trigger_list = pdata->trigger_list; > + st->registers = pdata->registers; > + > + return 0; > +} > + > +static const struct iio_info at91_adc_info = { > + .driver_module = THIS_MODULE, > + .read_raw =&at91_adc_read_raw, > +}; > + > +static int __devinit at91_adc_probe(struct platform_device *pdev) > +{ > + unsigned int prsc, mstrclk, ticks, adc_clk; > + int ret; > + struct iio_dev *idev; > + struct at91_adc_state *st; > + struct resource *res; > + > + idev = iio_device_alloc(sizeof(struct at91_adc_state)); > + if (idev == NULL) { > + ret = -ENOMEM; > + goto error_ret; > + } > + > + st = iio_priv(idev); > + > + ret = at91_adc_probe_pdata(st, pdev); > + if (ret) { > + dev_err(&pdev->dev, "No platform data available.\n"); > + ret = -EINVAL; > + goto error_free_device; > + } > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + if (!res) { > + dev_err(&pdev->dev, "No resource defined\n"); > + ret = -ENXIO; > + 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 =&at91_adc_info; > + > + 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 > + */ > + at91_adc_writel(st, AT91_ADC_CR, AT91_ADC_SWRST); > + at91_adc_writel(st, AT91_ADC_IDR, 0xFFFFFFFF); > + ret = request_irq(st->irq, > + at91_adc_eoc_trigger, > + 0, > + pdev->dev.driver->name, > + idev); > + 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; > + } > + > + ret = clk_prepare(st->clk); > + if (ret) { > + dev_err(&pdev->dev, "Could not prepare the clock.\n"); > + goto error_free_clk; > + } > + > + ret = clk_enable(st->clk); > + if (ret) { > + dev_err(&pdev->dev, "Could not enable the clock.\n"); > + goto error_unprepare_clk; > + } > + > + st->adc_clk = clk_get(&pdev->dev, "adc_op_clk"); > + if (IS_ERR(st->adc_clk)) { > + dev_err(&pdev->dev, "Failed to get the ADC clock.\n"); > + ret = PTR_ERR(st->clk); > + goto error_disable_clk; > + } > + > + ret = clk_prepare(st->adc_clk); > + if (ret) { > + dev_err(&pdev->dev, "Could not prepare the ADC clock.\n"); > + goto error_free_adc_clk; > + } > + > + ret = clk_enable(st->adc_clk); > + if (ret) { > + dev_err(&pdev->dev, "Could not enable the ADC clock.\n"); > + goto error_unprepare_adc_clk; > + } > + > + /* > + * Prescaler rate computation using the formula from the Atmel's > + * datasheet : ADC Clock = MCK / ((Prescaler + 1) * 2), ADC Clock being > + * specified by the electrical characteristics of the board. > + */ > + mstrclk = clk_get_rate(st->clk); > + adc_clk = clk_get_rate(st->adc_clk); > + prsc = (mstrclk / (2 * adc_clk)) - 1; > + > + if (!st->startup_time) { > + dev_err(&pdev->dev, "No startup time available.\n"); > + ret = -EINVAL; > + goto error_disable_adc_clk; > + } > + > + /* > + * Number of ticks needed to cover the startup time of the ADC as > + * defined in the electrical characteristics of the board, divided by 8. > + * The formula thus is : Startup Time = (ticks + 1) * 8 / ADC Clock > + */ > + ticks = round_up((st->startup_time * adc_clk / > + 1000000) - 1, 8) / 8; > + at91_adc_writel(st, 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 */ > + ret = at91_adc_channel_init(idev); > + if (ret< 0) { > + dev_err(&pdev->dev, "Couldn't initialize the channels.\n"); > + goto error_disable_adc_clk; > + } > + > + init_waitqueue_head(&st->wq_data_avail); > + mutex_init(&st->lock); > + > + ret = at91_adc_buffer_init(idev); > + if (ret< 0) { > + dev_err(&pdev->dev, "Couldn't initialize the buffer.\n"); > + goto error_disable_adc_clk; > + } > + > + ret = at91_adc_trigger_init(idev); > + if (ret< 0) { > + dev_err(&pdev->dev, "Couldn't setup the triggers.\n"); > + goto error_unregister_buffer; > + } > + > + ret = iio_device_register(idev); > + if (ret< 0) { > + dev_err(&pdev->dev, "Couldn't register the device.\n"); > + goto error_remove_triggers; > + } > + > + return 0; > + > +error_remove_triggers: > + at91_adc_trigger_remove(idev); > +error_unregister_buffer: > + at91_adc_buffer_remove(idev); > +error_disable_adc_clk: > + clk_disable(st->adc_clk); > +error_unprepare_adc_clk: > + clk_unprepare(st->adc_clk); > +error_free_adc_clk: > + clk_put(st->adc_clk); > +error_disable_clk: > + clk_disable(st->clk); > +error_unprepare_clk: > + clk_unprepare(st->clk); > +error_free_clk: > + clk_put(st->clk); > +error_free_irq: > + free_irq(st->irq, idev); > +error_unmap_reg: > + iounmap(st->reg_base); > +error_release_mem: > + release_mem_region(res->start, resource_size(res)); > +error_free_device: > + iio_device_free(idev); > +error_ret: > + return ret; > +} > + > +static int __devexit at91_adc_remove(struct platform_device *pdev) > +{ > + struct iio_dev *idev = platform_get_drvdata(pdev); > + struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + struct at91_adc_state *st = iio_priv(idev); > + > + iio_device_unregister(idev); > + at91_adc_trigger_remove(idev); > + at91_adc_buffer_remove(idev); There is clk_disable_unprepare... Could use it though then it's not quite as trivial to check everything lines up between probe and remove. > + clk_disable(st->adc_clk); > + clk_unprepare(st->adc_clk); > + clk_put(st->adc_clk); > + clk_disable(st->clk); > + clk_unprepare(st->clk); > + clk_put(st->clk); > + free_irq(st->irq, idev); > + iounmap(st->reg_base); > + release_mem_region(res->start, resource_size(res)); > + iio_device_free(idev); > + > + return 0; > +} > + > +static struct platform_driver at91_adc_driver = { > + .probe = at91_adc_probe, > + .remove = __devexit_p(at91_adc_remove), > + .driver = { > + .name = "at91_adc", > + }, > +}; > + > +module_platform_driver(at91_adc_driver); > + > +MODULE_LICENSE("GPL"); > +MODULE_DESCRIPTION("Atmel AT91 ADC Driver"); > +MODULE_AUTHOR("Maxime Ripard"); From mboxrd@z Thu Jan 1 00:00:00 1970 From: jic23@cam.ac.uk (Jonathan Cameron) Date: Wed, 09 May 2012 15:25:45 +0100 Subject: [PATCH 2/9] ARM: AT91: IIO: Add AT91 ADC driver. In-Reply-To: <1336568528-29877-3-git-send-email-maxime.ripard@free-electrons.com> References: <1336568528-29877-1-git-send-email-maxime.ripard@free-electrons.com> <1336568528-29877-3-git-send-email-maxime.ripard@free-electrons.com> Message-ID: <4FAA7E69.5050200@cam.ac.uk> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 5/9/2012 2:02 PM, Maxime Ripard wrote: > Add the ADC driver for Atmel's AT91SAM9G20-EK, AT91SAM9M10G45-EK > and AT91SAM9X5 family boards. > > It has support for both software and hardware triggers. Totally trivial, but please add a newline where it says below. Otherwise, looks fine to me. Couple of other passing comments inline, but nothing else to change unless you particularly want to. > Signed-off-by: Maxime Ripard > Acked-by: Nicolas Ferre Acked-by: Jonathan Cameron > --- > drivers/iio/Kconfig | 2 + > drivers/iio/Makefile | 2 + > drivers/iio/adc/Kconfig | 16 ++ > drivers/iio/adc/Makefile | 5 + > drivers/iio/adc/at91_adc.c | 673 ++++++++++++++++++++++++++++++++++++++++++++ > 5 files changed, 698 insertions(+) > create mode 100644 drivers/iio/adc/Kconfig > create mode 100644 drivers/iio/adc/Makefile > create mode 100644 drivers/iio/adc/at91_adc.c > > diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig > index 3ab7d48..47e0920 100644 > --- a/drivers/iio/Kconfig > +++ b/drivers/iio/Kconfig > @@ -48,4 +48,6 @@ config IIO_CONSUMERS_PER_TRIGGER > This value controls the maximum number of consumers that a > given trigger may handle. Default is 2. > > +source "drivers/iio/adc/Kconfig" > + > endif # IIO > diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile > index d5fc57d..eb926a2 100644 > --- a/drivers/iio/Makefile > +++ b/drivers/iio/Makefile > @@ -8,3 +8,5 @@ industrialio-$(CONFIG_IIO_BUFFER) += industrialio-buffer.o > industrialio-$(CONFIG_IIO_TRIGGER) += industrialio-trigger.o > > obj-$(CONFIG_IIO_KFIFO_BUF) += kfifo_buf.o > + > +obj-y += adc/ > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig > new file mode 100644 > index 0000000..9a0df81 > --- /dev/null > +++ b/drivers/iio/adc/Kconfig > @@ -0,0 +1,16 @@ > +# > +# ADC drivers > +# > +menu "Analog to digital converters" > + > +config AT91_ADC > + tristate "Atmel AT91 ADC" > + depends on ARCH_AT91 > + select IIO_BUFFER > + select IIO_KFIFO_BUF > + select IIO_TRIGGER > + select SYSFS > + help > + Say yes here to build support for Atmel AT91 ADC. > + > +endmenu > diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile > new file mode 100644 > index 0000000..b62d488 > --- /dev/null > +++ b/drivers/iio/adc/Makefile > @@ -0,0 +1,5 @@ > +# > +# Makefile for IIO ADC drivers > +# > + > +obj-$(CONFIG_AT91_ADC) += at91_adc.o Please add a newline here. > \ No newline at end of file > diff --git a/drivers/iio/adc/at91_adc.c b/drivers/iio/adc/at91_adc.c > new file mode 100644 > index 0000000..e5d73b1 > --- /dev/null > +++ b/drivers/iio/adc/at91_adc.c > @@ -0,0 +1,673 @@ > +/* > + * Driver for the ADC present in the Atmel AT91 evaluation boards. > + * > + * Copyright 2011 Free Electrons > + * > + * Licensed under the GPLv2 or later. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > + > +#include > +#include > +#include > +#include > +#include > + > +#include > + > +#define AT91_ADC_CHAN(st, ch) \ > + (st->registers->channel_base + (ch * 4)) > +#define at91_adc_readl(st, reg) \ > + (readl_relaxed(st->reg_base + reg)) > +#define at91_adc_writel(st, reg, val) \ > + (writel_relaxed(val, st->reg_base + reg)) > + > +struct at91_adc_state { > + struct clk *adc_clk; > + u16 *buffer; > + unsigned long channels_mask; > + struct clk *clk; > + bool done; > + int irq; > + bool irq_enabled; > + u16 last_value; > + struct mutex lock; > + u8 num_channels; > + void __iomem *reg_base; > + struct at91_adc_reg_desc *registers; > + u8 startup_time; > + struct iio_trigger **trig; > + struct at91_adc_trigger *trigger_list; > + u32 trigger_number; > + bool use_external; > + u32 vref_mv; > + wait_queue_head_t wq_data_avail; > +}; > + > +static irqreturn_t at91_adc_trigger_handler(int irq, void *p) > +{ > + struct iio_poll_func *pf = p; > + struct iio_dev *idev = pf->indio_dev; > + struct at91_adc_state *st = iio_priv(idev); > + struct iio_buffer *buffer = idev->buffer; > + int i, j = 0; > + > + for (i = 0; i< idev->masklength; i++) { > + if (!test_bit(i, idev->active_scan_mask)) > + continue; > + st->buffer[j] = at91_adc_readl(st, AT91_ADC_CHAN(st, i)); > + j++; > + } > + > + if (idev->scan_timestamp) { > + s64 *timestamp = (s64 *)((u8 *)st->buffer + > + ALIGN(j, sizeof(s64))); > + *timestamp = pf->timestamp; > + } > + > + buffer->access->store_to(buffer, (u8 *)st->buffer, pf->timestamp); > + > + iio_trigger_notify_done(idev->trig); > + st->irq_enabled = true; > + > + /* Needed to ACK the DRDY interruption */ > + at91_adc_readl(st, AT91_ADC_LCDR); > + > + enable_irq(st->irq); > + > + return IRQ_HANDLED; > +} > + > +static irqreturn_t at91_adc_eoc_trigger(int irq, void *private) > +{ > + struct iio_dev *idev = private; > + struct at91_adc_state *st = iio_priv(idev); > + u32 status = at91_adc_readl(st, st->registers->status_register); > + > + if (!(status& st->registers->drdy_mask)) > + return IRQ_HANDLED; > + > + if (iio_buffer_enabled(idev)) { > + disable_irq_nosync(irq); > + st->irq_enabled = false; > + iio_trigger_poll(idev->trig, iio_get_time_ns()); > + } else { > + st->last_value = at91_adc_readl(st, AT91_ADC_LCDR); > + st->done = true; > + wake_up_interruptible(&st->wq_data_avail); > + } > + > + return IRQ_HANDLED; > +} > + > +static int at91_adc_channel_init(struct iio_dev *idev) > +{ > + struct at91_adc_state *st = iio_priv(idev); > + struct iio_chan_spec *chan_array, *timestamp; > + int bit, idx = 0; > + > + idev->num_channels = bitmap_weight(&st->channels_mask, > + st->num_channels) + 1; > + > + chan_array = devm_kzalloc(&idev->dev, > + ((idev->num_channels + 1) * > + sizeof(struct iio_chan_spec)), > + GFP_KERNEL); > + > + if (!chan_array) > + return -ENOMEM; > + > + for_each_set_bit(bit,&st->channels_mask, st->num_channels) { > + struct iio_chan_spec *chan = chan_array + idx; > + > + chan->type = IIO_VOLTAGE; > + chan->indexed = 1; > + chan->channel = bit; > + chan->scan_index = idx; > + chan->scan_type.sign = 'u'; > + chan->scan_type.realbits = 10; > + chan->scan_type.storagebits = 16; > + chan->info_mask = IIO_CHAN_INFO_SCALE_SHARED_BIT | > + IIO_CHAN_INFO_RAW_SEPARATE_BIT; > + idx++; > + } > + timestamp = chan_array + idx; > + > + timestamp->type = IIO_TIMESTAMP; > + timestamp->channel = -1; > + timestamp->scan_index = idx; > + timestamp->scan_type.sign = 's'; > + timestamp->scan_type.realbits = 64; > + timestamp->scan_type.storagebits = 64; > + > + idev->channels = chan_array; > + return idev->num_channels; > +} > + > +static u8 at91_adc_get_trigger_value_by_name(struct iio_dev *idev, > + struct at91_adc_trigger *triggers, > + const char *trigger_name) > +{ > + struct at91_adc_state *st = iio_priv(idev); > + u8 value = 0; > + int i; > + > + for (i = 0; i< st->trigger_number; i++) { > + char *name = kasprintf(GFP_KERNEL, > + "%s-dev%d-%s", > + idev->name, > + idev->id, > + triggers[i].name); > + if (!name) > + return -ENOMEM; > + > + if (strcmp(trigger_name, name) == 0) { > + value = triggers[i].value; > + kfree(name); > + break; > + } > + > + kfree(name); > + } > + > + return value; > +} > + > +static int at91_adc_configure_trigger(struct iio_trigger *trig, bool state) > +{ > + struct iio_dev *idev = trig->private_data; > + struct at91_adc_state *st = iio_priv(idev); > + struct iio_buffer *buffer = idev->buffer; > + struct at91_adc_reg_desc *reg = st->registers; > + u32 status = at91_adc_readl(st, reg->trigger_register); > + u8 value; > + u8 bit; > + > + value = at91_adc_get_trigger_value_by_name(idev, > + st->trigger_list, > + idev->trig->name); > + if (value == 0) > + return -EINVAL; > + > + if (state) { > + st->buffer = kmalloc(idev->scan_bytes, GFP_KERNEL); > + if (st->buffer == NULL) > + return -ENOMEM; > + > + at91_adc_writel(st, reg->trigger_register, > + status | value); > + > + for_each_set_bit(bit, buffer->scan_mask, > + st->num_channels) { > + struct iio_chan_spec const *chan = idev->channels + bit; > + at91_adc_writel(st, AT91_ADC_CHER, > + AT91_ADC_CH(chan->channel)); > + } > + > + at91_adc_writel(st, AT91_ADC_IER, reg->drdy_mask); > + > + } else { > + at91_adc_writel(st, AT91_ADC_IDR, reg->drdy_mask); > + > + at91_adc_writel(st, reg->trigger_register, > + status& ~value); > + > + for_each_set_bit(bit, buffer->scan_mask, > + st->num_channels) { > + struct iio_chan_spec const *chan = idev->channels + bit; > + at91_adc_writel(st, AT91_ADC_CHDR, > + AT91_ADC_CH(chan->channel)); > + } > + kfree(st->buffer); > + } > + > + return 0; > +} > + > +static const struct iio_trigger_ops at91_adc_trigger_ops = { > + .owner = THIS_MODULE, > + .set_trigger_state =&at91_adc_configure_trigger, > +}; > + > +static struct iio_trigger *at91_adc_allocate_trigger(struct iio_dev *idev, > + struct at91_adc_trigger *trigger) > +{ > + struct iio_trigger *trig; > + int ret; > + > + trig = iio_trigger_alloc("%s-dev%d-%s", idev->name, > + idev->id, trigger->name); > + if (trig == NULL) > + return NULL; > + > + trig->dev.parent = idev->dev.parent; > + trig->private_data = idev; > + trig->ops =&at91_adc_trigger_ops; > + > + ret = iio_trigger_register(trig); > + if (ret) > + return NULL; > + > + return trig; > +} > + > +static int at91_adc_trigger_init(struct iio_dev *idev) > +{ > + struct at91_adc_state *st = iio_priv(idev); > + int i, ret; > + > + st->trig = devm_kzalloc(&idev->dev, > + st->trigger_number * sizeof(st->trig), > + GFP_KERNEL); > + > + if (st->trig == NULL) { > + ret = -ENOMEM; > + goto error_ret; > + } > + > + for (i = 0; i< st->trigger_number; i++) { > + if (st->trigger_list[i].is_external&& !(st->use_external)) > + continue; > + > + st->trig[i] = at91_adc_allocate_trigger(idev, > + st->trigger_list + i); > + if (st->trig[i] == NULL) { > + dev_err(&idev->dev, > + "Could not allocate trigger %d\n", i); > + ret = -ENOMEM; > + goto error_trigger; > + } > + } > + > + return 0; > + > +error_trigger: > + for (i--; i>= 0; i--) { > + iio_trigger_unregister(st->trig[i]); > + iio_trigger_free(st->trig[i]); > + } > +error_ret: > + return ret; > +} > + > +static void at91_adc_trigger_remove(struct iio_dev *idev) > +{ > + struct at91_adc_state *st = iio_priv(idev); > + int i; > + > + for (i = 0; i< st->trigger_number; i++) { > + iio_trigger_unregister(st->trig[i]); > + iio_trigger_free(st->trig[i]); > + } > +} > + > +static const struct iio_buffer_setup_ops at91_adc_buffer_ops = { > + .preenable =&iio_sw_buffer_preenable, > + .postenable =&iio_triggered_buffer_postenable, > + .predisable =&iio_triggered_buffer_predisable, > +}; > + > +static int at91_adc_buffer_init(struct iio_dev *idev) > +{ > + int ret; > + > + idev->buffer = iio_kfifo_allocate(idev); > + if (!idev->buffer) { > + ret = -ENOMEM; > + goto error_ret; > + } > + > + idev->pollfunc = iio_alloc_pollfunc(&iio_pollfunc_store_time, > + &at91_adc_trigger_handler, > + IRQF_ONESHOT, > + idev, > + "%s-consumer%d", > + idev->name, > + idev->id); > + if (idev->pollfunc == NULL) { > + ret = -ENOMEM; > + goto error_pollfunc; > + } > + > + idev->setup_ops =&at91_adc_buffer_ops; > + idev->modes |= INDIO_BUFFER_TRIGGERED; > + > + ret = iio_buffer_register(idev, > + idev->channels, > + idev->num_channels); > + if (ret) > + goto error_register; > + > + return 0; > + > +error_register: > + iio_dealloc_pollfunc(idev->pollfunc); > +error_pollfunc: > + iio_kfifo_free(idev->buffer); > +error_ret: > + return ret; > +} > + > +static void at91_adc_buffer_remove(struct iio_dev *idev) > +{ > + iio_buffer_unregister(idev); > + iio_dealloc_pollfunc(idev->pollfunc); > + iio_kfifo_free(idev->buffer); > +} > + > +static int at91_adc_read_raw(struct iio_dev *idev, > + struct iio_chan_spec const *chan, > + int *val, int *val2, long mask) > +{ > + struct at91_adc_state *st = iio_priv(idev); > + int ret; > + > + switch (mask) { > + case IIO_CHAN_INFO_RAW: > + mutex_lock(&st->lock); > + > + at91_adc_writel(st, AT91_ADC_CHER, > + AT91_ADC_CH(chan->channel)); > + at91_adc_writel(st, AT91_ADC_IER, st->registers->drdy_mask); > + at91_adc_writel(st, AT91_ADC_CR, AT91_ADC_START); > + > + ret = wait_event_interruptible_timeout(st->wq_data_avail, > + st->done, > + msecs_to_jiffies(1000)); > + if (ret == 0) > + return -ETIMEDOUT; > + else if (ret< 0) > + return ret; > + > + *val = st->last_value; > + > + at91_adc_writel(st, AT91_ADC_CHDR, > + AT91_ADC_CH(chan->channel)); > + at91_adc_writel(st, AT91_ADC_IDR, st->registers->drdy_mask); > + > + st->last_value = 0; > + st->done = false; > + mutex_unlock(&st->lock); > + return IIO_VAL_INT; > + > + case IIO_CHAN_INFO_SCALE: > + *val = (st->vref_mv * 1000)>> chan->scan_type.realbits; > + *val2 = 0; > + return IIO_VAL_INT_PLUS_MICRO; > + default: > + break; > + } > + return -EINVAL; > +} > + > +static int at91_adc_probe_pdata(struct at91_adc_state *st, > + struct platform_device *pdev) > +{ > + struct at91_adc_data *pdata = pdev->dev.platform_data; > + > + if (!pdata) > + return -EINVAL; > + This does look rather like it would be better to just keep a copy of pdata around... Still, not critical at all. > + st->use_external = pdata->use_external_triggers; > + st->vref_mv = pdata->vref; > + st->channels_mask = pdata->channels_used; > + st->num_channels = pdata->num_channels; > + st->startup_time = pdata->startup_time; > + st->trigger_number = pdata->trigger_number; > + st->trigger_list = pdata->trigger_list; > + st->registers = pdata->registers; > + > + return 0; > +} > + > +static const struct iio_info at91_adc_info = { > + .driver_module = THIS_MODULE, > + .read_raw =&at91_adc_read_raw, > +}; > + > +static int __devinit at91_adc_probe(struct platform_device *pdev) > +{ > + unsigned int prsc, mstrclk, ticks, adc_clk; > + int ret; > + struct iio_dev *idev; > + struct at91_adc_state *st; > + struct resource *res; > + > + idev = iio_device_alloc(sizeof(struct at91_adc_state)); > + if (idev == NULL) { > + ret = -ENOMEM; > + goto error_ret; > + } > + > + st = iio_priv(idev); > + > + ret = at91_adc_probe_pdata(st, pdev); > + if (ret) { > + dev_err(&pdev->dev, "No platform data available.\n"); > + ret = -EINVAL; > + goto error_free_device; > + } > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + if (!res) { > + dev_err(&pdev->dev, "No resource defined\n"); > + ret = -ENXIO; > + 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 =&at91_adc_info; > + > + 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 > + */ > + at91_adc_writel(st, AT91_ADC_CR, AT91_ADC_SWRST); > + at91_adc_writel(st, AT91_ADC_IDR, 0xFFFFFFFF); > + ret = request_irq(st->irq, > + at91_adc_eoc_trigger, > + 0, > + pdev->dev.driver->name, > + idev); > + 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; > + } > + > + ret = clk_prepare(st->clk); > + if (ret) { > + dev_err(&pdev->dev, "Could not prepare the clock.\n"); > + goto error_free_clk; > + } > + > + ret = clk_enable(st->clk); > + if (ret) { > + dev_err(&pdev->dev, "Could not enable the clock.\n"); > + goto error_unprepare_clk; > + } > + > + st->adc_clk = clk_get(&pdev->dev, "adc_op_clk"); > + if (IS_ERR(st->adc_clk)) { > + dev_err(&pdev->dev, "Failed to get the ADC clock.\n"); > + ret = PTR_ERR(st->clk); > + goto error_disable_clk; > + } > + > + ret = clk_prepare(st->adc_clk); > + if (ret) { > + dev_err(&pdev->dev, "Could not prepare the ADC clock.\n"); > + goto error_free_adc_clk; > + } > + > + ret = clk_enable(st->adc_clk); > + if (ret) { > + dev_err(&pdev->dev, "Could not enable the ADC clock.\n"); > + goto error_unprepare_adc_clk; > + } > + > + /* > + * Prescaler rate computation using the formula from the Atmel's > + * datasheet : ADC Clock = MCK / ((Prescaler + 1) * 2), ADC Clock being > + * specified by the electrical characteristics of the board. > + */ > + mstrclk = clk_get_rate(st->clk); > + adc_clk = clk_get_rate(st->adc_clk); > + prsc = (mstrclk / (2 * adc_clk)) - 1; > + > + if (!st->startup_time) { > + dev_err(&pdev->dev, "No startup time available.\n"); > + ret = -EINVAL; > + goto error_disable_adc_clk; > + } > + > + /* > + * Number of ticks needed to cover the startup time of the ADC as > + * defined in the electrical characteristics of the board, divided by 8. > + * The formula thus is : Startup Time = (ticks + 1) * 8 / ADC Clock > + */ > + ticks = round_up((st->startup_time * adc_clk / > + 1000000) - 1, 8) / 8; > + at91_adc_writel(st, 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 */ > + ret = at91_adc_channel_init(idev); > + if (ret< 0) { > + dev_err(&pdev->dev, "Couldn't initialize the channels.\n"); > + goto error_disable_adc_clk; > + } > + > + init_waitqueue_head(&st->wq_data_avail); > + mutex_init(&st->lock); > + > + ret = at91_adc_buffer_init(idev); > + if (ret< 0) { > + dev_err(&pdev->dev, "Couldn't initialize the buffer.\n"); > + goto error_disable_adc_clk; > + } > + > + ret = at91_adc_trigger_init(idev); > + if (ret< 0) { > + dev_err(&pdev->dev, "Couldn't setup the triggers.\n"); > + goto error_unregister_buffer; > + } > + > + ret = iio_device_register(idev); > + if (ret< 0) { > + dev_err(&pdev->dev, "Couldn't register the device.\n"); > + goto error_remove_triggers; > + } > + > + return 0; > + > +error_remove_triggers: > + at91_adc_trigger_remove(idev); > +error_unregister_buffer: > + at91_adc_buffer_remove(idev); > +error_disable_adc_clk: > + clk_disable(st->adc_clk); > +error_unprepare_adc_clk: > + clk_unprepare(st->adc_clk); > +error_free_adc_clk: > + clk_put(st->adc_clk); > +error_disable_clk: > + clk_disable(st->clk); > +error_unprepare_clk: > + clk_unprepare(st->clk); > +error_free_clk: > + clk_put(st->clk); > +error_free_irq: > + free_irq(st->irq, idev); > +error_unmap_reg: > + iounmap(st->reg_base); > +error_release_mem: > + release_mem_region(res->start, resource_size(res)); > +error_free_device: > + iio_device_free(idev); > +error_ret: > + return ret; > +} > + > +static int __devexit at91_adc_remove(struct platform_device *pdev) > +{ > + struct iio_dev *idev = platform_get_drvdata(pdev); > + struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + struct at91_adc_state *st = iio_priv(idev); > + > + iio_device_unregister(idev); > + at91_adc_trigger_remove(idev); > + at91_adc_buffer_remove(idev); There is clk_disable_unprepare... Could use it though then it's not quite as trivial to check everything lines up between probe and remove. > + clk_disable(st->adc_clk); > + clk_unprepare(st->adc_clk); > + clk_put(st->adc_clk); > + clk_disable(st->clk); > + clk_unprepare(st->clk); > + clk_put(st->clk); > + free_irq(st->irq, idev); > + iounmap(st->reg_base); > + release_mem_region(res->start, resource_size(res)); > + iio_device_free(idev); > + > + return 0; > +} > + > +static struct platform_driver at91_adc_driver = { > + .probe = at91_adc_probe, > + .remove = __devexit_p(at91_adc_remove), > + .driver = { > + .name = "at91_adc", > + }, > +}; > + > +module_platform_driver(at91_adc_driver); > + > +MODULE_LICENSE("GPL"); > +MODULE_DESCRIPTION("Atmel AT91 ADC Driver"); > +MODULE_AUTHOR("Maxime Ripard");