HID: microsoft: Add Surface Pro 4 type cover support

PATCH In review Zhang Rui 2 participants 7 messages Dec 1, 2016 – Dec 6, 2016

Subsystems (2)

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

lore

Share & tools

lore

Zhang Rui

author

2016-12-01 11:17 UTC #0 lore raw

PATCH 1/1 HID: microsoft: Add Surface Pro 4 type cover support

Quoted 1 line

From edb2fc779c3e9c118686a3c177cfa304e95e0220 Mon Sep 17 00:00:00 2001

From: Zhang Rui <[email protected]>
Date: Wed, 30 Nov 2016 20:09:11 +0800
Subject: [PATCH] HID: microsoft: Add Surface Pro 4 type cover support

Adding support for the Microsoft Surface Pro 4 Type Cover.

This patch has been tested on my surface pro 4, keyboard starts working, while the touchpad does not.

I don't know which type of surface pro 4 I have, but the lsusb output shows it's ID is 07e8, and I use USB_DEVICE_ID_MS_TYPE_COVER_4_1 as the name, following the patch at
https://bugzilla.kernel.org/show_bug.cgi?id=135561#c0

At the same time, this patch also fixes a long suspend/resume delay problem when type cover is attached.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=135561
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=186181

Signed-off-by Zhang Rui <[email protected]>

---
drivers/hid/hid-core.c          | 2 ++
drivers/hid/hid-ids.h           | 1 +
drivers/hid/hid-microsoft.c     | 2 ++
drivers/hid/usbhid/hid-quirks.c | 1 +

 4 files changed, 6 insertions(+)

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

index 2b89c70..3c4085d 100644
@@ -728,6 +728,7 @@ static void hid_scan_collection(struct hid_parser *parser, unsigned type)

 	     hid->product == USB_DEVICE_ID_MS_TYPE_COVER_PRO_3_2 ||
 	     hid->product == USB_DEVICE_ID_MS_TYPE_COVER_PRO_3_JP ||
 	     hid->product == USB_DEVICE_ID_MS_TYPE_COVER_PRO_4_JP ||

+      hid->product == USB_DEVICE_ID_MS_TYPE_COVER_PRO_4_1 ||
     hid->product == USB_DEVICE_ID_MS_TYPE_COVER_3 ||
     hid->product == USB_DEVICE_ID_MS_POWER_COVER) &&
    hid->group == HID_GROUP_MULTITOUCH)
@@ -1984,6 +1985,7 @@ static const struct hid_device_id hid_have_special_driver[] = { { HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_TYPE_COVER_PRO_3_2) }, { HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_TYPE_COVER_PRO_3_JP) }, { HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_TYPE_COVER_PRO_4_JP) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_TYPE_COVER_PRO_4_1) },
{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_TYPE_COVER_3) }, { HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_DIGITAL_MEDIA_7K) }, { HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_DIGITAL_MEDIA_600) },

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

index 575aa65..fe19866 100644
@@ -719,6 +719,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_PRO_4_JP 0x07e9 +#define USB_DEVICE_ID_MS_TYPE_COVER_PRO_4_1 0x07e8 #define USB_DEVICE_ID_MS_TYPE_COVER_3 0x07de #define USB_DEVICE_ID_MS_POWER_COVER 0x07da

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

index c6cd392..df06e6c 100644
@@ -282,6 +282,8 @@ static const struct hid_device_id ms_devices[] = {

.driver_data = MS_HIDINPUT },
{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_TYPE_COVER_PRO_4_JP), .driver_data = MS_HIDINPUT },
+ { HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_TYPE_COVER_PRO_4_1),
+ .driver_data = MS_HIDINPUT },
{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_TYPE_COVER_3), .driver_data = MS_HIDINPUT },
{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_POWER_COVER),

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

index e6cfd32..48d0ce8 100644
@@ -102,6 +102,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_PRO_4_JP, HID_QUIRK_NO_INIT_REPORTS },
+ { USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_TYPE_COVER_PRO_4_1, 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_POWER_COVER, HID_QUIRK_NO_INIT_REPORTS }, { USB_VENDOR_ID_MSI, USB_DEVICE_ID_MSI_GT683R_LED_PANEL, HID_QUIRK_NO_INIT_REPORTS }, --
2.7.4

Benjamin Tissoires

2016-12-01 12:53 UTC #1 lore raw

Quoted 10 lines · Zhang Rui

From edb2fc779c3e9c118686a3c177cfa304e95e0220 Mon Sep 17 00:00:00 2001
From: Zhang Rui <[email protected]>
Date: Wed, 30 Nov 2016 20:09:11 +0800
Subject: [PATCH] HID: microsoft: Add Surface Pro 4 type cover support
Adding support for the Microsoft Surface Pro 4 Type Cover.
This patch has been tested on my surface pro 4, keyboard starts working, while the touchpad does not.

Quoted 7 lines

I don't know which type of surface pro 4 I have, but the lsusb output shows it's ID is 07e8, and I use USB_DEVICE_ID_MS_TYPE_COVER_4_1 as the name, following the patch at
https://bugzilla.kernel.org/show_bug.cgi?id=135561#c0
At the same time, this patch also fixes a long suspend/resume delay problem when type cover is attached.

Hi,

would you mind testing the for-next branch of the HID tree without your patch?
https://git.kernel.org/cgit/linux/kernel/git/jikos/hid.git/log/drivers/hid?h=for-next

This branch makes the Surface 3 typeCover working out of the box without the need for specific quirks. I would hope that this way, you don't need the quirks for the Surface Pro 4 as well and forget about these cover :)

Cheers,
Benjamin

Quoted 71 lines

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=135561
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=186181

Signed-off-by Zhang Rui <[email protected]>

