Re: [v4 1/3] 1/3 Touchscreen: Cypress TTSP G3 Core Driver

[Henrik Rydberg]

> Cypress TTSP Gen3 Core Driver.
> Core Driver includes platform data definition file,
> core driver definition file, and core touchscreen
> touch handling of device data. Generates
> multi-touch input events.
> 
> Signed-off-by: Kevin McNeely <kev@xxxxxxxxxxx>
> ---

Hi Kevin,

> @@ -124,6 +124,11 @@ config TOUCHSCREEN_CY8CTMG110
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called cy8ctmg110_ts.
>  
> +config TOUCHSCREEN_CYTTSP_CORE
> +	tristate "Cypress TTSP touchscreen core"
> +	help
> +	  Always activated for Cypress TTSP touchscreen
> +

This one needs a bit of elaboration, see patch at the end.

> +static const u8 bl_command[] = {
> +	0x00,			/* file offset */
> +	0xFF,			/* command */
> +	0xA5,			/* exit bootloader command */
> +	0, 1, 2, 3, 4, 5, 6, 7	/* default keys */
> +};

Nice.

> +/* process current touches */
> +#define CY_MAX_TCH	2
> +static int cyttsp_xy_worker(struct cyttsp *ts)
> +{
> +	u8 num_cur_tch;
> +	u16 x[CY_MAX_TCH];
> +	u16 y[CY_MAX_TCH];
> +	u8 z[CY_MAX_TCH];
> +	bool t[CY_MAX_TCH];
> +	int i;

This became more complicated than the previous patch...

> +	/* send touches */
> +	if (!num_cur_tch)
> +		/* terminate previous active touch */
> +		input_mt_sync(ts->input);
> +	else {
> +		/* get touch 1 */
> +		/*
> +		 * If there is only one current active touch,
> +		 * it will be reported in the touch 1 regardless
> +		 * if it was reported in the touch 2 previously
> +		 */

This statement says that t[] is not needed - the idea was to simply
set all values in x[], y[] and z[], and then loop over the num_cur_tch
touches.

> +		x[0] = (ts->xy_data.tch1_xhi << 8) + ts->xy_data.tch1_xlo;
> +		y[0] = (ts->xy_data.tch1_yhi << 8) + ts->xy_data.tch1_ylo;
> +		z[0] = ts->xy_data.tch1_z;
> +		t[0] = true;
> +
> +		/* get touch 2 */
> +		if (num_cur_tch == CY_MAX_TCH) {
> +			x[1] = (ts->xy_data.tch2_xhi << 8) +
> +				ts->xy_data.tch2_xlo;
> +			y[1] = (ts->xy_data.tch2_yhi << 8) +
> +				ts->xy_data.tch2_ylo;
> +			z[1] = ts->xy_data.tch2_z;
> +			t[1] = true;
> +		}
> +
> +		for (i = 0; i < CY_MAX_TCH; i++) {

Using num_cur_tch here,

> +			if (t[i]) {

and removing t[i] here makes the comment above obvious.

> +				input_report_abs(ts->input,
> +					ABS_MT_POSITION_X, x[i]);
> +				input_report_abs(ts->input,
> +					ABS_MT_POSITION_Y, y[i]);
> +				input_report_abs(ts->input,
> +					ABS_MT_TOUCH_MAJOR, z[i]);
> +				input_mt_sync(ts->input);
> +			}
> +		}
> +	}
> +
> +	input_sync(ts->input);
> +
> +	return 0;
> +}

All in all, it seems we have yet to arrive at the most comfortable
solution, in particular if there is a plan to magically enable more
contacts.  Looking again at the data from the device, perhaps one can
simplify things a bit more.  How about the patch below, does that
still work for you?

Thanks,
Henrik
---