Surface Pro 2 USB composite device handling

PATCH New Derya 3 participants 6 messages Mar 9, 2014 – Mar 11, 2014

Subsystems (2)

HID USB HID/hidbp drivers (USB keyboards, mice, remote controls, ...)

lore

Share & tools

lore

Derya

author

2014-03-09 17:51 UTC #0 lore raw

PATCH v2 1/1 Surface Pro 2 USB composite device handling

This patch set is a reworked version of the one, that I submitted on Friday. Followed the Benjamin's suggestions (many thanks)

- The first patch reverses commit 117309c51dca42121f70cacec801511b76acf75c
- The second readds the product ids and ensures the load of hid-core instead of hid-multiouch

Derya

author

2014-03-09 17:53 UTC #1 lore raw

PATCH 1/2 Revert "HID: microsoft: Add ID's for Surface Type/Touch, Cover 2"

This reverts commit 117309c51dca42121f70cacec801511b76acf75c.

The MS Surface Pro 2 has an USB composite device with 3 interfaces
- interface 0 - sensor hub
- interface 1 - wacom digitizer
- interface 2 - the keyboard cover, if one is attached
This USB composite device changes it product id dependent on if and which keyboard cover is attached. Adding the covers to hid_have_special_driver prevents loading the right hid drivers for the other two interfaces, all 3 get loaded with hid-microsoft. We don't even need hid-microsoft for the keyboards. We have to revert this to load the right hid modules for each interface.

Signed-off-by Derya <[email protected]>

---
 drivers/hid/hid-core.c      | 2 --
 drivers/hid/hid-ids.h       | 2 --
 drivers/hid/hid-microsoft.c | 4 ----
 3 files changed, 8 deletions(-)

drivers/hid/hid-core.c +0 -0

index cc32a6f..bb5c494 100644
@@ -1780,8 +1780,6 @@ static const struct hid_device_id hid_have_special_driver[] = {
     { HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT,

USB_DEVICE_ID_MS_PRESENTER_8K_USB) },
{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT,
USB_DEVICE_ID_MS_DIGITAL_MEDIA_3K) },
{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT,
USB_DEVICE_ID_WIRELESS_OPTICAL_DESKTOP_3_0) },
- { HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_TYPE_COVER_2) },
- { HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_TOUCH_COVER_2) },
{ HID_USB_DEVICE(USB_VENDOR_ID_MONTEREY,
USB_DEVICE_ID_GENIUS_KB29E) },
{ HID_USB_DEVICE(USB_VENDOR_ID_NTRIG,
USB_DEVICE_ID_NTRIG_TOUCH_SCREEN) },
{ HID_USB_DEVICE(USB_VENDOR_ID_NTRIG,
USB_DEVICE_ID_NTRIG_TOUCH_SCREEN_1) },

drivers/hid/hid-ids.h +0 -2

index 22f28d6..07cd28c 100644
@@ -624,8 +624,6 @@
 #define USB_DEVICE_ID_MS_PRESENTER_8K_USB    0x0713
 #define USB_DEVICE_ID_MS_DIGITAL_MEDIA_3K    0x0730
 #define USB_DEVICE_ID_MS_COMFORT_MOUSE_4500    0x076c
-#define USB_DEVICE_ID_MS_TOUCH_COVER_2    0x07a7
-#define USB_DEVICE_ID_MS_TYPE_COVER_2    0x07a9
 
 #define USB_VENDOR_ID_MOJO        0x8282
 #define USB_DEVICE_ID_RETRO_ADAPTER    0x3201

drivers/hid/hid-microsoft.c +0 -0

index 404a3a8..c6ef6ee 100644
@@ -208,10 +208,6 @@ static const struct hid_device_id ms_devices[] = {
         .driver_data = MS_NOGET },
     { HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT,

USB_DEVICE_ID_MS_COMFORT_MOUSE_4500),
.driver_data = MS_DUPLICATE_USAGES },
- { HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_TYPE_COVER_2),
- .driver_data = 0 },
- { HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_TOUCH_COVER_2),
- .driver_data = 0 },

{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_MICROSOFT,
USB_DEVICE_ID_MS_PRESENTER_8K_BT),
.driver_data = MS_PRESENTER },

Signature · 1 line

1.8.3.2

Benjamin Tissoires

Acked-by on v1 1/2

2014-03-10 20:22 UTC #2 lore raw

Re: [PATCH 1/2] Revert "HID: microsoft: Add ID's for Surface Type/Touch, Cover 2"

