Re: [PATCH v2] Driver for AUO In-Cell touchscreens using pixcir ICs

["Heiko Stübner" <heiko@sntech.de>]

Am Mittwoch 07 Dezember 2011, 15:31:20 schrieb Heiko Stübner:
> Hi Dmitry,
> 
> thanks for the review
> 
> Am Dienstag, 6. Dezember 2011, 06:56:37 schrieb Dmitry Torokhov:
> > On Mon, Dec 05, 2011 at 09:05:30PM +0100, Heiko Stübner wrote:
> > > Some displays from AUO have a so called in-cell touchscreen, meaning it
> > > is built directly into the display unit.
> > > 
> > > Touchdata is gathered through PIXCIR Tango-ICs and processed in an
> > > Atmel ATmega168P with custom firmware. Communication between the host
> > > system and ATmega is done via I2C.
> > > 
> > > Devices using this touch solution include the Dell Streak5 and the
> > > family of Qisda ebook readers.
> > > 
> > > The driver reports single- and multi-touch events including touch area
> > > values.
> > > 
> > > Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> > > ---
> > > changes since v1: address comments from Christoph Fritz
> > > (wrong type for error checks, wrong call to input_free_device)
> > > 
> > >  drivers/input/touchscreen/Kconfig         |   14 +
> > >  drivers/input/touchscreen/Makefile        |    1 +
> > >  drivers/input/touchscreen/auo-pixcir-ts.c |  656
> > >  +++++++++++++++++++++++++++++ include/linux/input/auo-pixcir-ts.h
> > >  
> > >  |   57 +++
> > >  
> > >  4 files changed, 728 insertions(+), 0 deletions(-)
> > >  create mode 100644 drivers/input/touchscreen/auo-pixcir-ts.c
> > >  create mode 100644 include/linux/input/auo-pixcir-ts.h
> 
> [ ... small corrections, thanks for finding them ... ]
> 
> > > +
> > > +		for (i = 0; i < AUO_PIXCIR_REPORT_POINTS; i++) {
> > > +			if (point[i].coord_x > 0 || point[i].coord_y > 0) {
> > > +				input_report_abs(ts->input, ABS_MT_POSITION_X,
> > > +						 point[i].coord_x);
> > > +				input_report_abs(ts->input, ABS_MT_POSITION_Y,
> > > +						 point[i].coord_y);
> > > +				input_report_abs(ts->input, ABS_MT_TOUCH_MAJOR,
> > > +						 point[i].area_major);
> > > +				input_report_abs(ts->input, ABS_MT_TOUCH_MINOR,
> > > +						 point[i].area_minor);
> > > +				input_report_abs(ts->input, ABS_MT_ORIENTATION,
> > > +						 point[i].orientation);
> > > +				input_mt_sync(ts->input);
> > 
> > Is there a way to track the contacts and switch to protocol B by any
> > chance?
> 
> I'm not sure, but will check how the device handles changing the number of
> touching fingers and if it would be possible to switch.

Sadly, the device does not seem capable of really tracking the contacts 
individually.
My testcase:
- p0 <-> finger0, p1 <-> finger1
- finger0 is lifted 
in todays test: new mapping finger1 <-> p0
some time ago during another test sometimes it did this switch and sometimes 
not, so the mapping reported point <-> finger is probably not stable.

> > > +
> > > +				/* use first finger as source for singletouch */
> > > +				if (fingers == 0)
> > > +					abs = i;
> > > +
> > > +				/* number of touch points could also be queried
> > > +				 * via i2c but would require an additional call
> > > +				 */
> > > +				fingers++;
> > > +			}
> > > +		}
> 
> [ ... more small stuff for me to fix ... ]
> 
> > > +
> > > +	ret = input_register_device(input_dev);
> > > +	if (ret) {
> > > +		dev_err(&client->dev, "could not register input device\n");
> > > +		goto err_input_register;
> > > +	}
> > > +
> > > +	input_set_drvdata(ts->input, ts);
> > 
> > Just do this earlier and then auo_pixcir_stop() is enough, you won't
> > need to call auo_pixcir_int_enable() before registering IRQ.
> 
> Just to be sure, you mean moving the input_set_drvdata before the irq-
> registration?

seems to work

> > > +
> > > +	i2c_set_clientdata(client, ts);
> > > +
> > > +	return 0;
> > > +
> 
> [ ... and still more small stuff ]
> 
> Thanks
> Heiko

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