diff mbox series

[v2,2/2] Input: goodix - Ignore spurious interrupts

Message ID 20200312145009.27449-2-dmastykin@astralinux.ru (mailing list archive)
State Superseded
Headers show
Series [v2,1/2] Input: goodix - Add support for more then one touch-key | expand

Commit Message

Dmitry Mastykin March 12, 2020, 2:50 p.m. UTC
The goodix panel sends spurious interrupts after a 'finger up' event,
which always cause a timeout.
The timeout was reported as touch_num == 0 and caused reading of
not ready buffer and false key release event.
In this patch the timeout is reported as ENOMSG and not processed.

Signed-off-by: Dmitry Mastykin <dmastykin@astralinux.ru>
---
Changes in v2:
- Improve commit message 
---
 drivers/input/touchscreen/goodix.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Hans de Goede March 13, 2020, 11:10 a.m. UTC | #1
Hi,

On 3/12/20 3:50 PM, Dmitry Mastykin wrote:
> The goodix panel sends spurious interrupts after a 'finger up' event,
> which always cause a timeout.
> The timeout was reported as touch_num == 0 and caused reading of
> not ready buffer and false key release event.
> In this patch the timeout is reported as ENOMSG and not processed.
> 
> Signed-off-by: Dmitry Mastykin <dmastykin@astralinux.ru>

Patch looks good to me:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans




> ---
> Changes in v2:
> - Improve commit message
> ---
>   drivers/input/touchscreen/goodix.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
> index daf1781..0e14719 100644
> --- a/drivers/input/touchscreen/goodix.c
> +++ b/drivers/input/touchscreen/goodix.c
> @@ -329,7 +329,7 @@ static int goodix_ts_read_input_report(struct goodix_ts_data *ts, u8 *data)
>   	 * The Goodix panel will send spurious interrupts after a
>   	 * 'finger up' event, which will always cause a timeout.
>   	 */
> -	return 0;
> +	return -ENOMSG;
>   }
>   
>   static void goodix_ts_report_touch_8b(struct goodix_ts_data *ts, u8 *coor_data)
>
Bastien Nocera March 13, 2020, 1:28 p.m. UTC | #2
On Thu, 2020-03-12 at 17:50 +0300, Dmitry Mastykin wrote:
> The goodix panel sends spurious interrupts after a 'finger up' event,
> which always cause a timeout.
> The timeout was reported as touch_num == 0 and caused reading of
> not ready buffer and false key release event.
> In this patch the timeout is reported as ENOMSG and not processed.

I think a better commit message would be:
"
Input: goodix - Fix spurious key release events

The goodix panel sends spurious interrupts after a 'finger up' event,
which always cause a timeout.
We were exiting the interrupt handler by reporting touch_num == 0, but
this was still processed as valid and caused the code to use the
uninitialised point_data, creating spurious key release events.

Report an error from the interrupt handler so as to avoid processing
invalid point_data further.
"

Looks good otherwise.

> 
> Signed-off-by: Dmitry Mastykin <dmastykin@astralinux.ru>
> ---
> Changes in v2:
> - Improve commit message 
> ---
>  drivers/input/touchscreen/goodix.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/input/touchscreen/goodix.c
> b/drivers/input/touchscreen/goodix.c
> index daf1781..0e14719 100644
> --- a/drivers/input/touchscreen/goodix.c
> +++ b/drivers/input/touchscreen/goodix.c
> @@ -329,7 +329,7 @@ static int goodix_ts_read_input_report(struct
> goodix_ts_data *ts, u8 *data)
>  	 * The Goodix panel will send spurious interrupts after a
>  	 * 'finger up' event, which will always cause a timeout.
>  	 */
> -	return 0;
> +	return -ENOMSG;
>  }
>  
>  static void goodix_ts_report_touch_8b(struct goodix_ts_data *ts, u8
> *coor_data)
Dmitry Mastykin March 13, 2020, 2:20 p.m. UTC | #3
Hi,
Agree, it's better like this, especially the header.
Kind regards
Dmitry Mastykin

On 3/13/20 4:28 PM, Bastien Nocera wrote:
> On Thu, 2020-03-12 at 17:50 +0300, Dmitry Mastykin wrote:
>> The goodix panel sends spurious interrupts after a 'finger up' event,
>> which always cause a timeout.
>> The timeout was reported as touch_num == 0 and caused reading of
>> not ready buffer and false key release event.
>> In this patch the timeout is reported as ENOMSG and not processed.
> 
> I think a better commit message would be:
> "
> Input: goodix - Fix spurious key release events
> 
> The goodix panel sends spurious interrupts after a 'finger up' event,
> which always cause a timeout.
> We were exiting the interrupt handler by reporting touch_num == 0, but
> this was still processed as valid and caused the code to use the
> uninitialised point_data, creating spurious key release events.
> 
> Report an error from the interrupt handler so as to avoid processing
> invalid point_data further.
> "
> 
> Looks good otherwise.
> 
>>
>> Signed-off-by: Dmitry Mastykin <dmastykin@astralinux.ru>
>> ---
>> Changes in v2:
>> - Improve commit message
>> ---
>>   drivers/input/touchscreen/goodix.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/input/touchscreen/goodix.c
>> b/drivers/input/touchscreen/goodix.c
>> index daf1781..0e14719 100644
>> --- a/drivers/input/touchscreen/goodix.c
>> +++ b/drivers/input/touchscreen/goodix.c
>> @@ -329,7 +329,7 @@ static int goodix_ts_read_input_report(struct
>> goodix_ts_data *ts, u8 *data)
>>   	 * The Goodix panel will send spurious interrupts after a
>>   	 * 'finger up' event, which will always cause a timeout.
>>   	 */
>> -	return 0;
>> +	return -ENOMSG;
>>   }
>>   
>>   static void goodix_ts_report_touch_8b(struct goodix_ts_data *ts, u8
>> *coor_data)
>
diff mbox series

Patch

diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
index daf1781..0e14719 100644
--- a/drivers/input/touchscreen/goodix.c
+++ b/drivers/input/touchscreen/goodix.c
@@ -329,7 +329,7 @@  static int goodix_ts_read_input_report(struct goodix_ts_data *ts, u8 *data)
 	 * The Goodix panel will send spurious interrupts after a
 	 * 'finger up' event, which will always cause a timeout.
 	 */
-	return 0;
+	return -ENOMSG;
 }
 
 static void goodix_ts_report_touch_8b(struct goodix_ts_data *ts, u8 *coor_data)