[PATCH v2 0/5] Touchscreen performance related fixes

[PATCH v2 0/5] Touchscreen performance related fixes

[Vignesh R]

This series of patches fix TSC defects related to lag in touchscreen
performance and cursor jump at touch release. The lag was result of
udelay in TSC interrupt handler. Cursor jump due to false pen-up event.
The patches implement Advisory 1.0.31 in silicon errata of am335x-evm
to avoid false pen-up events and remove udelay. The advisory says to use
steps 1 to 4 for ADC and 5 to 16 for TSC (assuming 4 wire TSC and 4 channel
ADC). Further the X co-ordinate must be the last one to be sampled just
before charge step. The first two patches implement the required changes.

A DT parameter to configure the duration of tsc charge step. It represents
number of ADC clock cycles to wait between applying the step configuration
registers and going back to the IDLE state. The charge delay value can vary
across boards. Configuring correct value of charge delay is important to avoid
false pen-up events. Hence it is necessary to expose charge-delay value as
DT parameter. The pen-up detection happens immediately after the charge step
so this does in fact function as a hardware knob for adjusting the amount of
settling time.

After applying these changes false pen-up events have not be observed and
smooth circles can be drawn on touch screen. The performance is much better
in recognizing quick movement across the screen. No lag or cursor jump is
observed.

Change log:

v2:
 - Addressed comments by Hartmut Knaack
 - patch 2 was split into two as per Lee Jones comment

Brad Griffis (2):
  input: touchscreen: ti_am335x_tsc Interchange touchscreen and ADC
    steps
  input: touchscreen: ti_am335x_tsc: Remove udelay in interrupt handler

Vignesh R (3):
  mfd: ti_am335x_tscadc: Remove unwanted reg_se_cache save
  ARM: dts: AM335x: Make charge delay a DT parameter for tsc
  input: touchscreen: ti_am335x_tsc: Use charge delay DT parameter

 .../bindings/input/touchscreen/ti-tsc-adc.txt      |  15 +++
 arch/arm/boot/dts/am335x-evm.dts                   |   1 +
 drivers/iio/adc/ti_am335x_adc.c                    |   5 +-
 drivers/input/touchscreen/ti_am335x_tsc.c          | 106 ++++++++++-----------
 drivers/mfd/ti_am335x_tscadc.c                     |   7 +-
 include/linux/mfd/ti_am335x_tscadc.h               |   4 +-
 6 files changed, 79 insertions(+), 59 deletions(-)

-- 
1.9.1

[PATCH 1/5] input: touchscreen: ti_am335x_tsc Interchange touchscreen and ADC steps

[Vignesh R]

From: Brad Griffis <bgriffis@ti.com>

This patch makes the initial changes required to workaround TSC-false
pen-up interrupts. It is required to implement these changes in order to
remove udelay in the TSC interrupt handler and false pen-up events.
The charge step is to be executed immediately after sampling X+. Hence
TSC is made to use higher numbered steps (steps 5 to 16 for 5 co-ordinate
readouts, 4 wire TSC configuration) and ADC to use lower ones. Further
X co-ordinate readouts must be the last to be sampled, thus co-ordinates
are sampled in the order Y-Z-X.

Signed-off-by: Brad Griffis <bgriffis@ti.com>
[vigneshr@ti.com: Ported the patch from v3.12 to v3.18rc2]

Signed-off-by: Vignesh R <vigneshr@ti.com>
---
 drivers/iio/adc/ti_am335x_adc.c           |  5 ++--
 drivers/input/touchscreen/ti_am335x_tsc.c | 42 ++++++++++++++++++-------------
 2 files changed, 26 insertions(+), 21 deletions(-)

diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c
index b730864731e8..adba23246474 100644
--- a/drivers/iio/adc/ti_am335x_adc.c
+++ b/drivers/iio/adc/ti_am335x_adc.c
@@ -86,19 +86,18 @@ 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 = 0;
 
 	/*
 	 * There are 16 configurable steps and 8 analog input
 	 * lines available which are shared between Touchscreen and ADC.
 	 *
-	 * Steps backwards i.e. from 16 towards 0 are used by ADC
+	 * Steps forwards i.e. from 0 towards 16 are used by ADC
 	 * depending on number of input lines needed.
 	 * Channel would represent which analog input
 	 * needs to be given to ADC to digitalize data.
 	 */
 
-	steps = TOTAL_STEPS - adc_dev->channels;
 	if (iio_buffer_enabled(indio_dev))
 		stepconfig = STEPCONFIG_AVG_16 | STEPCONFIG_FIFO1
 					| STEPCONFIG_MODE_SWCNT;
