Thread (11 messages) 11 messages, 3 authors, 2014-11-10

Re: [PATCH 3/5] mfd: ti_am335x_tscadc: Remove unwanted reg_se_cache save

From: Lee Jones <hidden>
Date: 2014-11-10 09:26:57
Also in: linux-arm-kernel, linux-devicetree, linux-iio, linux-omap, lkml

On Mon, 10 Nov 2014, Lee Jones wrote:
On Fri, 07 Nov 2014, Vignesh R wrote:
quoted
In one shot mode, sequencer automatically disables all enabled steps at
the end of each cycle. (both ADC steps and TSC steps) Hence these steps
need not be saved in reg_se_cache for clearing these steps at a later
stage.
Also, when ADC wakes up Sequencer should not be busy executing any of the
config steps except for the charge step. Previously charge step was 1 ADC
clock cycle and hence it was ignored.

Signed-off-by: Vignesh R <redacted>
---
 drivers/mfd/ti_am335x_tscadc.c       | 7 +++++--
 include/linux/mfd/ti_am335x_tscadc.h | 1 +
 2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c
index d877e777cce6..94ef8992f46b 100644
--- a/drivers/mfd/ti_am335x_tscadc.c
+++ b/drivers/mfd/ti_am335x_tscadc.c
@@ -86,8 +86,12 @@ static void am335x_tscadc_need_adc(struct ti_tscadc_dev *tsadc)
 		spin_lock_irq(&tsadc->reg_lock);
 		finish_wait(&tsadc->reg_se_wait, &wait);
 
+		/*
+		 * Sequencer should either be idle or
+		 * busy applying the charge step.
+		 */
 		reg = tscadc_readl(tsadc, REG_ADCFSM);
-		WARN_ON(reg & SEQ_STATUS);
+		WARN_ON(reg & SEQ_STATUS & (!CHARGE_STEP));
This is almost certainly not correct.

Please take another look at the logic.

I'm _assuming_ you mean (reg & SEQ_STATUS && !CHARGE_STEP).
So I just saw that CHARGE_STEP is actually the new macro below.

So you're currently ANDing these together.

xxxxxxxx xxxxxxxx xxxxxxxx xxxxxxxx  reg
00000000 00000000 00000000 00100000  #define SEQ_STATUS   BIT(5)
00000000 00000000 00000000 00010001  #define CHARGE_STEP  0x11

... which will always equate to 0.
quoted
 		tsadc->adc_waiting = false;
 	}
 	tsadc->adc_in_use = true;
@@ -96,7 +100,6 @@ static void am335x_tscadc_need_adc(struct ti_tscadc_dev *tsadc)
 void am335x_tsc_se_set_once(struct ti_tscadc_dev *tsadc, u32 val)
 {
 	spin_lock_irq(&tsadc->reg_lock);
-	tsadc->reg_se_cache |= val;
Didn't you add this line a little over 1 month ago?

Why the change of heart?
quoted
 	am335x_tscadc_need_adc(tsadc);
 
 	tscadc_writel(tsadc, REG_SE, val);
diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
index c99be5dc0f5c..fcce182e4a35 100644
--- a/include/linux/mfd/ti_am335x_tscadc.h
+++ b/include/linux/mfd/ti_am335x_tscadc.h
@@ -128,6 +128,7 @@
 
 /* Sequencer Status */
 #define SEQ_STATUS BIT(5)
+#define CHARGE_STEP		0x11
 
 #define ADC_CLK			3000000
 #define TOTAL_STEPS		16
-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help