Re: [PATCH v2] HID: ite: Only bind to keyboard USB interface on Acer SW5-012 keyboard dock

[Hans de Goede <hdegoede@redhat.com>]

HI,

On 03-02-2020 16:14, Benjamin Tissoires wrote:
> On Sat, Feb 1, 2020 at 12:56 PM Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> Commit 8f18eca9ebc5 ("HID: ite: Add USB id match for Acer SW5-012 keyboard
>> dock") added the USB id for the Acer SW5-012's keyboard dock to the
>> hid-ite driver to fix the rfkill driver not working.
>>
>> Most keyboard docks with an ITE 8595 keyboard/touchpad controller have the
>> "Wireless Radio Control" bits which need the special hid-ite driver on the
>> second USB interface (the mouse interface) and their touchpad only supports
>> mouse emulation, so using generic hid-input handling for anything but
>> the "Wireless Radio Control" bits is fine. On these devices we simply bind
>> to all USB interfaces.
>>
>> But unlike other ITE8595 using keyboard docks, the Acer Aspire Switch 10
>> (SW5-012)'s touchpad not only does mouse emulation it also supports
>> HID-multitouch and all the keys including the "Wireless Radio Control"
>> bits have been moved to the first USB interface (the keyboard intf).
>>
>> So we need hid-ite to handle the first (keyboard) USB interface and have
>> it NOT bind to the second (mouse) USB interface so that that can be
>> handled by hid-multitouch.c and we get proper multi-touch support.
>>
>> This commit changes the hid_device_id for the SW5-012 keyboard dock to
>> only match on hid devices from the HID_GROUP_GENERIC group, this way
>> hid-ite will not bind the the mouse/multi-touch interface which has
>> HID_GROUP_MULTITOUCH_WIN_8 as group.
>> This fixes the regression to mouse-emulation mode introduced by adding
>> the keyboard dock USB id.
>>
>> Cc: stable@vger.kernel.org
>> Fixes: 8f18eca9ebc5 ("HID: ite: Add USB id match for Acer SW5-012 keyboard dock")
>> Reported-by: Zdeněk Rampas <zdenda.rampas@gmail.com>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>> Changes in v2:
>> - Extend hid_device_id to also match on the HID_GROUP_GENERIC group,
>>    instead of adding a match callback which peeks at the USB descriptors
> 
> Thanks for the quick revision.
> 
> Applied to for-5.6/upstream-fixes
> 
> And for the record, 2 MR have been added to hid-tools for regression testing:
> - https://gitlab.freedesktop.org/libevdev/hid-tools/merge_requests/70
> (keyboard and wifi key)
> - https://gitlab.freedesktop.org/libevdev/hid-tools/merge_requests/69
> (touchpad, which currently fails on Linux master unless this patch
> gets in)

Cool, thank you for doing that.

Regards,

Hans


>> ---
>>   drivers/hid/hid-ite.c | 5 +++--
>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/hid/hid-ite.c b/drivers/hid/hid-ite.c
>> index c436e12feb23..6c55682c5974 100644
>> --- a/drivers/hid/hid-ite.c
>> +++ b/drivers/hid/hid-ite.c
>> @@ -41,8 +41,9 @@ static const struct hid_device_id ite_devices[] = {
>>          { HID_USB_DEVICE(USB_VENDOR_ID_ITE, USB_DEVICE_ID_ITE8595) },
>>          { HID_USB_DEVICE(USB_VENDOR_ID_258A, USB_DEVICE_ID_258A_6A88) },
>>          /* ITE8595 USB kbd ctlr, with Synaptics touchpad connected to it. */
>> -       { HID_USB_DEVICE(USB_VENDOR_ID_SYNAPTICS,
>> -                        USB_DEVICE_ID_SYNAPTICS_ACER_SWITCH5_012) },
>> +       { HID_DEVICE(BUS_USB, HID_GROUP_GENERIC,
>> +                    USB_VENDOR_ID_SYNAPTICS,
>> +                    USB_DEVICE_ID_SYNAPTICS_ACER_SWITCH5_012) },
>>          { }
>>   };
>>   MODULE_DEVICE_TABLE(hid, ite_devices);
>> --
>> 2.23.0
>>
>