Quoted 13 lines · Derya

This reverts commit 117309c51dca42121f70cacec801511b76acf75c.
The MS Surface Pro 2 has an USB composite device with 3 interfaces
- interface 0 - sensor hub
- interface 1 - wacom digitizer
- interface 2 - the keyboard cover, if one is attached
This USB composite device changes it product id dependent on if and which keyboard cover is attached. Adding the covers to hid_have_special_driver prevents loading the right hid drivers for the other two interfaces, all 3 get loaded with hid-microsoft. We don't even need hid-microsoft for the keyboards. We have to revert this to load the right hid modules for each interface.

Quoted 1 line

Signed-off-by Derya <[email protected]>

Acked-by Benjamin Tissoires <[email protected]>

Hoewever, Derya, you should reconsider re-sending the whole series through "git send-email" or configuring your e-mail client according to Documentation/email-clients.txt . This series has the lines wrapped and tabs are converted to spaces, which means that Jiri ca not take the patch without reformatting them to his tree.

Thanks,
Benjamin

Quoted 62 lines

---
 drivers/hid/hid-core.c      | 2 --
 drivers/hid/hid-ids.h       | 2 --
 drivers/hid/hid-microsoft.c | 4 ----
 3 files changed, 8 deletions(-)

drivers/hid/hid-core.c +0 -0

index cc32a6f..bb5c494 100644
@@ -1780,8 +1780,6 @@ static const struct hid_device_id
 hid_have_special_driver[] = {
     { HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT,
 USB_DEVICE_ID_MS_PRESENTER_8K_USB) },
     { HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT,
 USB_DEVICE_ID_MS_DIGITAL_MEDIA_3K) },
     { HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT,

USB_DEVICE_ID_WIRELESS_OPTICAL_DESKTOP_3_0) },
- { HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT,
USB_DEVICE_ID_MS_TYPE_COVER_2) },
- { HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT,
USB_DEVICE_ID_MS_TOUCH_COVER_2) },
{ HID_USB_DEVICE(USB_VENDOR_ID_MONTEREY, USB_DEVICE_ID_GENIUS_KB29E) }, { HID_USB_DEVICE(USB_VENDOR_ID_NTRIG, USB_DEVICE_ID_NTRIG_TOUCH_SCREEN)
},
{ HID_USB_DEVICE(USB_VENDOR_ID_NTRIG,
USB_DEVICE_ID_NTRIG_TOUCH_SCREEN_1) },

drivers/hid/hid-ids.h +0 -2

index 22f28d6..07cd28c 100644
@@ -624,8 +624,6 @@
 #define USB_DEVICE_ID_MS_PRESENTER_8K_USB    0x0713
 #define USB_DEVICE_ID_MS_DIGITAL_MEDIA_3K    0x0730
 #define USB_DEVICE_ID_MS_COMFORT_MOUSE_4500    0x076c
-#define USB_DEVICE_ID_MS_TOUCH_COVER_2    0x07a7
-#define USB_DEVICE_ID_MS_TYPE_COVER_2    0x07a9
 
 #define USB_VENDOR_ID_MOJO        0x8282
 #define USB_DEVICE_ID_RETRO_ADAPTER    0x3201

drivers/hid/hid-microsoft.c +0 -4

index 404a3a8..c6ef6ee 100644
@@ -208,10 +208,6 @@ static const struct hid_device_id ms_devices[] = {
         .driver_data = MS_NOGET },
     { HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT,
 USB_DEVICE_ID_MS_COMFORT_MOUSE_4500),
         .driver_data = MS_DUPLICATE_USAGES },
-    { HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT,
 USB_DEVICE_ID_MS_TYPE_COVER_2),
-        .driver_data = 0 },
-    { HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT,
 USB_DEVICE_ID_MS_TOUCH_COVER_2),
-        .driver_data = 0 },

{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_MICROSOFT,
USB_DEVICE_ID_MS_PRESENTER_8K_BT),
.driver_data = MS_PRESENTER },

Signature · 1 line

1.8.3.2

Jiri Kosina

2014-03-11 15:54 UTC #3 lore raw

Re: [PATCH 1/2] Revert "HID: microsoft: Add ID's for Surface Type/Touch, Cover 2"

Quoted 13 lines · Benjamin Tissoires

