2013-09-19 06:24:49

by Zubair Lutfullah :

[permalink] [raw]
Subject: [PATCH V11 0/3] iio: input: ti_am335x_adc: Add continuous sampling support

Hi,

These apply to the togreg branch in iio.

Round 11
- Optimized memory usage based on list feedback
- Changed devm_*_irq to normal *_irq functions

Round 10 updates
- Removed devm_free as not needed

Round 9 updates

- Removed Trigger.
- Restructured everything for INDIO_BUFFERED_HARDWARE mode.
- Threaded IRQ with top/bottom halfs. Top half HW IRQ wakes
bottom half threaded worker that pushes samples to iio/userspace.
- Removed some of the patchwork to the driver that was irrelevant to the
continuous sampling patch.

Cleanup will be done separately once this goes through.
Hope these patches meet the mighty kernel standards :)

Zubair

Zubair Lutfullah (3):
input: ti_am335x_tsc: Enable shared IRQ for TSC
iio: ti_am335x_adc: optimize memory usage.
iio: ti_am335x_adc: Add continuous sampling support

drivers/iio/adc/ti_am335x_adc.c | 217 ++++++++++++++++++++++++++++-
drivers/input/touchscreen/ti_am335x_tsc.c | 12 +-
include/linux/mfd/ti_am335x_tscadc.h | 9 ++
3 files changed, 228 insertions(+), 10 deletions(-)

--
1.7.9.5


2013-09-19 06:24:54

by Zubair Lutfullah :

[permalink] [raw]
Subject: [PATCH 1/3] input: ti_am335x_tsc: Enable shared IRQ for TSC

Enable shared IRQ to allow ADC to share IRQ line from
parent MFD core. Only FIFO0 IRQs are for TSC and handled
on the TSC side.

Step mask would be updated from cached variable only previously.
In rare cases when both TSC and ADC are used, the cached
variable gets mixed up.
The step mask is written with the required mask every time.

Rachna Patil (TI) laid ground work for shared IRQ.

Signed-off-by: Zubair Lutfullah <[email protected]>
Acked-by: Dmitry Torokhov <[email protected]>
---
drivers/input/touchscreen/ti_am335x_tsc.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/input/touchscreen/ti_am335x_tsc.c b/drivers/input/touchscreen/ti_am335x_tsc.c
index e1c5300..24e625c 100644
--- a/drivers/input/touchscreen/ti_am335x_tsc.c
+++ b/drivers/input/touchscreen/ti_am335x_tsc.c
@@ -52,6 +52,7 @@ struct titsc {
u32 config_inp[4];
u32 bit_xp, bit_xn, bit_yp, bit_yn;
u32 inp_xp, inp_xn, inp_yp, inp_yn;
+ u32 step_mask;
};

static unsigned int titsc_readl(struct titsc *ts, unsigned int reg)
@@ -196,7 +197,8 @@ static void titsc_step_config(struct titsc *ts_dev)

/* The steps1 … end and bit 0 for TS_Charge */
stepenable = (1 << (end_step + 2)) - 1;
- am335x_tsc_se_set(ts_dev->mfd_tscadc, stepenable);
+ ts_dev->step_mask = stepenable;
+ am335x_tsc_se_set(ts_dev->mfd_tscadc, ts_dev->step_mask);
}

static void titsc_read_coordinates(struct titsc *ts_dev,
@@ -260,6 +262,10 @@ static irqreturn_t titsc_irq(int irq, void *dev)
unsigned int fsm;

status = titsc_readl(ts_dev, REG_IRQSTATUS);
+ /*
+ * ADC and touchscreen share the IRQ line.
+ * FIFO1 interrupts are used by ADC. Handle FIFO0 IRQs here only
+ */
if (status & IRQENB_FIFO0THRES) {

titsc_read_coordinates(ts_dev, &x, &y, &z1, &z2);
@@ -316,7 +322,7 @@ static irqreturn_t titsc_irq(int irq, void *dev)

if (irqclr) {
titsc_writel(ts_dev, REG_IRQSTATUS, irqclr);
- am335x_tsc_se_update(ts_dev->mfd_tscadc);
+ am335x_tsc_se_set(ts_dev->mfd_tscadc, ts_dev->step_mask);
return IRQ_HANDLED;
}
return IRQ_NONE;
@@ -389,7 +395,7 @@ static int titsc_probe(struct platform_device *pdev)
}

err = request_irq(ts_dev->irq, titsc_irq,
- 0, pdev->dev.driver->name, ts_dev);
+ IRQF_SHARED, pdev->dev.driver->name, ts_dev);
if (err) {
dev_err(&pdev->dev, "failed to allocate irq.\n");
goto err_free_mem;
--
1.7.9.5

2013-09-19 06:25:07

by Zubair Lutfullah :

[permalink] [raw]
Subject: [PATCH 3/3] iio: ti_am335x_adc: Add continuous sampling support

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 <[email protected]>
Acked-by: Greg Kroah-Hartman <[email protected]>
Signed-off-by: Russ Dill <[email protected]>
Acked-by: Lee Jones <[email protected]>
Acked-by: Sebastian Andrzej Siewior <[email protected]>
---
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
--
1.7.9.5

2013-09-19 06:25:05

by Zubair Lutfullah :

[permalink] [raw]
Subject: [PATCH 2/3] iio: ti_am335x_adc: optimize memory usage

12 bit ADC data is stored in 32 bits of storage.
Change from u32 to u16 to reduce wasted memory.

Signed-off-by: Zubair Lutfullah <[email protected]>
---
drivers/iio/adc/ti_am335x_adc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c
index a952538..ebe93eb 100644
--- a/drivers/iio/adc/ti_am335x_adc.c
+++ b/drivers/iio/adc/ti_am335x_adc.c
@@ -122,7 +122,7 @@ static int tiadc_channel_init(struct iio_dev *indio_dev, int channels)
chan->datasheet_name = chan_name_ain[chan->channel];
chan->scan_type.sign = 'u';
chan->scan_type.realbits = 12;
- chan->scan_type.storagebits = 32;
+ chan->scan_type.storagebits = 16;
}