---
drivers/hid/hid-core.c          | 2 ++
drivers/hid/hid-ids.h           | 1 +
drivers/hid/hid-microsoft.c     | 2 ++
drivers/hid/usbhid/hid-quirks.c | 1 +

 4 files changed, 6 insertions(+)

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

index 2b89c70..3c4085d 100644
@@ -728,6 +728,7 @@ static void hid_scan_collection(struct hid_parser *parser, unsigned type)

 	     hid->product == USB_DEVICE_ID_MS_TYPE_COVER_PRO_3_2 ||
 	     hid->product == USB_DEVICE_ID_MS_TYPE_COVER_PRO_3_JP ||
 	     hid->product == USB_DEVICE_ID_MS_TYPE_COVER_PRO_4_JP ||

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

+	     hid->product == USB_DEVICE_ID_MS_TYPE_COVER_PRO_4_1 ||

hid->product == USB_DEVICE_ID_MS_TYPE_COVER_3 || hid->product == USB_DEVICE_ID_MS_POWER_COVER) && hid->group == HID_GROUP_MULTITOUCH)

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

@@ -1984,6 +1985,7 @@ static const struct hid_device_id hid_have_special_driver[] = {
  	{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_TYPE_COVER_PRO_3_2) },
  	{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_TYPE_COVER_PRO_3_JP) },
  	{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_TYPE_COVER_PRO_4_JP) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_TYPE_COVER_PRO_4_1) },
  	{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_TYPE_COVER_3) },
  	{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_DIGITAL_MEDIA_7K) },
  	{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_DIGITAL_MEDIA_600) },

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

index 575aa65..fe19866 100644
@@ -719,6 +719,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_PRO_4_JP 0x07e9
+#define USB_DEVICE_ID_MS_TYPE_COVER_PRO_4_1 0x07e8
  #define USB_DEVICE_ID_MS_TYPE_COVER_3    0x07de
  #define USB_DEVICE_ID_MS_POWER_COVER     0x07da

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

index c6cd392..df06e6c 100644
@@ -282,6 +282,8 @@ static const struct hid_device_id ms_devices[] = {
  		.driver_data = MS_HIDINPUT },
  	{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_TYPE_COVER_PRO_4_JP),
  		.driver_data = MS_HIDINPUT },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_TYPE_COVER_PRO_4_1),
+		.driver_data = MS_HIDINPUT },
  	{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_TYPE_COVER_3),
  		.driver_data = MS_HIDINPUT },
  	{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_POWER_COVER),

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

index e6cfd32..48d0ce8 100644
@@ -102,6 +102,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_PRO_4_JP, HID_QUIRK_NO_INIT_REPORTS },
+	{ USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_TYPE_COVER_PRO_4_1, 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_POWER_COVER, HID_QUIRK_NO_INIT_REPORTS },
  	{ USB_VENDOR_ID_MSI, USB_DEVICE_ID_MSI_GT683R_LED_PANEL, HID_QUIRK_NO_INIT_REPORTS },

--
2.7.4

Zhang Rui

author

2016-12-05 08:00 UTC #2 lore raw

Quoted 22 lines · Benjamin Tissoires

Quoted 13 lines · Zhang Rui
From edb2fc779c3e9c118686a3c177cfa304e95e0220 Mon Sep 17 00:00:00 2001
From: Zhang Rui <[email protected]>
Date: Wed, 30 Nov 2016 20:09:11 +0800
Subject: [PATCH] HID: microsoft: Add Surface Pro 4 type cover support
Adding support for the Microsoft Surface Pro 4 Type Cover.
This patch has been tested on my surface pro 4, keyboard starts working, while the touchpad does not.
Quoted 8 lines
I don't know which type of surface pro 4 I have, but the lsusb output
shows it's ID is 07e8, and I use USB_DEVICE_ID_MS_TYPE_COVER_4_1 as the name, following the patch at
https://bugzilla.kernel.org/show_bug.cgi?id=135561#c0
At the same time, this patch also fixes a long suspend/resume delay problem when type cover is attached.

Quoted 8 lines

Hi,
would you mind testing the for-next branch of the HID tree without your
patch?
https://git.kernel.org/cgit/linux/kernel/git/jikos/hid.git/log/driver
s/hid?h=for-next

yes, I did.

Quoted 6 lines

This branch makes the Surface 3 typeCover working out of the box without
the need for specific quirks. I would hope that this way, you don't need
the quirks for the Surface Pro 4 as well and forget about these cover :)

yes, the keyboard works as expected.
But unfortunately, I experienced a system hang, in like an hour, with both your next tree, and upstream kernel plus my original type cover patch... At the same time, I also noticed that, when I type something, occasionally, the character keeps on popping up just like the key is not released.
Now I switch back to a clean upstream 4.9-rc6 kernel, and use it for half a day, no system hang so far.

thanks,
rui

Quoted 103 lines

Cheers,
Benjamin
Quoted 99 lines
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=135561
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=186181
Signed-off-by Zhang Rui <[email protected]>
---
drivers/hid/hid-core.c          | 2 ++
drivers/hid/hid-ids.h           | 1 +
drivers/hid/hid-microsoft.c     | 2 ++
drivers/hid/usbhid/hid-quirks.c | 1 +
 4 files changed, 6 insertions(+)
drivers/hid/hid-core.c +0 -0
index 2b89c70..3c4085d 100644
@@ -728,6 +728,7 @@ static void hid_scan_collection(struct
 hid_parser *parser, unsigned type)
     hid->product == USB_DEVICE_ID_MS_TYPE_COVER_PRO_3_2 ||
     hid->product == USB_DEVICE_ID_MS_TYPE_COVER_PRO_3_JP ||
     hid->product == USB_DEVICE_ID_MS_TYPE_COVER_PRO_4_JP ||
