* [PATCH v2 1/1] HID: multitouch: enable input devices on Surface Book
@ 2016-05-11 13:01 Andy Shevchenko
2016-05-11 15:33 ` Benjamin Tissoires
0 siblings, 1 reply; 2+ messages in thread
From: Andy Shevchenko @ 2016-05-11 13:01 UTC (permalink / raw)
To: linux-input, Benjamin Tissoires, Jiri Kosina, Mika Westerberg
Cc: Andy Shevchenko
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>
---
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 0x8282
diff --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
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH v2 1/1] HID: multitouch: enable input devices on Surface Book
2016-05-11 13:01 [PATCH v2 1/1] HID: multitouch: enable input devices on Surface Book Andy Shevchenko
@ 2016-05-11 15:33 ` Benjamin Tissoires
0 siblings, 0 replies; 2+ messages in thread
From: Benjamin Tissoires @ 2016-05-11 15:33 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: linux-input, Jiri Kosina, Mika Westerberg
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
> ---
> 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 0x8282
> diff --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
>
^ permalink raw reply related [flat|nested] 2+ messages in thread
end of thread, other threads:[~2016-05-11 15:33 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-05-11 13:01 [PATCH v2 1/1] HID: multitouch: enable input devices on Surface Book Andy Shevchenko
2016-05-11 15:33 ` Benjamin Tissoires
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).