Quoted 11 lines
The MS Surface Pro 2 has an USB composite device with 3 interfaces
- interface 0 - sensor hub
- interface 1 - wacom digitizer
- interface 2 - the keyboard cover, if one is attached
This USB composite device changes it product id dependent on if and which keyboard cover is attached. Adding the covers to hid_have_special_driver prevents loading the right hid drivers for the other two interfaces, all 3 get loaded with hid-microsoft. We don't even need hid-microsoft for the keyboards. We have to revert this to load the right hid modules for each interface.
Quoted 1 line
Signed-off-by Derya <[email protected]>

Quoted 7 lines

Acked-by Benjamin Tissoires <[email protected]>
Hoewever, Derya, you should reconsider re-sending the whole series through "git send-email" or configuring your e-mail client according to Documentation/email-clients.txt . This series has the lines wrapped and tabs are converted to spaces, which means that Jiri ca not take the patch without reformatting them to his tree.

Indeed, the patches have been mangled by your mail client beyond repair.

Could you please resend them (and include Benjamin's Ack), so that I can queue them?

Thanks!

Signature · 2 lines

Jiri Kosina
SUSE Labs

Derya

author

2014-03-09 17:56 UTC #4 lore raw

PATCH 2/2 Surface Pro 2 USB composite device handling

In reply to Derya

Surface Pro 2 USB composite device handling

(device changes it product id with attached keyboard cover)
- Adds MS Surface Pro 2's composite device ids, with and without attached keyboard, to the hid-ids
- Ensures to load hid-core instead of hid-multitouch for the keyboard covers
- Adds HID_QUIRK_NO_INIT_INPUT_REPORTS to prevent USB submit urb failure during keyboard cover (de)attaching

Signed-off-by Derya <[email protected]>

---
 drivers/hid/hid-core.c          | 9 +++++++++
 drivers/hid/hid-ids.h           | 3 +++
 drivers/hid/usbhid/hid-quirks.c | 3 +++
 3 files changed, 15 insertions(+)

drivers/hid/hid-core.c +9 -0

index bb5c494..f4e4820 100644
@@ -775,6 +775,15 @@ static int hid_scan_report(struct hid_device *hid)
     if ((parser->scan_flags & HID_SCAN_FLAG_MT_WIN_8) &&
         (hid->group == HID_GROUP_MULTITOUCH))
         hid->group = HID_GROUP_MULTITOUCH_WIN_8;
+
+    /*
+     * Handle vendor specific handlings
+     */
+    if ((hid->vendor == USB_VENDOR_ID_MICROSOFT) &&
+        (hid->product == USB_DEVICE_ID_MS_TYPE_COVER_2 ||
+        hid->product == USB_DEVICE_ID_MS_TOUCH_COVER_2) &&
+        (hid->group == HID_GROUP_MULTITOUCH))
+        hid->group = HID_GROUP_GENERIC;
 
     vfree(parser);
     return 0;

drivers/hid/hid-ids.h +3 -0

index 07cd28c..f61c0d8 100644
@@ -624,6 +624,9 @@
 #define USB_DEVICE_ID_MS_PRESENTER_8K_USB    0x0713
 #define USB_DEVICE_ID_MS_DIGITAL_MEDIA_3K    0x0730
 #define USB_DEVICE_ID_MS_COMFORT_MOUSE_4500    0x076c
+#define USB_DEVICE_ID_MS_SURFACE_PRO_2    0x0799
+#define USB_DEVICE_ID_MS_TOUCH_COVER_2    0x07a7
+#define USB_DEVICE_ID_MS_TYPE_COVER_2    0x07a9
 
 #define USB_VENDOR_ID_MOJO        0x8282
 #define USB_DEVICE_ID_RETRO_ADAPTER    0x3201

drivers/hid/usbhid/hid-quirks.c +0 -0

index dbd8387..7c4f86e 100644
@@ -73,6 +73,9 @@ static const struct hid_blacklist {
     { USB_VENDOR_ID_FORMOSA, USB_DEVICE_ID_FORMOSA_IR_RECEIVER,

HID_QUIRK_NO_INIT_REPORTS },
{ USB_VENDOR_ID_FREESCALE, USB_DEVICE_ID_FREESCALE_MX28,
HID_QUIRK_NOGET },
{ USB_VENDOR_ID_MGE, USB_DEVICE_ID_MGE_UPS, HID_QUIRK_NOGET },

+    { USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_SURFACE_PRO_2, HID_QUIRK_NO_INIT_INPUT_REPORTS },
+    { USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_TYPE_COVER_2, HID_QUIRK_NO_INIT_INPUT_REPORTS },
+    { USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_TOUCH_COVER_2, HID_QUIRK_NO_INIT_INPUT_REPORTS },
     { USB_VENDOR_ID_MSI, USB_DEVICE_ID_MSI_GX680R_LED_PANEL,

HID_QUIRK_NO_INIT_REPORTS },
{ USB_VENDOR_ID_NEXIO, USB_DEVICE_ID_NEXIO_MULTITOUCH_PTI0750,
HID_QUIRK_NO_INIT_REPORTS },
{ USB_VENDOR_ID_NOVATEK, USB_DEVICE_ID_NOVATEK_MOUSE,
HID_QUIRK_NO_INIT_REPORTS },

Signature · 1 line

1.8.3.2

Benjamin Tissoires

Reviewed-by on v1 2/2

2014-03-10 20:23 UTC #5 lore raw

Quoted 10 lines · Derya

Surface Pro 2 USB composite device handling
(device changes it product id with attached keyboard cover)
- Adds MS Surface Pro 2's composite device ids, with and without attached keyboard, to the hid-ids
- Ensures to load hid-core instead of hid-multitouch for the keyboard covers
- Adds HID_QUIRK_NO_INIT_INPUT_REPORTS to prevent USB submit urb failure during keyboard cover (de)attaching
Signed-off-by Derya <[email protected]>

Reviewed-by Benjamin Tissoires <[email protected]>

Cheers,
Benjamin

Quoted 66 lines

---
 drivers/hid/hid-core.c          | 9 +++++++++
 drivers/hid/hid-ids.h           | 3 +++
 drivers/hid/usbhid/hid-quirks.c | 3 +++
 3 files changed, 15 insertions(+)

drivers/hid/hid-core.c +9 -0

index bb5c494..f4e4820 100644
@@ -775,6 +775,15 @@ static int hid_scan_report(struct hid_device *hid)
     if ((parser->scan_flags & HID_SCAN_FLAG_MT_WIN_8) &&
         (hid->group == HID_GROUP_MULTITOUCH))
         hid->group = HID_GROUP_MULTITOUCH_WIN_8;
+
+    /*
+     * Handle vendor specific handlings
+     */
+    if ((hid->vendor == USB_VENDOR_ID_MICROSOFT) &&
+        (hid->product == USB_DEVICE_ID_MS_TYPE_COVER_2 ||
+        hid->product == USB_DEVICE_ID_MS_TOUCH_COVER_2) &&
+        (hid->group == HID_GROUP_MULTITOUCH))
+        hid->group = HID_GROUP_GENERIC;
 
     vfree(parser);
     return 0;

drivers/hid/hid-ids.h +3 -0

index 07cd28c..f61c0d8 100644
@@ -624,6 +624,9 @@
 #define USB_DEVICE_ID_MS_PRESENTER_8K_USB    0x0713
 #define USB_DEVICE_ID_MS_DIGITAL_MEDIA_3K    0x0730
 #define USB_DEVICE_ID_MS_COMFORT_MOUSE_4500    0x076c
+#define USB_DEVICE_ID_MS_SURFACE_PRO_2    0x0799
+#define USB_DEVICE_ID_MS_TOUCH_COVER_2    0x07a7
+#define USB_DEVICE_ID_MS_TYPE_COVER_2    0x07a9
 
 #define USB_VENDOR_ID_MOJO        0x8282
 #define USB_DEVICE_ID_RETRO_ADAPTER    0x3201

drivers/hid/usbhid/hid-quirks.c +2 -0

index dbd8387..7c4f86e 100644
@@ -73,6 +73,9 @@ static const struct hid_blacklist {
     { USB_VENDOR_ID_FORMOSA, USB_DEVICE_ID_FORMOSA_IR_RECEIVER,
 HID_QUIRK_NO_INIT_REPORTS },
     { USB_VENDOR_ID_FREESCALE, USB_DEVICE_ID_FREESCALE_MX28,
 HID_QUIRK_NOGET },
     { USB_VENDOR_ID_MGE, USB_DEVICE_ID_MGE_UPS, HID_QUIRK_NOGET },
+    { USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_SURFACE_PRO_2,
 HID_QUIRK_NO_INIT_INPUT_REPORTS },
+    { USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_TYPE_COVER_2,

HID_QUIRK_NO_INIT_INPUT_REPORTS },
+ { USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_TOUCH_COVER_2,
HID_QUIRK_NO_INIT_INPUT_REPORTS },

     { USB_VENDOR_ID_MSI, USB_DEVICE_ID_MSI_GX680R_LED_PANEL,

Signature · 1 line

1.8.3.2