diff --git a/drivers/input/touchscreen/ti_am335x_tsc.c b/drivers/input/touchscreen/ti_am335x_tsc.c
index 2ce649520fe0..1aeac9675fe7 100644
--- a/drivers/input/touchscreen/ti_am335x_tsc.c
+++ b/drivers/input/touchscreen/ti_am335x_tsc.c
@@ -121,7 +121,7 @@ static void titsc_step_config(struct titsc *ts_dev)
 {
 	unsigned int	config;
 	int i;
-	int end_step;
+	int end_step, first_step, tsc_steps;
 	u32 stepenable;
 
 	config = STEPCONFIG_MODE_HWSYNC |
@@ -140,9 +140,11 @@ static void titsc_step_config(struct titsc *ts_dev)
 		break;
 	}
 
-	/* 1 … coordinate_readouts is for X */
-	end_step = ts_dev->coordinate_readouts;
-	for (i = 0; i < end_step; i++) {
+	tsc_steps = ts_dev->coordinate_readouts * 2 + 2;
+	first_step = TOTAL_STEPS - tsc_steps;
+	/* Steps 16 to 16-coordinate_readouts is for X */
+	end_step = first_step + tsc_steps;
+	for (i = end_step - ts_dev->coordinate_readouts; i < end_step; i++) {
 		titsc_writel(ts_dev, REG_STEPCONFIG(i), config);
 		titsc_writel(ts_dev, REG_STEPDELAY(i), STEPCONFIG_OPENDLY);
 	}
@@ -164,9 +166,9 @@ static void titsc_step_config(struct titsc *ts_dev)
 		break;
 	}
 
-	/* coordinate_readouts … coordinate_readouts * 2 is for Y */
-	end_step = ts_dev->coordinate_readouts * 2;
-	for (i = ts_dev->coordinate_readouts; i < end_step; i++) {
+	/* 1 ... coordinate_readouts is for Y */
+	end_step = first_step + ts_dev->coordinate_readouts;
+	for (i = first_step; i < end_step; i++) {
 		titsc_writel(ts_dev, REG_STEPCONFIG(i), config);
 		titsc_writel(ts_dev, REG_STEPDELAY(i), STEPCONFIG_OPENDLY);
 	}
@@ -179,7 +181,7 @@ static void titsc_step_config(struct titsc *ts_dev)
 	titsc_writel(ts_dev, REG_CHARGECONFIG, config);
 	titsc_writel(ts_dev, REG_CHARGEDELAY, CHARGEDLY_OPENDLY);
 
-	/* coordinate_readouts * 2 … coordinate_readouts * 2 + 2 is for Z */
+	/* coordinate_readouts + 1 ... coordinate_readouts + 2 is for Z */
 	config = STEPCONFIG_MODE_HWSYNC |
 			STEPCONFIG_AVG_16 | ts_dev->bit_yp |
 			ts_dev->bit_xn | STEPCONFIG_INM_ADCREFM |
@@ -194,8 +196,11 @@ static void titsc_step_config(struct titsc *ts_dev)
 	titsc_writel(ts_dev, REG_STEPDELAY(end_step),
 			STEPCONFIG_OPENDLY);
 
-	/* The steps1 … end and bit 0 for TS_Charge */
-	stepenable = (1 << (end_step + 2)) - 1;
+	/* The steps end ... end - readouts * 2 + 2 and bit 0 for TS_Charge */
+	stepenable = 1;
+	for (i = 0; i < tsc_steps; i++)
+		stepenable |= 1 << (first_step + i + 1);
+
 	ts_dev->step_mask = stepenable;
 	am335x_tsc_se_set_cache(ts_dev->mfd_tscadc, ts_dev->step_mask);
 }
@@ -209,6 +214,7 @@ static void titsc_read_coordinates(struct titsc *ts_dev,
 	unsigned int read, diff;
 	unsigned int i, channel;
 	unsigned int creads = ts_dev->coordinate_readouts;
+	unsigned int first_step = TOTAL_STEPS - (creads * 2 + 2);
 
 	*z1 = *z2 = 0;
 	if (fifocount % (creads * 2 + 2))
@@ -226,7 +232,7 @@ static void titsc_read_coordinates(struct titsc *ts_dev,
 
 		channel = (read & 0xf0000) >> 16;
 		read &= 0xfff;
-		if (channel < creads) {
+		if (channel > first_step + creads + 2) {
 			diff = abs(read - prev_val_x);
 			if (diff < prev_diff_x) {
 				prev_diff_x = diff;
@@ -234,19 +240,19 @@ static void titsc_read_coordinates(struct titsc *ts_dev,
 			}
 			prev_val_x = read;
 
-		} else if (channel < creads * 2) {
+		} else if (channel == first_step + creads + 1) {
+			*z1 = read;
+
+		} else if (channel == first_step + creads + 2) {
+			*z2 = read;
+
+		} else if (channel > first_step) {
 			diff = abs(read - prev_val_y);
 			if (diff < prev_diff_y) {
 				prev_diff_y = diff;
 				*y = read;
 			}
 			prev_val_y = read;
-
-		} else if (channel < creads * 2 + 1) {
-			*z1 = read;
-
-		} else if (channel < creads * 2 + 2) {
-			*z2 = read;
 		}
 	}
 }
-- 
1.9.1

[PATCH 2/5] input: touchscreen: ti_am335x_tsc: Remove udelay in interrupt handler

[Vignesh R]

From: Brad Griffis <bgriffis@ti.com>

TSC interrupt handler had udelay to avoid reporting of false pen-up
interrupt to user space. This patch implements workaround suggesting in
Advisory 1.0.31 of silicon errata for am335x, thus eliminating udelay
and touchscreen lag. This also improves performance of touchscreen and
eliminates sudden jump of cursor at touch release.

IDLECONFIG and CHARGECONFIG registers are to be configured
with same values in order to eliminate false pen-up events. This
workaround may result in false pen-down to be detected, hence considerable
charge step delay needs to be added. The charge delay is set to 0xB000
(in terms of ADC clock cycles) by default.

TSC steps are disabled at the end of every sampling cycle and EOS bit is
set. Once the EOS bit is set, the TSC steps need to be re-enabled to begin
next sampling cycle.

Signed-off-by: Brad Griffis <bgriffis@ti.com>
[vigneshr@ti.com: Ported the patch from v3.12 to v3.18rc2]

Signed-off-by: Vignesh R <vigneshr@ti.com>
---
 drivers/input/touchscreen/ti_am335x_tsc.c | 56 ++++++++++++-------------------
 include/linux/mfd/ti_am335x_tscadc.h      |  3 +-
 2 files changed, 24 insertions(+), 35 deletions(-)

diff --git a/drivers/input/touchscreen/ti_am335x_tsc.c b/drivers/input/touchscreen/ti_am335x_tsc.c
index 1aeac9675fe7..483fd97c0e0c 100644
--- a/drivers/input/touchscreen/ti_am335x_tsc.c
+++ b/drivers/input/touchscreen/ti_am335x_tsc.c
@@ -173,11 +173,9 @@ static void titsc_step_config(struct titsc *ts_dev)
 		titsc_writel(ts_dev, REG_STEPDELAY(i), STEPCONFIG_OPENDLY);
 	}
 
-	/* Charge step configuration */
-	config = ts_dev->bit_xp | ts_dev->bit_yn |
-			STEPCHARGE_RFP_XPUL | STEPCHARGE_RFM_XNUR |
-			STEPCHARGE_INM_AN1 | STEPCHARGE_INP(ts_dev->inp_yp);
+	/* Make CHARGECONFIG same as IDLECONFIG */
 
+	config = titsc_readl(ts_dev, REG_IDLECONFIG);
 	titsc_writel(ts_dev, REG_CHARGECONFIG, config);
 	titsc_writel(ts_dev, REG_CHARGEDELAY, CHARGEDLY_OPENDLY);
 
@@ -264,9 +262,26 @@ static irqreturn_t titsc_irq(int irq, void *dev)
 	unsigned int status, irqclr = 0;
 	unsigned int x = 0, y = 0;
 	unsigned int z1, z2, z;
-	unsigned int fsm;
 
-	status = titsc_readl(ts_dev, REG_IRQSTATUS);
+	status = titsc_readl(ts_dev, REG_RAWIRQSTATUS);
+	if (status & IRQENB_HW_PEN) {
+		ts_dev->pen_down = true;
+		titsc_writel(ts_dev, REG_IRQWAKEUP, 0x00);
+		titsc_writel(ts_dev, REG_IRQCLR, IRQENB_HW_PEN);
+		irqclr |= IRQENB_HW_PEN;
+	}
+
+	if (status & IRQENB_PENUP) {
+		ts_dev->pen_down = false;
+		input_report_key(input_dev, BTN_TOUCH, 0);
+		input_report_abs(input_dev, ABS_PRESSURE, 0);
+		input_sync(input_dev);
+		irqclr |= IRQENB_PENUP;
+	}
+
+	if (status & IRQENB_EOS)
+		irqclr |= IRQENB_EOS;
+
 	/*
 	 * ADC and touchscreen share the IRQ line.
 	 * FIFO1 interrupts are used by ADC. Handle FIFO0 IRQs here only
@@ -297,34 +312,6 @@ static irqreturn_t titsc_irq(int irq, void *dev)
 		}
 		irqclr |= IRQENB_FIFO0THRES;
 	}
-
-	/*
-	 * Time for sequencer to settle, to read
-	 * correct state of the sequencer.
-	 */
-	udelay(SEQ_SETTLE);
-
-	status = titsc_readl(ts_dev, REG_RAWIRQSTATUS);
-	if (status & IRQENB_PENUP) {
-		/* Pen up event */
-		fsm = titsc_readl(ts_dev, REG_ADCFSM);
-		if (fsm == ADCFSM_STEPID) {
-			ts_dev->pen_down = false;
-			input_report_key(input_dev, BTN_TOUCH, 0);
-			input_report_abs(input_dev, ABS_PRESSURE, 0);
-			input_sync(input_dev);
-		} else {
-			ts_dev->pen_down = true;
-		}
-		irqclr |= IRQENB_PENUP;
-	}
-
-	if (status & IRQENB_HW_PEN) {
-
-		titsc_writel(ts_dev, REG_IRQWAKEUP, 0x00);
-		titsc_writel(ts_dev, REG_IRQCLR, IRQENB_HW_PEN);
-	}
-
 	if (irqclr) {
 		titsc_writel(ts_dev, REG_IRQSTATUS, irqclr);
 		am335x_tsc_se_set_cache(ts_dev->mfd_tscadc, ts_dev->step_mask);
@@ -417,6 +404,7 @@ static int titsc_probe(struct platform_device *pdev)
 	}
 
 	titsc_writel(ts_dev, REG_IRQENABLE, IRQENB_FIFO0THRES);
+	titsc_writel(ts_dev, REG_IRQENABLE, IRQENB_EOS);
 	err = titsc_config_wires(ts_dev);
 	if (err) {
 		dev_err(&pdev->dev, "wrong i/p wire configuration\n");
diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
index e2e70053470e..c99be5dc0f5c 100644
--- a/include/linux/mfd/ti_am335x_tscadc.h
+++ b/include/linux/mfd/ti_am335x_tscadc.h
@@ -52,6 +52,7 @@
 
 /* IRQ enable */
 #define IRQENB_HW_PEN		BIT(0)
+#define IRQENB_EOS		BIT(1)
 #define IRQENB_FIFO0THRES	BIT(2)
 #define IRQENB_FIFO0OVRRUN	BIT(3)
 #define IRQENB_FIFO0UNDRFLW	BIT(4)
@@ -107,7 +108,7 @@
 /* Charge delay */
 #define CHARGEDLY_OPEN_MASK	(0x3FFFF << 0)
 #define CHARGEDLY_OPEN(val)	((val) << 0)
-#define CHARGEDLY_OPENDLY	CHARGEDLY_OPEN(1)
+#define CHARGEDLY_OPENDLY	CHARGEDLY_OPEN(0xB000)
 
 /* Control register */
 #define CNTRLREG_TSCSSENB	BIT(0)
-- 
1.9.1

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

[Vignesh R]

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 <vigneshr-l0cyMroinI0@public.gmane.org>
---
 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));
 		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;
 	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
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 4/5] ARM: dts: AM335x: Make charge delay a DT parameter for tsc

[Vignesh R]

The charge delay value is by default 0xB000. But it can be set to lower
values on some boards as long as false pen-ups are avoided. Lowering the
value increases the sampling rate (though current sampling rate is
sufficient for tsc operation). Hence charge delay has been made a DT
parameter.

Signed-off-by: Vignesh R <vigneshr@ti.com>
---
 .../devicetree/bindings/input/touchscreen/ti-tsc-adc.txt  | 15 +++++++++++++++
 arch/arm/boot/dts/am335x-evm.dts                          |  1 +
 2 files changed, 16 insertions(+)

diff --git a/Documentation/devicetree/bindings/input/touchscreen/ti-tsc-adc.txt b/Documentation/devicetree/bindings/input/touchscreen/ti-tsc-adc.txt
index 878549ba814d..b87574bae009 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/ti-tsc-adc.txt
+++ b/Documentation/devicetree/bindings/input/touchscreen/ti-tsc-adc.txt
@@ -28,6 +28,20 @@ Required properties:
 	ti,adc-channels: List of analog inputs available for ADC.
 			 AIN0 = 0, AIN1 = 1 and so on till AIN7 = 7.
 
+Optional properties:
+- child "tsc"
+	ti,charge-delay: Length of touch screen charge delay step in terms of
+			 ADC clock cycles. Charge delay value should be large
+			 in order to avoid false pen-up events. This value
+			 affects the overall sampling speed, hence need to be
+			 kept as low as possible, while avoiding false pen-up
+			 event. Start from a lower value, say 0x400, and
+			 increase value until false pen-up events are avoided.
+			 The pen-up detection happens immediately after the
+			 charge step, so this does in fact function as a
+			 hardware knob for adjusting the amount of "settling
+			 time".
+
 Example:
 	tscadc: tscadc@44e0d000 {
 		compatible = "ti,am3359-tscadc";
@@ -36,6 +50,7 @@ Example:
 			ti,x-plate-resistance = <200>;
 			ti,coordiante-readouts = <5>;
 			ti,wire-config = <0x00 0x11 0x22 0x33>;
+			ti,charge-delay = <0xB000>;
 		};
 
 		adc {
diff --git a/arch/arm/boot/dts/am335x-evm.dts b/arch/arm/boot/dts/am335x-evm.dts
index e2156a583de7..80be0462298b 100644
--- a/arch/arm/boot/dts/am335x-evm.dts
+++ b/arch/arm/boot/dts/am335x-evm.dts
@@ -641,6 +641,7 @@
 		ti,x-plate-resistance = <200>;
 		ti,coordinate-readouts = <5>;
 		ti,wire-config = <0x00 0x11 0x22 0x33>;
+		ti,charge-delay = <0xB000>;
 	};
 
 	adc {
-- 
1.9.1

[PATCH 5/5] input: touchscreen: ti_am335x_tsc: Use charge delay DT parameter

[Vignesh R]

This patch reads charge delay from tsc DT node and writes to
REG_CHARGEDELAY register. If the charge delay is not specified in DT
then default value of 0xB000(CHARGEDLY_OPENDLY) is used.

Signed-off-by: Vignesh R <vigneshr@ti.com>
---
 drivers/input/touchscreen/ti_am335x_tsc.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/input/touchscreen/ti_am335x_tsc.c b/drivers/input/touchscreen/ti_am335x_tsc.c
index 483fd97c0e0c..20ce76b1b6e7 100644
--- a/drivers/input/touchscreen/ti_am335x_tsc.c
+++ b/drivers/input/touchscreen/ti_am335x_tsc.c
@@ -52,6 +52,7 @@ struct titsc {
 	u32			bit_xp, bit_xn, bit_yp, bit_yn;
 	u32			inp_xp, inp_xn, inp_yp, inp_yn;
 	u32			step_mask;
+	u32			charge_delay;
 };
 
 static unsigned int titsc_readl(struct titsc *ts, unsigned int reg)
@@ -177,7 +178,7 @@ static void titsc_step_config(struct titsc *ts_dev)
 
 	config = titsc_readl(ts_dev, REG_IDLECONFIG);
 	titsc_writel(ts_dev, REG_CHARGECONFIG, config);
-	titsc_writel(ts_dev, REG_CHARGEDELAY, CHARGEDLY_OPENDLY);
+	titsc_writel(ts_dev, REG_CHARGEDELAY, ts_dev->charge_delay);
 
 	/* coordinate_readouts + 1 ... coordinate_readouts + 2 is for Z */
 	config = STEPCONFIG_MODE_HWSYNC |
@@ -361,6 +362,11 @@ static int titsc_parse_dt(struct platform_device *pdev,
 	if (err < 0)
 		return err;
 
+	err = of_property_read_u32(node, "ti,charge-delay",
+				   &ts_dev->charge_delay);
+	if (err < 0)
+		ts_dev->charge_delay = CHARGEDLY_OPENDLY;
+
 	return of_property_read_u32_array(node, "ti,wire-config",
 			ts_dev->config_inp, ARRAY_SIZE(ts_dev->config_inp));
 }
-- 
1.9.1

Re: [PATCH 1/5] input: touchscreen: ti_am335x_tsc Interchange touchscreen and ADC steps

[Jonathan Cameron]

On 07/11/14 05:49, Vignesh R wrote:
> From: Brad Griffis <bgriffis-l0cyMroinI0@public.gmane.org>
> 
> This patch makes the initial changes required to workaround TSC-false
> pen-up interrupts. It is required to implement these changes in order to
> remove udelay in the TSC interrupt handler and false pen-up events.
> The charge step is to be executed immediately after sampling X+. Hence
> TSC is made to use higher numbered steps (steps 5 to 16 for 5 co-ordinate
> readouts, 4 wire TSC configuration) and ADC to use lower ones. Further
> X co-ordinate readouts must be the last to be sampled, thus co-ordinates
> are sampled in the order Y-Z-X.
> 
> Signed-off-by: Brad Griffis <bgriffis-l0cyMroinI0@public.gmane.org>
> [vigneshr-l0cyMroinI0@public.gmane.org: Ported the patch from v3.12 to v3.18rc2]
> 
> Signed-off-by: Vignesh R <vigneshr-l0cyMroinI0@public.gmane.org>
For the tiny bit in IIO
Acked-by: Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

All yours Lee/Dmitry.

Jonathan
> ---
>  drivers/iio/adc/ti_am335x_adc.c           |  5 ++--
>  drivers/input/touchscreen/ti_am335x_tsc.c | 42 ++++++++++++++++++-------------
>  2 files changed, 26 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c
> index b730864731e8..adba23246474 100644
> --- a/drivers/iio/adc/ti_am335x_adc.c
> +++ b/drivers/iio/adc/ti_am335x_adc.c
> @@ -86,19 +86,18 @@ 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 = 0;
>  
>  	/*
>  	 * There are 16 configurable steps and 8 analog input
>  	 * lines available which are shared between Touchscreen and ADC.
>  	 *
> -	 * Steps backwards i.e. from 16 towards 0 are used by ADC
> +	 * Steps forwards i.e. from 0 towards 16 are used by ADC
>  	 * depending on number of input lines needed.
>  	 * Channel would represent which analog input
>  	 * needs to be given to ADC to digitalize data.
>  	 */
>  
> -	steps = TOTAL_STEPS - adc_dev->channels;
>  	if (iio_buffer_enabled(indio_dev))
>  		stepconfig = STEPCONFIG_AVG_16 | STEPCONFIG_FIFO1
>  					| STEPCONFIG_MODE_SWCNT;
> diff --git a/drivers/input/touchscreen/ti_am335x_tsc.c b/drivers/input/touchscreen/ti_am335x_tsc.c
> index 2ce649520fe0..1aeac9675fe7 100644
> --- a/drivers/input/touchscreen/ti_am335x_tsc.c
> +++ b/drivers/input/touchscreen/ti_am335x_tsc.c
> @@ -121,7 +121,7 @@ static void titsc_step_config(struct titsc *ts_dev)
>  {
>  	unsigned int	config;
>  	int i;
> -	int end_step;
> +	int end_step, first_step, tsc_steps;
>  	u32 stepenable;
>  
>  	config = STEPCONFIG_MODE_HWSYNC |
> @@ -140,9 +140,11 @@ static void titsc_step_config(struct titsc *ts_dev)
>  		break;
>  	}
>  
> -	/* 1 … coordinate_readouts is for X */
> -	end_step = ts_dev->coordinate_readouts;
> -	for (i = 0; i < end_step; i++) {
> +	tsc_steps = ts_dev->coordinate_readouts * 2 + 2;
> +	first_step = TOTAL_STEPS - tsc_steps;
> +	/* Steps 16 to 16-coordinate_readouts is for X */
> +	end_step = first_step + tsc_steps;
> +	for (i = end_step - ts_dev->coordinate_readouts; i < end_step; i++) {
>  		titsc_writel(ts_dev, REG_STEPCONFIG(i), config);
>  		titsc_writel(ts_dev, REG_STEPDELAY(i), STEPCONFIG_OPENDLY);
>  	}
> @@ -164,9 +166,9 @@ static void titsc_step_config(struct titsc *ts_dev)
>  		break;
>  	}
>  
> -	/* coordinate_readouts … coordinate_readouts * 2 is for Y */
> -	end_step = ts_dev->coordinate_readouts * 2;
> -	for (i = ts_dev->coordinate_readouts; i < end_step; i++) {
> +	/* 1 ... coordinate_readouts is for Y */
> +	end_step = first_step + ts_dev->coordinate_readouts;
> +	for (i = first_step; i < end_step; i++) {
>  		titsc_writel(ts_dev, REG_STEPCONFIG(i), config);
>  		titsc_writel(ts_dev, REG_STEPDELAY(i), STEPCONFIG_OPENDLY);
>  	}
> @@ -179,7 +181,7 @@ static void titsc_step_config(struct titsc *ts_dev)
>  	titsc_writel(ts_dev, REG_CHARGECONFIG, config);
>  	titsc_writel(ts_dev, REG_CHARGEDELAY, CHARGEDLY_OPENDLY);
>  
> -	/* coordinate_readouts * 2 … coordinate_readouts * 2 + 2 is for Z */
> +	/* coordinate_readouts + 1 ... coordinate_readouts + 2 is for Z */
>  	config = STEPCONFIG_MODE_HWSYNC |
>  			STEPCONFIG_AVG_16 | ts_dev->bit_yp |
>  			ts_dev->bit_xn | STEPCONFIG_INM_ADCREFM |
> @@ -194,8 +196,11 @@ static void titsc_step_config(struct titsc *ts_dev)
>  	titsc_writel(ts_dev, REG_STEPDELAY(end_step),
>  			STEPCONFIG_OPENDLY);
>  
> -	/* The steps1 … end and bit 0 for TS_Charge */
> -	stepenable = (1 << (end_step + 2)) - 1;
> +	/* The steps end ... end - readouts * 2 + 2 and bit 0 for TS_Charge */
> +	stepenable = 1;
> +	for (i = 0; i < tsc_steps; i++)
> +		stepenable |= 1 << (first_step + i + 1);
> +
>  	ts_dev->step_mask = stepenable;
>  	am335x_tsc_se_set_cache(ts_dev->mfd_tscadc, ts_dev->step_mask);
>  }
> @@ -209,6 +214,7 @@ static void titsc_read_coordinates(struct titsc *ts_dev,
>  	unsigned int read, diff;
>  	unsigned int i, channel;
>  	unsigned int creads = ts_dev->coordinate_readouts;
> +	unsigned int first_step = TOTAL_STEPS - (creads * 2 + 2);
>  
>  	*z1 = *z2 = 0;
>  	if (fifocount % (creads * 2 + 2))
> @@ -226,7 +232,7 @@ static void titsc_read_coordinates(struct titsc *ts_dev,
>  
>  		channel = (read & 0xf0000) >> 16;
>  		read &= 0xfff;
> -		if (channel < creads) {
> +		if (channel > first_step + creads + 2) {
>  			diff = abs(read - prev_val_x);
>  			if (diff < prev_diff_x) {
>  				prev_diff_x = diff;
> @@ -234,19 +240,19 @@ static void titsc_read_coordinates(struct titsc *ts_dev,
>  			}
>  			prev_val_x = read;
>  
> -		} else if (channel < creads * 2) {
> +		} else if (channel == first_step + creads + 1) {
> +			*z1 = read;
> +
> +		} else if (channel == first_step + creads + 2) {
> +			*z2 = read;
> +
> +		} else if (channel > first_step) {
>  			diff = abs(read - prev_val_y);
>  			if (diff < prev_diff_y) {
>  				prev_diff_y = diff;
>  				*y = read;
>  			}
>  			prev_val_y = read;
> -
> -		} else if (channel < creads * 2 + 1) {
> -			*z1 = read;
> -
> -		} else if (channel < creads * 2 + 2) {
> -			*z2 = read;
>  		}
>  	}
>  }
> 

Re: [PATCH 2/5] input: touchscreen: ti_am335x_tsc: Remove udelay in interrupt handler

[Lee Jones]

On Fri, 07 Nov 2014, Vignesh R wrote:

> From: Brad Griffis <bgriffis@ti.com>
> 
> TSC interrupt handler had udelay to avoid reporting of false pen-up
> interrupt to user space. This patch implements workaround suggesting in
> Advisory 1.0.31 of silicon errata for am335x, thus eliminating udelay
> and touchscreen lag. This also improves performance of touchscreen and
> eliminates sudden jump of cursor at touch release.
> 
> IDLECONFIG and CHARGECONFIG registers are to be configured
> with same values in order to eliminate false pen-up events. This
> workaround may result in false pen-down to be detected, hence considerable
> charge step delay needs to be added. The charge delay is set to 0xB000
> (in terms of ADC clock cycles) by default.
> 
> TSC steps are disabled at the end of every sampling cycle and EOS bit is
> set. Once the EOS bit is set, the TSC steps need to be re-enabled to begin
> next sampling cycle.
> 
> Signed-off-by: Brad Griffis <bgriffis@ti.com>
> [vigneshr@ti.com: Ported the patch from v3.12 to v3.18rc2]
> 
> Signed-off-by: Vignesh R <vigneshr@ti.com>
> ---
>  drivers/input/touchscreen/ti_am335x_tsc.c | 56 ++++++++++++-------------------
>  include/linux/mfd/ti_am335x_tscadc.h      |  3 +-

Acked-by: Lee Jones <lee.jones@linaro.org>

>  2 files changed, 24 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/input/touchscreen/ti_am335x_tsc.c b/drivers/input/touchscreen/ti_am335x_tsc.c
> index 1aeac9675fe7..483fd97c0e0c 100644
> --- a/drivers/input/touchscreen/ti_am335x_tsc.c
> +++ b/drivers/input/touchscreen/ti_am335x_tsc.c
> @@ -173,11 +173,9 @@ static void titsc_step_config(struct titsc *ts_dev)
>  		titsc_writel(ts_dev, REG_STEPDELAY(i), STEPCONFIG_OPENDLY);
>  	}
>  
> -	/* Charge step configuration */
> -	config = ts_dev->bit_xp | ts_dev->bit_yn |
> -			STEPCHARGE_RFP_XPUL | STEPCHARGE_RFM_XNUR |
> -			STEPCHARGE_INM_AN1 | STEPCHARGE_INP(ts_dev->inp_yp);
> +	/* Make CHARGECONFIG same as IDLECONFIG */
>  
> +	config = titsc_readl(ts_dev, REG_IDLECONFIG);
>  	titsc_writel(ts_dev, REG_CHARGECONFIG, config);
>  	titsc_writel(ts_dev, REG_CHARGEDELAY, CHARGEDLY_OPENDLY);
>  
> @@ -264,9 +262,26 @@ static irqreturn_t titsc_irq(int irq, void *dev)
>  	unsigned int status, irqclr = 0;
>  	unsigned int x = 0, y = 0;
>  	unsigned int z1, z2, z;
> -	unsigned int fsm;
>  
> -	status = titsc_readl(ts_dev, REG_IRQSTATUS);
> +	status = titsc_readl(ts_dev, REG_RAWIRQSTATUS);
> +	if (status & IRQENB_HW_PEN) {
> +		ts_dev->pen_down = true;
> +		titsc_writel(ts_dev, REG_IRQWAKEUP, 0x00);
> +		titsc_writel(ts_dev, REG_IRQCLR, IRQENB_HW_PEN);
> +		irqclr |= IRQENB_HW_PEN;
> +	}
> +
> +	if (status & IRQENB_PENUP) {
> +		ts_dev->pen_down = false;
> +		input_report_key(input_dev, BTN_TOUCH, 0);
> +		input_report_abs(input_dev, ABS_PRESSURE, 0);
> +		input_sync(input_dev);
> +		irqclr |= IRQENB_PENUP;
> +	}
> +
> +	if (status & IRQENB_EOS)
> +		irqclr |= IRQENB_EOS;
> +
>  	/*
>  	 * ADC and touchscreen share the IRQ line.
>  	 * FIFO1 interrupts are used by ADC. Handle FIFO0 IRQs here only
> @@ -297,34 +312,6 @@ static irqreturn_t titsc_irq(int irq, void *dev)
>  		}
>  		irqclr |= IRQENB_FIFO0THRES;
>  	}
> -
> -	/*
> -	 * Time for sequencer to settle, to read
> -	 * correct state of the sequencer.
> -	 */
> -	udelay(SEQ_SETTLE);
> -
> -	status = titsc_readl(ts_dev, REG_RAWIRQSTATUS);
> -	if (status & IRQENB_PENUP) {
> -		/* Pen up event */
> -		fsm = titsc_readl(ts_dev, REG_ADCFSM);
> -		if (fsm == ADCFSM_STEPID) {
> -			ts_dev->pen_down = false;
> -			input_report_key(input_dev, BTN_TOUCH, 0);
> -			input_report_abs(input_dev, ABS_PRESSURE, 0);
> -			input_sync(input_dev);
> -		} else {
> -			ts_dev->pen_down = true;
> -		}
> -		irqclr |= IRQENB_PENUP;
> -	}
> -
> -	if (status & IRQENB_HW_PEN) {
> -
> -		titsc_writel(ts_dev, REG_IRQWAKEUP, 0x00);
> -		titsc_writel(ts_dev, REG_IRQCLR, IRQENB_HW_PEN);
> -	}
> -
>  	if (irqclr) {
>  		titsc_writel(ts_dev, REG_IRQSTATUS, irqclr);
>  		am335x_tsc_se_set_cache(ts_dev->mfd_tscadc, ts_dev->step_mask);
> @@ -417,6 +404,7 @@ static int titsc_probe(struct platform_device *pdev)
>  	}
>  
>  	titsc_writel(ts_dev, REG_IRQENABLE, IRQENB_FIFO0THRES);
> +	titsc_writel(ts_dev, REG_IRQENABLE, IRQENB_EOS);
>  	err = titsc_config_wires(ts_dev);
>  	if (err) {
>  		dev_err(&pdev->dev, "wrong i/p wire configuration\n");
> diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
> index e2e70053470e..c99be5dc0f5c 100644
> --- a/include/linux/mfd/ti_am335x_tscadc.h
> +++ b/include/linux/mfd/ti_am335x_tscadc.h
> @@ -52,6 +52,7 @@
>  
>  /* IRQ enable */
>  #define IRQENB_HW_PEN		BIT(0)
> +#define IRQENB_EOS		BIT(1)
>  #define IRQENB_FIFO0THRES	BIT(2)
>  #define IRQENB_FIFO0OVRRUN	BIT(3)
>  #define IRQENB_FIFO0UNDRFLW	BIT(4)
> @@ -107,7 +108,7 @@
>  /* Charge delay */
>  #define CHARGEDLY_OPEN_MASK	(0x3FFFF << 0)
>  #define CHARGEDLY_OPEN(val)	((val) << 0)
> -#define CHARGEDLY_OPENDLY	CHARGEDLY_OPEN(1)
> +#define CHARGEDLY_OPENDLY	CHARGEDLY_OPEN(0xB000)
>  
>  /* Control register */
>  #define CNTRLREG_TSCSSENB	BIT(0)

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

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

[Lee Jones]

On Fri, 07 Nov 2014, Vignesh R wrote:

> 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 <vigneshr@ti.com>
> ---
>  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).

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

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

[Lee Jones]

On Mon, 10 Nov 2014, Lee Jones wrote:

> On Fri, 07 Nov 2014, Vignesh R wrote:
> 
> > 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 <vigneshr-l0cyMroinI0@public.gmane.org>
> > ---
> >  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.

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

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

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

[Vignesh R]

On Monday 10 November 2014 02:56 PM, Lee Jones wrote:
> On Mon, 10 Nov 2014, Lee Jones wrote:
> 
>> On Fri, 07 Nov 2014, Vignesh R wrote:
>>
>>> 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 <vigneshr@ti.com>
>>> ---
>>>  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.
> 

Oops.. Sorry.. I meant ((reg & SEQ_STATUS) && !(reg & CHARGE_STEP)).
I will fix this.


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

Previously, TSC did not reliably re-enable its steps as the TSC irq
handler received false pen up events. Hence, in order to use TSC after
ADC operation, it was necessary to save and re-enable TSC steps
(basically, to keep TSC steps enabled always).
The change was more of a workaround to overcome limitation of TSC irq
handler. With this series of patches, TSC irq handler is very reliable
and the workaround is no longer required.

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