drivers/hid/hid-core.c +1 -0
+	     hid->product == USB_DEVICE_ID_MS_TYPE_COVER_PRO_4_1
||
 	     hid->product == USB_DEVICE_ID_MS_TYPE_COVER_3 ||
hid->product == USB_DEVICE_ID_MS_POWER_COVER) && hid->group == HID_GROUP_MULTITOUCH)
drivers/hid/hid-core.c +0 -0
@@ -1984,6 +1985,7 @@ static const struct hid_device_id
 hid_have_special_driver[] = {
  	{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT,
 USB_DEVICE_ID_MS_TYPE_COVER_PRO_3_2) },
  	{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT,
 USB_DEVICE_ID_MS_TYPE_COVER_PRO_3_JP) },
  	{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT,
USB_DEVICE_ID_MS_TYPE_COVER_PRO_4_JP) },
drivers/hid/hid-core.c +1 -0
+	{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT,
USB_DEVICE_ID_MS_TYPE_COVER_PRO_4_1) },
{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT,
USB_DEVICE_ID_MS_TYPE_COVER_3) },
{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT,
USB_DEVICE_ID_MS_DIGITAL_MEDIA_7K) },
{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT,
USB_DEVICE_ID_MS_DIGITAL_MEDIA_600) },
drivers/hid/hid-ids.h +1 -0
index 575aa65..fe19866 100644
@@ -719,6 +719,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_PRO_4_JP 0x07e9
+#define USB_DEVICE_ID_MS_TYPE_COVER_PRO_4_1 0x07e8
  #define USB_DEVICE_ID_MS_TYPE_COVER_3    0x07de
  #define USB_DEVICE_ID_MS_POWER_COVER     0x07da
drivers/hid/hid- renamed from drivers/hid/hid-microsoft.c +0 -0
microsoft.c
index c6cd392..df06e6c 100644
drivers/hid/hid-microsoft.c +2 -0
@@ -282,6 +282,8 @@ static const struct hid_device_id ms_devices[]
 = {
  		.driver_data = MS_HIDINPUT },
  	{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT,
 USB_DEVICE_ID_MS_TYPE_COVER_PRO_4_JP),
  		.driver_data = MS_HIDINPUT },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT,
 USB_DEVICE_ID_MS_TYPE_COVER_PRO_4_1),
+		.driver_data = MS_HIDINPUT },
{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT,
USB_DEVICE_ID_MS_TYPE_COVER_3),
.driver_data = MS_HIDINPUT },
{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT,
USB_DEVICE_ID_MS_POWER_COVER),
drivers/hid/usbhid/hid-quirks.c +1 -0
index e6cfd32..48d0ce8 100644
@@ -102,6 +102,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_PRO_4_JP, HID_QUIRK_NO_INIT_REPORTS },
+	{ USB_VENDOR_ID_MICROSOFT,
USB_DEVICE_ID_MS_TYPE_COVER_PRO_4_1, 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_POWER_COVER, HID_QUIRK_NO_INIT_REPORTS },
{ USB_VENDOR_ID_MSI, USB_DEVICE_ID_MSI_GT683R_LED_PANEL, HID_QUIRK_NO_INIT_REPORTS },
--
2.7.4

Benjamin Tissoires

2016-12-05 08:18 UTC #3 lore raw

Hi,

Quoted 40 lines · Zhang Rui

Quoted 22 lines · Benjamin Tissoires
Quoted 13 lines · Zhang Rui
From edb2fc779c3e9c118686a3c177cfa304e95e0220 Mon Sep 17 00:00:00 2001
From: Zhang Rui <[email protected]>
Date: Wed, 30 Nov 2016 20:09:11 +0800
Subject: [PATCH] HID: microsoft: Add Surface Pro 4 type cover support
Adding support for the Microsoft Surface Pro 4 Type Cover.
This patch has been tested on my surface pro 4, keyboard starts working, while the touchpad does not.
Quoted 8 lines
I don't know which type of surface pro 4 I have, but the lsusb output
shows it's ID is 07e8, and I use USB_DEVICE_ID_MS_TYPE_COVER_4_1 as the name, following the patch at
https://bugzilla.kernel.org/show_bug.cgi?id=135561#c0
At the same time, this patch also fixes a long suspend/resume delay problem when type cover is attached.
Quoted 8 lines
Hi,
would you mind testing the for-next branch of the HID tree without your
patch?
https://git.kernel.org/cgit/linux/kernel/git/jikos/hid.git/log/driver
s/hid?h=for-next
yes, I did.
Quoted 6 lines
This branch makes the Surface 3 typeCover working out of the box without
the need for specific quirks. I would hope that this way, you don't need
the quirks for the Surface Pro 4 as well and forget about these cover :)

Quoted 8 lines

yes, the keyboard works as expected.
But unfortunately, I experienced a system hang, in like an hour, with both your next tree, and upstream kernel plus my original type cover patch... At the same time, I also noticed that, when I type something, occasionally, the character keeps on popping up just like the key is not released.
Now I switch back to a clean upstream 4.9-rc6 kernel, and use it for half a day, no system hang so far.

I am not so sure I followed your tests. Could you confirm and complete that:
- upstream + your patch: keyboard works, 2 finger scroll doesn't but it hangs after an hour or so, and keys are repeated from time to time
- hid for-next branch: keyboard works, 2 finger scroll too, but it hangs after an hour or so, and keys are repeated from time to time
- vanilla kernel 4.9-rc6: no hangs, but what is the status of the keyboard/touchpad?

I have the impression that when the keyboard is disabled (plain v4.9-rc6) you don't experience hangs. So if that is the case, then maybe the keyboard is somewhat messing up the system. I don't know if that would be counted as a regression :/

