From fb7f8ce3bcd12bdfa0940c96ba1d2eddba88d000 Mon Sep 17 00:00:00 2001 From: Sebastian Andrzej Siewior Date: Thu, 19 Dec 2013 16:28:27 +0100 Subject: iio: ti_am335x_adc: Adjust the closing bracket in tiadc_read_raw() It somehow looks like the ending bracket belongs to the if statement but it does belong to the while loop. This patch moves the bracket where it belongs. Signed-off-by: Sebastian Andrzej Siewior Acked-by: Jonathan Cameron Signed-off-by: Lee Jones --- drivers/iio/adc/ti_am335x_adc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c index 728411ec7642..ce8d03ac900f 100644 --- a/drivers/iio/adc/ti_am335x_adc.c +++ b/drivers/iio/adc/ti_am335x_adc.c @@ -338,7 +338,7 @@ static int tiadc_read_raw(struct iio_dev *indio_dev, while (tiadc_readl(adc_dev, REG_ADCFSM) & SEQ_STATUS) { if (time_after(jiffies, timeout)) return -EAGAIN; - } + } map_val = chan->channel + TOTAL_CHANNELS; /* -- cgit v1.2.3 From 3466bd2273b81a0a29d0e134ba1c78b64b84f40b Mon Sep 17 00:00:00 2001 From: Sebastian Andrzej Siewior Date: Thu, 19 Dec 2013 16:28:28 +0100 Subject: mfd: ti_am335x_tscadc: Make am335x_tsc_se_update() local Since the "recent" changes, am335x_tsc_se_update() has no longer any users outside of this file so make it local. Signed-off-by: Sebastian Andrzej Siewior Signed-off-by: Lee Jones --- drivers/mfd/ti_am335x_tscadc.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'drivers') diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c index 88718abfb9ba..67d0eb469a45 100644 --- a/drivers/mfd/ti_am335x_tscadc.c +++ b/drivers/mfd/ti_am335x_tscadc.c @@ -48,11 +48,10 @@ static const struct regmap_config tscadc_regmap_config = { .val_bits = 32, }; -void am335x_tsc_se_update(struct ti_tscadc_dev *tsadc) +static void am335x_tsc_se_update(struct ti_tscadc_dev *tsadc) { tscadc_writel(tsadc, REG_SE, tsadc->reg_se_cache); } -EXPORT_SYMBOL_GPL(am335x_tsc_se_update); void am335x_tsc_se_set(struct ti_tscadc_dev *tsadc, u32 val) { -- cgit v1.2.3 From 7e170c6e4f7501bea900aa66b2b27a6ce5001e25 Mon Sep 17 00:00:00 2001 From: Sebastian Andrzej Siewior Date: Thu, 19 Dec 2013 16:28:29 +0100 Subject: mfd: ti_am335x_tscadc: Don't read back REG_SE The purpose of reg_se_cache has been defeated. It should avoid the read-back of the register to avoid the latency and the fact that the bits are reset to 0 after the individual conversation took place. The reason why this is required like this to work, is that read-back of the register removes the bits of the ADC so they do not start another conversation after the register is re-written from the TSC side for the update. To avoid the not required read-back I introduce a "set once" variant which does not update the cache mask. After the conversation completes, the bit is removed from the SE register anyway and we don't plan a new conversation "any time soon". The current set function is renamed to set_cache to distinguish the two operations. This is a small preparation for a larger sync-rework. Signed-off-by: Sebastian Andrzej Siewior Acked-by: Dmitry Torokhov Acked-by: Jonathan Cameron Signed-off-by: Lee Jones --- drivers/iio/adc/ti_am335x_adc.c | 4 ++-- drivers/input/touchscreen/ti_am335x_tsc.c | 4 ++-- drivers/mfd/ti_am335x_tscadc.c | 16 ++++++++++++---- 3 files changed, 16 insertions(+), 8 deletions(-) (limited to 'drivers') diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c index ce8d03ac900f..95eef8e89979 100644 --- a/drivers/iio/adc/ti_am335x_adc.c +++ b/drivers/iio/adc/ti_am335x_adc.c @@ -181,7 +181,7 @@ static int tiadc_buffer_postenable(struct iio_dev *indio_dev) 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); + am335x_tsc_se_set_cache(adc_dev->mfd_tscadc, enb); tiadc_writel(adc_dev, REG_IRQSTATUS, IRQENB_FIFO1THRES | IRQENB_FIFO1OVRRUN | IRQENB_FIFO1UNDRFLW); @@ -332,7 +332,7 @@ static int tiadc_read_raw(struct iio_dev *indio_dev, return -EBUSY; step_en = get_adc_step_mask(adc_dev); - am335x_tsc_se_set(adc_dev->mfd_tscadc, step_en); + am335x_tsc_se_set_once(adc_dev->mfd_tscadc, step_en); /* Wait for ADC sequencer to complete sampling */ while (tiadc_readl(adc_dev, REG_ADCFSM) & SEQ_STATUS) { diff --git a/drivers/input/touchscreen/ti_am335x_tsc.c b/drivers/input/touchscreen/ti_am335x_tsc.c index 68beadaabceb..2ca5a7bee04e 100644 --- a/drivers/input/touchscreen/ti_am335x_tsc.c +++ b/drivers/input/touchscreen/ti_am335x_tsc.c @@ -198,7 +198,7 @@ static void titsc_step_config(struct titsc *ts_dev) /* The steps1 … end and bit 0 for TS_Charge */ stepenable = (1 << (end_step + 2)) - 1; ts_dev->step_mask = stepenable; - am335x_tsc_se_set(ts_dev->mfd_tscadc, ts_dev->step_mask); + am335x_tsc_se_set_cache(ts_dev->mfd_tscadc, ts_dev->step_mask); } static void titsc_read_coordinates(struct titsc *ts_dev, @@ -322,7 +322,7 @@ static irqreturn_t titsc_irq(int irq, void *dev) if (irqclr) { titsc_writel(ts_dev, REG_IRQSTATUS, irqclr); - am335x_tsc_se_set(ts_dev->mfd_tscadc, ts_dev->step_mask); + am335x_tsc_se_set_cache(ts_dev->mfd_tscadc, ts_dev->step_mask); return IRQ_HANDLED; } return IRQ_NONE; diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c index 67d0eb469a45..cb0c211fc7d8 100644 --- a/drivers/mfd/ti_am335x_tscadc.c +++ b/drivers/mfd/ti_am335x_tscadc.c @@ -53,24 +53,32 @@ static void am335x_tsc_se_update(struct ti_tscadc_dev *tsadc) tscadc_writel(tsadc, REG_SE, tsadc->reg_se_cache); } -void am335x_tsc_se_set(struct ti_tscadc_dev *tsadc, u32 val) +void am335x_tsc_se_set_cache(struct ti_tscadc_dev *tsadc, u32 val) { unsigned long flags; spin_lock_irqsave(&tsadc->reg_lock, flags); - tsadc->reg_se_cache = tscadc_readl(tsadc, REG_SE); tsadc->reg_se_cache |= val; am335x_tsc_se_update(tsadc); spin_unlock_irqrestore(&tsadc->reg_lock, flags); } -EXPORT_SYMBOL_GPL(am335x_tsc_se_set); +EXPORT_SYMBOL_GPL(am335x_tsc_se_set_cache); + +void am335x_tsc_se_set_once(struct ti_tscadc_dev *tsadc, u32 val) +{ + unsigned long flags; + + spin_lock_irqsave(&tsadc->reg_lock, flags); + tscadc_writel(tsadc, REG_SE, tsadc->reg_se_cache | val); + spin_unlock_irqrestore(&tsadc->reg_lock, flags); +} +EXPORT_SYMBOL_GPL(am335x_tsc_se_set_once); void am335x_tsc_se_clr(struct ti_tscadc_dev *tsadc, u32 val) { unsigned long flags; spin_lock_irqsave(&tsadc->reg_lock, flags); - tsadc->reg_se_cache = tscadc_readl(tsadc, REG_SE); tsadc->reg_se_cache &= ~val; am335x_tsc_se_update(tsadc); spin_unlock_irqrestore(&tsadc->reg_lock, flags); -- cgit v1.2.3 From 3954b7bfc665fed878cabe57342bae34d2391478 Mon Sep 17 00:00:00 2001 From: Sebastian Andrzej Siewior Date: Thu, 19 Dec 2013 16:28:30 +0100 Subject: mfd: ti_am335x: Drop am335x_tsc_se_update() from resume path The update of the SE register in MFD doesn't look right as it has nothing to do with it. The better place to do it is in TSC driver (which is already doing it) and in the ADC driver which needs this only in the continues mode. Signed-off-by: Sebastian Andrzej Siewior Acked-by: Jonathan Cameron Signed-off-by: Lee Jones --- drivers/iio/adc/ti_am335x_adc.c | 2 ++ drivers/mfd/ti_am335x_tscadc.c | 1 - 2 files changed, 2 insertions(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c index 95eef8e89979..e0dc2d0e7590 100644 --- a/drivers/iio/adc/ti_am335x_adc.c +++ b/drivers/iio/adc/ti_am335x_adc.c @@ -199,6 +199,7 @@ static int tiadc_buffer_predisable(struct iio_dev *indio_dev) 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); + adc_dev->buffer_en_ch_steps = 0; /* Flush FIFO of leftover data in the time it takes to disable adc */ fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT); @@ -491,6 +492,7 @@ static int tiadc_resume(struct device *dev) tiadc_writel(adc_dev, REG_CTRL, restore); tiadc_step_config(indio_dev); + am335x_tsc_se_set(adc_dev->mfd_tscadc, adc_dev->buffer_en_ch_steps); return 0; } diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c index cb0c211fc7d8..157f5699a33c 100644 --- a/drivers/mfd/ti_am335x_tscadc.c +++ b/drivers/mfd/ti_am335x_tscadc.c @@ -309,7 +309,6 @@ static int tscadc_resume(struct device *dev) if (tscadc_dev->tsc_cell != -1) tscadc_idle_config(tscadc_dev); - am335x_tsc_se_update(tscadc_dev); restore = tscadc_readl(tscadc_dev, REG_CTRL); tscadc_writel(tscadc_dev, REG_CTRL, (restore | CNTRLREG_TSCSSENB)); -- cgit v1.2.3 From 7ca6740cd1cd410828a01151a044b51910d06eff Mon Sep 17 00:00:00 2001 From: Sebastian Andrzej Siewior Date: Thu, 19 Dec 2013 16:28:31 +0100 Subject: mfd: input: iio: ti_amm335x: Rework TSC/ADC synchronization The ADC driver always programs all possible ADC values and discards them except for the value IIO asked for. On the am335x-evm the driver programs four values and it takes 500us to gather them. Reducing the number of conversations down to the (required) one also reduces the busy loop down to 125us. This leads to another error, namely the FIFOCOUNT register is sometimes (like one out of 10 attempts) not updated in time leading to EBUSY. The next read has the FIFOCOUNT register updated. Checking for the ADCSTAT register for being idle isn't a good choice either. The problem is that if TSC is used at the same time, the HW completes the conversation for ADC *and* before the driver noticed it, the HW begins to perform a TSC conversation and so the driver never seen the HW idle. The next time we would have two values in the FIFO but since the driver reads everything we always see the current one. So instead of polling for the IDLE bit in ADCStatus register, we should check the FIFOCOUNT register. It should be one instead of zero because we request one value. This change in turn leads to another error. Sometimes if TSC & ADC are used together the TSC starts generating interrupts even if nobody actually touched the touchscreen. The interrupts seem valid because TSC's FIFO is filled with values for each channel of the TSC. This condition stops after a few ADC reads but will occur again. Not good. On top of this (even without the changes I just mentioned) there is a ADC & TSC lockup condition which was reported to me by Jeff Lance including the following test case: A busy loop of "cat /sys/bus/iio/devices/iio\:device0/in_voltage4_raw" and a mug on touch screen. With this setup, the hardware will lockup after something between 20 minutes and it could take up to a couple of hours. During that lockup, the ADCSTAT register says 0x30 (or 0x70) which means STEP_ID = IDLE and FSM_BUSY = yes. That means the hardware says that it is idle and busy at the same time which is an invalid condition. For all this reasons I decided to rework this TSC/ADC part and add a handshake / synchronization here: First the ADC signals that it needs the HW and writes a 0 mask into the SE register. The HW (if active) will complete the current conversation and become idle. The TSC driver will gather the values from the FIFO (woken up by an interrupt) and won't "enable" another conversation. Instead it will wake up the ADC driver which is already waiting. The ADC driver will start "its" conversation and once it is done, it will enable the TSC steps so the TSC will work again. After this rework I haven't observed the lockup so far. Plus the busy loop has been reduced from 500us to 125us. The continues-read mode remains unchanged. Signed-off-by: Sebastian Andrzej Siewior Acked-by: Jonathan Cameron Signed-off-by: Lee Jones --- drivers/iio/adc/ti_am335x_adc.c | 64 +++++++++++++++++++++++++++++------------ drivers/mfd/ti_am335x_tscadc.c | 63 +++++++++++++++++++++++++++++++++------- 2 files changed, 99 insertions(+), 28 deletions(-) (limited to 'drivers') diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c index e0dc2d0e7590..dff7343405e2 100644 --- a/drivers/iio/adc/ti_am335x_adc.c +++ b/drivers/iio/adc/ti_am335x_adc.c @@ -60,6 +60,24 @@ static u32 get_adc_step_mask(struct tiadc_device *adc_dev) return step_en; } +static u32 get_adc_chan_step_mask(struct tiadc_device *adc_dev, + struct iio_chan_spec const *chan) +{ + int i; + + for (i = 0; i < ARRAY_SIZE(adc_dev->channel_step); i++) { + if (chan->channel == adc_dev->channel_line[i]) { + u32 step; + + step = adc_dev->channel_step[i]; + /* +1 for the charger */ + return 1 << (step + 1); + } + } + WARN_ON(1); + return 0; +} + static u32 get_adc_step_bit(struct tiadc_device *adc_dev, int chan) { return 1 << adc_dev->channel_step[chan]; @@ -326,34 +344,43 @@ static int tiadc_read_raw(struct iio_dev *indio_dev, unsigned int fifo1count, read, stepid; bool found = false; u32 step_en; - unsigned long timeout = jiffies + usecs_to_jiffies - (IDLE_TIMEOUT * adc_dev->channels); + unsigned long timeout; if (iio_buffer_enabled(indio_dev)) return -EBUSY; - step_en = get_adc_step_mask(adc_dev); + step_en = get_adc_chan_step_mask(adc_dev, chan); + if (!step_en) + return -EINVAL; + + fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT); + while (fifo1count--) + tiadc_readl(adc_dev, REG_FIFO1); + am335x_tsc_se_set_once(adc_dev->mfd_tscadc, step_en); - /* Wait for ADC sequencer to complete sampling */ - while (tiadc_readl(adc_dev, REG_ADCFSM) & SEQ_STATUS) { - if (time_after(jiffies, timeout)) + timeout = jiffies + usecs_to_jiffies + (IDLE_TIMEOUT * adc_dev->channels); + /* Wait for Fifo threshold interrupt */ + while (1) { + fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT); + if (fifo1count) + break; + + if (time_after(jiffies, timeout)) { + am335x_tsc_se_adc_done(adc_dev->mfd_tscadc); return -EAGAIN; + } } map_val = chan->channel + TOTAL_CHANNELS; /* - * When the sub-system is first enabled, - * the sequencer will always start with the - * lowest step (1) and continue until step (16). - * For ex: If we have enabled 4 ADC channels and - * currently use only 1 out of them, the - * sequencer still configures all the 4 steps, - * leading to 3 unwanted data. - * Hence we need to flush out this data. + * We check the complete FIFO. We programmed just one entry but in case + * something went wrong we left empty handed (-EAGAIN previously) and + * then the value apeared somehow in the FIFO we would have two entries. + * Therefore we read every item and keep only the latest version of the + * requested channel. */ - - fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT); for (i = 0; i < fifo1count; i++) { read = tiadc_readl(adc_dev, REG_FIFO1); stepid = read & FIFOREAD_CHNLID_MASK; @@ -365,6 +392,7 @@ static int tiadc_read_raw(struct iio_dev *indio_dev, *val = (u16) read; } } + am335x_tsc_se_adc_done(adc_dev->mfd_tscadc); if (found == false) return -EBUSY; @@ -492,8 +520,8 @@ static int tiadc_resume(struct device *dev) tiadc_writel(adc_dev, REG_CTRL, restore); tiadc_step_config(indio_dev); - am335x_tsc_se_set(adc_dev->mfd_tscadc, adc_dev->buffer_en_ch_steps); - + am335x_tsc_se_set_cache(adc_dev->mfd_tscadc, + adc_dev->buffer_en_ch_steps); return 0; } diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c index 157f5699a33c..d4e860413bb5 100644 --- a/drivers/mfd/ti_am335x_tscadc.c +++ b/drivers/mfd/ti_am335x_tscadc.c @@ -24,6 +24,7 @@ #include #include #include +#include #include @@ -48,31 +49,71 @@ static const struct regmap_config tscadc_regmap_config = { .val_bits = 32, }; -static void am335x_tsc_se_update(struct ti_tscadc_dev *tsadc) -{ - tscadc_writel(tsadc, REG_SE, tsadc->reg_se_cache); -} - void am335x_tsc_se_set_cache(struct ti_tscadc_dev *tsadc, u32 val) { unsigned long flags; spin_lock_irqsave(&tsadc->reg_lock, flags); - tsadc->reg_se_cache |= val; - am335x_tsc_se_update(tsadc); + tsadc->reg_se_cache = val; + if (tsadc->adc_waiting) + wake_up(&tsadc->reg_se_wait); + else if (!tsadc->adc_in_use) + tscadc_writel(tsadc, REG_SE, val); + spin_unlock_irqrestore(&tsadc->reg_lock, flags); } EXPORT_SYMBOL_GPL(am335x_tsc_se_set_cache); +static void am335x_tscadc_need_adc(struct ti_tscadc_dev *tsadc) +{ + DEFINE_WAIT(wait); + u32 reg; + + /* + * disable TSC steps so it does not run while the ADC is using it. If + * write 0 while it is running (it just started or was already running) + * then it completes all steps that were enabled and stops then. + */ + tscadc_writel(tsadc, REG_SE, 0); + reg = tscadc_readl(tsadc, REG_ADCFSM); + if (reg & SEQ_STATUS) { + tsadc->adc_waiting = true; + prepare_to_wait(&tsadc->reg_se_wait, &wait, + TASK_UNINTERRUPTIBLE); + spin_unlock_irq(&tsadc->reg_lock); + + schedule(); + + spin_lock_irq(&tsadc->reg_lock); + finish_wait(&tsadc->reg_se_wait, &wait); + + reg = tscadc_readl(tsadc, REG_ADCFSM); + WARN_ON(reg & SEQ_STATUS); + tsadc->adc_waiting = false; + } + tsadc->adc_in_use = true; +} + void am335x_tsc_se_set_once(struct ti_tscadc_dev *tsadc, u32 val) +{ + spin_lock_irq(&tsadc->reg_lock); + am335x_tscadc_need_adc(tsadc); + + tscadc_writel(tsadc, REG_SE, val); + spin_unlock_irq(&tsadc->reg_lock); +} +EXPORT_SYMBOL_GPL(am335x_tsc_se_set_once); + +void am335x_tsc_se_adc_done(struct ti_tscadc_dev *tsadc) { unsigned long flags; spin_lock_irqsave(&tsadc->reg_lock, flags); - tscadc_writel(tsadc, REG_SE, tsadc->reg_se_cache | val); + tsadc->adc_in_use = false; + tscadc_writel(tsadc, REG_SE, tsadc->reg_se_cache); spin_unlock_irqrestore(&tsadc->reg_lock, flags); } -EXPORT_SYMBOL_GPL(am335x_tsc_se_set_once); +EXPORT_SYMBOL_GPL(am335x_tsc_se_adc_done); void am335x_tsc_se_clr(struct ti_tscadc_dev *tsadc, u32 val) { @@ -80,7 +121,7 @@ void am335x_tsc_se_clr(struct ti_tscadc_dev *tsadc, u32 val) spin_lock_irqsave(&tsadc->reg_lock, flags); tsadc->reg_se_cache &= ~val; - am335x_tsc_se_update(tsadc); + tscadc_writel(tsadc, REG_SE, tsadc->reg_se_cache); spin_unlock_irqrestore(&tsadc->reg_lock, flags); } EXPORT_SYMBOL_GPL(am335x_tsc_se_clr); @@ -188,6 +229,8 @@ static int ti_tscadc_probe(struct platform_device *pdev) } spin_lock_init(&tscadc->reg_lock); + init_waitqueue_head(&tscadc->reg_se_wait); + pm_runtime_enable(&pdev->dev); pm_runtime_get_sync(&pdev->dev); -- cgit v1.2.3