| Message ID | 20161105164458.GF2927@TopQuark.net (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
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...
On Sun, Nov 06, 2016 at 08:59:34PM +0100, Pali Rohár wrote: > 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 this part actually functions properly as-is because alps_process_packet_ss4_v2() always calls input_sync() after alps_decode_ss4_v2() calls input_report_*(). But I definitely agree this code path is confusing and inconsistent with the rest of alps.c. I'll take another stab at cleaning this up. Thanks, -Paul -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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) { + 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);
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> -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html