Unless the keyboard is already working with a plain v4.9-rc6 but just doesn't after suspend/resume...

Cheers,
Benjamin

Quoted 106 lines

thanks,
rui
Quoted 103 lines
Cheers,
Benjamin
Quoted 99 lines
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=135561
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=186181
Signed-off-by Zhang Rui <[email protected]>
---
drivers/hid/hid-core.c          | 2 ++
drivers/hid/hid-ids.h           | 1 +
drivers/hid/hid-microsoft.c     | 2 ++
drivers/hid/usbhid/hid-quirks.c | 1 +
 4 files changed, 6 insertions(+)
drivers/hid/hid-core.c +0 -0
index 2b89c70..3c4085d 100644
@@ -728,6 +728,7 @@ static void hid_scan_collection(struct
 hid_parser *parser, unsigned type)
     hid->product == USB_DEVICE_ID_MS_TYPE_COVER_PRO_3_2 ||
     hid->product == USB_DEVICE_ID_MS_TYPE_COVER_PRO_3_JP ||
     hid->product == USB_DEVICE_ID_MS_TYPE_COVER_PRO_4_JP ||
drivers/hid/hid-core.c +1 -0
+	     hid->product == USB_DEVICE_ID_MS_TYPE_COVER_PRO_4_1
||
 	     hid->product == USB_DEVICE_ID_MS_TYPE_COVER_3 ||
hid->product == USB_DEVICE_ID_MS_POWER_COVER) && hid->group == HID_GROUP_MULTITOUCH)
drivers/hid/hid-core.c +0 -0
@@ -1984,6 +1985,7 @@ static const struct hid_device_id
 hid_have_special_driver[] = {
  	{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT,
 USB_DEVICE_ID_MS_TYPE_COVER_PRO_3_2) },
  	{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT,
 USB_DEVICE_ID_MS_TYPE_COVER_PRO_3_JP) },
  	{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT,
USB_DEVICE_ID_MS_TYPE_COVER_PRO_4_JP) },
drivers/hid/hid-core.c +1 -0
+	{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT,
USB_DEVICE_ID_MS_TYPE_COVER_PRO_4_1) },
{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT,
USB_DEVICE_ID_MS_TYPE_COVER_3) },
{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT,
USB_DEVICE_ID_MS_DIGITAL_MEDIA_7K) },
{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT,
USB_DEVICE_ID_MS_DIGITAL_MEDIA_600) },
drivers/hid/hid-ids.h +1 -0
index 575aa65..fe19866 100644
@@ -719,6 +719,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_PRO_4_JP 0x07e9
+#define USB_DEVICE_ID_MS_TYPE_COVER_PRO_4_1 0x07e8
  #define USB_DEVICE_ID_MS_TYPE_COVER_3    0x07de
  #define USB_DEVICE_ID_MS_POWER_COVER     0x07da
drivers/hid/hid- renamed from drivers/hid/hid-microsoft.c +0 -0
microsoft.c
index c6cd392..df06e6c 100644
drivers/hid/hid-microsoft.c +2 -0
@@ -282,6 +282,8 @@ static const struct hid_device_id ms_devices[]
 = {
  		.driver_data = MS_HIDINPUT },
  	{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT,
 USB_DEVICE_ID_MS_TYPE_COVER_PRO_4_JP),
  		.driver_data = MS_HIDINPUT },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT,
 USB_DEVICE_ID_MS_TYPE_COVER_PRO_4_1),
+		.driver_data = MS_HIDINPUT },
{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT,
USB_DEVICE_ID_MS_TYPE_COVER_3),
.driver_data = MS_HIDINPUT },
{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT,
USB_DEVICE_ID_MS_POWER_COVER),
drivers/hid/usbhid/hid-quirks.c +1 -0
index e6cfd32..48d0ce8 100644
@@ -102,6 +102,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_PRO_4_JP, HID_QUIRK_NO_INIT_REPORTS },
+	{ USB_VENDOR_ID_MICROSOFT,
USB_DEVICE_ID_MS_TYPE_COVER_PRO_4_1, 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_POWER_COVER, HID_QUIRK_NO_INIT_REPORTS },
{ USB_VENDOR_ID_MSI, USB_DEVICE_ID_MSI_GT683R_LED_PANEL, HID_QUIRK_NO_INIT_REPORTS },
--
2.7.4

Zhang Rui

author

2016-12-05 08:41 UTC #4 lore raw

Quoted 66 lines · Benjamin Tissoires

Hi,
Quoted 50 lines · Zhang Rui
Quoted 29 lines · Benjamin Tissoires
Quoted 22 lines · Zhang Rui
From edb2fc779c3e9c118686a3c177cfa304e95e0220 Mon Sep 17 00:00:00
2001
From: Zhang Rui <[email protected]>
Date: Wed, 30 Nov 2016 20:09:11 +0800
Subject: [PATCH] HID: microsoft: Add Surface Pro 4 type cover support
Adding support for the Microsoft Surface Pro 4 Type Cover.
This patch has been tested on my surface pro 4, keyboard starts working, while the touchpad does not.
I don't know which type of surface pro 4 I have, but the lsusb output
shows it's ID is 07e8, and I use USB_DEVICE_ID_MS_TYPE_COVER_4_1 as
the name, following the patch at
https://bugzilla.kernel.org/show_bug.cgi?id=135561#c0
Quoted 3 lines
At the same time, this patch also fixes a long suspend/resume delay
problem when type cover is attached.
Hi,
Quoted 8 lines
would you mind testing the for-next branch of the HID tree without
your
patch?
https://git.kernel.org/cgit/linux/kernel/git/jikos/hid.git/log/dr
iver
s/hid?h=for-next
yes, I did.
Quoted 9 lines
This branch makes the Surface 3 typeCover working out of the box without
the need for specific quirks. I would hope that this way, you don't
need
the quirks for the Surface Pro 4 as well and forget about these cover
:)
Quoted 13 lines
yes, the keyboard works as expected.
But unfortunately, I experienced a system hang, in like an hour, with
both your next tree, and upstream kernel plus my original type cover
patch... At the same time, I also noticed that, when I type something,
occasionally, the character keeps on popping up just like the key is
not released.
Now I switch back to a clean upstream 4.9-rc6 kernel, and use it for
half a day, no system hang so far.

