Re: [PATCH v2] Input: ALPS - Fix TrackStick support for SS5 hardware

["Pali Rohár" <pali.rohar@gmail.com>]

[-- Attachment #1: Type: Text/Plain, Size: 3153 bytes --]

On Saturday 05 November 2016 17:44:58 Paul Donohue wrote:
> The current Alps SS5 (SS4 v2) code generates bogus TouchPad events when 
> TrackStick packets are processed.
> 
> This causes the xorg synaptics driver to print 
> "unable to find touch point 0" and 
> "BUG: triggered 'if (priv->num_active_touches > priv->num_slots)'" 
> messages.  It also causes unexpected TouchPad button release and reclick 
> event sequences if the TrackStick is moved while holding a TouchPad 
> button.
> 
> This commit corrects the problem by adjusting
> alps_process_packet_ss4_v2() so that it only sends TrackStick reports
> when processing TrackStick packets.
> 
> Signed-off-by: Paul Donohue <linux-kernel@PaulSD.com>
> 
> diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
> index 6d7de9b..1d0302b 100644
> --- a/drivers/input/mouse/alps.c
> +++ b/drivers/input/mouse/alps.c
> @@ -1279,6 +1279,8 @@ static int alps_decode_ss4_v2(struct alps_fields *f,
>                         input_report_rel(priv->dev2, REL_Y, -y);
>                         input_report_abs(priv->dev2, ABS_PRESSURE, pressure);
>                 }
> +               f->first_mp = 0;
> +               f->is_mp = 0;
>                 break;
>  
>         case SS4_PACKET_ID_IDLE:
> @@ -1346,18 +1348,18 @@ static void alps_process_packet_ss4_v2(struct psmouse *psmouse)
>  
>         priv->multi_packet = 0;
>  
> -       alps_report_mt_data(psmouse, (f->fingers <= 4) ? f->fingers : 4);
> +       if ((packet[3] & 0x30) != 0x20) {

Ideally adds comment that this is test for SS4_PACKET_ID_STICK.

> +               alps_report_mt_data(psmouse, (f->fingers <= 4) ? f->fingers : 4);
>  
> -       input_mt_report_finger_count(dev, f->fingers);
> +               input_mt_report_finger_count(dev, f->fingers);
>  
> -       input_report_key(dev, BTN_LEFT, f->left);
> -       input_report_key(dev, BTN_RIGHT, f->right);
> -       input_report_key(dev, BTN_MIDDLE, f->middle);
> +               input_report_key(dev, BTN_LEFT, f->left);
> +               input_report_key(dev, BTN_RIGHT, f->right);
> +               input_report_key(dev, BTN_MIDDLE, f->middle);
>  
> -       input_report_abs(dev, ABS_PRESSURE, f->pressure);
> -       input_sync(dev);
> -
> -       if (priv->flags & ALPS_DUALPOINT) {
> +               input_report_abs(dev, ABS_PRESSURE, f->pressure);
> +               input_sync(dev);
> +       } else if (priv->flags & ALPS_DUALPOINT) {
>                 input_report_key(dev2, BTN_LEFT, f->ts_left);
>                 input_report_key(dev2, BTN_RIGHT, f->ts_right);
>                 input_report_key(dev2, BTN_MIDDLE, f->ts_middle);
> 

Otherwise this patch looks OK.

Anyway, I looked into alps.c source code and I see there another
problem with ss4 devices. Function alps_decode_ss4_v2() calls
input_report_*() functions without input_sync(). But all those
input_report_*() should be in alps_process_packet_ss4_v2().

Would you like to fix this problem?

I think due to this problem, original code contains that BUG reported
by you...

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]