indio_dev->channels = chan_array;
@@ -186,7 +186,7 @@ static int tiadc_read_raw(struct iio_dev *indio_dev,
if (stepid == map_val) {
read = read & FIFOREAD_DATA_MASK;
found = true;
- *val = read;
+ *val = (u16) read;
}
}

--
1.7.9.5

2013-09-21 09:52:04

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH V11 0/3] iio: input: ti_am335x_adc: Add continuous sampling support

Hi Zubair,

Thanks for persevering with this patch set. Now this
gets to be the example for dealing with hardware buffer
equipped devices ;)

Applied to the togreg branch of
git://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git

On 09/19/13 07:24, Zubair Lutfullah wrote:
> Hi,
>
> These apply to the togreg branch in iio.
>
> Round 11
> - Optimized memory usage based on list feedback
> - Changed devm_*_irq to normal *_irq functions
>
> Round 10 updates
> - Removed devm_free as not needed
>
> Round 9 updates
>
> - Removed Trigger.
> - Restructured everything for INDIO_BUFFERED_HARDWARE mode.
> - Threaded IRQ with top/bottom halfs. Top half HW IRQ wakes
> bottom half threaded worker that pushes samples to iio/userspace.
> - Removed some of the patchwork to the driver that was irrelevant to the
> continuous sampling patch.
>
> Cleanup will be done separately once this goes through.
> Hope these patches meet the mighty kernel standards :)
>
> Zubair
>
> Zubair Lutfullah (3):
> input: ti_am335x_tsc: Enable shared IRQ for TSC
> iio: ti_am335x_adc: optimize memory usage.
> iio: ti_am335x_adc: Add continuous sampling support
>
> drivers/iio/adc/ti_am335x_adc.c | 217 ++++++++++++++++++++++++++++-
> drivers/input/touchscreen/ti_am335x_tsc.c | 12 +-
> include/linux/mfd/ti_am335x_tscadc.h | 9 ++
> 3 files changed, 228 insertions(+), 10 deletions(-)
>

2013-09-21 17:28:16

by Jonathan Cameron

[permalink] [raw]
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 <[email protected]>
> Acked-by: Greg Kroah-Hartman <[email protected]>
> Signed-off-by: Russ Dill <[email protected]>
> Acked-by: Lee Jones <[email protected]>
> Acked-by: Sebastian Andrzej Siewior <[email protected]>
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
>

2013-09-22 04:42:32

by Zubair Lutfullah :

[permalink] [raw]
Subject: Re: [PATCH V11 0/3] iio: input: ti_am335x_adc: Add continuous sampling support

On Sat, Sep 21, 2013 at 11:52:21AM +0100, Jonathan Cameron wrote:
> Hi Zubair,
>
> Thanks for persevering with this patch set. Now this
> gets to be the example for dealing with hardware buffer
> equipped devices ;)
>
> Applied to the togreg branch of
> git://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git

Thank-you very much for the quick feedbacks and accepting these patches!
:)

Zubair

>
> On 09/19/13 07:24, Zubair Lutfullah wrote:
> > Hi,
> >
> > These apply to the togreg branch in iio.
> >
> > Round 11
> > - Optimized memory usage based on list feedback
> > - Changed devm_*_irq to normal *_irq functions
> >
> > Round 10 updates
> > - Removed devm_free as not needed
> >
> > Round 9 updates
> >
> > - Removed Trigger.
> > - Restructured everything for INDIO_BUFFERED_HARDWARE mode.
> > - Threaded IRQ with top/bottom halfs. Top half HW IRQ wakes
> > bottom half threaded worker that pushes samples to iio/userspace.
> > - Removed some of the patchwork to the driver that was irrelevant to the
> > continuous sampling patch.
> >
> > Cleanup will be done separately once this goes through.
> > Hope these patches meet the mighty kernel standards :)
> >
> > Zubair
> >
> > Zubair Lutfullah (3):
> > input: ti_am335x_tsc: Enable shared IRQ for TSC
> > iio: ti_am335x_adc: optimize memory usage.
> > iio: ti_am335x_adc: Add continuous sampling support
> >
> > drivers/iio/adc/ti_am335x_adc.c | 217 ++++++++++++++++++++++++++++-
> > drivers/input/touchscreen/ti_am335x_tsc.c | 12 +-
> > include/linux/mfd/ti_am335x_tscadc.h | 9 ++
> > 3 files changed, 228 insertions(+), 10 deletions(-)
> >

2013-09-22 04:46:32

by Zubair Lutfullah :

[permalink] [raw]
Subject: Re: [PATCH 3/3] iio: ti_am335x_adc: Add continuous sampling support

On Sat, Sep 21, 2013 at 07:28:32PM +0100, Jonathan Cameron wrote:
> 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.
...
> I've added a SELECT IIO_KFIFO_BUF after the autobuilders picked
> up that was missing.
>
Thank-you.

> 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?
>

Sure.

Zubair

> Thanks,
>
> Jonathan
> > ---
> > drivers/iio/adc/ti_am335x_adc.c | 213 +++++++++++++++++++++++++++++++++-
> > include/linux/mfd/ti_am335x_tscadc.h | 9 ++