Quoted 5 lines

I am not so sure I followed your tests. Could you confirm and complete that:
- upstream + your patch: keyboard works, 2 finger scroll doesn't but it
hangs after an hour or so, and keys are repeated from time to time

yes.

Quoted 3 lines

- hid for-next branch: keyboard works, 2 finger scroll too, but it hangs
after an hour or so, and keys are repeated from time to time

yes.

Quoted 3 lines

- vanilla kernel 4.9-rc6: no hangs, but what is the status of the keyboard/touchpad?

No hangs, keyboard does not work at all.

Quoted 5 lines

I have the impression that when the keyboard is disabled (plain v4.9-rc6) you don't experience hangs. So if that is the case, then maybe
the keyboard is somewhat messing up the system. I don't know if that would be counted as a regression :/

Right, that's also my concern.
would you like to catch something for hid for-next branch? say, when the key repeats, I usually press some other key to stop the storm, would you like me to get some information after that?

I can switch back to hid for-next to confirm the symptom if needed.

Quoted 4 lines

Unless the keyboard is already working with a plain v4.9-rc6 but just doesn't after suspend/resume...

No, keyboard does not work with upstream kernel.

thanks,
rui

Quoted 132 lines

Cheers,
Benjamin
Quoted 129 lines
thanks,
rui
Quoted 125 lines
Cheers,
Benjamin
Quoted 120 lines
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=135561
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=186181
Signed-off-by Zhang Rui <[email protected]>
---
drivers/hid/hid-core.c | 2 ++ drivers/hid/hid-ids.h | 1 + drivers/hid/hid-microsoft.c | 2 ++ drivers/hid/usbhid/hid-quirks.c | 1 +
 4 files changed, 6 insertions(+)
drivers/hid/hid-core.c +0 -0
index 2b89c70..3c4085d 100644
@@ -728,6 +728,7 @@ static void hid_scan_collection(struct
 hid_parser *parser, unsigned type)
     hid->product == USB_DEVICE_ID_MS_TYPE_COVER_PRO_3_2
Quoted 3 lines
Quoted 3 lines
     hid->product == USB_DEVICE_ID_MS_TYPE_COVER_PRO_3_JP
Quoted 3 lines
Quoted 3 lines
     hid->product == USB_DEVICE_ID_MS_TYPE_COVER_PRO_4_JP
Quoted 3 lines
Quoted 3 lines
drivers/hid/hid-core.c +1 -0
+	     hid->product ==
USB_DEVICE_ID_MS_TYPE_COVER_PRO_4_1
Quoted 3 lines
Quoted 3 lines
hid->product == USB_DEVICE_ID_MS_TYPE_COVER_3 || hid->product == USB_DEVICE_ID_MS_POWER_COVER) && hid->group == HID_GROUP_MULTITOUCH)
drivers/hid/hid-core.c +0 -0
@@ -1984,6 +1985,7 @@ static const struct hid_device_id
 hid_have_special_driver[] = {
  	{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT,
 USB_DEVICE_ID_MS_TYPE_COVER_PRO_3_2) },
  	{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT,
 USB_DEVICE_ID_MS_TYPE_COVER_PRO_3_JP) },
  	{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT,
USB_DEVICE_ID_MS_TYPE_COVER_PRO_4_JP) },
drivers/hid/hid-core.c +1 -0
+	{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT,
USB_DEVICE_ID_MS_TYPE_COVER_PRO_4_1) },
{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_TYPE_COVER_3) },
{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_DIGITAL_MEDIA_7K) },
{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_DIGITAL_MEDIA_600) },
drivers/hid/hid-ids.h +1 -0
index 575aa65..fe19866 100644
@@ -719,6 +719,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_PRO_4_JP 0x07e9
+#define USB_DEVICE_ID_MS_TYPE_COVER_PRO_4_1 0x07e8
  #define USB_DEVICE_ID_MS_TYPE_COVER_3    0x07de
  #define USB_DEVICE_ID_MS_POWER_COVER     0x07da
drivers/hid/hid- renamed from drivers/hid/hid-microsoft.c +0 -0
microsoft.c
index c6cd392..df06e6c 100644
drivers/hid/hid-microsoft.c +0 -0
@@ -282,6 +282,8 @@ static const struct hid_device_id
ms_devices[]
= {
.driver_data = MS_HIDINPUT }, { HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_TYPE_COVER_PRO_4_JP), .driver_data = MS_HIDINPUT },
drivers/hid/hid-microsoft.c +1 -0
+	{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT,
USB_DEVICE_ID_MS_TYPE_COVER_PRO_4_1),
drivers/hid/hid-microsoft.c +1 -0
+		.driver_data = MS_HIDINPUT },
{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_TYPE_COVER_3), .driver_data = MS_HIDINPUT }, { HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_POWER_COVER),
drivers/hid/usbhid/hid-quirks.c +0 -0
index e6cfd32..48d0ce8 100644
@@ -102,6 +102,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_PRO_4_JP, HID_QUIRK_NO_INIT_REPORTS },
drivers/hid/usbhid/hid-quirks.c +1 -0
+	{ USB_VENDOR_ID_MICROSOFT,
USB_DEVICE_ID_MS_TYPE_COVER_PRO_4_1, 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_POWER_COVER, HID_QUIRK_NO_INIT_REPORTS },
{ USB_VENDOR_ID_MSI, USB_DEVICE_ID_MSI_GT683R_LED_PANEL, HID_QUIRK_NO_INIT_REPORTS },
--
2.7.4

