* [PATCH V6 0/2] iio: input: ti_am335x_adc: Add continuous sampling support @ 2013-08-25 22:45 Zubair Lutfullah 2013-08-25 22:45 ` [PATCH 1/2] input: ti_am335x_tsc: Enable shared IRQ for TSC Zubair Lutfullah [not found] ` <1377470724-15710-1-git-send-email-zubair.lutfullah-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 0 siblings, 2 replies; 21+ messages in thread From: Zubair Lutfullah @ 2013-08-25 22:45 UTC (permalink / raw) To: jic23-KWPb1pKIrIJaa/9Udqfwiw, lee.jones-QSEj5FYQhm4dnm+yROfE0A Cc: bigeasy-hfZtesqFncYOwBW4kG4KsQ, linux-iio-u79uwXL29TY76Z2rM5mHXA, linux-input-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r This applies to togreg branch in iio. Round 6 updates Fixed trigger the way iio list wanted. Driver has its own trigger. Triggers at every FIFO Threshold IRQ and pushes samples to iio buffer. Went through the driver and cleaned it up quite a bit. Squashed patches together instead of having multiple tiny patches. Zubair Lutfullah (2): input: ti_am335x_tsc: Enable shared IRQ for TSC iio: ti_am335x_adc: Add continuous sampling support drivers/iio/adc/ti_am335x_adc.c | 254 ++++++++++++++++++++++++++--- drivers/input/touchscreen/ti_am335x_tsc.c | 24 ++- include/linux/mfd/ti_am335x_tscadc.h | 13 ++ 3 files changed, 264 insertions(+), 27 deletions(-) -- 1.7.9.5 ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 1/2] input: ti_am335x_tsc: Enable shared IRQ for TSC 2013-08-25 22:45 [PATCH V6 0/2] iio: input: ti_am335x_adc: Add continuous sampling support Zubair Lutfullah @ 2013-08-25 22:45 ` Zubair Lutfullah [not found] ` <1377470724-15710-2-git-send-email-zubair.lutfullah-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> [not found] ` <1377470724-15710-1-git-send-email-zubair.lutfullah-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 1 sibling, 1 reply; 21+ messages in thread From: Zubair Lutfullah @ 2013-08-25 22:45 UTC (permalink / raw) To: jic23, lee.jones; +Cc: bigeasy, linux-iio, linux-input, linux-kernel, gregkh 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 <zubair.lutfullah@gmail.com> --- drivers/input/touchscreen/ti_am335x_tsc.c | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/drivers/input/touchscreen/ti_am335x_tsc.c b/drivers/input/touchscreen/ti_am335x_tsc.c index e1c5300..4124e580 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); @@ -315,11 +321,17 @@ 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); - return IRQ_HANDLED; + titsc_writel(ts_dev, REG_IRQSTATUS, (status | irqclr)); + am335x_tsc_se_set(ts_dev->mfd_tscadc, ts_dev->step_mask); } - return IRQ_NONE; + + /* If any IRQ flags left, return none. So ADC can handle its IRQs */ + status = titsc_readl(ts_dev, REG_IRQSTATUS); + if (status == false) + return IRQ_HANDLED; + else + return IRQ_NONE; + } static int titsc_parse_dt(struct platform_device *pdev, @@ -389,7 +401,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 ^ permalink raw reply related [flat|nested] 21+ messages in thread
[parent not found: <1377470724-15710-2-git-send-email-zubair.lutfullah-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]
* Re: [PATCH 1/2] input: ti_am335x_tsc: Enable shared IRQ for TSC [not found] ` <1377470724-15710-2-git-send-email-zubair.lutfullah-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> @ 2013-08-28 10:42 ` Sebastian Andrzej Siewior 2013-08-28 18:49 ` Zubair Lutfullah : 0 siblings, 1 reply; 21+ messages in thread From: Sebastian Andrzej Siewior @ 2013-08-28 10:42 UTC (permalink / raw) To: Zubair Lutfullah Cc: jic23-KWPb1pKIrIJaa/9Udqfwiw, lee.jones-QSEj5FYQhm4dnm+yROfE0A, linux-iio-u79uwXL29TY76Z2rM5mHXA, linux-input-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r * Zubair Lutfullah | 2013-08-25 23:45:23 [+0100]: >diff --git a/drivers/input/touchscreen/ti_am335x_tsc.c b/drivers/input/touchscreen/ti_am335x_tsc.c >index e1c5300..4124e580 100644 >--- a/drivers/input/touchscreen/ti_am335x_tsc.c >+++ b/drivers/input/touchscreen/ti_am335x_tsc.c >@@ -315,11 +321,17 @@ 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); >- return IRQ_HANDLED; >+ titsc_writel(ts_dev, REG_IRQSTATUS, (status | irqclr)); >+ am335x_tsc_se_set(ts_dev->mfd_tscadc, ts_dev->step_mask); > } >- return IRQ_NONE; >+ >+ /* If any IRQ flags left, return none. So ADC can handle its IRQs */ >+ status = titsc_readl(ts_dev, REG_IRQSTATUS); >+ if (status == false) >+ return IRQ_HANDLED; >+ else >+ return IRQ_NONE; If I understand this correctly you return IRQ_NONE the TSC interrupt has been handled and no ADC interrupt is outstanding. This is bad because if you received 1k _only_ TSC interrupts you return always IRQ_NONE and the irq-core will disable the interrupt line. You don't want this. Now, if you handle an interrupt at the TSC level and return IRQ_HANDLED then the second handler, the ADC in your case, is also invoked. So you can drop this and return IRQ_HANDLED if you *did* something here and NONE if didn't do anything. Basides that, I'm fine with it. >+ > } Sebastian ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/2] input: ti_am335x_tsc: Enable shared IRQ for TSC 2013-08-28 10:42 ` Sebastian Andrzej Siewior @ 2013-08-28 18:49 ` Zubair Lutfullah : 0 siblings, 0 replies; 21+ messages in thread From: Zubair Lutfullah : @ 2013-08-28 18:49 UTC (permalink / raw) To: Sebastian Andrzej Siewior Cc: Zubair Lutfullah, jic23, lee.jones, linux-iio, linux-input, linux-kernel, gregkh On Wed, Aug 28, 2013 at 12:42:11PM +0200, Sebastian Andrzej Siewior wrote: > * Zubair Lutfullah | 2013-08-25 23:45:23 [+0100]: > > >diff --git a/drivers/input/touchscreen/ti_am335x_tsc.c b/drivers/input/touchscreen/ti_am335x_tsc.c > >index e1c5300..4124e580 100644 > >--- a/drivers/input/touchscreen/ti_am335x_tsc.c > >+++ b/drivers/input/touchscreen/ti_am335x_tsc.c > >@@ -315,11 +321,17 @@ static irqreturn_t titsc_irq(int irq, void *dev) ... > >+ /* If any IRQ flags left, return none. So ADC can handle its IRQs */ > >+ status = titsc_readl(ts_dev, REG_IRQSTATUS); > >+ if (status == false) > >+ return IRQ_HANDLED; > >+ else > >+ return IRQ_NONE; > > If I understand this correctly you return IRQ_NONE the TSC interrupt has > been handled and no ADC interrupt is outstanding. Its actually the opposite. TSC handler checks if there are any ADC IRQ flags outstanding. If there is no outstanding, then IRQ_HANDLED is returned. If there is ADC IRQ outstanding, then IRQ_NONE is returned. ZubairLK ^ permalink raw reply [flat|nested] 21+ messages in thread
[parent not found: <1377470724-15710-1-git-send-email-zubair.lutfullah-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]
* [PATCH 2/2] iio: ti_am335x_adc: Add continuous sampling support [not found] ` <1377470724-15710-1-git-send-email-zubair.lutfullah-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> @ 2013-08-25 22:45 ` Zubair Lutfullah 2013-08-27 8:42 ` Lee Jones ` (2 more replies) 0 siblings, 3 replies; 21+ messages in thread From: Zubair Lutfullah @ 2013-08-25 22:45 UTC (permalink / raw) To: jic23-KWPb1pKIrIJaa/9Udqfwiw, lee.jones-QSEj5FYQhm4dnm+yROfE0A Cc: bigeasy-hfZtesqFncYOwBW4kG4KsQ, linux-iio-u79uwXL29TY76Z2rM5mHXA, linux-input-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r Previously the driver had only one-shot reading functionality. This patch adds triggered buffer support to the driver. Continuous sampling starts when buffer is enabled. And samples are pushed to userpace by the trigger which triggers automatically at every hardware interrupt of FIFO1 filling with samples upto threshold value. Userspace responsibility to stop sampling by writing zero in the buffer enable file. 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 trigger support. Signed-off-by: Zubair Lutfullah <zubair.lutfullah-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> Acked-by: Greg Kroah-Hartman <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org> Signed-off-by: Russ Dill <Russ.Dill-l0cyMroinI0@public.gmane.org> --- drivers/iio/adc/ti_am335x_adc.c | 254 +++++++++++++++++++++++++++++++--- include/linux/mfd/ti_am335x_tscadc.h | 13 ++ 2 files changed, 246 insertions(+), 21 deletions(-) diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c index a952538..ae2202b 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/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 irq; + int buffer_en_ch_steps; + struct iio_trigger *trig; + u32 *data; }; static unsigned int tiadc_readl(struct tiadc_device *adc, unsigned int reg) @@ -56,10 +64,11 @@ 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 void tiadc_step_config(struct iio_dev *indio_dev) { + struct tiadc_device *adc_dev = iio_priv(indio_dev); unsigned int stepconfig; - int i, steps; + int i, steps, chan; /* * There are 16 configurable steps and 8 analog input @@ -72,11 +81,13 @@ 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; - chan = adc_dev->channel_line[i]; tiadc_writel(adc_dev, REG_STEPCONFIG(steps), stepconfig | STEPCONFIG_INP(chan)); @@ -85,7 +96,175 @@ static void tiadc_step_config(struct tiadc_device *adc_dev) adc_dev->channel_step[i] = steps; steps++; } +} + +static irqreturn_t tiadc_irq(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)); + } else if (status & IRQENB_FIFO1THRES) { + /* Trigger to push FIFO data to iio buffer */ + tiadc_writel(adc_dev, REG_IRQCLR, IRQENB_FIFO1THRES); + iio_trigger_poll(indio_dev->trig, iio_get_time_ns()); + } else + return IRQ_NONE; + + /* If any IRQ flags left, return none. So TSC can handle its IRQs */ + status = tiadc_readl(adc_dev, REG_IRQSTATUS); + if (status == false) + return IRQ_HANDLED; + else + return IRQ_NONE; +} + +static irqreturn_t tiadc_trigger_h(int irq, void *p) +{ + struct iio_poll_func *pf = p; + struct iio_dev *indio_dev = pf->indio_dev; + struct tiadc_device *adc_dev = iio_priv(indio_dev); + int i, k, fifo1count, read; + u32 *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)/4; 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); + + iio_trigger_notify_done(indio_dev->trig); + + 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 before starting sampling */ + 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, stepnum; + u8 bit; + + adc_dev->data = kmalloc(indio_dev->scan_bytes, GFP_KERNEL); + if (adc_dev->data == NULL) + return -ENOMEM; + + tiadc_step_config(indio_dev); + for_each_set_bit(bit, buffer->scan_mask, + adc_dev->channels) { + struct iio_chan_spec const *chan = indio_dev->channels + bit; + /* + * There are a total of 16 steps available + * that are shared between ADC and touchscreen. + * We start configuring from step 16 to 0 incase of + * ADC. Hence the relation between input channel + * and step for ADC would be as below. + */ + stepnum = chan->channel + 9; + enb |= (1 << stepnum); + } + + 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 iio_triggered_buffer_postenable(indio_dev); +} + +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 any leftover data */ + fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT); + for (i = 0; i < fifo1count; i++) + read = tiadc_readl(adc_dev, REG_FIFO1); + + return iio_triggered_buffer_predisable(indio_dev); +} + +static int tiadc_buffer_postdisable(struct iio_dev *indio_dev) +{ + struct tiadc_device *adc_dev = iio_priv(indio_dev); + + tiadc_step_config(indio_dev); + kfree(adc_dev->data); + 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, +}; + +static const struct iio_trigger_ops tiadc_trigger_ops = { + .owner = THIS_MODULE, +}; + +static struct iio_trigger *tiadc_iio_allocate_trigger(struct iio_dev *indio_dev) +{ + struct iio_trigger *trig; + int ret; + + trig = iio_trigger_alloc("%s-dev%d", indio_dev->name, indio_dev->id); + if (trig == NULL) + return NULL; + + trig->dev.parent = indio_dev->dev.parent; + trig->ops = &tiadc_trigger_ops; + iio_trigger_set_drvdata(trig, indio_dev); + + ret = iio_trigger_register(trig); + if (ret) + return NULL; + + return trig; } static const char * const chan_name_ain[] = { @@ -120,6 +299,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 = 32; @@ -142,11 +322,14 @@ static int tiadc_read_raw(struct iio_dev *indio_dev, struct tiadc_device *adc_dev = iio_priv(indio_dev); int i, map_val; unsigned int fifo1count, read, stepid; - u32 step = UINT_MAX; bool found = false; 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); @@ -168,15 +351,6 @@ static int tiadc_read_raw(struct iio_dev *indio_dev, * Hence we need to flush out this data. */ - for (i = 0; i < ARRAY_SIZE(adc_dev->channel_step); i++) { - if (chan->channel == adc_dev->channel_line[i]) { - step = adc_dev->channel_step[i]; - break; - } - } - if (WARN_ON_ONCE(step == UINT_MAX)) - return -EINVAL; - fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT); for (i = 0; i < fifo1count; i++) { read = tiadc_readl(adc_dev, REG_FIFO1); @@ -220,7 +394,8 @@ static int tiadc_probe(struct platform_device *pdev) sizeof(struct tiadc_device)); if (indio_dev == NULL) { dev_err(&pdev->dev, "failed to allocate iio device\n"); - return -ENOMEM; + err = -ENOMEM; + goto err_ret; } adc_dev = iio_priv(indio_dev); @@ -231,28 +406,56 @@ static int tiadc_probe(struct platform_device *pdev) channels++; } adc_dev->channels = channels; + adc_dev->irq = adc_dev->mfd_tscadc->irq; indio_dev->dev.parent = &pdev->dev; indio_dev->name = dev_name(&pdev->dev); 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; + goto err_free_device; + + adc_dev->trig = tiadc_iio_allocate_trigger(indio_dev); + if (adc_dev->trig == NULL) { + err = -ENOMEM; + goto err_free_channels; + } + + err = request_irq(adc_dev->irq, tiadc_irq, IRQF_SHARED, + indio_dev->name, indio_dev); + if (err) + goto err_free_trigger; + + err = iio_triggered_buffer_setup(indio_dev, NULL, + &tiadc_trigger_h, &tiadc_buffer_setup_ops); + if (err) + goto err_free_irq; err = iio_device_register(indio_dev); if (err) - goto err_free_channels; + goto err_buffer_unregister; platform_set_drvdata(pdev, indio_dev); return 0; +err_buffer_unregister: + iio_buffer_unregister(indio_dev); +err_free_irq: + free_irq(adc_dev->irq, indio_dev); +err_free_trigger: + iio_trigger_unregister(adc_dev->trig); + iio_trigger_free(adc_dev->trig); err_free_channels: tiadc_channels_remove(indio_dev); +err_free_device: + iio_device_free(indio_dev); +err_ret: return err; } @@ -262,11 +465,14 @@ static int tiadc_remove(struct platform_device *pdev) struct tiadc_device *adc_dev = iio_priv(indio_dev); u32 step_en; + free_irq(adc_dev->irq, indio_dev); iio_device_unregister(indio_dev); + iio_buffer_unregister(indio_dev); tiadc_channels_remove(indio_dev); step_en = get_adc_step_mask(adc_dev); am335x_tsc_se_clr(adc_dev->mfd_tscadc, step_en); + iio_device_free(indio_dev); return 0; } @@ -298,10 +504,16 @@ static int tiadc_resume(struct device *dev) /* Make sure ADC is powered up */ restore = tiadc_readl(adc_dev, REG_CTRL); - restore &= ~(CNTRLREG_POWERDOWN); + restore &= ~(CNTRLREG_TSCSSENB); tiadc_writel(adc_dev, REG_CTRL, restore); - tiadc_step_config(adc_dev); + tiadc_writel(adc_dev, REG_FIFO1THR, FIFO1_THRESHOLD); + tiadc_step_config(indio_dev); + + /* Make sure ADC is powered up */ + restore &= ~(CNTRLREG_POWERDOWN); + restore |= CNTRLREG_TSCSSENB; + tiadc_writel(adc_dev, REG_CTRL, restore); return 0; } diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h index db1791b..a372ebf 100644 --- a/include/linux/mfd/ti_am335x_tscadc.h +++ b/include/linux/mfd/ti_am335x_tscadc.h @@ -46,17 +46,25 @@ /* 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_HWSYNC STEPCONFIG_MODE(2) +#define STEPCONFIG_MODE_SWCNT STEPCONFIG_MODE(1) #define STEPCONFIG_AVG_MASK (7 << 2) #define STEPCONFIG_AVG(val) ((val) << 2) #define STEPCONFIG_AVG_16 STEPCONFIG_AVG(4) @@ -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 @@ -153,6 +162,10 @@ struct ti_tscadc_dev { /* adc device */ struct adc_device *adc; + + /* Context save */ + unsigned int irqstat; + unsigned int ctrl; }; static inline struct ti_tscadc_dev *ti_tscadc_dev_get(struct platform_device *p) -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] iio: ti_am335x_adc: Add continuous sampling support 2013-08-25 22:45 ` [PATCH 2/2] iio: ti_am335x_adc: Add continuous sampling support Zubair Lutfullah @ 2013-08-27 8:42 ` Lee Jones [not found] ` <1377470724-15710-3-git-send-email-zubair.lutfullah-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 2013-08-28 13:01 ` Sebastian Andrzej Siewior 2 siblings, 0 replies; 21+ messages in thread From: Lee Jones @ 2013-08-27 8:42 UTC (permalink / raw) To: Zubair Lutfullah Cc: jic23, bigeasy, linux-iio, linux-input, linux-kernel, gregkh On Sun, 25 Aug 2013, Zubair Lutfullah wrote: > Previously the driver had only one-shot reading functionality. > This patch adds triggered buffer support to the driver. > > Continuous sampling starts when buffer is enabled. > And samples are pushed to userpace by the trigger which > triggers automatically at every hardware interrupt > of FIFO1 filling with samples upto threshold value. > > Userspace responsibility to stop sampling by writing zero > in the buffer enable file. > > 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 trigger support. > > Signed-off-by: Zubair Lutfullah <zubair.lutfullah@gmail.com> > Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Signed-off-by: Russ Dill <Russ.Dill@ti.com> > --- > drivers/iio/adc/ti_am335x_adc.c | 254 +++++++++++++++++++++++++++++++--- > include/linux/mfd/ti_am335x_tscadc.h | 13 ++ > 2 files changed, 246 insertions(+), 21 deletions(-) MFD stuff looks okay to me. Acked-by: Lee Jones <lee.jones@linaro.org> <snip> > diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h > index db1791b..a372ebf 100644 > --- a/include/linux/mfd/ti_am335x_tscadc.h > +++ b/include/linux/mfd/ti_am335x_tscadc.h > @@ -46,17 +46,25 @@ > /* 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_HWSYNC STEPCONFIG_MODE(2) > +#define STEPCONFIG_MODE_SWCNT STEPCONFIG_MODE(1) > #define STEPCONFIG_AVG_MASK (7 << 2) > #define STEPCONFIG_AVG(val) ((val) << 2) > #define STEPCONFIG_AVG_16 STEPCONFIG_AVG(4) > @@ -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 > @@ -153,6 +162,10 @@ struct ti_tscadc_dev { > > /* adc device */ > struct adc_device *adc; > + > + /* Context save */ > + unsigned int irqstat; > + unsigned int ctrl; > }; > > static inline struct ti_tscadc_dev *ti_tscadc_dev_get(struct platform_device *p) -- Lee Jones Linaro ST-Ericsson Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 21+ messages in thread
[parent not found: <1377470724-15710-3-git-send-email-zubair.lutfullah-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]
* [PATCH] iio: am335x-iio-adc: select triggered buffer [not found] ` <1377470724-15710-3-git-send-email-zubair.lutfullah-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> @ 2013-08-28 10:31 ` Sebastian Andrzej Siewior [not found] ` <20130828103134.GA14111-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org> 2013-08-28 14:18 ` [PATCH 2/2] iio: ti_am335x_adc: Add continuous sampling support Sebastian Andrzej Siewior 2013-08-28 16:43 ` Sebastian Andrzej Siewior 2 siblings, 1 reply; 21+ messages in thread From: Sebastian Andrzej Siewior @ 2013-08-28 10:31 UTC (permalink / raw) To: Zubair Lutfullah Cc: jic23-KWPb1pKIrIJaa/9Udqfwiw, lee.jones-QSEj5FYQhm4dnm+yROfE0A, linux-iio-u79uwXL29TY76Z2rM5mHXA, linux-input-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r "triggered buffer" isn't selected so we end up with: |ERROR: "iio_triggered_buffer_setup" [drivers/iio/adc/ti_am335x_adc.ko] undefined! Signed-off-by: Sebastian Andrzej Siewior <bigeasy-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org> --- Please merge this into _this_ patch, it is still required. drivers/iio/adc/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig index 09371cb..cfa2039 100644 --- a/drivers/iio/adc/Kconfig +++ b/drivers/iio/adc/Kconfig @@ -167,6 +167,7 @@ config TI_ADC081C config TI_AM335X_ADC tristate "TI's AM335X ADC driver" depends on MFD_TI_AM335X_TSCADC + select IIO_TRIGGERED_BUFFER help Say yes here to build support for Texas Instruments ADC driver which is also a MFD client. -- 1.8.4.rc3 ^ permalink raw reply related [flat|nested] 21+ messages in thread
[parent not found: <20130828103134.GA14111-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>]
* Re: [PATCH] iio: am335x-iio-adc: select triggered buffer [not found] ` <20130828103134.GA14111-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org> @ 2013-08-28 18:32 ` Zubair Lutfullah : 0 siblings, 0 replies; 21+ messages in thread From: Zubair Lutfullah : @ 2013-08-28 18:32 UTC (permalink / raw) To: Sebastian Andrzej Siewior Cc: Zubair Lutfullah, jic23-KWPb1pKIrIJaa/9Udqfwiw, lee.jones-QSEj5FYQhm4dnm+yROfE0A, linux-iio-u79uwXL29TY76Z2rM5mHXA, linux-input-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r On Wed, Aug 28, 2013 at 12:31:34PM +0200, Sebastian Andrzej Siewior wrote: > "triggered buffer" isn't selected so we end up with: > |ERROR: "iio_triggered_buffer_setup" [drivers/iio/adc/ti_am335x_adc.ko] undefined! > > Signed-off-by: Sebastian Andrzej Siewior <bigeasy-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org> > --- > Please merge this into _this_ patch, it is still required. > > drivers/iio/adc/Kconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig > index 09371cb..cfa2039 100644 > --- a/drivers/iio/adc/Kconfig > +++ b/drivers/iio/adc/Kconfig > @@ -167,6 +167,7 @@ config TI_ADC081C > config TI_AM335X_ADC > tristate "TI's AM335X ADC driver" > depends on MFD_TI_AM335X_TSCADC > + select IIO_TRIGGERED_BUFFER Thanks for pointing it out. Added. ZubairLK ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] iio: ti_am335x_adc: Add continuous sampling support [not found] ` <1377470724-15710-3-git-send-email-zubair.lutfullah-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 2013-08-28 10:31 ` [PATCH] iio: am335x-iio-adc: select triggered buffer Sebastian Andrzej Siewior @ 2013-08-28 14:18 ` Sebastian Andrzej Siewior [not found] ` <20130828141835.GD14111-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org> 2013-08-28 16:43 ` Sebastian Andrzej Siewior 2 siblings, 1 reply; 21+ messages in thread From: Sebastian Andrzej Siewior @ 2013-08-28 14:18 UTC (permalink / raw) To: Zubair Lutfullah Cc: jic23-KWPb1pKIrIJaa/9Udqfwiw, lee.jones-QSEj5FYQhm4dnm+yROfE0A, linux-iio-u79uwXL29TY76Z2rM5mHXA, linux-input-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r * Zubair Lutfullah | 2013-08-25 23:45:24 [+0100]: >diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c >index a952538..ae2202b 100644 >--- a/drivers/iio/adc/ti_am335x_adc.c >+++ b/drivers/iio/adc/ti_am335x_adc.c … >+static struct iio_trigger *tiadc_iio_allocate_trigger(struct iio_dev *indio_dev) >+{ >+ struct iio_trigger *trig; >+ int ret; >+ >+ trig = iio_trigger_alloc("%s-dev%d", indio_dev->name, indio_dev->id); >+ if (trig == NULL) >+ return NULL; >+ >+ trig->dev.parent = indio_dev->dev.parent; >+ trig->ops = &tiadc_trigger_ops; >+ iio_trigger_set_drvdata(trig, indio_dev); >+ >+ ret = iio_trigger_register(trig); >+ if (ret) shouldn't you free the trigger / undo iio_trigger_alloc() here? >+ return NULL; >+ >+ return trig; > } > > static const char * const chan_name_ain[] = { >@@ -220,7 +394,8 @@ static int tiadc_probe(struct platform_device *pdev) > sizeof(struct tiadc_device)); > if (indio_dev == NULL) { > dev_err(&pdev->dev, "failed to allocate iio device\n"); >- return -ENOMEM; >+ err = -ENOMEM; >+ goto err_ret; Why the jump instead of leave right away? > } > adc_dev = iio_priv(indio_dev); > >@@ -262,11 +465,14 @@ static int tiadc_remove(struct platform_device *pdev) > struct tiadc_device *adc_dev = iio_priv(indio_dev); > u32 step_en; > >+ free_irq(adc_dev->irq, indio_dev); > iio_device_unregister(indio_dev); >+ iio_buffer_unregister(indio_dev); > tiadc_channels_remove(indio_dev); I think you need also to iio_trigger_unregister(adc_dev->trig); iio_trigger_free(adc_dev->trig); here. > step_en = get_adc_step_mask(adc_dev); > am335x_tsc_se_clr(adc_dev->mfd_tscadc, step_en); >+ iio_device_free(indio_dev); > > return 0; > } Sebastian ^ permalink raw reply [flat|nested] 21+ messages in thread
[parent not found: <20130828141835.GD14111-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>]
* Re: [PATCH 2/2] iio: ti_am335x_adc: Add continuous sampling support [not found] ` <20130828141835.GD14111-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org> @ 2013-08-28 18:22 ` Zubair Lutfullah : 0 siblings, 0 replies; 21+ messages in thread From: Zubair Lutfullah : @ 2013-08-28 18:22 UTC (permalink / raw) To: Sebastian Andrzej Siewior Cc: Zubair Lutfullah, jic23-KWPb1pKIrIJaa/9Udqfwiw, lee.jones-QSEj5FYQhm4dnm+yROfE0A, linux-iio-u79uwXL29TY76Z2rM5mHXA, linux-input-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r On Wed, Aug 28, 2013 at 04:18:35PM +0200, Sebastian Andrzej Siewior wrote: > * Zubair Lutfullah | 2013-08-25 23:45:24 [+0100]: > > >diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c > >index a952538..ae2202b 100644 > >--- a/drivers/iio/adc/ti_am335x_adc.c > >+++ b/drivers/iio/adc/ti_am335x_adc.c > … > > >+static struct iio_trigger *tiadc_iio_allocate_trigger(struct iio_dev *indio_dev) ... > >+ trig = iio_trigger_alloc("%s-dev%d", indio_dev->name, indio_dev->id); > >+ if (trig == NULL) > >+ return NULL; > >+ ... > >+ ret = iio_trigger_register(trig); > >+ if (ret) > > shouldn't you free the trigger / undo iio_trigger_alloc() here? > > >+ return NULL; > >+ ... Possible. It's done in the probe section just to keep error handling bunched there. > > > > > static const char * const chan_name_ain[] = { > >@@ -220,7 +394,8 @@ static int tiadc_probe(struct platform_device *pdev) > > sizeof(struct tiadc_device)); > > if (indio_dev == NULL) { > > dev_err(&pdev->dev, "failed to allocate iio device\n"); > >- return -ENOMEM; > >+ err = -ENOMEM; > >+ goto err_ret; > Why the jump instead of leave right away? > > > } Again. Error handling all in one place. > > > >@@ -262,11 +465,14 @@ static int tiadc_remove(struct platform_device *pdev) > > struct tiadc_device *adc_dev = iio_priv(indio_dev); > > u32 step_en; > > > >+ free_irq(adc_dev->irq, indio_dev); > > iio_device_unregister(indio_dev); > >+ iio_buffer_unregister(indio_dev); > > tiadc_channels_remove(indio_dev); > > I think you need also to > > iio_trigger_unregister(adc_dev->trig); > iio_trigger_free(adc_dev->trig); > > here. Indeed. Thanks for pointing it out. Zubair ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] iio: ti_am335x_adc: Add continuous sampling support [not found] ` <1377470724-15710-3-git-send-email-zubair.lutfullah-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 2013-08-28 10:31 ` [PATCH] iio: am335x-iio-adc: select triggered buffer Sebastian Andrzej Siewior 2013-08-28 14:18 ` [PATCH 2/2] iio: ti_am335x_adc: Add continuous sampling support Sebastian Andrzej Siewior @ 2013-08-28 16:43 ` Sebastian Andrzej Siewior [not found] ` <20130828164308.GE14111-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org> 2 siblings, 1 reply; 21+ messages in thread From: Sebastian Andrzej Siewior @ 2013-08-28 16:43 UTC (permalink / raw) To: Zubair Lutfullah Cc: jic23-KWPb1pKIrIJaa/9Udqfwiw, lee.jones-QSEj5FYQhm4dnm+yROfE0A, linux-iio-u79uwXL29TY76Z2rM5mHXA, linux-input-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r * Zubair Lutfullah | 2013-08-25 23:45:24 [+0100]: >diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c >index a952538..ae2202b 100644 >--- a/drivers/iio/adc/ti_am335x_adc.c >+++ b/drivers/iio/adc/ti_am335x_adc.c >@@ -231,28 +406,56 @@ static int tiadc_probe(struct platform_device *pdev) … >+err_free_device: >+ iio_device_free(indio_dev); I am not sure about this one. >+err_ret: > return err; > } > >@@ -262,11 +465,14 @@ static int tiadc_remove(struct platform_device *pdev) > struct tiadc_device *adc_dev = iio_priv(indio_dev); > u32 step_en; > >+ free_irq(adc_dev->irq, indio_dev); > iio_device_unregister(indio_dev); >+ iio_buffer_unregister(indio_dev); > tiadc_channels_remove(indio_dev); > > step_en = get_adc_step_mask(adc_dev); > am335x_tsc_se_clr(adc_dev->mfd_tscadc, step_en); >+ iio_device_free(indio_dev); But this one is wrong. The will be removed via dev_res() and if you do it here as well then dev_res() will decrement the reference of an unused object. > > return 0; > } Sebastian ^ permalink raw reply [flat|nested] 21+ messages in thread
[parent not found: <20130828164308.GE14111-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>]
* Re: [PATCH 2/2] iio: ti_am335x_adc: Add continuous sampling support [not found] ` <20130828164308.GE14111-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org> @ 2013-08-28 18:19 ` Zubair Lutfullah : 0 siblings, 0 replies; 21+ messages in thread From: Zubair Lutfullah : @ 2013-08-28 18:19 UTC (permalink / raw) To: Sebastian Andrzej Siewior Cc: Zubair Lutfullah, jic23-KWPb1pKIrIJaa/9Udqfwiw, lee.jones-QSEj5FYQhm4dnm+yROfE0A, linux-iio-u79uwXL29TY76Z2rM5mHXA, linux-input-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r On Wed, Aug 28, 2013 at 06:43:08PM +0200, Sebastian Andrzej Siewior wrote: > * Zubair Lutfullah | 2013-08-25 23:45:24 [+0100]: > > >diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c > >index a952538..ae2202b 100644 > >--- a/drivers/iio/adc/ti_am335x_adc.c > >+++ b/drivers/iio/adc/ti_am335x_adc.c > >@@ -231,28 +406,56 @@ static int tiadc_probe(struct platform_device *pdev) > … > >+err_free_device: > >+ iio_device_free(indio_dev); > > I am not sure about this one. If I understand correctly, if devm_iio_device_alloc is successful earlier in the code and subsequent stuff fails. Then the code jumps to err_free_device and this is needed. > > >+err_ret: > > return err; > > } > > > >@@ -262,11 +465,14 @@ static int tiadc_remove(struct platform_device *pdev) > > struct tiadc_device *adc_dev = iio_priv(indio_dev); > > u32 step_en; > > > >+ free_irq(adc_dev->irq, indio_dev); > > iio_device_unregister(indio_dev); > >+ iio_buffer_unregister(indio_dev); > > tiadc_channels_remove(indio_dev); > > > > step_en = get_adc_step_mask(adc_dev); > > am335x_tsc_se_clr(adc_dev->mfd_tscadc, step_en); > >+ iio_device_free(indio_dev); > > But this one is wrong. The will be removed via dev_res() and if you do > it here as well then dev_res() will decrement the reference of an unused > object. I was unaware of this aspect. *but* iio_simple_dummy.c has it in its remove function. And I just checked and found it in several drivers as well. I'll leave this to the more experienced folks on the list.. > > > > return 0; > > } > > Sebastian ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] iio: ti_am335x_adc: Add continuous sampling support 2013-08-25 22:45 ` [PATCH 2/2] iio: ti_am335x_adc: Add continuous sampling support Zubair Lutfullah 2013-08-27 8:42 ` Lee Jones [not found] ` <1377470724-15710-3-git-send-email-zubair.lutfullah-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> @ 2013-08-28 13:01 ` Sebastian Andrzej Siewior 2013-08-28 18:43 ` Zubair Lutfullah : 2 siblings, 1 reply; 21+ messages in thread From: Sebastian Andrzej Siewior @ 2013-08-28 13:01 UTC (permalink / raw) To: Zubair Lutfullah Cc: jic23, lee.jones, linux-iio, linux-input, linux-kernel, gregkh * Zubair Lutfullah | 2013-08-25 23:45:24 [+0100]: I am mostly happy with it. There are just two things I pointed out. Besides that, it looks good from my side. >diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c >index a952538..ae2202b 100644 >--- a/drivers/iio/adc/ti_am335x_adc.c >+++ b/drivers/iio/adc/ti_am335x_adc.c >@@ -85,7 +96,175 @@ static void tiadc_step_config(struct tiadc_device *adc_dev) > adc_dev->channel_step[i] = steps; > steps++; > } >+} >+ >+static irqreturn_t tiadc_irq(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)); >+ } else if (status & IRQENB_FIFO1THRES) { >+ /* Trigger to push FIFO data to iio buffer */ >+ tiadc_writel(adc_dev, REG_IRQCLR, IRQENB_FIFO1THRES); >+ iio_trigger_poll(indio_dev->trig, iio_get_time_ns()); >+ } else >+ return IRQ_NONE; >+ >+ /* If any IRQ flags left, return none. So TSC can handle its IRQs */ >+ status = tiadc_readl(adc_dev, REG_IRQSTATUS); >+ if (status == false) >+ return IRQ_HANDLED; >+ else >+ return IRQ_NONE; As I said in 1/2 of this series, you shouldn't do this. Both handlers of a shared line are invoked once an interrupt is detected. … >+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, stepnum; >+ u8 bit; >+ >+ adc_dev->data = kmalloc(indio_dev->scan_bytes, GFP_KERNEL); >+ if (adc_dev->data == NULL) >+ return -ENOMEM; >+ >+ tiadc_step_config(indio_dev); >+ for_each_set_bit(bit, buffer->scan_mask, >+ adc_dev->channels) { >+ struct iio_chan_spec const *chan = indio_dev->channels + bit; >+ /* >+ * There are a total of 16 steps available >+ * that are shared between ADC and touchscreen. >+ * We start configuring from step 16 to 0 incase of >+ * ADC. Hence the relation between input channel >+ * and step for ADC would be as below. >+ */ >+ stepnum = chan->channel + 9; >+ enb |= (1 << stepnum); Hmm. This looks odd. In tiadc_step_config() we do: | steps = TOTAL_STEPS - adc_dev->channels; |… | for (i = 0; i < adc_dev->channels; i++) { | chan = adc_dev->channel_line[i]; | tiadc_writel(adc_dev, | REG_STEPCONFIG(steps), | stepconfig | | | STEPCONFIG_INP(chan)); | tiadc_writel(adc_dev, | REG_STEPDELAY(steps), | STEPCONFIG_OPENDLY); | adc_dev->channel_step[i] | = | steps; | steps++; | } That means if we have only one channel we enable the last step bit / topmost that is bit 16. For the "default" four channels we get 0x1e000 as mask which means bit 13 to 16. If you do have only one channel with the number 4 you then get_adc_step_mask() will compute the correct step mask but here enable step 14 instead of step 16. What about the following as replacement? index 08681d3..3bfcf1b 100644 --- a/drivers/iio/adc/ti_am335x_adc.c +++ b/drivers/iio/adc/ti_am335x_adc.c @@ -65,6 +65,11 @@ static u32 get_adc_step_mask(struct tiadc_device *adc_dev) return step_en; } +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); @@ -179,7 +187,7 @@ 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, stepnum; + unsigned int enb = 0; u8 bit; adc_dev->data = kmalloc(indio_dev->scan_bytes, GFP_KERNEL); @@ -187,19 +195,8 @@ static int tiadc_buffer_postenable(struct iio_dev *indio_dev) return -ENOMEM; tiadc_step_config(indio_dev); - for_each_set_bit(bit, buffer->scan_mask, - adc_dev->channels) { - struct iio_chan_spec const *chan = indio_dev->channels + bit; - /* - * There are a total of 16 steps available - * that are shared between ADC and touchscreen. - * We start configuring from step 16 to 0 incase of - * ADC. Hence the relation between input channel - * and step for ADC would be as below. - */ - stepnum = chan->channel + 9; - enb |= (1 << stepnum); - } + for_each_set_bit(bit, buffer->scan_mask, adc_dev->channels) + enb |= get_adc_step_bit(adc_dev, bit); adc_dev->buffer_en_ch_steps = enb; am335x_tsc_se_set(adc_dev->mfd_tscadc, enb); Would it work? This is untested but it could work :) Sebastian ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] iio: ti_am335x_adc: Add continuous sampling support 2013-08-28 13:01 ` Sebastian Andrzej Siewior @ 2013-08-28 18:43 ` Zubair Lutfullah : 0 siblings, 0 replies; 21+ messages in thread From: Zubair Lutfullah : @ 2013-08-28 18:43 UTC (permalink / raw) To: Sebastian Andrzej Siewior Cc: Zubair Lutfullah, jic23, lee.jones, linux-iio, linux-input, linux-kernel, gregkh On Wed, Aug 28, 2013 at 03:01:24PM +0200, Sebastian Andrzej Siewior wrote: > * Zubair Lutfullah | 2013-08-25 23:45:24 [+0100]: > > I am mostly happy with it. There are just two things I pointed out. > Besides that, it looks good from my side. Sign-off? > >+static irqreturn_t tiadc_irq(int irq, void *private) > >+{ ... > >+ /* If any IRQ flags left, return none. So TSC can handle its IRQs */ > >+ status = tiadc_readl(adc_dev, REG_IRQSTATUS); > >+ if (status == false) > >+ return IRQ_HANDLED; > >+ else > >+ return IRQ_NONE; > > As I said in 1/2 of this series, you shouldn't do this. Both handlers of a > shared line are invoked once an interrupt is detected. > > … I somehow missed sending these patches to Dmitry.. I'll do it next series. He originally suggested this way. An alternative was to handle irqs in mfd core which would seriously complicate code and patches.. Whenever I change the handlers to the way you suggest. i.e. Handle what each irq handler does and return IRQ_HANDLED if the irq handler handled nothing then return IRQ_NONE. The driver hangs when I'm brutal while touching the TSC and continuously sampling the ADC. Not always. But happens. When I check /proc/interrupts the hardware irqs seem to be firing. But for some reason the whole thing messes up.. > >+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, stepnum; > >+ u8 bit; > >+ > >+ adc_dev->data = kmalloc(indio_dev->scan_bytes, GFP_KERNEL); > >+ if (adc_dev->data == NULL) > >+ return -ENOMEM; > >+ > >+ tiadc_step_config(indio_dev); > >+ for_each_set_bit(bit, buffer->scan_mask, > >+ adc_dev->channels) { > >+ struct iio_chan_spec const *chan = indio_dev->channels + bit; > >+ /* > >+ * There are a total of 16 steps available > >+ * that are shared between ADC and touchscreen. > >+ * We start configuring from step 16 to 0 incase of > >+ * ADC. Hence the relation between input channel > >+ * and step for ADC would be as below. > >+ */ > >+ stepnum = chan->channel + 9; > >+ enb |= (1 << stepnum); > > Hmm. This looks odd. > > In tiadc_step_config() we do: > | steps = TOTAL_STEPS - adc_dev->channels; > |… > | for (i = 0; i < adc_dev->channels; i++) { > | chan = adc_dev->channel_line[i]; > | tiadc_writel(adc_dev, > | REG_STEPCONFIG(steps), > | stepconfig > | | > | STEPCONFIG_INP(chan)); > | tiadc_writel(adc_dev, > | REG_STEPDELAY(steps), > | STEPCONFIG_OPENDLY); > | adc_dev->channel_step[i] > | = > | steps; > | steps++; > | } > > That means if we have only one channel we enable the last step bit / > topmost that is bit 16. For the "default" four channels we get 0x1e000 > as mask which means bit 13 to 16. > If you do have only one channel with the number 4 you then > get_adc_step_mask() will compute the correct step mask but here enable > step 14 instead of step 16. > > What about the following as replacement? I sense a mismatch. Probably because of different styles of coding these bit handling :). I'll look into it and the alternative way you suggested as well. Thanks ZubairLK ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH V7 0/2] iio: input: ti_am335x_tscadc: Add continuous sampling support to adc @ 2013-09-01 11:07 Zubair Lutfullah 2013-09-01 11:07 ` [PATCH 1/2] input: ti_am335x_tsc: Enable shared IRQ for TSC Zubair Lutfullah 0 siblings, 1 reply; 21+ messages in thread From: Zubair Lutfullah @ 2013-09-01 11:07 UTC (permalink / raw) To: jic23, dmitry.torokhov Cc: linux-iio, linux-input, linux-kernel, bigeasy, gregkh, zubair.lutfullah Round 7 updates - Fixes to error handling path for trigger register - Optimization of step enable calculations based on input from Sebastian - trigger unregister added to remove section. was missing - missing entry in Kconfig for triggered buffer support. added - TSC/ADC IRQ handling was based on wierd ack system. not required. removed. each irq handler deals with its own stuff *only* Thanks for all the input Regards Zubair Lutfullah Kakakhel Zubair Lutfullah (2): input: ti_am335x_tsc: Enable shared IRQ for TSC iio: ti_am335x_adc: Add continuous sampling support drivers/iio/adc/Kconfig | 1 + drivers/iio/adc/ti_am335x_adc.c | 243 ++++++++++++++++++++++++++--- drivers/input/touchscreen/ti_am335x_tsc.c | 12 +- include/linux/mfd/ti_am335x_tscadc.h | 13 ++ 4 files changed, 246 insertions(+), 23 deletions(-) -- 1.7.9.5 ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 1/2] input: ti_am335x_tsc: Enable shared IRQ for TSC 2013-09-01 11:07 [PATCH V7 0/2] iio: input: ti_am335x_tscadc: Add continuous sampling support to adc Zubair Lutfullah @ 2013-09-01 11:07 ` Zubair Lutfullah 0 siblings, 0 replies; 21+ messages in thread From: Zubair Lutfullah @ 2013-09-01 11:07 UTC (permalink / raw) To: jic23, dmitry.torokhov Cc: linux-iio, linux-input, linux-kernel, bigeasy, gregkh, zubair.lutfullah 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 <zubair.lutfullah@gmail.com> --- 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 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH V8 0/2] iio: input: ti_am335x_tscadc: Add continuous sampling support to adc @ 2013-09-01 11:17 Zubair Lutfullah [not found] ` <1378034277-26728-1-git-send-email-zubair.lutfullah-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 0 siblings, 1 reply; 21+ messages in thread From: Zubair Lutfullah @ 2013-09-01 11:17 UTC (permalink / raw) To: jic23, dmitry.torokhov Cc: linux-iio, linux-input, linux-kernel, bigeasy, gregkh, zubair.lutfullah Round 8 updates Rebased to togreg branch of IIO.. Round 7 updates - Fixes to error handling path for trigger register - Optimization of step enable calculations based on input from Sebastian - trigger unregister added to remove section. was missing - missing entry in Kconfig for triggered buffer support. added - TSC/ADC IRQ handling was based on wierd ack system. not required. removed. each irq handler deals with its own stuff *only* Thanks for all the input Regards Zubair Lutfullah Kakakhel Zubair Lutfullah (2): input: ti_am335x_tsc: Enable shared IRQ for TSC iio: ti_am335x_adc: Add continuous sampling support drivers/iio/adc/Kconfig | 1 + drivers/iio/adc/ti_am335x_adc.c | 243 ++++++++++++++++++++++++++--- drivers/input/touchscreen/ti_am335x_tsc.c | 12 +- include/linux/mfd/ti_am335x_tscadc.h | 13 ++ 4 files changed, 246 insertions(+), 23 deletions(-) -- 1.7.9.5 ^ permalink raw reply [flat|nested] 21+ messages in thread
[parent not found: <1378034277-26728-1-git-send-email-zubair.lutfullah-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]
* [PATCH 1/2] input: ti_am335x_tsc: Enable shared IRQ for TSC [not found] ` <1378034277-26728-1-git-send-email-zubair.lutfullah-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> @ 2013-09-01 11:17 ` Zubair Lutfullah [not found] ` <1378034277-26728-2-git-send-email-zubair.lutfullah-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 0 siblings, 1 reply; 21+ messages in thread From: Zubair Lutfullah @ 2013-09-01 11:17 UTC (permalink / raw) To: jic23-KWPb1pKIrIJaa/9Udqfwiw, dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA, linux-input-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, bigeasy-hfZtesqFncYOwBW4kG4KsQ, gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r, zubair.lutfullah-Re5JQEeQqe8AvxtiuMwx3w 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 <zubair.lutfullah-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> --- 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 ^ permalink raw reply related [flat|nested] 21+ messages in thread
[parent not found: <1378034277-26728-2-git-send-email-zubair.lutfullah-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]
* Re: [PATCH 1/2] input: ti_am335x_tsc: Enable shared IRQ for TSC [not found] ` <1378034277-26728-2-git-send-email-zubair.lutfullah-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> @ 2013-09-08 11:29 ` Jonathan Cameron [not found] ` <522C5F96.20001-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> 0 siblings, 1 reply; 21+ messages in thread From: Jonathan Cameron @ 2013-09-08 11:29 UTC (permalink / raw) To: Zubair Lutfullah Cc: dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w, linux-iio-u79uwXL29TY76Z2rM5mHXA, linux-input-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, bigeasy-hfZtesqFncYOwBW4kG4KsQ, gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r On 09/01/13 12:17, Zubair Lutfullah wrote: > 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 <zubair.lutfullah-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> Whilst I would have prefered an mfd under these drivers and elegant handling of the interrupts, I guess if this works it is at least not terribly invasive. However, this does need an Ack from Dmitry before I can take it (or for Dmitry to take it himself?) > --- > 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; > ^ permalink raw reply [flat|nested] 21+ messages in thread
[parent not found: <522C5F96.20001-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>]
* Re: [PATCH 1/2] input: ti_am335x_tsc: Enable shared IRQ for TSC [not found] ` <522C5F96.20001-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> @ 2013-09-09 16:12 ` Dmitry Torokhov [not found] ` <20130909161230.GA25107-WlK9ik9hQGAhIp7JRqBPierSzoNAToWh@public.gmane.org> 0 siblings, 1 reply; 21+ messages in thread From: Dmitry Torokhov @ 2013-09-09 16:12 UTC (permalink / raw) To: Jonathan Cameron Cc: Zubair Lutfullah, linux-iio-u79uwXL29TY76Z2rM5mHXA, linux-input-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, bigeasy-hfZtesqFncYOwBW4kG4KsQ, gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r On Sun, Sep 08, 2013 at 12:29:26PM +0100, Jonathan Cameron wrote: > On 09/01/13 12:17, Zubair Lutfullah wrote: > > 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 <zubair.lutfullah-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> > Whilst I would have prefered an mfd under these drivers and elegant handling > of the interrupts, I guess if this works it is at least not terribly invasive. > > However, this does need an Ack from Dmitry before I can take it (or for > Dmitry to take it himself?) I completely agree with Jonathan, more elegant handling would be nice but current one will do for now. Since most of the work on the driver is going through IIO tree I think it would make sense for this patch to go through it as well. Acked-by: Dmitry Torokhov <dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> Thanks. -- Dmitry ^ permalink raw reply [flat|nested] 21+ messages in thread
[parent not found: <20130909161230.GA25107-WlK9ik9hQGAhIp7JRqBPierSzoNAToWh@public.gmane.org>]
* Re: [PATCH 1/2] input: ti_am335x_tsc: Enable shared IRQ for TSC [not found] ` <20130909161230.GA25107-WlK9ik9hQGAhIp7JRqBPierSzoNAToWh@public.gmane.org> @ 2013-09-11 16:08 ` Zubair Lutfullah : 0 siblings, 0 replies; 21+ messages in thread From: Zubair Lutfullah : @ 2013-09-11 16:08 UTC (permalink / raw) To: Dmitry Torokhov Cc: Jonathan Cameron, Zubair Lutfullah, linux-iio-u79uwXL29TY76Z2rM5mHXA, linux-input-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, bigeasy-hfZtesqFncYOwBW4kG4KsQ, gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r On Mon, Sep 09, 2013 at 09:12:30AM -0700, Dmitry Torokhov wrote: > On Sun, Sep 08, 2013 at 12:29:26PM +0100, Jonathan Cameron wrote: > > On 09/01/13 12:17, Zubair Lutfullah wrote: > > > 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 <zubair.lutfullah-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> > > Whilst I would have prefered an mfd under these drivers and elegant handling > > of the interrupts, I guess if this works it is at least not terribly invasive. > > > > However, this does need an Ack from Dmitry before I can take it (or for > > Dmitry to take it himself?) > > I completely agree with Jonathan, more elegant handling would be nice > but current one will do for now. > > Since most of the work on the driver is going through IIO tree I think > it would make sense for this patch to go through it as well. > > Acked-by: Dmitry Torokhov <dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> Thank-you. ZubairLK ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH V9 0/2] iio: input: ti_am335x_adc: Add continuous sampling support @ 2013-09-17 4:44 Zubair Lutfullah 2013-09-17 4:44 ` [PATCH 1/2] input: ti_am335x_tsc: Enable shared IRQ for TSC Zubair Lutfullah 0 siblings, 1 reply; 21+ messages in thread From: Zubair Lutfullah @ 2013-09-17 4:44 UTC (permalink / raw) To: jic23-KWPb1pKIrIJaa/9Udqfwiw, dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA, linux-input-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, bigeasy-hfZtesqFncYOwBW4kG4KsQ, gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r, zubair.lutfullah-Re5JQEeQqe8AvxtiuMwx3w Hi, This applies to togreg branch in iio. 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 (2): input: ti_am335x_tsc: Enable shared IRQ for TSC iio: ti_am335x_adc: Add continuous sampling support drivers/iio/adc/ti_am335x_adc.c | 222 ++++++++++++++++++++++++++++- drivers/input/touchscreen/ti_am335x_tsc.c | 12 +- include/linux/mfd/ti_am335x_tscadc.h | 9 ++ 3 files changed, 235 insertions(+), 8 deletions(-) -- 1.7.9.5 ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 1/2] input: ti_am335x_tsc: Enable shared IRQ for TSC 2013-09-17 4:44 [PATCH V9 0/2] iio: input: ti_am335x_adc: Add continuous sampling support Zubair Lutfullah @ 2013-09-17 4:44 ` Zubair Lutfullah 0 siblings, 0 replies; 21+ messages in thread From: Zubair Lutfullah @ 2013-09-17 4:44 UTC (permalink / raw) To: jic23, dmitry.torokhov Cc: linux-iio, linux-input, linux-kernel, bigeasy, gregkh, zubair.lutfullah 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 <zubair.lutfullah@gmail.com> Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> --- 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 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH V10 0/2] iio: input: ti_am335x_adc: Add continuous sampling support @ 2013-09-18 11:23 Zubair Lutfullah [not found] ` <1379503383-17086-1-git-send-email-zubair.lutfullah-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 0 siblings, 1 reply; 21+ messages in thread From: Zubair Lutfullah @ 2013-09-18 11:23 UTC (permalink / raw) To: jic23, dmitry.torokhov Cc: linux-iio, linux-input, linux-kernel, bigeasy, gregkh, zubair.lutfullah Hi, These apply to togreg branch in iio. 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 (2): input: ti_am335x_tsc: Enable shared IRQ for TSC iio: ti_am335x_adc: Add continuous sampling support drivers/iio/adc/ti_am335x_adc.c | 216 ++++++++++++++++++++++++++++- drivers/input/touchscreen/ti_am335x_tsc.c | 12 +- include/linux/mfd/ti_am335x_tscadc.h | 9 ++ 3 files changed, 229 insertions(+), 8 deletions(-) -- 1.7.9.5 ^ permalink raw reply [flat|nested] 21+ messages in thread
[parent not found: <1379503383-17086-1-git-send-email-zubair.lutfullah-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]
* [PATCH 1/2] input: ti_am335x_tsc: Enable shared IRQ for TSC [not found] ` <1379503383-17086-1-git-send-email-zubair.lutfullah-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> @ 2013-09-18 11:23 ` Zubair Lutfullah 0 siblings, 0 replies; 21+ messages in thread From: Zubair Lutfullah @ 2013-09-18 11:23 UTC (permalink / raw) To: jic23-KWPb1pKIrIJaa/9Udqfwiw, dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA, linux-input-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, bigeasy-hfZtesqFncYOwBW4kG4KsQ, gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r, zubair.lutfullah-Re5JQEeQqe8AvxtiuMwx3w 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 <zubair.lutfullah-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> Acked-by: Dmitry Torokhov <dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> --- 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 ^ permalink raw reply related [flat|nested] 21+ messages in thread
end of thread, other threads:[~2013-09-18 11:23 UTC | newest] Thread overview: 21+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-08-25 22:45 [PATCH V6 0/2] iio: input: ti_am335x_adc: Add continuous sampling support Zubair Lutfullah 2013-08-25 22:45 ` [PATCH 1/2] input: ti_am335x_tsc: Enable shared IRQ for TSC Zubair Lutfullah [not found] ` <1377470724-15710-2-git-send-email-zubair.lutfullah-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 2013-08-28 10:42 ` Sebastian Andrzej Siewior 2013-08-28 18:49 ` Zubair Lutfullah : [not found] ` <1377470724-15710-1-git-send-email-zubair.lutfullah-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 2013-08-25 22:45 ` [PATCH 2/2] iio: ti_am335x_adc: Add continuous sampling support Zubair Lutfullah 2013-08-27 8:42 ` Lee Jones [not found] ` <1377470724-15710-3-git-send-email-zubair.lutfullah-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 2013-08-28 10:31 ` [PATCH] iio: am335x-iio-adc: select triggered buffer Sebastian Andrzej Siewior [not found] ` <20130828103134.GA14111-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org> 2013-08-28 18:32 ` Zubair Lutfullah : 2013-08-28 14:18 ` [PATCH 2/2] iio: ti_am335x_adc: Add continuous sampling support Sebastian Andrzej Siewior [not found] ` <20130828141835.GD14111-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org> 2013-08-28 18:22 ` Zubair Lutfullah : 2013-08-28 16:43 ` Sebastian Andrzej Siewior [not found] ` <20130828164308.GE14111-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org> 2013-08-28 18:19 ` Zubair Lutfullah : 2013-08-28 13:01 ` Sebastian Andrzej Siewior 2013-08-28 18:43 ` Zubair Lutfullah : -- strict thread matches above, loose matches on Subject: below -- 2013-09-01 11:07 [PATCH V7 0/2] iio: input: ti_am335x_tscadc: Add continuous sampling support to adc Zubair Lutfullah 2013-09-01 11:07 ` [PATCH 1/2] input: ti_am335x_tsc: Enable shared IRQ for TSC Zubair Lutfullah 2013-09-01 11:17 [PATCH V8 0/2] iio: input: ti_am335x_tscadc: Add continuous sampling support to adc Zubair Lutfullah [not found] ` <1378034277-26728-1-git-send-email-zubair.lutfullah-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 2013-09-01 11:17 ` [PATCH 1/2] input: ti_am335x_tsc: Enable shared IRQ for TSC Zubair Lutfullah [not found] ` <1378034277-26728-2-git-send-email-zubair.lutfullah-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 2013-09-08 11:29 ` Jonathan Cameron [not found] ` <522C5F96.20001-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> 2013-09-09 16:12 ` Dmitry Torokhov [not found] ` <20130909161230.GA25107-WlK9ik9hQGAhIp7JRqBPierSzoNAToWh@public.gmane.org> 2013-09-11 16:08 ` Zubair Lutfullah : 2013-09-17 4:44 [PATCH V9 0/2] iio: input: ti_am335x_adc: Add continuous sampling support Zubair Lutfullah 2013-09-17 4:44 ` [PATCH 1/2] input: ti_am335x_tsc: Enable shared IRQ for TSC Zubair Lutfullah 2013-09-18 11:23 [PATCH V10 0/2] iio: input: ti_am335x_adc: Add continuous sampling support Zubair Lutfullah [not found] ` <1379503383-17086-1-git-send-email-zubair.lutfullah-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 2013-09-18 11:23 ` [PATCH 1/2] input: ti_am335x_tsc: Enable shared IRQ for TSC Zubair Lutfullah
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).