On Mon, Mar 10, 2014 at 10:57:10AM +0200, Roger Quadros wrote:
> Hi Henrik,
> 
> On 03/08/2014 05:11 PM, Henrik Rydberg wrote:
> > Hi Roger,
> > 
> > the MT implementation seems mostly fine, just one curiosity:
> > 
> >>  static irqreturn_t pixcir_ts_isr(int irq, void *dev_id)
> >>  {
> >>  	struct pixcir_i2c_ts_data *tsdata = dev_id;
> >>  	const struct pixcir_ts_platform_data *pdata = tsdata->chip;
> >> +	struct pixcir_report_data report;
> >>  
> >>  	while (!tsdata->exiting) {
> >> -		pixcir_ts_poscheck(tsdata);
> >> -
> >> -		if (gpio_get_value(pdata->gpio_attb))
> >> +		/* parse packet */
> >> +		pixcir_ts_parse(tsdata, &report);
> >> +
> >> +		/* report it */
> >> +		pixcir_ts_report(tsdata, &report);
> >> +
> >> +		if (gpio_get_value(pdata->gpio_attb)) {
> >> +			if (report.num_touches) {
> >> +				/*
> >> +				 * Last report with no finger up?
> >> +				 * Do it now then.
> >> +				 */
> >> +				input_mt_sync_frame(tsdata->input);
> >> +				input_sync(tsdata->input);
> > 
> > Why is this special handling needed?
> 
> This is needed because the controller doesn't always report when all fingers
> have left the screen. e.g. report might contain 3 fingers touched and then
> gpio_attb line is de-asserted. There's no report with 0 fingers touched even
> if the user's fingers have left the screen. So we never detect a BUTTON_UP.
> 
> Without this s/w workaround we observe side effects like buttons being pressed
> but not released. To me it looks like a bug in the controller.

the other way would be to *also* use IRQF_TRIGGER_RISING, then you get
an IRQ when fingers leave the screen. No ?

-- 
balbi