[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <523DE550.4070107@kernel.org>
Date: Sat, 21 Sep 2013 19:28:32 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: Zubair Lutfullah <zubair.lutfullah@...il.com>
CC: dmitry.torokhov@...il.com, linux-iio@...r.kernel.org,
linux-input@...r.kernel.org, linux-kernel@...r.kernel.org,
bigeasy@...utronix.de, gregkh@...uxfoundation.org
Subject: Re: [PATCH 3/3] iio: ti_am335x_adc: Add continuous sampling support
On 09/19/13 07:24, Zubair Lutfullah wrote:
> Previously the driver had only one-shot reading functionality.
> This patch adds continuous sampling support to the driver.
>
> Continuous sampling starts when buffer is enabled.
> HW IRQ wakes worker thread that pushes samples to userspace.
> Sampling stops when buffer is disabled by userspace.
>
> Patil Rachna (TI) laid the ground work for ADC HW register access.
> Russ Dill (TI) fixed bugs in the driver relevant to FIFOs and IRQs.
>
> I fixed channel scanning so multiple ADC channels can be read
> simultaneously and pushed to userspace.
> Restructured the driver to fit IIO ABI.
> And added INDIO_BUFFER_HARDWARE mode.
>
> Signed-off-by: Zubair Lutfullah <zubair.lutfullah@...il.com>
> Acked-by: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> Signed-off-by: Russ Dill <Russ.Dill@...com>
> Acked-by: Lee Jones <lee.jones@...aro.org>
> Acked-by: Sebastian Andrzej Siewior <bigeasy@...utronix.de>
I've added a SELECT IIO_KFIFO_BUF after the autobuilders picked
up that was missing.
One other thing I'd missed until I was reviewing the updated patch.
Do you still need the trigger headers? I can't immediately see why.
If not, could you post a follow up patch to drop them?
Thanks,
Jonathan
> ---
> drivers/iio/adc/ti_am335x_adc.c | 213 +++++++++++++++++++++++++++++++++-
> include/linux/mfd/ti_am335x_tscadc.h | 9 ++
> 2 files changed, 217 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c
> index ebe93eb..5287bff 100644
> --- a/drivers/iio/adc/ti_am335x_adc.c
> +++ b/drivers/iio/adc/ti_am335x_adc.c
> @@ -28,12 +28,20 @@
> #include <linux/iio/driver.h>
>
> #include <linux/mfd/ti_am335x_tscadc.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/kfifo_buf.h>
> +#include <linux/iio/trigger.h>
> +#include <linux/iio/trigger_consumer.h>
> +#include <linux/iio/triggered_buffer.h>
>
> struct tiadc_device {
> struct ti_tscadc_dev *mfd_tscadc;
> int channels;
> u8 channel_line[8];
> u8 channel_step[8];
> + int buffer_en_ch_steps;
> + struct iio_trigger *trig;
> + u16 data[8];
> };
>
> static unsigned int tiadc_readl(struct tiadc_device *adc, unsigned int reg)
> @@ -56,8 +64,14 @@ static u32 get_adc_step_mask(struct tiadc_device *adc_dev)
> return step_en;
> }
>
> -static void tiadc_step_config(struct tiadc_device *adc_dev)
> +static u32 get_adc_step_bit(struct tiadc_device *adc_dev, int chan)
> {
> + return 1 << adc_dev->channel_step[chan];
> +}
> +
> +static void tiadc_step_config(struct iio_dev *indio_dev)
> +{
> + struct tiadc_device *adc_dev = iio_priv(indio_dev);
> unsigned int stepconfig;
> int i, steps;
>
> @@ -72,7 +86,11 @@ static void tiadc_step_config(struct tiadc_device *adc_dev)
> */
>
> steps = TOTAL_STEPS - adc_dev->channels;
> - stepconfig = STEPCONFIG_AVG_16 | STEPCONFIG_FIFO1;
> + if (iio_buffer_enabled(indio_dev))
> + stepconfig = STEPCONFIG_AVG_16 | STEPCONFIG_FIFO1
> + | STEPCONFIG_MODE_SWCNT;
> + else
> + stepconfig = STEPCONFIG_AVG_16 | STEPCONFIG_FIFO1;
>
> for (i = 0; i < adc_dev->channels; i++) {
> int chan;
> @@ -85,9 +103,175 @@ static void tiadc_step_config(struct tiadc_device *adc_dev)
> adc_dev->channel_step[i] = steps;
> steps++;
> }
> +}
> +
> +static irqreturn_t tiadc_irq_h(int irq, void *private)
> +{
> + struct iio_dev *indio_dev = private;
> + struct tiadc_device *adc_dev = iio_priv(indio_dev);
> + unsigned int status, config;
> + status = tiadc_readl(adc_dev, REG_IRQSTATUS);
> +
> + /*
> + * ADC and touchscreen share the IRQ line.
> + * FIFO0 interrupts are used by TSC. Handle FIFO1 IRQs here only
> + */
> + if (status & IRQENB_FIFO1OVRRUN) {
> + /* FIFO Overrun. Clear flag. Disable/Enable ADC to recover */
> + config = tiadc_readl(adc_dev, REG_CTRL);
> + config &= ~(CNTRLREG_TSCSSENB);
> + tiadc_writel(adc_dev, REG_CTRL, config);
> + tiadc_writel(adc_dev, REG_IRQSTATUS, IRQENB_FIFO1OVRRUN
> + | IRQENB_FIFO1UNDRFLW | IRQENB_FIFO1THRES);
> + tiadc_writel(adc_dev, REG_CTRL, (config | CNTRLREG_TSCSSENB));
> + return IRQ_HANDLED;
> + } else if (status & IRQENB_FIFO1THRES) {
> + /* Disable irq and wake worker thread */
> + tiadc_writel(adc_dev, REG_IRQCLR, IRQENB_FIFO1THRES);
> + return IRQ_WAKE_THREAD;
> + }
> +
> + return IRQ_NONE;
> +}
> +
> +static irqreturn_t tiadc_worker_h(int irq, void *private)
> +{
> + struct iio_dev *indio_dev = private;
> + struct tiadc_device *adc_dev = iio_priv(indio_dev);
> + int i, k, fifo1count, read;
> + u16 *data = adc_dev->data;
> +
> + fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
> + for (k = 0; k < fifo1count; k = k + i) {
> + for (i = 0; i < (indio_dev->scan_bytes)/2; i++) {
> + read = tiadc_readl(adc_dev, REG_FIFO1);
> + data[i] = read & FIFOREAD_DATA_MASK;
> + }
> + iio_push_to_buffers(indio_dev, (u8 *) data);
> + }
> +
> + tiadc_writel(adc_dev, REG_IRQSTATUS, IRQENB_FIFO1THRES);
> + tiadc_writel(adc_dev, REG_IRQENABLE, IRQENB_FIFO1THRES);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int tiadc_buffer_preenable(struct iio_dev *indio_dev)
> +{
> + struct tiadc_device *adc_dev = iio_priv(indio_dev);
> + int i, fifo1count, read;
> +
> + tiadc_writel(adc_dev, REG_IRQCLR, (IRQENB_FIFO1THRES |
> + IRQENB_FIFO1OVRRUN |
> + IRQENB_FIFO1UNDRFLW));
> +
> + /* Flush FIFO. Needed in corner cases in simultaneous tsc/adc use */
> + fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
> + for (i = 0; i < fifo1count; i++)
> + read = tiadc_readl(adc_dev, REG_FIFO1);
> +
> + return iio_sw_buffer_preenable(indio_dev);
> +}
> +
> +static int tiadc_buffer_postenable(struct iio_dev *indio_dev)
> +{
> + struct tiadc_device *adc_dev = iio_priv(indio_dev);
> + struct iio_buffer *buffer = indio_dev->buffer;
> + unsigned int enb = 0;
> + u8 bit;
> +
> + tiadc_step_config(indio_dev);
> + for_each_set_bit(bit, buffer->scan_mask, adc_dev->channels)
> + enb |= (get_adc_step_bit(adc_dev, bit) << 1);
> + adc_dev->buffer_en_ch_steps = enb;
> +
> + am335x_tsc_se_set(adc_dev->mfd_tscadc, enb);
> +
> + tiadc_writel(adc_dev, REG_IRQSTATUS, IRQENB_FIFO1THRES
> + | IRQENB_FIFO1OVRRUN | IRQENB_FIFO1UNDRFLW);
> + tiadc_writel(adc_dev, REG_IRQENABLE, IRQENB_FIFO1THRES
> + | IRQENB_FIFO1OVRRUN);
> +
> + return 0;
> +}
> +
> +static int tiadc_buffer_predisable(struct iio_dev *indio_dev)
> +{
> + struct tiadc_device *adc_dev = iio_priv(indio_dev);
> + int fifo1count, i, read;
> +
> + tiadc_writel(adc_dev, REG_IRQCLR, (IRQENB_FIFO1THRES |
> + IRQENB_FIFO1OVRRUN | IRQENB_FIFO1UNDRFLW));
> + am335x_tsc_se_clr(adc_dev->mfd_tscadc, adc_dev->buffer_en_ch_steps);
>
> + /* Flush FIFO of leftover data in the time it takes to disable adc */
> + fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
> + for (i = 0; i < fifo1count; i++)
> + read = tiadc_readl(adc_dev, REG_FIFO1);
> +
> + return 0;
> }
>
> +static int tiadc_buffer_postdisable(struct iio_dev *indio_dev)
> +{
> + tiadc_step_config(indio_dev);
> +
> + return 0;
> +}
> +
> +static const struct iio_buffer_setup_ops tiadc_buffer_setup_ops = {
> + .preenable = &tiadc_buffer_preenable,
> + .postenable = &tiadc_buffer_postenable,
> + .predisable = &tiadc_buffer_predisable,
> + .postdisable = &tiadc_buffer_postdisable,
> +};
> +
> +int tiadc_iio_buffered_hardware_setup(struct iio_dev *indio_dev,
> + irqreturn_t (*pollfunc_bh)(int irq, void *p),
> + irqreturn_t (*pollfunc_th)(int irq, void *p),
> + int irq,
> + unsigned long flags,
> + const struct iio_buffer_setup_ops *setup_ops)
> +{
> + int ret;
> +
> + indio_dev->buffer = iio_kfifo_allocate(indio_dev);
> + if (!indio_dev->buffer)
> + return -ENOMEM;
> +
> + ret = request_threaded_irq(irq, pollfunc_th, pollfunc_bh,
> + flags, indio_dev->name, indio_dev);
> + if (ret)
> + goto error_kfifo_free;
> +
> + indio_dev->setup_ops = setup_ops;
> + indio_dev->modes |= INDIO_BUFFER_HARDWARE;
> +
> + ret = iio_buffer_register(indio_dev,
> + indio_dev->channels,
> + indio_dev->num_channels);
> + if (ret)
> + goto error_free_irq;
> +
> + return 0;
> +
> +error_free_irq:
> + free_irq(irq, indio_dev);
> +error_kfifo_free:
> + iio_kfifo_free(indio_dev->buffer);
> + return ret;
> +}
> +
> +static void tiadc_iio_buffered_hardware_remove(struct iio_dev *indio_dev)
> +{
> + struct tiadc_device *adc_dev = iio_priv(indio_dev);
> +
> + free_irq(adc_dev->mfd_tscadc->irq, indio_dev);
> + iio_kfifo_free(indio_dev->buffer);
> + iio_buffer_unregister(indio_dev);
> +}
> +
> +
> static const char * const chan_name_ain[] = {
> "AIN0",
> "AIN1",
> @@ -120,6 +304,7 @@ static int tiadc_channel_init(struct iio_dev *indio_dev, int channels)
> chan->channel = adc_dev->channel_line[i];
> chan->info_mask_separate = BIT(IIO_CHAN_INFO_RAW);
> chan->datasheet_name = chan_name_ain[chan->channel];
> + chan->scan_index = i;
> chan->scan_type.sign = 'u';
> chan->scan_type.realbits = 12;
> chan->scan_type.storagebits = 16;
> @@ -147,6 +332,10 @@ static int tiadc_read_raw(struct iio_dev *indio_dev,
> u32 step_en;
> unsigned long timeout = jiffies + usecs_to_jiffies
> (IDLE_TIMEOUT * adc_dev->channels);
> +
> + if (iio_buffer_enabled(indio_dev))
> + return -EBUSY;
> +
> step_en = get_adc_step_mask(adc_dev);
> am335x_tsc_se_set(adc_dev->mfd_tscadc, step_en);
>
> @@ -237,20 +426,33 @@ static int tiadc_probe(struct platform_device *pdev)
> indio_dev->modes = INDIO_DIRECT_MODE;
> indio_dev->info = &tiadc_info;
>
> - tiadc_step_config(adc_dev);
> + tiadc_step_config(indio_dev);
> + tiadc_writel(adc_dev, REG_FIFO1THR, FIFO1_THRESHOLD);
>
> err = tiadc_channel_init(indio_dev, adc_dev->channels);
> if (err < 0)
> return err;
>
> - err = iio_device_register(indio_dev);
> + err = tiadc_iio_buffered_hardware_setup(indio_dev,
> + &tiadc_worker_h,
> + &tiadc_irq_h,
> + adc_dev->mfd_tscadc->irq,
> + IRQF_SHARED,
> + &tiadc_buffer_setup_ops);
> +
> if (err)
> goto err_free_channels;
>
> + err = iio_device_register(indio_dev);
> + if (err)
> + goto err_buffer_unregister;
> +
> platform_set_drvdata(pdev, indio_dev);
>
> return 0;
>
> +err_buffer_unregister:
> + tiadc_iio_buffered_hardware_remove(indio_dev);
> err_free_channels:
> tiadc_channels_remove(indio_dev);
> return err;
> @@ -263,6 +465,7 @@ static int tiadc_remove(struct platform_device *pdev)
> u32 step_en;
>
> iio_device_unregister(indio_dev);
> + tiadc_iio_buffered_hardware_remove(indio_dev);
> tiadc_channels_remove(indio_dev);
>
> step_en = get_adc_step_mask(adc_dev);
> @@ -301,7 +504,7 @@ static int tiadc_resume(struct device *dev)
> restore &= ~(CNTRLREG_POWERDOWN);
> tiadc_writel(adc_dev, REG_CTRL, restore);
>
> - tiadc_step_config(adc_dev);
> + tiadc_step_config(indio_dev);
>
> return 0;
> }
> diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
> index db1791b..7d98562 100644
> --- a/include/linux/mfd/ti_am335x_tscadc.h
> +++ b/include/linux/mfd/ti_am335x_tscadc.h
> @@ -46,16 +46,24 @@
> /* Step Enable */
> #define STEPENB_MASK (0x1FFFF << 0)
> #define STEPENB(val) ((val) << 0)
> +#define ENB(val) (1 << (val))
> +#define STPENB_STEPENB STEPENB(0x1FFFF)
> +#define STPENB_STEPENB_TC STEPENB(0x1FFF)
>
> /* IRQ enable */
> #define IRQENB_HW_PEN BIT(0)
> #define IRQENB_FIFO0THRES BIT(2)
> +#define IRQENB_FIFO0OVRRUN BIT(3)
> +#define IRQENB_FIFO0UNDRFLW BIT(4)
> #define IRQENB_FIFO1THRES BIT(5)
> +#define IRQENB_FIFO1OVRRUN BIT(6)
> +#define IRQENB_FIFO1UNDRFLW BIT(7)
> #define IRQENB_PENUP BIT(9)
>
> /* Step Configuration */
> #define STEPCONFIG_MODE_MASK (3 << 0)
> #define STEPCONFIG_MODE(val) ((val) << 0)
> +#define STEPCONFIG_MODE_SWCNT STEPCONFIG_MODE(1)
> #define STEPCONFIG_MODE_HWSYNC STEPCONFIG_MODE(2)
> #define STEPCONFIG_AVG_MASK (7 << 2)
> #define STEPCONFIG_AVG(val) ((val) << 2)
> @@ -124,6 +132,7 @@
> #define MAX_CLK_DIV 7
> #define TOTAL_STEPS 16
> #define TOTAL_CHANNELS 8
> +#define FIFO1_THRESHOLD 19
>
> /*
> * ADC runs at 3MHz, and it takes
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists