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:
v3:
- Replace delta filtering logic in TSC driver with median filtering
as suggested by Richard.
- Addressed Lee Jones comments.
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 (4):
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
input: touchscreen: ti_am335x_tsc: Replace delta filtering with median
filtering
.../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 | 186 +++++++++++----------
drivers/mfd/ti_am335x_tscadc.c | 7 +-
include/linux/mfd/ti_am335x_tscadc.h | 4 +-
6 files changed, 127 insertions(+), 91 deletions(-)
--
1.9.1
From: Brad Griffis <[email protected]>
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 <[email protected]>
[[email protected]: Ported the patch from v3.12 to v3.18rc2]
Signed-off-by: Vignesh R <[email protected]>
Acked-by: Jonathan Cameron <[email protected]>
---
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
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 <[email protected]>
---
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..110038859a8d 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) && !(reg & 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 3f4e994ace2b..1fd50dcfe47c 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
The charge delay value is by default 0x400. 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). In some boards, the value has to be
increased to avoid false pen-up events. Hence, charge delay has been
made a DT parameter.
Signed-off-by: Vignesh R <[email protected]>
---
v4:
- Set charge delay to 0x400 as default. Most devices function normally
at this value
.../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..6df5028a4ad3 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 = <0x400>;
};
adc {
diff --git a/arch/arm/boot/dts/am335x-evm.dts b/arch/arm/boot/dts/am335x-evm.dts
index e2156a583de7..ce12f6ef1e28 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 = <0x400>;
};
adc {
--
1.9.1
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 0x400(CHARGEDLY_OPENDLY) is used.
Signed-off-by: Vignesh R <[email protected]>
---
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 b94719fc8849..e64055a8cd48 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 |
@@ -366,6 +367,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
Previously, delta filtering was applied TSC co-ordinate readouts before
reporting a single value to user space. This patch replaces delta filtering
with median filtering. Median filtering sorts co-ordinate readouts, drops min
and max values, and reports the average of remaining values. This method is
more sensible than delta filering. Median filtering is applied only if number
of readouts is greater than 3 else just average of co-ordinate readouts is
reported.
Signed-off-by: Vignesh R <[email protected]>
---
drivers/input/touchscreen/ti_am335x_tsc.c | 91 +++++++++++++++++--------------
1 file changed, 51 insertions(+), 40 deletions(-)
diff --git a/drivers/input/touchscreen/ti_am335x_tsc.c b/drivers/input/touchscreen/ti_am335x_tsc.c
index e64055a8cd48..b84493fc8a78 100644
--- a/drivers/input/touchscreen/ti_am335x_tsc.c
+++ b/drivers/input/touchscreen/ti_am335x_tsc.c
@@ -26,6 +26,7 @@
#include <linux/delay.h>
#include <linux/of.h>
#include <linux/of_device.h>
+#include <linux/sort.h>
#include <linux/mfd/ti_am335x_tscadc.h>
@@ -204,56 +205,61 @@ static void titsc_step_config(struct titsc *ts_dev)
am335x_tsc_se_set_cache(ts_dev->mfd_tscadc, ts_dev->step_mask);
}
+static int titsc_cmp_coord(const void *a, const void *b)
+{
+ return *(int *)a - *(int *)b;
+}
+
static void titsc_read_coordinates(struct titsc *ts_dev,
u32 *x, u32 *y, u32 *z1, u32 *z2)
{
- unsigned int fifocount = titsc_readl(ts_dev, REG_FIFO0CNT);
- unsigned int prev_val_x = ~0, prev_val_y = ~0;
- unsigned int prev_diff_x = ~0, prev_diff_y = ~0;
- unsigned int read, diff;
- unsigned int i, channel;
+ unsigned int yvals[7], xvals[7];
+ unsigned int i, xsum = 0, ysum = 0;
unsigned int creads = ts_dev->coordinate_readouts;
- unsigned int first_step = TOTAL_STEPS - (creads * 2 + 2);
- *z1 = *z2 = 0;
- if (fifocount % (creads * 2 + 2))
- fifocount -= fifocount % (creads * 2 + 2);
- /*
- * Delta filter is used to remove large variations in sampled
- * values from ADC. The filter tries to predict where the next
- * coordinate could be. This is done by taking a previous
- * coordinate and subtracting it form current one. Further the
- * algorithm compares the difference with that of a present value,
- * if true the value is reported to the sub system.
- */
- for (i = 0; i < fifocount; i++) {
- read = titsc_readl(ts_dev, REG_FIFO0);
-
- channel = (read & 0xf0000) >> 16;
- read &= 0xfff;
- if (channel > first_step + creads + 2) {
- diff = abs(read - prev_val_x);
- if (diff < prev_diff_x) {
- prev_diff_x = diff;
- *x = read;
- }
- prev_val_x = read;
+ for (i = 0; i < creads; i++) {
+ yvals[i] = titsc_readl(ts_dev, REG_FIFO0);
+ yvals[i] &= 0xfff;
+ }
- } else if (channel == first_step + creads + 1) {
- *z1 = read;
+ *z1 = titsc_readl(ts_dev, REG_FIFO0);
+ *z1 &= 0xfff;
+ *z2 = titsc_readl(ts_dev, REG_FIFO0);
+ *z2 &= 0xfff;
- } else if (channel == first_step + creads + 2) {
- *z2 = read;
+ for (i = 0; i < creads; i++) {
+ xvals[i] = titsc_readl(ts_dev, REG_FIFO0);
+ xvals[i] &= 0xfff;
+ }
- } 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;
+ /*
+ * If co-ordinates readouts is less than 4 then
+ * report the average. In case of 4 or more
+ * readouts, sort the co-ordinate samples, drop
+ * min and max values and report the average of
+ * remaining values.
+ */
+ if (creads <= 3) {
+ for (i = 0; i < creads; i++) {
+ ysum += yvals[i];
+ xsum += xvals[i];
}
+ ysum /= creads;
+ xsum /= creads;
+ } else {
+ sort(yvals, creads, sizeof(unsigned int),
+ titsc_cmp_coord, NULL);
+ sort(xvals, creads, sizeof(unsigned int),
+ titsc_cmp_coord, NULL);
+ for (i = 1; i < creads - 1; i++) {
+ ysum += yvals[i];
+ xsum += xvals[i];
+ }
+ ysum /= creads - 2;
+ xsum /= creads - 2;
}
+ *y = ysum;
+ *x = xsum;
}
static irqreturn_t titsc_irq(int irq, void *dev)
@@ -367,6 +373,11 @@ static int titsc_parse_dt(struct platform_device *pdev,
if (err < 0)
return err;
+ if (ts_dev->coordinate_readouts <= 0) {
+ dev_warn(&pdev->dev, "invalid co-ordinate readouts, resetting it to 5\n");
+ ts_dev->coordinate_readouts = 5;
+ }
+
err = of_property_read_u32(node, "ti,charge-delay",
&ts_dev->charge_delay);
if (err < 0)
--
1.9.1
From: Brad Griffis <[email protected]>
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 <[email protected]>
[[email protected]: Ported the patch from v3.12 to v3.18rc2]
Signed-off-by: Vignesh R <[email protected]>
---
v4:
- check for PEN_IRQ bit in ADCFSM to avoid false-pen when ADC
and TSC are used together.
- Set charge delay to 0x400 as default. Most devices function
normally at this value.
drivers/input/touchscreen/ti_am335x_tsc.c | 63 ++++++++++++++-----------------
include/linux/mfd/ti_am335x_tscadc.h | 3 +-
2 files changed, 30 insertions(+), 36 deletions(-)
diff --git a/drivers/input/touchscreen/ti_am335x_tsc.c b/drivers/input/touchscreen/ti_am335x_tsc.c
index 1aeac9675fe7..b94719fc8849 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);
@@ -261,12 +259,34 @@ static irqreturn_t titsc_irq(int irq, void *dev)
{
struct titsc *ts_dev = dev;
struct input_dev *input_dev = ts_dev->input;
- unsigned int status, irqclr = 0;
+ unsigned int fsm, 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) {
+ 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_EOS)
+ irqclr |= IRQENB_EOS;
+
/*
* ADC and touchscreen share the IRQ line.
* FIFO1 interrupts are used by ADC. Handle FIFO0 IRQs here only
@@ -297,34 +317,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 +409,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..3f4e994ace2b 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(0x400)
/* Control register */
#define CNTRLREG_TSCSSENB BIT(0)
--
1.9.1
* Vignesh R <[email protected]> [141113 21:42]:
> The charge delay value is by default 0x400. 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). In some boards, the value has to be
> increased to avoid false pen-up events. Hence, charge delay has been
> made a DT parameter.
>
> Signed-off-by: Vignesh R <[email protected]>
As long as the input and DT people are OK with this, ack for
merging the .dts changes along with the input patches:
Acked-by: Tony Lindgren <[email protected]>
> ---
>
> v4:
> - Set charge delay to 0x400 as default. Most devices function normally
> at this value
>
> .../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..6df5028a4ad3 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 = <0x400>;
> };
>
> adc {
> diff --git a/arch/arm/boot/dts/am335x-evm.dts b/arch/arm/boot/dts/am335x-evm.dts
> index e2156a583de7..ce12f6ef1e28 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 = <0x400>;
> };
>
> adc {
> --
> 1.9.1
>
On Fri, Nov 14, 2014 at 10:37:27AM +0530, Vignesh R wrote:
> From: Brad Griffis <[email protected]>
>
> 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.
I back ported this series onto v3.15.1 in order to try this out on a
custom, beaglebone-like board. With this series, the touch is really
broken. (I had fixed the pen up problem in a totally different way for
a customer, and so I wanted to try out your solution.)
I will try to port the board code to a more recent kernel to try your
series again. With which kernel version did you test your patches?
And which board?
Thanks,
Richard
On Sunday 16 November 2014 12:57 AM, Richard Cochran wrote:
> On Fri, Nov 14, 2014 at 10:37:27AM +0530, Vignesh R wrote:
>> From: Brad Griffis <[email protected]>
>>
>> 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.
>
> I back ported this series onto v3.15.1 in order to try this out on a
> custom, beaglebone-like board. With this series, the touch is really
> broken. (I had fixed the pen up problem in a totally different way for
> a customer, and so I wanted to try out your solution.)
>
> I will try to port the board code to a more recent kernel to try your
> series again. With which kernel version did you test your patches?
>
> And which board?
>
Thanks for testing these patches.
My patches are based on v3.18rc2. I tested my patches on am335x-evm
using tslib.
If you are encountering false pen-ups, charge-delay parameter needs to
be tuned to your board. In some custom setups charge-delay of 0xB000
provided better performance.
If you are using ts_test (from tslib) for testing try
# ts_test > /dev/null
for better performance.
Please explain "touch is broken"? What is the behaviour of TSC?
Which ADC channels are being used for TSC?
Regards
Vignesh
> Thanks,
> Richard
>
>
On Mon, Nov 17, 2014 at 09:57:05AM +0530, Vignesh R wrote:
> My patches are based on v3.18rc2. I tested my patches on am335x-evm
> using tslib.
No beaglebone + cape testing?
> Please explain "touch is broken"? What is the behaviour of TSC?
With plain v3.17 plus your series, the cursor is almost never near the
pen. Mostly it jitters around the right hand edge.
My customer had already changed the step delay (I think by trial and
error, not sure) in order to get the cursor near the pen. I ported
this change onto your series (see patch, below), but still the pen up
event causes a huge cursor jump.
(Again, I did solve the pen up issue, but in a totally different
way. I never posted the fix, because I could not be sure that it would
work on a wide variety of boards.)
> Which ADC channels are being used for TSC?
&tscadc {
status = "okay";
tsc {
ti,wires = <4>;
ti,x-plate-resistance = <300>;
ti,coordinate-readouts = <5>;
ti,wire-config = <0x00 0x11 0x23 0x32>;
};
adc {
ti,adc-channels = <5 6 7>;
};
};
So for this particular design, your series really does not help, not
even a little. You did not test the series on many boards. I am
concerned that this series only works on the one board you did test,
and that it may break functionality on other people's boards.
Thanks,
Richard
---
diff --git a/drivers/input/touchscreen/ti_am335x_tsc.c b/drivers/input/touchscreen/ti_am335x_tsc.c
index b84493f..77a4883 100644
--- a/drivers/input/touchscreen/ti_am335x_tsc.c
+++ b/drivers/input/touchscreen/ti_am335x_tsc.c
@@ -148,7 +148,7 @@ static void titsc_step_config(struct titsc *ts_dev)
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);
+ titsc_writel(ts_dev, REG_STEPDELAY(i), STEPCONFIG_OPENDLY | STEPDELAY_SAMPLE(20));
}
config = 0;
@@ -172,7 +172,7 @@ static void titsc_step_config(struct titsc *ts_dev)
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);
+ titsc_writel(ts_dev, REG_STEPDELAY(i), STEPCONFIG_OPENDLY | STEPDELAY_SAMPLE(20));
}
/* Make CHARGECONFIG same as IDLECONFIG */
@@ -188,13 +188,13 @@ static void titsc_step_config(struct titsc *ts_dev)
STEPCONFIG_INP(ts_dev->inp_xp);
titsc_writel(ts_dev, REG_STEPCONFIG(end_step), config);
titsc_writel(ts_dev, REG_STEPDELAY(end_step),
- STEPCONFIG_OPENDLY);
+ STEPCONFIG_OPENDLY | STEPDELAY_SAMPLE(20));
end_step++;
config |= STEPCONFIG_INP(ts_dev->inp_yn);
titsc_writel(ts_dev, REG_STEPCONFIG(end_step), config);
titsc_writel(ts_dev, REG_STEPDELAY(end_step),
- STEPCONFIG_OPENDLY);
+ STEPCONFIG_OPENDLY | STEPDELAY_SAMPLE(20));
/* The steps end ... end - readouts * 2 + 2 and bit 0 for TS_Charge */
stepenable = 1;
On Monday 17 November 2014 01:53 PM, Richard Cochran wrote:
> On Mon, Nov 17, 2014 at 09:57:05AM +0530, Vignesh R wrote:
>> My patches are based on v3.18rc2. I tested my patches on am335x-evm
>> using tslib.
>
> No beaglebone + cape testing?
>
>> Please explain "touch is broken"? What is the behaviour of TSC?
>
> With plain v3.17 plus your series, the cursor is almost never near the
> pen. Mostly it jitters around the right hand edge.
>
> My customer had already changed the step delay (I think by trial and
> error, not sure) in order to get the cursor near the pen. I ported
> this change onto your series (see patch, below), but still the pen up
> event causes a huge cursor jump.
>
> (Again, I did solve the pen up issue, but in a totally different
> way. I never posted the fix, because I could not be sure that it would
> work on a wide variety of boards.)
>
>> Which ADC channels are being used for TSC?
>
> &tscadc {
> status = "okay";
> tsc {
> ti,wires = <4>;
> ti,x-plate-resistance = <300>;
> ti,coordinate-readouts = <5>;
> ti,wire-config = <0x00 0x11 0x23 0x32>;
> };
What is the ti,charge-delay value you have tried?
Try setting it to 0xB000 and experiment.
>
> adc {
> ti,adc-channels = <5 6 7>;
> };
> };
>
> So for this particular design, your series really does not help, not
> even a little. You did not test the series on many boards. I am
> concerned that this series only works on the one board you did test,
> and that it may break functionality on other people's boards.
>
I just have one am335x-evm available. I will search for a beaglebone
cape and test the patches. Let Sebastian, Johannes come back with their
results. If the series works on multiple boards then, I believe, this
series can go in.
Regards
Vignesh
> Thanks,
> Richard
>
> ---
> diff --git a/drivers/input/touchscreen/ti_am335x_tsc.c b/drivers/input/touchscreen/ti_am335x_tsc.c
> index b84493f..77a4883 100644
> --- a/drivers/input/touchscreen/ti_am335x_tsc.c
> +++ b/drivers/input/touchscreen/ti_am335x_tsc.c
> @@ -148,7 +148,7 @@ static void titsc_step_config(struct titsc *ts_dev)
> 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);
> + titsc_writel(ts_dev, REG_STEPDELAY(i), STEPCONFIG_OPENDLY | STEPDELAY_SAMPLE(20));
> }
>
> config = 0;
> @@ -172,7 +172,7 @@ static void titsc_step_config(struct titsc *ts_dev)
> 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);
> + titsc_writel(ts_dev, REG_STEPDELAY(i), STEPCONFIG_OPENDLY | STEPDELAY_SAMPLE(20));
> }
>
> /* Make CHARGECONFIG same as IDLECONFIG */
> @@ -188,13 +188,13 @@ static void titsc_step_config(struct titsc *ts_dev)
> STEPCONFIG_INP(ts_dev->inp_xp);
> titsc_writel(ts_dev, REG_STEPCONFIG(end_step), config);
> titsc_writel(ts_dev, REG_STEPDELAY(end_step),
> - STEPCONFIG_OPENDLY);
> + STEPCONFIG_OPENDLY | STEPDELAY_SAMPLE(20));
>
> end_step++;
> config |= STEPCONFIG_INP(ts_dev->inp_yn);
> titsc_writel(ts_dev, REG_STEPCONFIG(end_step), config);
> titsc_writel(ts_dev, REG_STEPDELAY(end_step),
> - STEPCONFIG_OPENDLY);
> + STEPCONFIG_OPENDLY | STEPDELAY_SAMPLE(20));
>
> /* The steps end ... end - readouts * 2 + 2 and bit 0 for TS_Charge */
> stepenable = 1;
>
On Fri, 14 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 <[email protected]>
> ---
> drivers/mfd/ti_am335x_tscadc.c | 7 +++++--
> include/linux/mfd/ti_am335x_tscadc.h | 1 +
> 2 files changed, 6 insertions(+), 2 deletions(-)
Code looks better now.
Acked-by: Lee Jones <[email protected]>
What's the plan for this series?
> diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c
> index d877e777cce6..110038859a8d 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) && !(reg & 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 3f4e994ace2b..1fd50dcfe47c 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
On Tue, Nov 18, 2014 at 02:30:05PM +0000, Lee Jones wrote:
> On Fri, 14 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 <[email protected]>
> > ---
> > drivers/mfd/ti_am335x_tscadc.c | 7 +++++--
> > include/linux/mfd/ti_am335x_tscadc.h | 1 +
> > 2 files changed, 6 insertions(+), 2 deletions(-)
>
> Code looks better now.
>
> Acked-by: Lee Jones <[email protected]>
>
> What's the plan for this series?
I am waiting for the interested parties to provide more feedback. So far
I have seen a few reports saying that they see issues with the series
applied.
--
Dmitry
On Tuesday 18 November 2014 10:42 PM, Dmitry Torokhov wrote:
> On Tue, Nov 18, 2014 at 02:30:05PM +0000, Lee Jones wrote:
>> On Fri, 14 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 <[email protected]>
>>> ---
>>> drivers/mfd/ti_am335x_tscadc.c | 7 +++++--
>>> include/linux/mfd/ti_am335x_tscadc.h | 1 +
>>> 2 files changed, 6 insertions(+), 2 deletions(-)
>>
>> Code looks better now.
>>
>> Acked-by: Lee Jones <[email protected]>
>>
>> What's the plan for this series?
>
> I am waiting for the interested parties to provide more feedback. So far
> I have seen a few reports saying that they see issues with the series
> applied.
I tested this using lcd7 cape connected to beaglebone black. The latest
kernel I could find on this board was a TI BSP based v3.14 kernel. So I
had to port these patches to that kernel. Cc Robert Nelson to see if he
knows about a more recent kernel supporting that cape.
I did not see any breakage with these patches applied although I did not
see any noticeable performance improvement as well.
I also tested along with continuous loop reading from
/sys/bus/iio/devices/iio:device0/in_voltage5_raw
I have pushed the kernel I used here in case anyone wants to take a look
at my porting.
http://git.ti.com/cgit/cgit.cgi/~nsekhar/ti-linux-kernel/nsekhar-ti-linux-kernel.git/log/?h=am335x-tsc-test
I also tested this series on AM335x EVM using the v3.18-rc5 kernel.
Again, no breakage but no improvement as well.
All testing was done using ts_test
FWIW, you can add:
Tested-by: Sekhar Nori <[email protected]>
Thanks,
Sekhar
> -----Original Message-----
> From: Nori, Sekhar
> Sent: Thursday, November 20, 2014 7:56 AM
>
> I also tested this series on AM335x EVM using the v3.18-rc5 kernel.
> Again, no breakage but no improvement as well.
The primary goal was not necessarily to improve performance of the touchscreen itself. It was to reduce unnecessary CPU overhead introduced by the 275us udelay in the ISR. On a related note, that 275us udelay is not something that worked for all boards. For example, see the following forum thread:
http://e2e.ti.com/support/arm/sitara_arm/f/791/p/217587/775152.aspx#775152
In that thread the user was registering multiple press events for a single press. By increasing the udelay to 1.5ms they were able to solve the problem. Of course, having a 1.5ms delay in your ISR is a really bad thing to do...
I have another customer that was experiencing the same issue of registering multiple press events, and sure enough the 1.5ms delay "fixed" their problem too. The patches allowed them to remove that gigantic delay from the ISR because that software delay has now become a hardware delay via CHARGECONFIG. That customer is the one that needed 0xB000 which is MUCH larger than the value of 0x400 that was working fine for me on the EVM.
Brad,
What you wrote is just the kind of thing one would like to see in the
cover letter or change log...
On Thu, Nov 20, 2014 at 02:23:30PM +0000, Griffis, Brad wrote:
> In that thread the user was registering multiple press events for a single press. By increasing the udelay to 1.5ms they were able to solve the problem. Of course, having a 1.5ms delay in your ISR is a really bad thing to do...
I fully support removing the aweful ISR delay, as long as the result
works!
> I have another customer that was experiencing the same issue of registering multiple press events, and sure enough the 1.5ms delay "fixed" their problem too. The patches allowed them to remove that gigantic delay from the ISR because that software delay has now become a hardware delay via CHARGECONFIG. That customer is the one that needed 0xB000 which is MUCH larger than the value of 0x400 that was working fine for me on the EVM.
It would be really nice for TI to explain to board designers how to
calculate the proper value.
Thanks,
Richard
On Thu, Nov 20, 2014 at 07:26:00PM +0530, Sekhar Nori wrote:
> I tested this using lcd7 cape connected to beaglebone black. The latest
> kernel I could find on this board was a TI BSP based v3.14 kernel. So I
> had to port these patches to that kernel. Cc Robert Nelson to see if he
> knows about a more recent kernel supporting that cape.
What is missing from mainline for the black?
(I thought it was fully supported by now.)
> I did not see any breakage with these patches applied although I did not
> see any noticeable performance improvement as well.
So, the jumping on pen-up persists, right? That means the patch
series does not provide a general fix for that issue.
> I also tested this series on AM335x EVM using the v3.18-rc5 kernel.
> Again, no breakage but no improvement as well.
You still have jumps on a pen up event?
Thanks,
Richard
On Thursday 20 November 2014 08:10 PM, Richard Cochran wrote:
> On Thu, Nov 20, 2014 at 07:26:00PM +0530, Sekhar Nori wrote:
>> I tested this using lcd7 cape connected to beaglebone black. The latest
>> kernel I could find on this board was a TI BSP based v3.14 kernel. So I
>> had to port these patches to that kernel. Cc Robert Nelson to see if he
>> knows about a more recent kernel supporting that cape.
>
> What is missing from mainline for the black?
>
> (I thought it was fully supported by now.)
Black is supported, but LCD7 cape is not.
>
>> I did not see any breakage with these patches applied although I did not
>> see any noticeable performance improvement as well.
>
> So, the jumping on pen-up persists, right? That means the patch
> series does not provide a general fix for that issue.
Not sure how to reproduce the jumping on pen-up. I was able to draw
circles, sign my name and actually today I wrote up the whole English
alphabet on the screen. Picture here.
https://drive.google.com/file/d/0ByWLwvGx8MeyVF9CTlhHU0FHMFk/view?usp=sharing
Is there any particular pattern you want me to test with?
I would still like to test with a more recent kernel on LCD7 cape. So,
if anyone has ideas (other than I porting the LCD7 support) please let
me know.
Thanks,
Sekhar
On Fri, Nov 21, 2014 at 05:40:12PM +0530, Sekhar Nori wrote:
> Not sure how to reproduce the jumping on pen-up.
Does the cursor stay in exactly the same spot when you lift up the
stylus? Then you don't have the issue.
On the BB white using the LCD4 cape and the shipped debian kernel, the
cursor *does* jump away, but not as often or as far as on the custom
design I was working with.
Thanks,
Richard
I tested version 4 of the patch series on a custom design and I saw
also the jumps as described by Richard. I played a little with the
sample delay and it got better but I couldn't completely remove the
jumps.
The other issue I had with version 3, the pen_ups during the busy
loop, is solved. I experienced some pen_up misses during the busy loop
if I used a too high sample delay, without the sample delay it was ok.
Thanks,
Hannes
2014-11-21 14:10 GMT+01:00 Richard Cochran <[email protected]>:
> On Fri, Nov 21, 2014 at 05:40:12PM +0530, Sekhar Nori wrote:
>> Not sure how to reproduce the jumping on pen-up.
>
> Does the cursor stay in exactly the same spot when you lift up the
> stylus? Then you don't have the issue.
>
> On the BB white using the LCD4 cape and the shipped debian kernel, the
> cursor *does* jump away, but not as often or as far as on the custom
> design I was working with.
>
> Thanks,
> Richard
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
On Friday 21 November 2014 08:41 PM, Johannes Pointner wrote:
> I tested version 4 of the patch series on a custom design and I saw
> also the jumps as described by Richard. I played a little with the
> sample delay and it got better but I couldn't completely remove the
> jumps.
And before the patches were applied, there were no jumps at all? Which
kernel are you using for testing?
Thanks,
Sekhar
Before the patches were also jumps but I thought it is something
Vignesh should know. Maybe there is some fix for that too?
As Richard also noted, it would be nice if ti could let us know how to
get the delay values right. By trial and error is IMHO not the best
way.
For the testing I used 3.16.7.
2014-11-21 16:37 GMT+01:00 Sekhar Nori <[email protected]>:
> On Friday 21 November 2014 08:41 PM, Johannes Pointner wrote:
>> I tested version 4 of the patch series on a custom design and I saw
>> also the jumps as described by Richard. I played a little with the
>> sample delay and it got better but I couldn't completely remove the
>> jumps.
>
> And before the patches were applied, there were no jumps at all? Which
> kernel are you using for testing?
>
> Thanks,
> Sekhar
On Fri, Nov 21, 2014 at 07:17:18PM +0100, Johannes Pointner wrote:
> Before the patches were also jumps but I thought it is something
> Vignesh should know. Maybe there is some fix for that too?
> As Richard also noted, it would be nice if ti could let us know how to
> get the delay values right. By trial and error is IMHO not the best
> way.
That is not only an opinion, it is a matter of fact. TI really dropped
the ball on this one. I thought the patch series was a sign they
finally were going to properly address this issue. Wrong again.
The data sheet for the TI part used to have a reference to an app-note
for the touch controller.
spruh73f page 1154
The Pen IRQ can only occur if the correct Pen Ctrl bits are high
(AFE Pen Ctrl bits 6:5 in the TSC_ADC control register) and if
the correct ground transistor biasing is set in the StepConfig
[N] Register. Refer to the application notes for the correct
settings.
I searched high and low for this application note. Then, the data
sheet got revised.
Thanks,
Richard
> -----Original Message-----
> From: Richard Cochran [mailto:[email protected]]
>
> On Fri, Nov 21, 2014 at 07:17:18PM +0100, Johannes Pointner wrote:
> > Before the patches were also jumps but I thought it is something
> > Vignesh should know. Maybe there is some fix for that too?
I believe I've seen the "jumping" behavior pop up at a couple random times in my testing. I think it relates to the hardware not properly registering a pen-up interrupt. When that's happening can you try running ts_calibrate? If you're not registering a pen-up event then you won't be able to advance through those touch points. Is this behavior related to capturing ADC samples, or do you see this behavior even without capturing ADC samples?
> > As Richard also noted, it would be nice if ti could let us know how to
> > get the delay values right. By trial and error is IMHO not the best
> > way.
>
> That is not only an opinion, it is a matter of fact. TI really dropped the ball on
> this one. I thought the patch series was a sign they finally were going to
> properly address this issue. Wrong again.
These patches originate from work I did on TI's SDK 7.00 (3.12 kernel). The touchscreen is working beautifully in that environment and we are putting forth significant effort to bring those improvements to the community.
Things do not seem to be working as well with the current mainline kernel. I've been reviewing various patches between 3.12 and the mainline to understand why it's different. I believe I have figured this out, and I have discussions going separately with individuals that contributed those patches in order to come up with the best possible solution, i.e. we likely need a few additional changes to this patch set.
The CHARGECONFIG delay serves precisely the same purpose as the udelay in the original code base. How did you determine the udelay value in the first place? I went through the effort of figuring out a way to make the delay into a HARDWARE delay instead of a software delay so that it could be removed from the ISR. That delay time relates to "settling time" and so it's going to be dependent on your PCB. You could hook up an oscilloscope and attempt to capture this parameter from the signals themselves. Personally I think that's a lot more difficult than just doing a few iterations of testing.
> The data sheet for the TI part used to have a reference to an app-note for
> the touch controller.
>
> spruh73f page 1154
>
> The Pen IRQ can only occur if the correct Pen Ctrl bits are high
> (AFE Pen Ctrl bits 6:5 in the TSC_ADC control register) and if
> the correct ground transistor biasing is set in the StepConfig
> [N] Register. Refer to the application notes for the correct
> settings.
>
> I searched high and low for this application note. Then, the data sheet got
> revised.
Such an application note does not exist, which explains why you didn't find it and why we have removed that reference from the TRM. Sorry for your trouble. We always strive for accurate documentation.
2014-11-21 23:25 GMT+01:00 Griffis, Brad <[email protected]>:
>
>> -----Original Message-----
>> From: Richard Cochran [mailto:[email protected]]
>>
>> On Fri, Nov 21, 2014 at 07:17:18PM +0100, Johannes Pointner wrote:
>> > Before the patches were also jumps but I thought it is something
>> > Vignesh should know. Maybe there is some fix for that too?
>
> I believe I've seen the "jumping" behavior pop up at a couple random times in my testing. I think it relates to the hardware not properly registering a pen-up interrupt. When that's happening can you try running ts_calibrate? If you're not registering a pen-up event then you won't be able to advance through those touch points. Is this behavior related to capturing ADC samples, or do you see this behavior even without capturing ADC samples?
Is it the right way to test? Because I think the ts_lib will cover
misbehavior of the touchscreen driver.
>
>> > As Richard also noted, it would be nice if ti could let us know how to
>> > get the delay values right. By trial and error is IMHO not the best
>> > way.
>>
>> That is not only an opinion, it is a matter of fact. TI really dropped the ball on
>> this one. I thought the patch series was a sign they finally were going to
>> properly address this issue. Wrong again.
>
> These patches originate from work I did on TI's SDK 7.00 (3.12 kernel). The touchscreen is working beautifully in that environment and we are putting forth significant effort to bring those improvements to the community.
>
> Things do not seem to be working as well with the current mainline kernel. I've been reviewing various patches between 3.12 and the mainline to understand why it's different. I believe I have figured this out, and I have discussions going separately with individuals that contributed those patches in order to come up with the best possible solution, i.e. we likely need a few additional changes to this patch set.
Ok, then I'm looking forward for a new patch set to test. Furthermore,
I would be interested which changes you are talking about.
>
> The CHARGECONFIG delay serves precisely the same purpose as the udelay in the original code base. How did you determine the udelay value in the first place? I went through the effort of figuring out a way to make the delay into a HARDWARE delay instead of a software delay so that it could be removed from the ISR. That delay time relates to "settling time" and so it's going to be dependent on your PCB. You could hook up an oscilloscope and attempt to capture this parameter from the signals themselves. Personally I think that's a lot more difficult than just doing a few iterations of testing.
I did measure with an oscilloscope to get the right charge delay and
it looks fine to me. But I can't affect the jumps with the charge
delay, the only thing that helped, was to add some sample delay. But
it didn't solve it.
>
>> The data sheet for the TI part used to have a reference to an app-note for
>> the touch controller.
>>
>> spruh73f page 1154
>>
>> The Pen IRQ can only occur if the correct Pen Ctrl bits are high
>> (AFE Pen Ctrl bits 6:5 in the TSC_ADC control register) and if
>> the correct ground transistor biasing is set in the StepConfig
>> [N] Register. Refer to the application notes for the correct
>> settings.
>>
>> I searched high and low for this application note. Then, the data sheet got
>> revised.
>
> Such an application note does not exist, which explains why you didn't find it and why we have removed that reference from the TRM. Sorry for your trouble. We always strive for accurate documentation.
Thanks,
Hannes
On 11/21/2014 02:10 PM, Richard Cochran wrote:
> On the BB white using the LCD4 cape and the shipped debian kernel, the
> cursor *does* jump away, but not as often or as far as on the custom
> design I was working with.
This sounds like the ADC is still sampling while the input data becomes
invalid. Usually there is a resistor on the wiper line and the
touchscreen manual says how much it should be at least. Could you
please check if this is properly installed?
>
> Thanks,
> Richard
Sebastian
On Mon, Nov 24, 2014 at 09:57:46AM +0100, Sebastian Andrzej Siewior wrote:
> On 11/21/2014 02:10 PM, Richard Cochran wrote:
>
> > On the BB white using the LCD4 cape and the shipped debian kernel, the
> > cursor *does* jump away, but not as often or as far as on the custom
> > design I was working with.
>
> This sounds like the ADC is still sampling while the input data becomes
> invalid. Usually there is a resistor on the wiper line and the
> touchscreen manual says how much it should be at least. Could you
> please check if this is properly installed?
Wiper line? This is a four wire device.
Thanks,
Richard
* Vignesh R | 2014-11-14 10:37:25 [+0530]:
>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
am335x not -evm. The am335x-evm is a board (with its own advisory
document) built around the SoC.
Just testing the v4. I can use now IIO and touchscren at the same time.
back at v1 I reported that it does not work, this has been fixed now.
I had it running for a few minutes, now I see one of WARN_ON() beeing
triggered (I've cut a few numbers so don't wonder about PID 2 and so on):
|dmesg |grep WARNING | wc -l
|10
| dmesg |grep WARNING
|[306.257995] WARNING: CPU: 0 PID: 97 at mfd/ti_am335x_tscadc.c:94 am335x_tsc_se_set_once+0xf8/0x104
|[365.469591] WARNING: CPU: 0 PID: 58 at mfd/ti_am335x_tscadc.c:94 am335x_tsc_se_set_once+0xf8/0x104
|[379.255904] WARNING: CPU: 0 PID: 24 at mfd/ti_am335x_tscadc.c:94 am335x_tsc_se_set_once+0xf8/0x104
|[426.230505] WARNING: CPU: 0 PID: 35 at mfd/ti_am335x_tscadc.c:94 am335x_tsc_se_set_once+0xf8/0x104
|[435.654091] WARNING: CPU: 0 PID: 28 at mfd/ti_am335x_tscadc.c:94 am335x_tsc_se_set_once+0xf8/0x104
|[438.897519] WARNING: CPU: 0 PID: 91 at mfd/ti_am335x_tscadc.c:94 am335x_tsc_se_set_once+0xf8/0x104
|[525.720193] WARNING: CPU: 0 PID: 88 at mfd/ti_am335x_tscadc.c:94 am335x_tsc_se_set_once+0xf8/0x104
|[527.644770] WARNING: CPU: 0 PID: 38 at mfd/ti_am335x_tscadc.c:94 am335x_tsc_se_set_once+0xf8/0x104
|[557.218349] WARNING: CPU: 0 PID: 56 at mfd/ti_am335x_tscadc.c:94 am335x_tsc_se_set_once+0xf8/0x104
|[610.077274] WARNING: CPU: 0 PID: 2 at mfd/ti_am335x_tscadc.c:94 am335x_tsc_se_set_once+0xf8/0x104
The complete trace:
|[610.110692] CPU: 0 PID: 4422 Comm: cat Tainted: G W 3.18.0-rc6+ #1745
|[610.118577] [<c00138ec>] (unwind_backtrace) from [<c0011544>] (show_stack+0x10/0x14)
|[610.126772] [<c0011544>] (show_stack) from [<c003c9b0>] (warn_slowpath_common+0x68/0x88)
|[610.135313] [<c003c9b0>] (warn_slowpath_common) from [<c003c9ec>] (warn_slowpath_null+0x1c/0x24)
|[610.144586] [<c003c9ec>] (warn_slowpath_null) from [<bf00569c>] (am335x_tsc_se_set_once+0xf8/0x104 [ti_am335x_tscadc])
|[610.155886] [<bf00569c>] (am335x_tsc_se_set_once [ti_am335x_tscadc]) from [<bf067494>] (tiadc_read_raw+0xbc/0x190 [ti_am335x_adc])
|[610.168326] [<bf067494>] (tiadc_read_raw [ti_am335x_adc]) from [<bf02dccc>] (iio_read_channel_info+0x9c/0xa4 [industrialio])
|[610.180191] [<bf02dccc>] (iio_read_channel_info [industrialio]) from [<c02a42d4>] (dev_attr_show+0x1c/0x48)
|[610.190477] [<c02a42d4>] (dev_attr_show) from [<c013d544>] (sysfs_kf_seq_show+0x8c/0x110)
|[610.199108] [<c013d544>] (sysfs_kf_seq_show) from [<c013c1c8>] (kernfs_seq_show+0x24/0x28)
|[610.207833] [<c013c1c8>] (kernfs_seq_show) from [<c0102658>] (seq_read+0x1b4/0x47c)
|[610.215922] [<c0102658>] (seq_read) from [<c00e6700>] (vfs_read+0x8c/0x148)
|[610.223269] [<c00e6700>] (vfs_read) from [<c00e67fc>] (SyS_read+0x40/0x8c)
|[610.230525] [<c00e67fc>] (SyS_read) from [<c000e640>] (ret_fast_syscall+0x0/0x30)
Could you please look at that one? (I tested it on am335x-evm btw).
Sebastian
On Monday 24 November 2014 05:21 PM, Sebastian Andrzej Siewior wrote:
> * Vignesh R | 2014-11-14 10:37:25 [+0530]:
>
>> 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
> am335x not -evm. The am335x-evm is a board (with its own advisory
> document) built around the SoC.
>
> Just testing the v4. I can use now IIO and touchscren at the same time.
> back at v1 I reported that it does not work, this has been fixed now.
> I had it running for a few minutes, now I see one of WARN_ON() beeing
> triggered (I've cut a few numbers so don't wonder about PID 2 and so on):
>
> |dmesg |grep WARNING | wc -l
> |10
> | dmesg |grep WARNING
> |[306.257995] WARNING: CPU: 0 PID: 97 at mfd/ti_am335x_tscadc.c:94 am335x_tsc_se_set_once+0xf8/0x104
> |[365.469591] WARNING: CPU: 0 PID: 58 at mfd/ti_am335x_tscadc.c:94 am335x_tsc_se_set_once+0xf8/0x104
> |[379.255904] WARNING: CPU: 0 PID: 24 at mfd/ti_am335x_tscadc.c:94 am335x_tsc_se_set_once+0xf8/0x104
> |[426.230505] WARNING: CPU: 0 PID: 35 at mfd/ti_am335x_tscadc.c:94 am335x_tsc_se_set_once+0xf8/0x104
> |[435.654091] WARNING: CPU: 0 PID: 28 at mfd/ti_am335x_tscadc.c:94 am335x_tsc_se_set_once+0xf8/0x104
> |[438.897519] WARNING: CPU: 0 PID: 91 at mfd/ti_am335x_tscadc.c:94 am335x_tsc_se_set_once+0xf8/0x104
> |[525.720193] WARNING: CPU: 0 PID: 88 at mfd/ti_am335x_tscadc.c:94 am335x_tsc_se_set_once+0xf8/0x104
> |[527.644770] WARNING: CPU: 0 PID: 38 at mfd/ti_am335x_tscadc.c:94 am335x_tsc_se_set_once+0xf8/0x104
> |[557.218349] WARNING: CPU: 0 PID: 56 at mfd/ti_am335x_tscadc.c:94 am335x_tsc_se_set_once+0xf8/0x104
> |[610.077274] WARNING: CPU: 0 PID: 2 at mfd/ti_am335x_tscadc.c:94 am335x_tsc_se_set_once+0xf8/0x104
>
> The complete trace:
>
> |[610.110692] CPU: 0 PID: 4422 Comm: cat Tainted: G W 3.18.0-rc6+ #1745
> |[610.118577] [<c00138ec>] (unwind_backtrace) from [<c0011544>] (show_stack+0x10/0x14)
> |[610.126772] [<c0011544>] (show_stack) from [<c003c9b0>] (warn_slowpath_common+0x68/0x88)
> |[610.135313] [<c003c9b0>] (warn_slowpath_common) from [<c003c9ec>] (warn_slowpath_null+0x1c/0x24)
> |[610.144586] [<c003c9ec>] (warn_slowpath_null) from [<bf00569c>] (am335x_tsc_se_set_once+0xf8/0x104 [ti_am335x_tscadc])
> |[610.155886] [<bf00569c>] (am335x_tsc_se_set_once [ti_am335x_tscadc]) from [<bf067494>] (tiadc_read_raw+0xbc/0x190 [ti_am335x_adc])
> |[610.168326] [<bf067494>] (tiadc_read_raw [ti_am335x_adc]) from [<bf02dccc>] (iio_read_channel_info+0x9c/0xa4 [industrialio])
> |[610.180191] [<bf02dccc>] (iio_read_channel_info [industrialio]) from [<c02a42d4>] (dev_attr_show+0x1c/0x48)
> |[610.190477] [<c02a42d4>] (dev_attr_show) from [<c013d544>] (sysfs_kf_seq_show+0x8c/0x110)
> |[610.199108] [<c013d544>] (sysfs_kf_seq_show) from [<c013c1c8>] (kernfs_seq_show+0x24/0x28)
> |[610.207833] [<c013c1c8>] (kernfs_seq_show) from [<c0102658>] (seq_read+0x1b4/0x47c)
> |[610.215922] [<c0102658>] (seq_read) from [<c00e6700>] (vfs_read+0x8c/0x148)
> |[610.223269] [<c00e6700>] (vfs_read) from [<c00e67fc>] (SyS_read+0x40/0x8c)
> |[610.230525] [<c00e67fc>] (SyS_read) from [<c000e640>] (ret_fast_syscall+0x0/0x30)
>
> Could you please look at that one? (I tested it on am335x-evm btw).
I have tried running both IIO and TSC at the same time. But I have never
seen WARN_ON() even after running for close to 30 min. Can you send me
the exact script, so that it will be easy to reproduce?
Regards
Vignesh
>
> Sebastian
>
On 11/24/2014 01:16 PM, Vignesh R wrote:
> I have tried running both IIO and TSC at the same time. But I have never
> seen WARN_ON() even after running for close to 30 min. Can you send me
> the exact script, so that it will be easy to reproduce?
Sure thing.
- one shell
evtest /dev/input/event2
- second shell
./iio-test.sh
with:
|#!/bin/sh
|
|while [ 1 = 1 ]
|do
| cat /sys/bus/iio/devices/iio\:device0/in_voltage4_raw
|done
the kernel config: https://breakpoint.cc/am335x-config
>
> Regards
> Vignesh
Sebastian
On Monday 24 November 2014 06:05 PM, Sebastian Andrzej Siewior wrote:
> On 11/24/2014 01:16 PM, Vignesh R wrote:
>
>> I have tried running both IIO and TSC at the same time. But I have never
>> seen WARN_ON() even after running for close to 30 min. Can you send me
>> the exact script, so that it will be easy to reproduce?
>
> Sure thing.
> - one shell
> evtest /dev/input/event2
> - second shell
> ./iio-test.sh
> with:
>
> |#!/bin/sh
> |
> |while [ 1 = 1 ]
> |do
> | cat /sys/bus/iio/devices/iio\:device0/in_voltage4_raw
> |done
>
>
> the kernel config: https://breakpoint.cc/am335x-config
>
I was able to reproduce this issue. When ADC hits WARN_ON(), the TSC
steps should be disabled. But, in this case, TSC is sampling the first Y
co-ordinate and all TSC steps are enabled.
REG_SE: 0x1ffc1
ADC_STAT: 0x24
Any idea why this may be happening?
Regards
Vignesh
Hi Sebastian,
On Monday 24 November 2014 05:21 PM, Sebastian Andrzej Siewior wrote:
> * Vignesh R | 2014-11-14 10:37:25 [+0530]:
>
>> 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
> am335x not -evm. The am335x-evm is a board (with its own advisory
> document) built around the SoC.
>
> Just testing the v4. I can use now IIO and touchscren at the same time.
> back at v1 I reported that it does not work, this has been fixed now.
> I had it running for a few minutes, now I see one of WARN_ON() beeing
> triggered (I've cut a few numbers so don't wonder about PID 2 and so on):
>
I fixed the issue that was triggering the WARN_ON(). I think it would be
better if you tested these patches, before I posted them on the
mainline. I don't want to clutter the list with too main versions. Here
is the link to the tree with latest changes:
git clone -b tsc-devel git://git.ti.com/kernel/tsc-adc.git
(tsc-devel branch)
Please test the above tree and let me know the results.
I will post new version(v5) if there are no issues wrt IIO and TSC
working at the same time.
The cursor jump at touch release has been fixed. But I do see random
cursor movement when dragging on the screen. This is not due to false
pen-up events. My patches do not aim at fixing them. (the aim is to
remove udelay in irq handler and fix false pen-ups).
With these changes, false pen-ups have been eliminated and there is a
significant improvement in the performance of TSC. I feel these patches
can go in, if there are no other serious issues.
I will work on random cursor jump in a separate patch series.
Regards
Vignesh
On 12/01/2014 10:53 AM, Vignesh R wrote:
> Hi Sebastian,
Hi Vignesh,
> I fixed the issue that was triggering the WARN_ON(). I think it would be
> better if you tested these patches, before I posted them on the
> mainline. I don't want to clutter the list with too main versions. Here
> is the link to the tree with latest changes:
awesome, thank you.
>
> git clone -b tsc-devel git://git.ti.com/kernel/tsc-adc.git
> (tsc-devel branch)
I will and let you know.
> Please test the above tree and let me know the results.
> I will post new version(v5) if there are no issues wrt IIO and TSC
> working at the same time.
Cool. Were you able to get in touch with someone who has a 5-wire touch?
> Regards
> Vignesh
>
Sebastian
Hi,
On Monday 01 December 2014 03:29 PM, Sebastian Andrzej Siewior wrote:
> On 12/01/2014 10:53 AM, Vignesh R wrote:
>> Hi Sebastian,
>
> Hi Vignesh,
>
>> I fixed the issue that was triggering the WARN_ON(). I think it would be
>> better if you tested these patches, before I posted them on the
>> mainline. I don't want to clutter the list with too main versions. Here
>> is the link to the tree with latest changes:
>
> awesome, thank you.
>
>>
>> git clone -b tsc-devel git://git.ti.com/kernel/tsc-adc.git
>> (tsc-devel branch)
>
> I will and let you know.
>
>> Please test the above tree and let me know the results.
>> I will post new version(v5) if there are no issues wrt IIO and TSC
>> working at the same time.
>
> Cool. Were you able to get in touch with someone who has a 5-wire touch?
I have asked Jeff Lance to verify 5-wire touch.
Regards
Vignesh
Hello,
Any updates on this series? I don't see it applied in [1] or [2].
[1] https://github.com/RobertCNelson/ti-linux-kernel/commits/ti-linux-3.14.y/drivers/input/touchscreen/ti_am335x_tsc.c
[2] https://github.com/RobertCNelson/ti-linux-kernel/commits/ti-linux-3.14.y-exp/drivers/input/touchscreen/ti_am335x_tsc.c
Regards,
Nicolae Rosia
On Mon, Dec 1, 2014 at 12:29 PM, Vignesh R <[email protected]> wrote:
>
> Hi,
>
> On Monday 01 December 2014 03:29 PM, Sebastian Andrzej Siewior wrote:
>> On 12/01/2014 10:53 AM, Vignesh R wrote:
>>> Hi Sebastian,
>>
>> Hi Vignesh,
>>
>>> I fixed the issue that was triggering the WARN_ON(). I think it would be
>>> better if you tested these patches, before I posted them on the
>>> mainline. I don't want to clutter the list with too main versions. Here
>>> is the link to the tree with latest changes:
>>
>> awesome, thank you.
>>
>>>
>>> git clone -b tsc-devel git://git.ti.com/kernel/tsc-adc.git
>>> (tsc-devel branch)
>>
>> I will and let you know.
>>
>>> Please test the above tree and let me know the results.
>>> I will post new version(v5) if there are no issues wrt IIO and TSC
>>> working at the same time.
>>
>> Cool. Were you able to get in touch with someone who has a 5-wire touch?
>
> I have asked Jeff Lance to verify 5-wire touch.
>
> Regards
> Vignesh
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
On 12/11/2014 09:34 PM, Nicolae Rosia wrote:
> Hello,
Hi,
> Any updates on this series? I don't see it applied in [1] or [2].
I manage to freeze am335x-evm with those patches and I think Vignesh
is looking into this.
> Regards,
> Nicolae Rosia
Sebastian
On Friday 12 December 2014 02:10 AM, Sebastian Andrzej Siewior wrote:
> On 12/11/2014 09:34 PM, Nicolae Rosia wrote:
>> Hello,
>
> Hi,
>
>> Any updates on this series? I don't see it applied in [1] or [2].
>
> I manage to freeze am335x-evm with those patches and I think Vignesh
> is looking into this.
I was not able to reproduce this issue with my kernel configurations.
I am yet to try with Sebastian's ".config" file. Not been able work on this.
>
>> Regards,
>> Nicolae Rosia
>
> Sebastian
>
Regards,
Vignesh
Hello,
I have tried your patches by cloning [1] and copying the
relevant files over the kernel I've cloned from [2].
It compiles & runs, but I'm seeing lots of pen-down/pen-up
events when I'm not touching the screen.
Any suggestions?
Hardware: Beaglebone Black with BB-View 4.3 Cape from Farnell
[1] git clone -b tsc-devel git://git.ti.com/kernel/tsc-adc.git
(tsc-devel branch)
[2] https://github.com/beagleboard/linux/tree/3.14
> -----Original Message-----
> From: Catalin Crenguta [mailto:[email protected]]
> I have tried your patches by cloning [1] and copying the relevant files over
> the kernel I've cloned from [2].
> It compiles & runs, but I'm seeing lots of pen-down/pen-up events when I'm
> not touching the screen.
> Any suggestions?
How are you configuring ti,charge-delay in your dts? I've seen this behavior on some custom boards where we were using a smaller charge delay (0x400) to begin with, and by upping it to 0xb000 we resolved the issue. These patches however already specified ti,charge-delay = 0xb000 by default so this would surprise me that it's still seeing that issue. Was the touchscreen working as expected before these new patches, or does it have issues both ways?
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?
On Fri, Dec 12, 2014 at 4:16 PM, Griffis, Brad <[email protected]> wrote:
> How are you configuring ti,charge-delay in your dts? I've seen this behavior on some custom boards where we were using a smaller charge delay (0x400) to begin with, and by upping it to 0xb000 we resolved the issue. These patches however already specified ti,charge-delay = 0xb000 by default so this would surprise me that it's still seeing that issue. Was the touchscreen working as expected before these new patches, or does it have issues both ways?
Initially I had set the charge-delay to 0x400 and I was seeing many
false pen-down/pen-up events With 0xb000, I was still getting many of
them but less than with 0x400.
On Fri, Dec 12, 2014 at 4:16 PM, Griffis, Brad <[email protected]> wrote:
> How are you configuring ti,charge-delay in your dts? I've seen this behavior on some custom boards where we were using a smaller charge delay (0x400) to begin with, and by upping it to 0xb000 we resolved the issue. These patches however already specified ti,charge-delay = 0xb000 by default so this would surprise me that it's still seeing that issue. Was the touchscreen working as expected before these new patches, or does it have issues both ways?
I have created a DTS for Beaglebone Black with 4 wire touchscreen and
display which works on 3.18 stable and tsc-devel, omap2plus_defconfig
with tsc and tilcdc support.
It seems that with lcdc disabled, I am not getting false reports but
when I'm activating the lcdc, I get a lot of false pen-downs/pen-ups.
Even with tilcdc enabled, after it prints "timeout waiting for
framedone" and the screen get blanked (white), the noise is gone.
Regards,
Catalin.
On Fri, Dec 12, 2014 at 4:16 PM, Griffis, Brad <[email protected]> wrote:
> How are you configuring ti,charge-delay in your dts? I've seen this behavior on some custom boards where we were using a smaller charge delay (0x400) to begin with, and by upping it to 0xb000 we resolved the issue. These patches however already specified ti,charge-delay = 0xb000 by default so this would surprise me that it's still seeing that issue. Was the touchscreen working as expected before these new patches, or does it have issues both ways?
It seems that because the ribbon cable has both the analog and digital
signals, the analog signals are affected by the digital ones (hence
the touchscreen was working OK when the display was disabled). Putting
decoupling capacitors on X+, X-, Y+, Y- reduces the noise and the
false events disappear.
I'm sorry for all the noise I have generated.
Best regards,
On Tue, Dec 16, 2014 at 10:31:47AM +0200, Catalin Crenguta wrote:
>
> It seems that because the ribbon cable has both the analog and digital
> signals, the analog signals are affected by the digital ones (hence
> the touchscreen was working OK when the display was disabled). Putting
> decoupling capacitors on X+, X-, Y+, Y- reduces the noise and the
> false events disappear.
IOW, the root cause is a design flaw in the BB-View 4.3 Cape?
Thanks,
Richard