From mboxrd@z Thu Jan 1 00:00:00 1970 From: Hui Wang Subject: Re: [PATCH] Input: synaptics - checking Extbit when passing ext_buttons to pt_port Date: Wed, 16 Mar 2016 09:33:21 +0800 Message-ID: <56E8B7E1.7030807@canonical.com> References: <1458013522-8139-1-git-send-email-hui.wang@canonical.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Return-path: Received: from youngberry.canonical.com ([91.189.89.112]:59219 "EHLO youngberry.canonical.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752756AbcCPBd1 (ORCPT ); Tue, 15 Mar 2016 21:33:27 -0400 In-Reply-To: Sender: linux-input-owner@vger.kernel.org List-Id: linux-input@vger.kernel.org To: Benjamin Tissoires Cc: Dmitry Torokhov , linux-input On 03/15/2016 10:21 PM, Benjamin Tissoires wrote: > Hi Hui, > > On Tue, Mar 15, 2016 at 4:45 AM, Hui Wang wrote: >> We found a problem on couples of Lenovo laptop models, when we press >> down the extend button of trackstick (IBM trackpoint), the driver will >> report both key_down and key_up events to the userspace, while we >> release that button, the driver won't report any events to the >> userspace. >> >> Through debugging, we found when we press down or release the extend >> button, the hardware will report more than one packets to the driver, >> only one packet is valid (Extbit is set to 1), the rest of the >> packets only contain zero. The current driver won't drop those >> invalid packets but regard them as ext_button packets and pass them >> to the pt_port, unfortunately the zero means key release, as a >> result the userspace get the key_up event immediately following the >> key_down event. >> >> Adding an Extbit check before passing ext_buttons to pt_port can >> drop all invalid packets and fix this problem. >> >> Signed-off-by: Hui Wang > Could you check if the patch I posted this morning fixes your problem?: > https://patchwork.kernel.org/patch/8587101/ > > The issue with yours is that it will change the behavior for all > generation of synaptics devices and might introduce regressions. > > Cheers, > Benjamin Yes, it can fix the problem on the machines I tested. By the way, according to the page 29 of Synaptics PS/2 TouchPad Interfacing Guide Rev. B, the extended button event is only reported when Extbit is set to 1, I think all generation firmware should follow this guide, why do we need to check the firmware version here? Thanks, Hui. > >> --- >> drivers/input/mouse/synaptics.c | 6 ++++-- >> 1 file changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c >> index 6025eb4..ad81140 100644 >> --- a/drivers/input/mouse/synaptics.c >> +++ b/drivers/input/mouse/synaptics.c >> @@ -880,9 +880,11 @@ static void synaptics_report_ext_buttons(struct psmouse *psmouse, >> /* >> * This generation of touchpads has the trackstick buttons >> * physically wired to the touchpad. Re-route them through >> - * the pass-through interface. >> + * the pass-through interface. And only pass extend button >> + * events to pt_port when ExtBit is 1. >> */ >> - if (!priv->pt_port) >> + if (!priv->pt_port || >> + !((psmouse->packet[0] ^ psmouse->packet[3]) & 0x02)) >> return; >> >> /* The trackstick expects at most 3 buttons */ >> -- >> 1.9.1 >> >> -- >> 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