linux-kernel - Re: [PATCH 25/51] Input: atmel_mxt_ts - Handle multiple input reports in one message

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 18 Jul 2013 19:18:24 +0200
From:	rydberg@...omail.se
To:	Nick Dyer <nick.dyer@...ev.co.uk>
Cc:	Dmitry Torokhov <dmitry.torokhov@...il.com>,
	Daniel Kurtz <djkurtz@...omium.org>,
	Joonyoung Shim <jy0922.shim@...sung.com>,
	Alan Bowens <Alan.Bowens@...el.com>,
	linux-input@...r.kernel.org, linux-kernel@...r.kernel.org,
	Peter Meerwald <pmeerw@...erw.net>,
	Benson Leung <bleung@...omium.org>,
	Olof Johansson <olofj@...omium.org>
Subject: Re: [PATCH 25/51] Input: atmel_mxt_ts - Handle multiple input
 reports in one message

Hi Nick,

> Signed-off-by: Nick Dyer <nick.dyer@...ev.co.uk>
> Acked-by: Benson Leung <bleung@...omium.org>
> ---
>  drivers/input/touchscreen/atmel_mxt_ts.c |   34 +++++++++++++++++++++++-------
>  1 file changed, 26 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
> index 5a16383..8632133 100644
> --- a/drivers/input/touchscreen/atmel_mxt_ts.c
> +++ b/drivers/input/touchscreen/atmel_mxt_ts.c
> @@ -628,6 +628,12 @@ static void mxt_input_button(struct mxt_data *data, struct mxt_message *message)
>  	}
>  }
>  
> +static void mxt_input_sync(struct input_dev *input_dev)
> +{
> +	input_mt_report_pointer_emulation(input_dev, false);
> +	input_sync(input_dev);
> +}

Why not handle the enable_reporting and update_input logic here as
well? The logic is inconsistent with the rest of the patchset.

> +
>  static void mxt_input_touchevent(struct mxt_data *data,
>  				      struct mxt_message *message, int id)
>  {
> @@ -645,10 +651,12 @@ static void mxt_input_touchevent(struct mxt_data *data,
>  
>  	x = (message->message[1] << 4) | ((message->message[3] >> 4) & 0xf);
>  	y = (message->message[2] << 4) | ((message->message[3] & 0xf));
> +
> +	/* Handle 10/12 bit switching */
>  	if (data->max_x < 1024)
> -		x = x >> 2;
> +		x >>= 2;
>  	if (data->max_y < 1024)
> -		y = y >> 2;
> +		y >>= 2;

Unrelated changes.

>  
>  	area = message->message[4];
>  	amplitude = message->message[5];
> @@ -667,14 +675,26 @@ static void mxt_input_touchevent(struct mxt_data *data,
>  		x, y, area, amplitude);
>  
>  	input_mt_slot(input_dev, id);
> -	input_mt_report_slot_state(input_dev, MT_TOOL_FINGER,
> -				   status & MXT_T9_DETECT);
>  
>  	if (status & MXT_T9_DETECT) {
> +		/* Multiple bits may be set if the host is slow to read the
> +		 * status messages, indicating all the events that have
> +		 * happened */
> +		if (status & MXT_T9_RELEASE) {
> +			input_mt_report_slot_state(input_dev,
> +						   MT_TOOL_FINGER, 0);
> +			mxt_input_sync(input_dev);

What are the guarantees that nobody else expects the frame to not be
cut off here? What is the update_input state after this operation?

> +		}
> +
> +		/* Touch active */
> +		input_mt_report_slot_state(input_dev, MT_TOOL_FINGER, 1);
>  		input_report_abs(input_dev, ABS_MT_POSITION_X, x);
>  		input_report_abs(input_dev, ABS_MT_POSITION_Y, y);
>  		input_report_abs(input_dev, ABS_MT_PRESSURE, amplitude);
>  		input_report_abs(input_dev, ABS_MT_TOUCH_MAJOR, area);
> +	} else {
> +		/* Touch no longer active, close out slot */
> +		input_mt_report_slot_state(input_dev, MT_TOOL_FINGER, 0);
>  	}
>  }
>  
> @@ -732,10 +752,8 @@ static irqreturn_t mxt_process_messages_until_invalid(struct mxt_data *data)
>  		}
>  	} while (reportid != 0xff);
>  
> -	if (data->enable_reporting && update_input) {
> -		input_mt_report_pointer_emulation(data->input_dev, false);
> -		input_sync(data->input_dev);
> -	}
> +	if (data->enable_reporting && update_input)
> +		mxt_input_sync(data->input_dev);
>  
>  	return IRQ_HANDLED;
>  }
> -- 
> 1.7.10.4
> 

Thanks,
Henrik
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/