Re: [PATCH v2] input: qt602240 - Add ATMEL QT602240 touchscreen driver

[Henrik Rydberg <rydberg@euromail.se>]

Hi Joonyoung,

some follow-up comments on the MT events.

> +static void qt602240_input_report(struct qt602240_data *data, int single_id)
> +{
> +	struct qt602240_finger *finger = data->finger;
> +	struct input_dev *input_dev = data->input_dev;
> +	int finger_num = 0;
> +	int id;
> +
> +	for (id = 0; id < QT602240_MAX_FINGER; id++) {
> +		if (!finger[id].status)
> +			continue;
> +
> +		input_report_abs(input_dev, ABS_MT_TOUCH_MAJOR,
> +				finger[id].area);
> +
> +		if (finger[id].status == QT602240_RELEASE)
> +			finger[id].status = 0;
> +		else {
> +			input_report_abs(input_dev, ABS_MT_POSITION_X,
> +					finger[id].x);
> +			input_report_abs(input_dev, ABS_MT_POSITION_Y,
> +					finger[id].y);
> +			finger_num++;
> +		}
> +
> +		input_mt_sync(input_dev);
> +	}
> +
> +	input_report_key(input_dev, BTN_TOUCH, finger_num > 0);
> +
> +	if (finger[single_id].status != QT602240_RELEASE) {
> +		input_report_abs(input_dev, ABS_X, finger[single_id].x);
> +		input_report_abs(input_dev, ABS_Y, finger[single_id].y);
> +	}
> +
> +	input_sync(input_dev);
> +}

The problem still persists here. I will try to explain in code form instead:

for (id = 0; id < QT602240_MAX_FINGER; id++) {
	if (!finger[id].status)
		continue;

	input_report_abs(input_dev, ABS_MT_TOUCH_MAJOR,
		finger[id].status != QT602240_RELEASE ? finger[id].area : 0);
	input_report_abs(input_dev, ABS_MT_POSITION_X,
			finger[id].x);
	input_report_abs(input_dev, ABS_MT_POSITION_Y,
			finger[id].y);
	input_mt_sync(input_dev);

	if (finger[id].status == QT602240_RELEASE)
		finger[id].status = 0;
	else
		finger_num++;
}

Regarding the single_id, I can understand the need for it, but the logic for a
single touch is slightly confusing. If single_id is to be interpreted as the
contact currently being tracked as the single pointer, and that single_id
changes as the number of fingers on the pad is reduced, until there is only one
left, then it would still be clearer logically to do something like this:

if (finger_num > 0) {
	input_report_key(input_dev, BTN_TOUCH, 1);
	input_report_abs(input_dev, ABS_X, finger[single_id].x);
	input_report_abs(input_dev, ABS_Y, finger[single_id].y);
} else {
	input_report_key(input_dev, BTN_TOUCH, 0);
}
input_sync(input_dev);

Would it not?

Thanks,
Henrik