| Message ID | 1369847397-27451-3-git-send-email-bigeasy@linutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, May 29, 2013 at 07:09:53PM +0200, Sebastian Andrzej Siewior wrote: > The driver programs a threshold of "steps-to-configure" say 5. The > REG_FIFO0THR registers says it should it be programmed to "threshold > minus one". The driver does not expect just 5 coordinates but 5 * 2 + 2. > Multiplied by two because 5 for X and 5 for Y. Plus 2 because we have > two Z. > The whole thing works because It reads the 5 coordinates for X and Y and > split over FIFO0 and FIFO1 and the last element in each FIFO is ignored > within the loop and read later. > Nothing guaranties that FIFO1 is ready by the time it is read. In fact I > could see that that FIFO1 reaturns for Y channels 8,9, 10, 12, 6 and for > Y channel 7 for Z. The problem is that channel 7 and channel 12 got > somehow mixed up. The variance filter here maybe tries to get rid of the > wrong Y values, dunno. > The other Problem is that FIFO1 is also used by the IIO part leading to > not wrong results if both (tsc & adc) are used. > > The patch tries to clean up the whole thing a little: > - Name it "coordiante-readouts" and not "steps-to-configure" and > document it properly. Point out that it the hardware does a total of > "5 * 2 + 2" reads / steps and not just what is configured. > > - Remove the +1 and -1 in REG_STEPCONFIG, REG_STEPDELAY and its counter > part in the for loop. This is just confusing. > > - Use only FIFO0 in TSC. The fifo has space for 64 entries so should be > fine. > > - Read the whole FIFO in one function and check the channel. > > - in case we dawdle around, make sure we only read a multiple of our > coordinate set. On the second interrupt we will cleanup the remaining > enties. > > Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> > --- > .../bindings/input/touchscreen/ti-tsc-adc.txt | 15 ++-- > arch/arm/boot/dts/am335x-evm.dts | 2 +- > drivers/iio/adc/ti_am335x_adc.c | 2 +- > drivers/input/touchscreen/ti_am335x_tsc.c | 87 ++++++++++---------- > include/linux/mfd/ti_am335x_tscadc.h | 4 +- > 5 files changed, 58 insertions(+), 52 deletions(-) > > diff --git a/Documentation/devicetree/bindings/input/touchscreen/ti-tsc-adc.txt b/Documentation/devicetree/bindings/input/touchscreen/ti-tsc-adc.txt > index e533e9d..80e04e3 100644 > --- a/Documentation/devicetree/bindings/input/touchscreen/ti-tsc-adc.txt > +++ b/Documentation/devicetree/bindings/input/touchscreen/ti-tsc-adc.txt > @@ -6,10 +6,15 @@ > ti,wires: Wires refer to application modes i.e. 4/5/8 wire touchscreen > support on the platform. > ti,x-plate-resistance: X plate resistance > - ti,steps-to-configure: The sequencer supports a total of 16 > - programmable steps. A step configured to read a > - single co-ordinate value. Can be applied more > - number of times for better results. > + ti,coordiante-readouts: The sequencer supports a total of 16 > + programmable steps each step is used to > + read a single coordinate. A single > + readout is enough but multiple reads can > + increase the quality. > + A value of 5 means, 5 reads for X, 5 for > + Y and 2 for Z (always). This utilises 12 > + of the 16 software steps available. The > + remaining 4 can be used by the ADC. > ti,wire-config: Different boards could have a different order for > connecting wires on touchscreen. We need to provide an > 8 bit number where in the 1st four bits represent the > @@ -28,7 +33,7 @@ > tsc { > ti,wires = <4>; > ti,x-plate-resistance = <200>; > - ti,steps-to-configure = <5>; > + ti,coordiante-readouts = <5>; > ti,wire-config = <0x00 0x11 0x22 0x33>; > }; > > diff --git a/arch/arm/boot/dts/am335x-evm.dts b/arch/arm/boot/dts/am335x-evm.dts > index be6a5b2..26fea97 100644 > --- a/arch/arm/boot/dts/am335x-evm.dts > +++ b/arch/arm/boot/dts/am335x-evm.dts > @@ -250,7 +250,7 @@ > tsc { > ti,wires = <4>; > ti,x-plate-resistance = <200>; > - ti,steps-to-configure = <5>; > + ti,coordiante-readouts = <5>; > ti,wire-config = <0x00 0x11 0x22 0x33>; > }; > > diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c > index b2f27de..2988840 100644 > --- a/drivers/iio/adc/ti_am335x_adc.c > +++ b/drivers/iio/adc/ti_am335x_adc.c > @@ -72,7 +72,7 @@ static void tiadc_step_config(struct tiadc_device *adc_dev) > > stepconfig = STEPCONFIG_AVG_16 | STEPCONFIG_FIFO1; > > - for (i = (steps + 1); i <= TOTAL_STEPS; i++) { > + for (i = steps; i < TOTAL_STEPS; i++) { > tiadc_writel(adc_dev, REG_STEPCONFIG(i), > stepconfig | STEPCONFIG_INP(channels)); > tiadc_writel(adc_dev, REG_STEPDELAY(i), > diff --git a/drivers/input/touchscreen/ti_am335x_tsc.c b/drivers/input/touchscreen/ti_am335x_tsc.c > index 2921163..7c97fc7 100644 > --- a/drivers/input/touchscreen/ti_am335x_tsc.c > +++ b/drivers/input/touchscreen/ti_am335x_tsc.c > @@ -58,7 +58,7 @@ struct titsc { > unsigned int bckup_x; > unsigned int bckup_y; > bool pen_down; > - int steps_to_configure; > + int coordiante_readouts; > u32 config_inp[4]; > int bit_xp, bit_xn, bit_yp, bit_yn; > int inp_xp, inp_xn, inp_yp, inp_yn; > @@ -168,11 +168,8 @@ static int titsc_config_wires(struct titsc *ts_dev) > static void titsc_step_config(struct titsc *ts_dev) > { > unsigned int config; > - unsigned int stepenable = 0; > - int i, total_steps; > - > - /* Configure the Step registers */ > - total_steps = 2 * ts_dev->steps_to_configure; > + int i; > + int end_step; > > config = STEPCONFIG_MODE_HWSYNC | > STEPCONFIG_AVG_16 | ts_dev->bit_xp; > @@ -190,7 +187,9 @@ static void titsc_step_config(struct titsc *ts_dev) > break; > } > > - for (i = 1; i <= ts_dev->steps_to_configure; i++) { > + /* 1 … coordiante_readouts is for X */ > + end_step = ts_dev->coordiante_readouts; > + for (i = 0; i < end_step; i++) { > titsc_writel(ts_dev, REG_STEPCONFIG(i), config); > titsc_writel(ts_dev, REG_STEPDELAY(i), STEPCONFIG_OPENDLY); > } > @@ -198,7 +197,7 @@ static void titsc_step_config(struct titsc *ts_dev) > config = 0; > config = STEPCONFIG_MODE_HWSYNC | > STEPCONFIG_AVG_16 | ts_dev->bit_yn | > - STEPCONFIG_INM_ADCREFM | STEPCONFIG_FIFO1; > + STEPCONFIG_INM_ADCREFM; > switch (ts_dev->wires) { > case 4: > config |= ts_dev->bit_yp | STEPCONFIG_INP(ts_dev->inp_xp); > @@ -212,12 +211,13 @@ static void titsc_step_config(struct titsc *ts_dev) > break; > } > > - for (i = (ts_dev->steps_to_configure + 1); i <= total_steps; i++) { > + /* coordiante_readouts … coordiante_readouts * 2 is for Y */ > + end_step = ts_dev->coordiante_readouts * 2; > + for (i = ts_dev->coordiante_readouts; i < end_step; i++) { > titsc_writel(ts_dev, REG_STEPCONFIG(i), config); > titsc_writel(ts_dev, REG_STEPDELAY(i), STEPCONFIG_OPENDLY); > } > > - config = 0; > /* Charge step configuration */ > config = ts_dev->bit_xp | ts_dev->bit_yn | > STEPCHARGE_RFP_XPUL | STEPCHARGE_RFM_XNUR | > @@ -226,37 +226,40 @@ static void titsc_step_config(struct titsc *ts_dev) > titsc_writel(ts_dev, REG_CHARGECONFIG, config); > titsc_writel(ts_dev, REG_CHARGEDELAY, CHARGEDLY_OPENDLY); > > - config = 0; > - /* Configure to calculate pressure */ > + /* coordiante_readouts * 2 … coordiante_readouts * 2 + 2 is for Z */ > config = STEPCONFIG_MODE_HWSYNC | > STEPCONFIG_AVG_16 | ts_dev->bit_yp | > ts_dev->bit_xn | STEPCONFIG_INM_ADCREFM | > STEPCONFIG_INP(ts_dev->inp_xp); > - titsc_writel(ts_dev, REG_STEPCONFIG(total_steps + 1), config); > - titsc_writel(ts_dev, REG_STEPDELAY(total_steps + 1), > + titsc_writel(ts_dev, REG_STEPCONFIG(end_step), config); > + titsc_writel(ts_dev, REG_STEPDELAY(end_step), > STEPCONFIG_OPENDLY); > > - config |= STEPCONFIG_INP(ts_dev->inp_yn) | STEPCONFIG_FIFO1; > - titsc_writel(ts_dev, REG_STEPCONFIG(total_steps + 2), config); > - titsc_writel(ts_dev, REG_STEPDELAY(total_steps + 2), > + 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); > > - for (i = 0; i <= (total_steps + 2); i++) > - stepenable |= 1 << i; > - ts_dev->enable_bits = stepenable; > + /* The steps1 … end and bit 0 for TS_Charge */ > + ts_dev->enable_bits = (1 << (end_step + 2)) - 1; > > titsc_writel(ts_dev, REG_SE, ts_dev->enable_bits); > } > > static void titsc_read_coordinates(struct titsc *ts_dev, > - unsigned int *x, unsigned int *y) > + 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 creads = ts_dev->coordiante_readouts; > > + *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 > @@ -265,32 +268,31 @@ static void titsc_read_coordinates(struct titsc *ts_dev, > * 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 - 1; i++) { > + for (i = 0; i < fifocount; i++) { > read = titsc_readl(ts_dev, REG_FIFO0); > - channel = read & 0xf0000; > - channel = channel >> 0x10; > - if ((channel >= 0) && (channel < ts_dev->steps_to_configure)) { > - read &= 0xfff; > + channel = (read & 0xf0000) >> 16; > + read &= 0xfff; > + if (channel < creads) { > diff = abs(read - prev_val_x); > if (diff < prev_diff_x) { > prev_diff_x = diff; > *x = read; > } > prev_val_x = read; > - } > > - read = titsc_readl(ts_dev, REG_FIFO1); > - channel = read & 0xf0000; > - channel = channel >> 0x10; > - if ((channel >= ts_dev->steps_to_configure) && > - (channel < (2 * ts_dev->steps_to_configure - 1))) { > - read &= 0xfff; > + } else if (channel < creads * 2) { > 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; > } > } > } > @@ -307,26 +309,24 @@ static irqreturn_t titsc_irq(int irq, void *dev) > > status = titsc_readl(ts_dev, REG_IRQSTATUS); > if (status & IRQENB_FIFO0THRES) { > - titsc_read_coordinates(ts_dev, &x, &y); > + > + titsc_read_coordinates(ts_dev, &x, &y, &z1, &z2); > > diffx = abs(x - (ts_dev->bckup_x)); > diffy = abs(y - (ts_dev->bckup_y)); > ts_dev->bckup_x = x; > ts_dev->bckup_y = y; > > - z1 = titsc_readl(ts_dev, REG_FIFO0) & 0xfff; > - z2 = titsc_readl(ts_dev, REG_FIFO1) & 0xfff; > - > if (ts_dev->pen_down && z1 != 0 && z2 != 0) { > /* > * Calculate pressure using formula > * Resistance(touch) = x plate resistance * > * x postion/4096 * ((z2 / z1) - 1) > */ > - z = z2 - z1; > + z = z1 - z2; > z *= x; > z *= ts_dev->x_plate_resistance; > - z /= z1; > + z /= z2; > z = (z + 2047) >> 12; > > if ((diffx < TSCADC_DELTA_X) && > @@ -399,8 +399,8 @@ static int titsc_parse_dt(struct ti_tscadc_dev *tscadc_dev, > if (err < 0) > return err; > > - err = of_property_read_u32(node, "ti,steps-to-configure", > - &ts_dev->steps_to_configure); > + err = of_property_read_u32(node, "ti,coordiante-readouts", > + &ts_dev->coordiante_readouts); > if (err < 0) > return err; > > @@ -454,7 +454,8 @@ static int titsc_probe(struct platform_device *pdev) > goto err_free_irq; > } > titsc_step_config(ts_dev); > - titsc_writel(ts_dev, REG_FIFO0THR, ts_dev->steps_to_configure); > + titsc_writel(ts_dev, REG_FIFO0THR, > + ts_dev->coordiante_readouts * 2 + 2 - 1); > > input_dev->name = "ti-tsc"; > input_dev->dev.parent = &pdev->dev; > @@ -526,7 +527,7 @@ static int titsc_resume(struct device *dev) > } > titsc_step_config(ts_dev); > titsc_writel(ts_dev, REG_FIFO0THR, > - ts_dev->steps_to_configure); > + ts_dev->coordiante_readouts * 2 + 2 - 1); > return 0; > } > > diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h > index c985262..4ec52b0 100644 > --- a/include/linux/mfd/ti_am335x_tscadc.h > +++ b/include/linux/mfd/ti_am335x_tscadc.h > @@ -30,8 +30,8 @@ > #define REG_IDLECONFIG 0x058 > #define REG_CHARGECONFIG 0x05C > #define REG_CHARGEDELAY 0x060 > -#define REG_STEPCONFIG(n) (0x64 + ((n - 1) * 8)) > -#define REG_STEPDELAY(n) (0x68 + ((n - 1) * 8)) > +#define REG_STEPCONFIG(n) (0x64 + ((n) * 8)) > +#define REG_STEPDELAY(n) (0x68 + ((n) * 8)) > #define REG_FIFO0CNT 0xE4 > #define REG_FIFO0THR 0xE8 > #define REG_FIFO1CNT 0xF0 > -- > 1.7.10.4 >
diff --git a/Documentation/devicetree/bindings/input/touchscreen/ti-tsc-adc.txt b/Documentation/devicetree/bindings/input/touchscreen/ti-tsc-adc.txt index e533e9d..80e04e3 100644 --- a/Documentation/devicetree/bindings/input/touchscreen/ti-tsc-adc.txt +++ b/Documentation/devicetree/bindings/input/touchscreen/ti-tsc-adc.txt @@ -6,10 +6,15 @@ ti,wires: Wires refer to application modes i.e. 4/5/8 wire touchscreen support on the platform. ti,x-plate-resistance: X plate resistance - ti,steps-to-configure: The sequencer supports a total of 16 - programmable steps. A step configured to read a - single co-ordinate value. Can be applied more - number of times for better results. + ti,coordiante-readouts: The sequencer supports a total of 16 + programmable steps each step is used to + read a single coordinate. A single + readout is enough but multiple reads can + increase the quality. + A value of 5 means, 5 reads for X, 5 for + Y and 2 for Z (always). This utilises 12 + of the 16 software steps available. The + remaining 4 can be used by the ADC. ti,wire-config: Different boards could have a different order for connecting wires on touchscreen. We need to provide an 8 bit number where in the 1st four bits represent the @@ -28,7 +33,7 @@ tsc { ti,wires = <4>; ti,x-plate-resistance = <200>; - ti,steps-to-configure = <5>; + ti,coordiante-readouts = <5>; ti,wire-config = <0x00 0x11 0x22 0x33>; }; diff --git a/arch/arm/boot/dts/am335x-evm.dts b/arch/arm/boot/dts/am335x-evm.dts index be6a5b2..26fea97 100644 --- a/arch/arm/boot/dts/am335x-evm.dts +++ b/arch/arm/boot/dts/am335x-evm.dts @@ -250,7 +250,7 @@ tsc { ti,wires = <4>; ti,x-plate-resistance = <200>; - ti,steps-to-configure = <5>; + ti,coordiante-readouts = <5>; ti,wire-config = <0x00 0x11 0x22 0x33>; }; diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c index b2f27de..2988840 100644 --- a/drivers/iio/adc/ti_am335x_adc.c +++ b/drivers/iio/adc/ti_am335x_adc.c @@ -72,7 +72,7 @@ static void tiadc_step_config(struct tiadc_device *adc_dev) stepconfig = STEPCONFIG_AVG_16 | STEPCONFIG_FIFO1; - for (i = (steps + 1); i <= TOTAL_STEPS; i++) { + for (i = steps; i < TOTAL_STEPS; i++) { tiadc_writel(adc_dev, REG_STEPCONFIG(i), stepconfig | STEPCONFIG_INP(channels)); tiadc_writel(adc_dev, REG_STEPDELAY(i), diff --git a/drivers/input/touchscreen/ti_am335x_tsc.c b/drivers/input/touchscreen/ti_am335x_tsc.c index 2921163..7c97fc7 100644 --- a/drivers/input/touchscreen/ti_am335x_tsc.c +++ b/drivers/input/touchscreen/ti_am335x_tsc.c @@ -58,7 +58,7 @@ struct titsc { unsigned int bckup_x; unsigned int bckup_y; bool pen_down; - int steps_to_configure; + int coordiante_readouts; u32 config_inp[4]; int bit_xp, bit_xn, bit_yp, bit_yn; int inp_xp, inp_xn, inp_yp, inp_yn; @@ -168,11 +168,8 @@ static int titsc_config_wires(struct titsc *ts_dev) static void titsc_step_config(struct titsc *ts_dev) { unsigned int config; - unsigned int stepenable = 0; - int i, total_steps; - - /* Configure the Step registers */ - total_steps = 2 * ts_dev->steps_to_configure; + int i; + int end_step; config = STEPCONFIG_MODE_HWSYNC | STEPCONFIG_AVG_16 | ts_dev->bit_xp; @@ -190,7 +187,9 @@ static void titsc_step_config(struct titsc *ts_dev) break; } - for (i = 1; i <= ts_dev->steps_to_configure; i++) { + /* 1 … coordiante_readouts is for X */ + end_step = ts_dev->coordiante_readouts; + for (i = 0; i < end_step; i++) { titsc_writel(ts_dev, REG_STEPCONFIG(i), config); titsc_writel(ts_dev, REG_STEPDELAY(i), STEPCONFIG_OPENDLY); } @@ -198,7 +197,7 @@ static void titsc_step_config(struct titsc *ts_dev) config = 0; config = STEPCONFIG_MODE_HWSYNC | STEPCONFIG_AVG_16 | ts_dev->bit_yn | - STEPCONFIG_INM_ADCREFM | STEPCONFIG_FIFO1; + STEPCONFIG_INM_ADCREFM; switch (ts_dev->wires) { case 4: config |= ts_dev->bit_yp | STEPCONFIG_INP(ts_dev->inp_xp); @@ -212,12 +211,13 @@ static void titsc_step_config(struct titsc *ts_dev) break; } - for (i = (ts_dev->steps_to_configure + 1); i <= total_steps; i++) { + /* coordiante_readouts … coordiante_readouts * 2 is for Y */ + end_step = ts_dev->coordiante_readouts * 2; + for (i = ts_dev->coordiante_readouts; i < end_step; i++) { titsc_writel(ts_dev, REG_STEPCONFIG(i), config); titsc_writel(ts_dev, REG_STEPDELAY(i), STEPCONFIG_OPENDLY); } - config = 0; /* Charge step configuration */ config = ts_dev->bit_xp | ts_dev->bit_yn | STEPCHARGE_RFP_XPUL | STEPCHARGE_RFM_XNUR | @@ -226,37 +226,40 @@ static void titsc_step_config(struct titsc *ts_dev) titsc_writel(ts_dev, REG_CHARGECONFIG, config); titsc_writel(ts_dev, REG_CHARGEDELAY, CHARGEDLY_OPENDLY); - config = 0; - /* Configure to calculate pressure */ + /* coordiante_readouts * 2 … coordiante_readouts * 2 + 2 is for Z */ config = STEPCONFIG_MODE_HWSYNC | STEPCONFIG_AVG_16 | ts_dev->bit_yp | ts_dev->bit_xn | STEPCONFIG_INM_ADCREFM | STEPCONFIG_INP(ts_dev->inp_xp); - titsc_writel(ts_dev, REG_STEPCONFIG(total_steps + 1), config); - titsc_writel(ts_dev, REG_STEPDELAY(total_steps + 1), + titsc_writel(ts_dev, REG_STEPCONFIG(end_step), config); + titsc_writel(ts_dev, REG_STEPDELAY(end_step), STEPCONFIG_OPENDLY); - config |= STEPCONFIG_INP(ts_dev->inp_yn) | STEPCONFIG_FIFO1; - titsc_writel(ts_dev, REG_STEPCONFIG(total_steps + 2), config); - titsc_writel(ts_dev, REG_STEPDELAY(total_steps + 2), + 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); - for (i = 0; i <= (total_steps + 2); i++) - stepenable |= 1 << i; - ts_dev->enable_bits = stepenable; + /* The steps1 … end and bit 0 for TS_Charge */ + ts_dev->enable_bits = (1 << (end_step + 2)) - 1; titsc_writel(ts_dev, REG_SE, ts_dev->enable_bits); } static void titsc_read_coordinates(struct titsc *ts_dev, - unsigned int *x, unsigned int *y) + 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 creads = ts_dev->coordiante_readouts; + *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 @@ -265,32 +268,31 @@ static void titsc_read_coordinates(struct titsc *ts_dev, * 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 - 1; i++) { + for (i = 0; i < fifocount; i++) { read = titsc_readl(ts_dev, REG_FIFO0); - channel = read & 0xf0000; - channel = channel >> 0x10; - if ((channel >= 0) && (channel < ts_dev->steps_to_configure)) { - read &= 0xfff; + channel = (read & 0xf0000) >> 16; + read &= 0xfff; + if (channel < creads) { diff = abs(read - prev_val_x); if (diff < prev_diff_x) { prev_diff_x = diff; *x = read; } prev_val_x = read; - } - read = titsc_readl(ts_dev, REG_FIFO1); - channel = read & 0xf0000; - channel = channel >> 0x10; - if ((channel >= ts_dev->steps_to_configure) && - (channel < (2 * ts_dev->steps_to_configure - 1))) { - read &= 0xfff; + } else if (channel < creads * 2) { 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; } } } @@ -307,26 +309,24 @@ static irqreturn_t titsc_irq(int irq, void *dev) status = titsc_readl(ts_dev, REG_IRQSTATUS); if (status & IRQENB_FIFO0THRES) { - titsc_read_coordinates(ts_dev, &x, &y); + + titsc_read_coordinates(ts_dev, &x, &y, &z1, &z2); diffx = abs(x - (ts_dev->bckup_x)); diffy = abs(y - (ts_dev->bckup_y)); ts_dev->bckup_x = x; ts_dev->bckup_y = y; - z1 = titsc_readl(ts_dev, REG_FIFO0) & 0xfff; - z2 = titsc_readl(ts_dev, REG_FIFO1) & 0xfff; - if (ts_dev->pen_down && z1 != 0 && z2 != 0) { /* * Calculate pressure using formula * Resistance(touch) = x plate resistance * * x postion/4096 * ((z2 / z1) - 1) */ - z = z2 - z1; + z = z1 - z2; z *= x; z *= ts_dev->x_plate_resistance; - z /= z1; + z /= z2; z = (z + 2047) >> 12; if ((diffx < TSCADC_DELTA_X) && @@ -399,8 +399,8 @@ static int titsc_parse_dt(struct ti_tscadc_dev *tscadc_dev, if (err < 0) return err; - err = of_property_read_u32(node, "ti,steps-to-configure", - &ts_dev->steps_to_configure); + err = of_property_read_u32(node, "ti,coordiante-readouts", + &ts_dev->coordiante_readouts); if (err < 0) return err; @@ -454,7 +454,8 @@ static int titsc_probe(struct platform_device *pdev) goto err_free_irq; } titsc_step_config(ts_dev); - titsc_writel(ts_dev, REG_FIFO0THR, ts_dev->steps_to_configure); + titsc_writel(ts_dev, REG_FIFO0THR, + ts_dev->coordiante_readouts * 2 + 2 - 1); input_dev->name = "ti-tsc"; input_dev->dev.parent = &pdev->dev; @@ -526,7 +527,7 @@ static int titsc_resume(struct device *dev) } titsc_step_config(ts_dev); titsc_writel(ts_dev, REG_FIFO0THR, - ts_dev->steps_to_configure); + ts_dev->coordiante_readouts * 2 + 2 - 1); return 0; } diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h index c985262..4ec52b0 100644 --- a/include/linux/mfd/ti_am335x_tscadc.h +++ b/include/linux/mfd/ti_am335x_tscadc.h @@ -30,8 +30,8 @@ #define REG_IDLECONFIG 0x058 #define REG_CHARGECONFIG 0x05C #define REG_CHARGEDELAY 0x060 -#define REG_STEPCONFIG(n) (0x64 + ((n - 1) * 8)) -#define REG_STEPDELAY(n) (0x68 + ((n - 1) * 8)) +#define REG_STEPCONFIG(n) (0x64 + ((n) * 8)) +#define REG_STEPDELAY(n) (0x68 + ((n) * 8)) #define REG_FIFO0CNT 0xE4 #define REG_FIFO0THR 0xE8 #define REG_FIFO1CNT 0xF0
The driver programs a threshold of "steps-to-configure" say 5. The REG_FIFO0THR registers says it should it be programmed to "threshold minus one". The driver does not expect just 5 coordinates but 5 * 2 + 2. Multiplied by two because 5 for X and 5 for Y. Plus 2 because we have two Z. The whole thing works because It reads the 5 coordinates for X and Y and split over FIFO0 and FIFO1 and the last element in each FIFO is ignored within the loop and read later. Nothing guaranties that FIFO1 is ready by the time it is read. In fact I could see that that FIFO1 reaturns for Y channels 8,9, 10, 12, 6 and for Y channel 7 for Z. The problem is that channel 7 and channel 12 got somehow mixed up. The variance filter here maybe tries to get rid of the wrong Y values, dunno. The other Problem is that FIFO1 is also used by the IIO part leading to not wrong results if both (tsc & adc) are used. The patch tries to clean up the whole thing a little: - Name it "coordiante-readouts" and not "steps-to-configure" and document it properly. Point out that it the hardware does a total of "5 * 2 + 2" reads / steps and not just what is configured. - Remove the +1 and -1 in REG_STEPCONFIG, REG_STEPDELAY and its counter part in the for loop. This is just confusing. - Use only FIFO0 in TSC. The fifo has space for 64 entries so should be fine. - Read the whole FIFO in one function and check the channel. - in case we dawdle around, make sure we only read a multiple of our coordinate set. On the second interrupt we will cleanup the remaining enties. Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> --- .../bindings/input/touchscreen/ti-tsc-adc.txt | 15 ++-- arch/arm/boot/dts/am335x-evm.dts | 2 +- drivers/iio/adc/ti_am335x_adc.c | 2 +- drivers/input/touchscreen/ti_am335x_tsc.c | 87 ++++++++++---------- include/linux/mfd/ti_am335x_tscadc.h | 4 +- 5 files changed, 58 insertions(+), 52 deletions(-)