Re: [PATCH v2 1/1] HID: multitouch: enable input devices on Surface Book
flat view
From: Benjamin Tissoires <hidden>
Date: 2016-05-11 15:33:53
Subsystem:
hid core layer, the rest · Maintainers:
Jiri Kosina, Benjamin Tissoires, Linus Torvalds
On May 11 2016 or thereabouts, Andy Shevchenko wrote:
Microsoft Surface Book has a nulti-functional input device attached to USB as a HID device, in particular it includes keyboard and touchpad. Enable it here. The change has been tested on bare metal. Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Thanks for the respin. I finally had a look at the type cover and understood why the LEDs are not working while everything else is working. The issue is that the quirk HID_QUIRK_MULTI_INPUT is incompatible with LEDs. With this quirk activated, we iterate over the INPUT reports, allocate a new input for each and register it. Then we iterate over the OUTPUT reports (which contains the LED), and allocate/register an input for it too. In the end we have 2 keyboard input devices, and the first one doesn't have the LED capabilities attached, which means X11 can't turn the LED on/off. A solution would be to rework the way we iterate over the INPUT/OUTPUT reports by making sure we associate INPUT & OUTPUT reports on the same hidinput node. It will require a little work in hid-input.c but should be beneficial for everybody. Also, now that I get this, I wouldn't mind having hid-multitouch accepting by default keyboards and consumer devices. A patch like ---
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index c741f5e..cd385c0 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c@@ -835,6 +835,8 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi, if (!td->mtclass.export_all_inputs && field->application != HID_DG_TOUCHSCREEN && field->application != HID_DG_PEN && + field->application != HID_GD_KEYBOARD && + field->application != HID_CP_CONSUMER_CONTROL && field->application != HID_DG_TOUCHPAD) return -1; ---
Would be acceptable as we would still remove the mouse collection but keep the rest. The good part is that you won't need to add a special entry for the surface cover as it will be handle properly directly. Once we fix hid-input to properly iterate over the reports, we should be able to move the other Type cover from hid-microsoft to hid-multitouch (i.e. remove their special handling in hid-core.c). Cheers, Benjamin
quoted hunk ↗ jump to hunk
--- In v2: - combine to one patch and remove unneded hunk drivers/hid/hid-ids.h | 1 + drivers/hid/hid-multitouch.c | 5 +++++ drivers/hid/usbhid/hid-quirks.c | 1 + 3 files changed, 7 insertions(+)diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h index 3eec09a1..1bbc795 100644 --- a/drivers/hid/hid-ids.h +++ b/drivers/hid/hid-ids.h@@ -704,6 +704,7 @@ #define USB_DEVICE_ID_MS_TYPE_COVER_PRO_3_2 0x07e2 #define USB_DEVICE_ID_MS_TYPE_COVER_PRO_3_JP 0x07dd #define USB_DEVICE_ID_MS_TYPE_COVER_3 0x07de +#define USB_DEVICE_ID_MS_SURFACE_BOOK 0x07cd #define USB_DEVICE_ID_MS_POWER_COVER 0x07da #define USB_VENDOR_ID_MOJO 0x8282diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c index c741f5e..c3df02d 100644 --- a/drivers/hid/hid-multitouch.c +++ b/drivers/hid/hid-multitouch.c@@ -1377,6 +1377,11 @@ static const struct hid_device_id mt_devices[] = { MT_USB_DEVICE(USB_VENDOR_ID_ILITEK, USB_DEVICE_ID_ILITEK_MULTITOUCH) }, + /* Microsoft Surface Book */ + { .driver_data = MT_CLS_EXPORT_ALL_INPUTS, + MT_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, + USB_DEVICE_ID_MS_SURFACE_BOOK) }, + /* MosArt panels */ { .driver_data = MT_CLS_CONFIDENCE_MINUS_ONE, MT_USB_DEVICE(USB_VENDOR_ID_ASUS,diff --git a/drivers/hid/usbhid/hid-quirks.c b/drivers/hid/usbhid/hid-quirks.c index b4b8c6a..6940956 100644 --- a/drivers/hid/usbhid/hid-quirks.c +++ b/drivers/hid/usbhid/hid-quirks.c@@ -99,6 +99,7 @@ static const struct hid_blacklist { { USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_TYPE_COVER_PRO_3_2, HID_QUIRK_NO_INIT_REPORTS }, { USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_TYPE_COVER_PRO_3_JP, HID_QUIRK_NO_INIT_REPORTS }, { USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_TYPE_COVER_3, HID_QUIRK_NO_INIT_REPORTS }, + { USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_SURFACE_BOOK, HID_QUIRK_NO_INIT_REPORTS }, { USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_POWER_COVER, HID_QUIRK_NO_INIT_REPORTS }, { USB_VENDOR_ID_MSI, USB_DEVICE_ID_MSI_GT683R_LED_PANEL, HID_QUIRK_NO_INIT_REPORTS }, { USB_VENDOR_ID_NEXIO, USB_DEVICE_ID_NEXIO_MULTITOUCH_PTI0750, HID_QUIRK_NO_INIT_REPORTS },-- 2.8.1