Benjamin Tissoires

2016-12-05 08:59 UTC #5 lore raw

Quoted 73 lines · Zhang Rui

Quoted 66 lines · Benjamin Tissoires
Hi,
Quoted 50 lines · Zhang Rui
Quoted 29 lines · Benjamin Tissoires
Quoted 22 lines · Zhang Rui
From edb2fc779c3e9c118686a3c177cfa304e95e0220 Mon Sep 17 00:00:00
2001
From: Zhang Rui <[email protected]>
Date: Wed, 30 Nov 2016 20:09:11 +0800
Subject: [PATCH] HID: microsoft: Add Surface Pro 4 type cover support
Adding support for the Microsoft Surface Pro 4 Type Cover.
This patch has been tested on my surface pro 4, keyboard starts working, while the touchpad does not.
I don't know which type of surface pro 4 I have, but the lsusb output
shows it's ID is 07e8, and I use USB_DEVICE_ID_MS_TYPE_COVER_4_1 as
the name, following the patch at
https://bugzilla.kernel.org/show_bug.cgi?id=135561#c0
Quoted 3 lines
At the same time, this patch also fixes a long suspend/resume delay
problem when type cover is attached.
Hi,
Quoted 8 lines
would you mind testing the for-next branch of the HID tree without
your
patch?
https://git.kernel.org/cgit/linux/kernel/git/jikos/hid.git/log/dr
iver
s/hid?h=for-next
yes, I did.
Quoted 9 lines
This branch makes the Surface 3 typeCover working out of the box without
the need for specific quirks. I would hope that this way, you don't
need
the quirks for the Surface Pro 4 as well and forget about these cover
:)
Quoted 13 lines
yes, the keyboard works as expected.
But unfortunately, I experienced a system hang, in like an hour, with
both your next tree, and upstream kernel plus my original type cover
patch... At the same time, I also noticed that, when I type something,
occasionally, the character keeps on popping up just like the key is
not released.
Now I switch back to a clean upstream 4.9-rc6 kernel, and use it for
half a day, no system hang so far.
Quoted 5 lines
I am not so sure I followed your tests. Could you confirm and complete that:
- upstream + your patch: keyboard works, 2 finger scroll doesn't but it
hangs after an hour or so, and keys are repeated from time to time
yes.

Quoted 8 lines

Quoted 3 lines
- hid for-next branch: keyboard works, 2 finger scroll too, but it hangs
after an hour or so, and keys are repeated from time to time
yes.
Quoted 3 lines
- vanilla kernel 4.9-rc6: no hangs, but what is the status of the keyboard/touchpad?
No hangs, keyboard does not work at all.

Ok, thanks for the confirmation.

Quoted 11 lines

Quoted 6 lines
I have the impression that when the keyboard is disabled (plain v4.9-rc6) you don't experience hangs. So if that is the case, then maybe
the keyboard is somewhat messing up the system. I don't know if that would be counted as a regression :/
Right, that's also my concern.
would you like to catch something for hid for-next branch? say, when the key repeats, I usually press some other key to stop the storm, would you like me to get some information after that?

Well, I would believe that we are missing the key release event. Given the way keys are sent from the keyboard, it seems normal that if you press one other key and release it, the kernel will release all of them. The keys are sent as an array, so if you send a new key event, the previous key will be cleared as it won't be part of the events.

One interesting bit could be to monitor the dmesg during these issues and see if any warning happens.

One other interesting bit could be to check whether disabling runtime suspend has an effect on the hangs.

Cheers,
Benjamin

Quoted 142 lines

I can switch back to hid for-next to confirm the symptom if needed.
Quoted 4 lines
Unless the keyboard is already working with a plain v4.9-rc6 but just doesn't after suspend/resume...
No, keyboard does not work with upstream kernel.
thanks,
rui
Quoted 132 lines
Cheers,
Benjamin
Quoted 129 lines
thanks,
rui
Quoted 125 lines
Cheers,
Benjamin
Quoted 120 lines
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=135561
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=186181
Signed-off-by Zhang Rui <[email protected]>
---
drivers/hid/hid-core.c | 2 ++ drivers/hid/hid-ids.h | 1 + drivers/hid/hid-microsoft.c | 2 ++ drivers/hid/usbhid/hid-quirks.c | 1 +
 4 files changed, 6 insertions(+)
drivers/hid/hid-core.c +0 -0
index 2b89c70..3c4085d 100644
@@ -728,6 +728,7 @@ static void hid_scan_collection(struct
 hid_parser *parser, unsigned type)
     hid->product == USB_DEVICE_ID_MS_TYPE_COVER_PRO_3_2
Quoted 3 lines
Quoted 3 lines
     hid->product == USB_DEVICE_ID_MS_TYPE_COVER_PRO_3_JP
Quoted 3 lines
Quoted 3 lines
     hid->product == USB_DEVICE_ID_MS_TYPE_COVER_PRO_4_JP
