| Message ID | 1432101429-28023-1-git-send-email-adam.lee@canonical.com |
|---|---|
| State | New |
| Headers | show |
On Wed, May 20, 2015 at 01:57:09PM +0800, Adam Lee wrote: > From: Benjamin Tissoires <benjamin.tissoires@redhat.com> > > BugLink: https://bugs.launchpad.net/bugs/1456881 > > Touchpads that have only one button are called clickpads and should > be advertised as such by the kernel. > > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> > Tested-by: Jason Ekstrand <jason@jlekstrand.net> > Signed-off-by: Jiri Kosina <jkosina@suse.cz> > (cherry picked from commit 015fdaa9f8edd89a456b3331088e1b77ebdad9d0) > Signed-off-by: Adam Lee <adam.lee@canonical.com> This seems fine. As noted on the bug there's also my patch "HID: multitouch: Add support for button type usage" which does the same thing via a different detection mechanism. It may be that one or the other doesn't work for some machines, but until we have an example of such a machine I think that taking either one is sufficient. Acked-by: Seth Forshee <seth.forshee@canonical.com>
Applied to Trusty and Utopic master-next branch.
On Thu, May 28, 2015 at 07:48:23AM -0500, Seth Forshee wrote: > This doesn't make any sense to me. All of the detection done by this > patch happens when the device is probed, and once INPUT_PROP_BUTTONPAD > is set it shouldn't get cleared later due to heavy load. The device is > defined in the ACPI tables, so it's not going to be vanishing then > reappearing to be reprobed. > > So I'm skeptical that this patch really fixes the problem. Not that I > think it will do any harm, but I can't understand how it would possibly > help. > > Seth Thanks, Seth. Will paste your comment into launchpad and let's see the following updates. (I don't have the physical access to that machine
On Fri, May 29, 2015 at 10:03:36AM +0800, Adam Lee wrote: > On Thu, May 28, 2015 at 07:48:23AM -0500, Seth Forshee wrote: > > This doesn't make any sense to me. All of the detection done by this > > patch happens when the device is probed, and once INPUT_PROP_BUTTONPAD > > is set it shouldn't get cleared later due to heavy load. The device is > > defined in the ACPI tables, so it's not going to be vanishing then > > reappearing to be reprobed. > > > > So I'm skeptical that this patch really fixes the problem. Not that I > > think it will do any harm, but I can't understand how it would possibly > > help. > > > > Seth > > Thanks, Seth. Will paste your comment into launchpad and let's see the > following updates. (I don't have the physical access to that machine If I'm understanding the bug comments it sounds like additional testing has shown that the patch doesn't actually fix the problem. Based on that I'm nacking these patches.
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c index d83b1e8..f5ff876 100644 --- a/drivers/hid/hid-multitouch.c +++ b/drivers/hid/hid-multitouch.c @@ -111,6 +111,7 @@ struct mt_device { __u8 touches_by_report; /* how many touches are present in one report: * 1 means we should use a serial protocol * > 1 means hybrid (multitouch) protocol */ + __u8 buttons_count; /* number of physical buttons per touchpad */ bool serial_maybe; /* need to check for serial protocol */ bool curvalid; /* is the current contact valid? */ unsigned mt_flags; /* flags to pass to input-mt */ @@ -418,6 +419,10 @@ static int mt_touch_input_mapping(struct hid_device *hdev, struct hid_input *hi, (usage->hid & HID_USAGE_PAGE) == HID_UP_BUTTON) td->mt_flags |= INPUT_MT_POINTER; + /* count the buttons on touchpads */ + if ((usage->hid & HID_USAGE_PAGE) == HID_UP_BUTTON) + td->buttons_count++; + if (usage->usage_index) prev_usage = &field->usage[usage->usage_index - 1]; @@ -767,6 +772,10 @@ static void mt_touch_input_configured(struct hid_device *hdev, if (cls->quirks & MT_QUIRK_NOT_SEEN_MEANS_UP) td->mt_flags |= INPUT_MT_DROP_UNUSED; + /* check for clickpads */ + if ((td->mt_flags & INPUT_MT_POINTER) && (td->buttons_count == 1)) + __set_bit(INPUT_PROP_BUTTONPAD, input->propbit); + input_mt_init_slots(input, td->maxcontacts, td->mt_flags); td->mt_flags = 0;