Quoted 3 lines
Quoted 3 lines
drivers/hid/hid-core.c +1 -0
+	     hid->product ==
USB_DEVICE_ID_MS_TYPE_COVER_PRO_4_1
Quoted 3 lines
Quoted 3 lines
hid->product == USB_DEVICE_ID_MS_TYPE_COVER_3 || hid->product == USB_DEVICE_ID_MS_POWER_COVER) && hid->group == HID_GROUP_MULTITOUCH)
drivers/hid/hid-core.c +0 -0
@@ -1984,6 +1985,7 @@ static const struct hid_device_id
 hid_have_special_driver[] = {
  	{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT,
 USB_DEVICE_ID_MS_TYPE_COVER_PRO_3_2) },
  	{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT,
 USB_DEVICE_ID_MS_TYPE_COVER_PRO_3_JP) },
  	{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT,
USB_DEVICE_ID_MS_TYPE_COVER_PRO_4_JP) },
drivers/hid/hid-core.c +1 -0
+	{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT,
USB_DEVICE_ID_MS_TYPE_COVER_PRO_4_1) },
{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_TYPE_COVER_3) },
{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_DIGITAL_MEDIA_7K) },
{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_DIGITAL_MEDIA_600) },
drivers/hid/hid-ids.h +1 -0
index 575aa65..fe19866 100644
@@ -719,6 +719,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_PRO_4_JP 0x07e9
+#define USB_DEVICE_ID_MS_TYPE_COVER_PRO_4_1 0x07e8
  #define USB_DEVICE_ID_MS_TYPE_COVER_3    0x07de
  #define USB_DEVICE_ID_MS_POWER_COVER     0x07da
drivers/hid/hid- renamed from drivers/hid/hid-microsoft.c +0 -0
microsoft.c
index c6cd392..df06e6c 100644
drivers/hid/hid-microsoft.c +0 -0
@@ -282,6 +282,8 @@ static const struct hid_device_id
ms_devices[]
= {
.driver_data = MS_HIDINPUT }, { HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_TYPE_COVER_PRO_4_JP), .driver_data = MS_HIDINPUT },
drivers/hid/hid-microsoft.c +1 -0
+	{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT,
USB_DEVICE_ID_MS_TYPE_COVER_PRO_4_1),
drivers/hid/hid-microsoft.c +1 -0
+		.driver_data = MS_HIDINPUT },
{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_TYPE_COVER_3), .driver_data = MS_HIDINPUT }, { HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_POWER_COVER),
drivers/hid/usbhid/hid-quirks.c +0 -0
index e6cfd32..48d0ce8 100644
@@ -102,6 +102,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_PRO_4_JP, HID_QUIRK_NO_INIT_REPORTS },
drivers/hid/usbhid/hid-quirks.c +1 -0
+	{ USB_VENDOR_ID_MICROSOFT,
USB_DEVICE_ID_MS_TYPE_COVER_PRO_4_1, 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_POWER_COVER, HID_QUIRK_NO_INIT_REPORTS },
{ USB_VENDOR_ID_MSI, USB_DEVICE_ID_MSI_GT683R_LED_PANEL, HID_QUIRK_NO_INIT_REPORTS },
--
2.7.4

Zhang Rui

author

2016-12-06 07:01 UTC #6 lore raw

Quoted 127 lines · Benjamin Tissoires

Quoted 95 lines · Zhang Rui
Quoted 80 lines · Benjamin Tissoires
Hi,
Quoted 60 lines · Zhang Rui
Quoted 34 lines · Benjamin Tissoires
Quoted 25 lines · Zhang Rui
From edb2fc779c3e9c118686a3c177cfa304e95e0220 Mon Sep 17 00:00:00
2001
From: Zhang Rui <[email protected]>
Date: Wed, 30 Nov 2016 20:09:11 +0800
Subject: [PATCH] HID: microsoft: Add Surface Pro 4 type cover
support
Adding support for the Microsoft Surface Pro 4 Type Cover.
This patch has been tested on my surface pro 4, keyboard starts working, while the touchpad does not.
I don't know which type of surface pro 4 I have, but the lsusb
output
shows it's ID is 07e8, and I use USB_DEVICE_ID_MS_TYPE_COVER_4_1 as
the name, following the patch at
https://bugzilla.kernel.org/show_bug.cgi?id=135561#c0
Quoted 4 lines
At the same time, this patch also fixes a long suspend/resume
delay
problem when type cover is attached.
Hi,
Quoted 9 lines
would you mind testing the for-next branch of the HID tree without
your
patch?
https://git.kernel.org/cgit/linux/kernel/git/jikos/hid.git/lo
g/dr
iver
s/hid?h=for-next
yes, I did.
Quoted 12 lines
This branch makes the Surface 3 typeCover working out of the box
without
the need for specific quirks. I would hope that this way, you don't
need
the quirks for the Surface Pro 4 as well and forget about these
cover
:)
Quoted 16 lines
yes, the keyboard works as expected.
But unfortunately, I experienced a system hang, in like an hour,
with
both your next tree, and upstream kernel plus my original type cover
patch... At the same time, I also noticed that, when I type something,
occasionally, the character keeps on popping up just like the key
is
not released.
Now I switch back to a clean upstream 4.9-rc6 kernel, and use it
for
half a day, no system hang so far.
Quoted 7 lines
I am not so sure I followed your tests. Could you confirm and complete that:
- upstream + your patch: keyboard works, 2 finger scroll doesn't but
it
hangs after an hour or so, and keys are repeated from time to time
yes.
Quoted 5 lines
- hid for-next branch: keyboard works, 2 finger scroll too, but it
hangs
after an hour or so, and keys are repeated from time to time
Quoted 6 lines
yes.
Quoted 4 lines
- vanilla kernel 4.9-rc6: no hangs, but what is the status of the keyboard/touchpad?
No hangs, keyboard does not work at all.
Ok, thanks for the confirmation.
Quoted 10 lines
Quoted 10 lines
I have the impression that when the keyboard is disabled (plain v4.9-rc6) you don't experience hangs. So if that is the case, then
maybe
the keyboard is somewhat messing up the system. I don't know if that
would be counted as a regression :/
Quoted 5 lines
Right, that's also my concern.
would you like to catch something for hid for-next branch? say, when
the key repeats, I usually press some other key to stop the storm, would you like me to get some information after that?
Well, I would believe that we are missing the key release event. Given
the way keys are sent from the keyboard, it seems normal that if you press one other key and release it, the kernel will release all of them.
The keys are sent as an array, so if you send a new key event, the previous key will be cleared as it won't be part of the events.

Quoted 6 lines

One interesting bit could be to monitor the dmesg during these issues and see if any warning happens.
One other interesting bit could be to check whether disabling runtime suspend has an effect on the hangs.

Well, something strange, I switched back to HID for-next kernel yesterday, and has the system running for over 20 hours, still working well. no hang, no repeat key. not sure what happened, I will stick with this kernel, and update if the problem is reproduced.

thanks,
rui

Quoted 169 lines

Cheers,
Benjamin
Quoted 166 lines
I can switch back to hid for-next to confirm the symptom if needed.
Quoted 6 lines
Unless the keyboard is already working with a plain v4.9-rc6 but just
doesn't after suspend/resume...
No, keyboard does not work with upstream kernel.
thanks,
rui
Quoted 153 lines
Cheers,
Benjamin
Quoted 149 lines
thanks,
rui
Quoted 144 lines
Cheers,
Benjamin
Quoted 138 lines
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=13556
1
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=18618
1
Signed-off-by Zhang Rui <[email protected]>
---
drivers/hid/hid-core.c | 2 ++ drivers/hid/hid-ids.h | 1 + drivers/hid/hid-microsoft.c | 2 ++ drivers/hid/usbhid/hid-quirks.c | 1 +
 4 files changed, 6 insertions(+)
drivers/hid/hid- renamed from drivers/hid/hid-core.c +0 -0
core.c
index 2b89c70..3c4085d 100644
drivers/hid/hid-core.c +0 -0
@@ -728,6 +728,7 @@ static void hid_scan_collection(struct
 hid_parser *parser, unsigned type)
     hid->product == USB_DEVICE_ID_MS_TYPE_COVER_PRO_3_2
Quoted 5 lines
Quoted 5 lines
     hid->product == USB_DEVICE_ID_MS_TYPE_COVER_PRO_3_JP
Quoted 5 lines
Quoted 5 lines
     hid->product == USB_DEVICE_ID_MS_TYPE_COVER_PRO_4_JP
Quoted 5 lines
Quoted 5 lines
drivers/hid/hid-core.c +1 -0
+	     hid->product ==
USB_DEVICE_ID_MS_TYPE_COVER_PRO_4_1
Quoted 5 lines
Quoted 5 lines
     hid->product == USB_DEVICE_ID_MS_TYPE_COVER_3 ||
     hid->product == USB_DEVICE_ID_MS_POWER_COVER) &&
    hid->group == HID_GROUP_MULTITOUCH)
drivers/hid/hid-core.c +0 -0
@@ -1984,6 +1985,7 @@ static const struct hid_device_id
 hid_have_special_driver[] = {
  	{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT,
 USB_DEVICE_ID_MS_TYPE_COVER_PRO_3_2) },
  	{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT,
 USB_DEVICE_ID_MS_TYPE_COVER_PRO_3_JP) },
  	{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT,
USB_DEVICE_ID_MS_TYPE_COVER_PRO_4_JP) },
drivers/hid/hid-core.c +1 -0
+	{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT,
USB_DEVICE_ID_MS_TYPE_COVER_PRO_4_1) },
{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_TYPE_COVER_3) },
{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_DIGITAL_MEDIA_7K) },
{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_DIGITAL_MEDIA_600) },
drivers/hid/hid-ids.h +1 -0
index 575aa65..fe19866 100644
@@ -719,6 +719,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_PRO_4_JP 0x07e9
+#define USB_DEVICE_ID_MS_TYPE_COVER_PRO_4_1 0x07e8
  #define USB_DEVICE_ID_MS_TYPE_COVER_3    0x07de
  #define USB_DEVICE_ID_MS_POWER_COVER     0x07da
drivers/hid/hid- renamed from drivers/hid/hid-microsoft.c +0 -0
microsoft.c
index c6cd392..df06e6c 100644
drivers/hid/hid-microsoft.c +0 -0
@@ -282,6 +282,8 @@ static const struct hid_device_id
ms_devices[]
= {
.driver_data = MS_HIDINPUT }, { HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_TYPE_COVER_PRO_4_JP), .driver_data = MS_HIDINPUT },
drivers/hid/hid-microsoft.c +1 -0
+	{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT,
USB_DEVICE_ID_MS_TYPE_COVER_PRO_4_1),
drivers/hid/hid-microsoft.c +1 -0
+		.driver_data = MS_HIDINPUT },
{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_TYPE_COVER_3), .driver_data = MS_HIDINPUT }, { HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_POWER_COVER),
drivers/hid/usbhid/hid-quirks.c +0 -0
index e6cfd32..48d0ce8 100644
@@ -102,6 +102,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_PRO_4_JP, HID_QUIRK_NO_INIT_REPORTS
},
drivers/hid/usbhid/hid-quirks.c +1 -0
+	{ USB_VENDOR_ID_MICROSOFT,
USB_DEVICE_ID_MS_TYPE_COVER_PRO_4_1, 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_POWER_COVER, HID_QUIRK_NO_INIT_REPORTS },
{ USB_VENDOR_ID_MSI, USB_DEVICE_ID_MSI_GT683R_LED_PANEL, HID_QUIRK_NO_INIT_REPORTS },
--
2.7.4