[PATCH] HID: multitouch: add support for Type Cover Pro 3

[PATCH] HID: multitouch: add support for Type Cover Pro 3

[Dennis Chen]

[-- Attachment #1.1: Type: text/plain, Size: 1811 bytes --]

Include Microsoft Type Cover 3 support into hid-multitouch.c
Allow touchpad device to have multitouch functionality.

Signed-off-by: Dennis Chen <barracks510@gmail.com>
---
 drivers/hid/Kconfig          |  1 +
 drivers/hid/hid-multitouch.c | 14 ++++++++++++++
 2 files changed, 15 insertions(+)

diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 5646ca4..5af0603 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -530,6 +530,7 @@ config HID_MULTITOUCH
 	  - IrTouch Infrared USB panels
 	  - LG Display panels (Dell ST2220Tc)
 	  - Lumio CrystalTouch panels
+	  - Microsoft Type Cover 3 touchpad
 	  - MosArt dual-touch panels
 	  - Panasonic multitouch panels
 	  - PenMount dual touch panels
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index c741f5e..f052ed2 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -1377,6 +1377,20 @@ static const struct hid_device_id mt_devices[] = {
 		MT_USB_DEVICE(USB_VENDOR_ID_ILITEK,
 			USB_DEVICE_ID_ILITEK_MULTITOUCH) },
 
+	/* Microsoft Type Cover 3 touchpad */
+	{ .driver_data = MT_CLS_EXPORT_ALL_INPUTS,
+		MT_USB_DEVICE(USB_VENDOR_ID_MICROSOFT,
+			USB_DEVICE_ID_MS_TYPE_COVER_PRO_3) },
+	{ .driver_data = MT_CLS_EXPORT_ALL_INPUTS,
+		MT_USB_DEVICE(USB_VENDOR_ID_MICROSOFT,
+			USB_DEVICE_ID_MS_TYPE_COVER_PRO_3_2) },
+	{ .driver_data = MT_CLS_EXPORT_ALL_INPUTS,
+		MT_USB_DEVICE(USB_VENDOR_ID_MICROSOFT,
+			USB_DEVICE_ID_MS_TYPE_COVER_PRO_3_JP) },
+	{ .driver_data = MT_CLS_EXPORT_ALL_INPUTS,
+		MT_USB_DEVICE(USB_VENDOR_ID_MICROSOFT,
+			USB_DEVICE_ID_MS_TYPE_COVER_3) },
+
 	/* MosArt panels */
 	{ .driver_data = MT_CLS_CONFIDENCE_MINUS_ONE,
 		MT_USB_DEVICE(USB_VENDOR_ID_ASUS,
-- 
2.5.5

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5755 bytes --]

Re: [PATCH] HID: multitouch: add support for Type Cover Pro 3

[Jiri Kosina]

On Fri, 17 Jun 2016, Dennis Chen wrote:

> Include Microsoft Type Cover 3 support into hid-multitouch.c
> Allow touchpad device to have multitouch functionality.
> 
> Signed-off-by: Dennis Chen <barracks510@gmail.com>
> ---
>  drivers/hid/Kconfig          |  1 +
>  drivers/hid/hid-multitouch.c | 14 ++++++++++++++
>  2 files changed, 15 insertions(+)
> 
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> index 5646ca4..5af0603 100644
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -530,6 +530,7 @@ config HID_MULTITOUCH
>  	  - IrTouch Infrared USB panels
>  	  - LG Display panels (Dell ST2220Tc)
>  	  - Lumio CrystalTouch panels
> +	  - Microsoft Type Cover 3 touchpad
>  	  - MosArt dual-touch panels
>  	  - Panasonic multitouch panels
>  	  - PenMount dual touch panels
> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
> index c741f5e..f052ed2 100644
> --- a/drivers/hid/hid-multitouch.c
> +++ b/drivers/hid/hid-multitouch.c
> @@ -1377,6 +1377,20 @@ static const struct hid_device_id mt_devices[] = {
>  		MT_USB_DEVICE(USB_VENDOR_ID_ILITEK,
>  			USB_DEVICE_ID_ILITEK_MULTITOUCH) },
>  
> +	/* Microsoft Type Cover 3 touchpad */
> +	{ .driver_data = MT_CLS_EXPORT_ALL_INPUTS,
> +		MT_USB_DEVICE(USB_VENDOR_ID_MICROSOFT,
> +			USB_DEVICE_ID_MS_TYPE_COVER_PRO_3) },
> +	{ .driver_data = MT_CLS_EXPORT_ALL_INPUTS,
> +		MT_USB_DEVICE(USB_VENDOR_ID_MICROSOFT,
> +			USB_DEVICE_ID_MS_TYPE_COVER_PRO_3_2) },
> +	{ .driver_data = MT_CLS_EXPORT_ALL_INPUTS,
> +		MT_USB_DEVICE(USB_VENDOR_ID_MICROSOFT,
> +			USB_DEVICE_ID_MS_TYPE_COVER_PRO_3_JP) },
> +	{ .driver_data = MT_CLS_EXPORT_ALL_INPUTS,
> +		MT_USB_DEVICE(USB_VENDOR_ID_MICROSOFT,
> +			USB_DEVICE_ID_MS_TYPE_COVER_3) },
> +
>  	/* MosArt panels */
>  	{ .driver_data = MT_CLS_CONFIDENCE_MINUS_ONE,

This would mean that both hid-multitouch and hid-microsoft would claim 
support for this device, which isn't really going to provide consistent 
user experience. Is MT_CLS_EXPORT_ALL_INPUTS sufficient to provide 
complete functionality by hid-multitouch? If so, the support from 
hid-microsoft should be dropped.

Thanks,

-- 
Jiri Kosina
SUSE Labs

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] HID: multitouch: add support for Type Cover Pro 3

[Benjamin Tissoires]

On Jun 17 2016 or thereabouts, Jiri Kosina wrote:
> On Fri, 17 Jun 2016, Dennis Chen wrote:
> 
> > Include Microsoft Type Cover 3 support into hid-multitouch.c
> > Allow touchpad device to have multitouch functionality.
> > 
> > Signed-off-by: Dennis Chen <barracks510@gmail.com>
> > ---
> >  drivers/hid/Kconfig          |  1 +
> >  drivers/hid/hid-multitouch.c | 14 ++++++++++++++
> >  2 files changed, 15 insertions(+)
> > 
> > diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> > index 5646ca4..5af0603 100644
> > --- a/drivers/hid/Kconfig
> > +++ b/drivers/hid/Kconfig
> > @@ -530,6 +530,7 @@ config HID_MULTITOUCH
> >  	  - IrTouch Infrared USB panels
> >  	  - LG Display panels (Dell ST2220Tc)
> >  	  - Lumio CrystalTouch panels
> > +	  - Microsoft Type Cover 3 touchpad
> >  	  - MosArt dual-touch panels
> >  	  - Panasonic multitouch panels
> >  	  - PenMount dual touch panels
> > diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
> > index c741f5e..f052ed2 100644
> > --- a/drivers/hid/hid-multitouch.c
> > +++ b/drivers/hid/hid-multitouch.c
> > @@ -1377,6 +1377,20 @@ static const struct hid_device_id mt_devices[] = {
> >  		MT_USB_DEVICE(USB_VENDOR_ID_ILITEK,
> >  			USB_DEVICE_ID_ILITEK_MULTITOUCH) },
> >  
> > +	/* Microsoft Type Cover 3 touchpad */
> > +	{ .driver_data = MT_CLS_EXPORT_ALL_INPUTS,
> > +		MT_USB_DEVICE(USB_VENDOR_ID_MICROSOFT,
> > +			USB_DEVICE_ID_MS_TYPE_COVER_PRO_3) },
> > +	{ .driver_data = MT_CLS_EXPORT_ALL_INPUTS,
> > +		MT_USB_DEVICE(USB_VENDOR_ID_MICROSOFT,
> > +			USB_DEVICE_ID_MS_TYPE_COVER_PRO_3_2) },
> > +	{ .driver_data = MT_CLS_EXPORT_ALL_INPUTS,
> > +		MT_USB_DEVICE(USB_VENDOR_ID_MICROSOFT,
> > +			USB_DEVICE_ID_MS_TYPE_COVER_PRO_3_JP) },
> > +	{ .driver_data = MT_CLS_EXPORT_ALL_INPUTS,
> > +		MT_USB_DEVICE(USB_VENDOR_ID_MICROSOFT,
> > +			USB_DEVICE_ID_MS_TYPE_COVER_3) },
> > +
> >  	/* MosArt panels */
> >  	{ .driver_data = MT_CLS_CONFIDENCE_MINUS_ONE,
> 
> This would mean that both hid-multitouch and hid-microsoft would claim 
> support for this device, which isn't really going to provide consistent 
> user experience. Is MT_CLS_EXPORT_ALL_INPUTS sufficient to provide 
> complete functionality by hid-multitouch? If so, the support from 
> hid-microsoft should be dropped.

Also, please see if the series with the 2 following patches is not
sufficient enough:
http://www.spinics.net/lists/linux-input/msg44576.html
http://www.spinics.net/lists/linux-input/msg44577.html

If you just adapt the second patch to add your ids, it should hopefully
be working (though the surface book is not for some unknown reasons
yet).

Cheers,
Benjamin

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] HID: multitouch: add support for Type Cover Pro 3

[Dennis Chen]

[-- Attachment #1.1: Type: text/plain, Size: 1175 bytes --]

On Fri, 2016-06-17 at 23:11 +0200, Benjamin Tissoires wrote:
> On Jun 17 2016 or thereabouts, Jiri Kosina wrote:
> > This would mean that both hid-multitouch and hid-microsoft would
> > claim support for this device, which isn't really going to provide
> > consistent user experience. Is MT_CLS_EXPORT_ALL_INPUTS sufficient
> > to provide complete functionality by hid-multitouch? If so, the
> > support from hid-microsoft should be dropped.

I find MT_CLS_EXPORT_ALL_INPUTS to make the Type Cover 3 near
completely functional. I'll send a PATCHv2 to drop hid-microsoft
support. However, hid-microsoft provided Caps-Lock LED control, which
hid-multitouch does not; I'm not sure how to fix this. 

> Also, please see if the series with the 2 following patches is not
> sufficient enough:
> http://www.spinics.net/lists/linux-input/msg44576.html
> http://www.spinics.net/lists/linux-input/msg44577.html
> 
> If you just adapt the second patch to add your ids, it should
> hopefully be working (though the surface book is not for some unknown
> reasons
> yet).

I'll test this out sometime this week, and get back to you. 

Sincerely,
Dennis Chen

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5755 bytes --]

Re: [PATCH] HID: multitouch: add support for Type Cover Pro 3

[Benjamin Tissoires]

On Jun 18 2016 or thereabouts, Dennis Chen wrote:
> On Fri, 2016-06-17 at 23:11 +0200, Benjamin Tissoires wrote:
> > On Jun 17 2016 or thereabouts, Jiri Kosina wrote:
> > > This would mean that both hid-multitouch and hid-microsoft would
> > > claim support for this device, which isn't really going to provide
> > > consistent user experience. Is MT_CLS_EXPORT_ALL_INPUTS sufficient
> > > to provide complete functionality by hid-multitouch? If so, the
> > > support from hid-microsoft should be dropped.
> 
> I find MT_CLS_EXPORT_ALL_INPUTS to make the Type Cover 3 near
> completely functional. I'll send a PATCHv2 to drop hid-microsoft
> support. However, hid-microsoft provided Caps-Lock LED control, which
> hid-multitouch does not; I'm not sure how to fix this. 

That's the entire purpose of the series I mentioned below. It adds
the keyboard support, caps lock and multitouch, so there is nothing more
to fix once it is in. The benefit is that MT_CLS_EXPORT_ALL_INPUTS
exports far too many input devices while the series exports only the
required ones.

> 
> > Also, please see if the series with the 2 following patches is not
> > sufficient enough:
> > http://www.spinics.net/lists/linux-input/msg44576.html
> > http://www.spinics.net/lists/linux-input/msg44577.html
> > 
> > If you just adapt the second patch to add your ids, it should
> > hopefully be working (though the surface book is not for some unknown
> > reasons
> > yet).
> 
> I'll test this out sometime this week, and get back to you. 
> 

Thanks.

Cheers,
Benjamin




--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] HID: multitouch: add support for Type Cover Pro 3

[Dennis Chen]

[-- Attachment #1.1: Type: text/plain, Size: 25 bytes --]


Sincerely,
Dennis Chen

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5755 bytes --]

Re: [PATCH] HID: multitouch: add support for Type Cover Pro 3

[Dennis Chen]

[-- Attachment #1.1: Type: text/plain, Size: 725 bytes --]

On Sat, 2016-06-18 at 21:45 +0200, Benjamin Tissoires wrote:
> 
> That's the entire purpose of the series I mentioned below. It adds
> the keyboard support, caps lock and multitouch, so there is nothing
> more to fix once it is in. The benefit is that
> MT_CLS_EXPORT_ALL_INPUTS exports far too many input devices while
> the series exports only the required ones.
> 

Ah. Sorry, I misunderstood. 

I've tested the patches you sent. There is one major issue: the
keyboard/touchpad no longer functions after a disconnect. However, on a
clean bootup of the Surface Pro 3, I can confirm that the keyboard
seems fully functional, i.e. touchpad works, CAPS LOCK LED is toggled,
etc. 

Sincerely,
Dennis Chen

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5755 bytes --]

Re: [PATCH] HID: multitouch: add support for Type Cover Pro 3

[Benjamin Tissoires]

On Jun 19 2016 or thereabouts, Dennis Chen wrote:
> On Sat, 2016-06-18 at 21:45 +0200, Benjamin Tissoires wrote:
> > 
> > That's the entire purpose of the series I mentioned below. It adds
> > the keyboard support, caps lock and multitouch, so there is nothing
> > more to fix once it is in. The benefit is that
> > MT_CLS_EXPORT_ALL_INPUTS exports far too many input devices while
> > the series exports only the required ones.
> > 
> 
> Ah. Sorry, I misunderstood. 

no worries.

> 
> I've tested the patches you sent. There is one major issue: the
> keyboard/touchpad no longer functions after a disconnect. However, on a
> clean bootup of the Surface Pro 3, I can confirm that the keyboard
> seems fully functional, i.e. touchpad works, CAPS LOCK LED is toggled,
> etc. 

When you say disconnect, you mean disconnecting the cover and
re-attaching it? 
If so, I can't understand why it would fail with my patches but
not without. Could you send the dmesg after disconnet/reconnect
and some hid-recorder[1] trace after re-attaching the device when
the keyboard is not working?

Cheers,
Benjamin


[1] http://bentiss.github.io/hid-replay-docs/


--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] HID: multitouch: add support for Type Cover Pro 3

[Dennis Chen]

[-- Attachment #1.1: Type: text/plain, Size: 4137 bytes --]

On Mon, 2016-06-20 at 11:59 +0200, Benjamin Tissoires wrote:
> Could you send the dmesg after disconnet/reconnect
> and some hid-recorder[1] trace after re-attaching the device when
> the keyboard is not working?
> 

I've noticed that the problem resolves itself after a while. Here are
the relevant dmseg lines.

---SNIP--
[ 2409.650260] usb 1-3: new full-speed USB device number 16 using xhci_hcd
[ 2409.815845] usb 1-3: New USB device found, idVendor=045e, idProduct=07dc
[ 2409.815853] usb 1-3: New USB device strings: Mfr=1, Product=2, SerialNumber=3
[ 2409.815858] usb 1-3: Product: Surface Type Cover
[ 2409.815861] usb 1-3: Manufacturer: Microsoft
[ 2409.815864] usb 1-3: SerialNumber: 041396550454
[ 2419.823679] hid-multitouch 0003:045E:07DC.0012: usb_submit_urb(ctrl) failed: -1
[ 2419.823710] hid-multitouch 0003:045E:07DC.0012: timeout initializing reports
[ 2419.824295] input: Microsoft Surface Type Cover Keyboard as /devices/pci0000:00/0000:00:14.0/usb1/1-3/1-3:1.0/0003:045E:07DC.0012/input/input179
[ 2419.875399] input: Microsoft Surface Type Cover Consumer Control as /devices/pci0000:00/0000:00:14.0/usb1/1-3/1-3:1.0/0003:045E:07DC.0012/input/input181
[ 2419.875837] input: Microsoft Surface Type Cover Touchpad as /devices/pci0000:00/0000:00:14.0/usb1/1-3/1-3:1.0/0003:045E:07DC.0012/input/input183
[ 2419.876411] hid-multitouch 0003:045E:07DC.0012: input,hiddev0,hidraw4: USB HID v1.11 Keyboard [Microsoft Surface Type Cover] on usb-0000:00:14.0-3/input0
[ 2419.876641] usb 1-3: USB disconnect, device number 16
[ 2419.934821] hid-multitouch 0003:045E:07DC.0012: usb_submit_urb(ctrl) failed: -19
[ 2420.191569] usb 1-3: new full-speed USB device number 17 using xhci_hcd
[ 2420.356899] usb 1-3: New USB device found, idVendor=045e, idProduct=07dc
[ 2420.356902] usb 1-3: New USB device strings: Mfr=1, Product=2, SerialNumber=3
[ 2420.356903] usb 1-3: Product: Surface Type Cover
[ 2420.356904] usb 1-3: Manufacturer: Microsoft
[ 2420.356905] usb 1-3: SerialNumber: 041396550454
[ 2430.361044] hid-multitouch 0003:045E:07DC.0013: usb_submit_urb(ctrl) failed: -1
[ 2430.361072] hid-multitouch 0003:045E:07DC.0013: timeout initializing reports
[ 2430.361967] input: Microsoft Surface Type Cover Keyboard as /devices/pci0000:00/0000:00:14.0/usb1/1-3/1-3:1.0/0003:045E:07DC.0013/input/input195
[ 2430.414002] input: Microsoft Surface Type Cover Consumer Control as /devices/pci0000:00/0000:00:14.0/usb1/1-3/1-3:1.0/0003:045E:07DC.0013/input/input197
[ 2430.414356] input: Microsoft Surface Type Cover Touchpad as /devices/pci0000:00/0000:00:14.0/usb1/1-3/1-3:1.0/0003:045E:07DC.0013/input/input199
[ 2430.415114] hid-multitouch 0003:045E:07DC.0013: input,hiddev0,hidraw4: USB HID v1.11 Keyboard [Microsoft Surface Type Cover] on usb-0000:00:14.0-3/input0
[ 2430.415365] usb 1-3: USB disconnect, device number 17
[ 2430.457425] hid-multitouch 0003:045E:07DC.0013: usb_submit_urb(ctrl)
failed: -19
[ 2430.714979] usb 1-3: new full-speed USB device number 18 using xhci_hcd
[ 2430.880521] usb 1-3: New USB device found, idVendor=045e, idProduct=07dc
[ 2430.880528] usb 1-3: New USB device strings: Mfr=1, Product=2, SerialNumber=3
[ 2430.880532] usb 1-3: Product: Surface Type Cover
[ 2430.880534] usb 1-3: Manufacturer: Microsoft
[ 2430.880537] usb 1-3: SerialNumber: 041396550454
[ 2434.993319] input: Microsoft Surface Type Cover Keyboard as /devices/pci0000:00/0000:00:14.0/usb1/1-3/1-3:1.0/0003:045E:07DC.0014/input/input211
[ 2435.045167] input: Microsoft Surface Type Cover Consumer Control as /devices/pci0000:00/0000:00:14.0/usb1/1-3/1-3:1.0/0003:045E:07DC.0014/input/input213
[ 2435.045506] input: Microsoft Surface Type Cover Touchpad as /devices/pci0000:00/0000:00:14.0/usb1/1-3/1-3:1.0/0003:045E:07DC.0014/input/input215
[ 2435.046174] hid-multitouch 0003:045E:07DC.0014: input,hiddev0,hidraw4: USB HID v1.11 Keyboard [Microsoft Surface Type Cover] on usb-0000:00:14.0-3/input0
---END---

Hopefully this is helpful. I've looked at the your patch, and - as
you've mentioned - have no idea why this only occurs with your patch. 

Sincerely,
Dennis Chen

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5755 bytes --]

Re: [PATCH] HID: multitouch: add support for Type Cover Pro 3

[Benjamin Tissoires]

On Jun 23 2016 or thereabouts, Dennis Chen wrote:
> On Mon, 2016-06-20 at 11:59 +0200, Benjamin Tissoires wrote:
> > Could you send the dmesg after disconnet/reconnect
> > and some hid-recorder[1] trace after re-attaching the device when
> > the keyboard is not working?
> > 
> 
> I've noticed that the problem resolves itself after a while. Here are
> the relevant dmseg lines.
> 
> ---SNIP--
> [ 2409.650260] usb 1-3: new full-speed USB device number 16 using xhci_hcd
> [ 2409.815845] usb 1-3: New USB device found, idVendor=045e, idProduct=07dc
> [ 2409.815853] usb 1-3: New USB device strings: Mfr=1, Product=2, SerialNumber=3
> [ 2409.815858] usb 1-3: Product: Surface Type Cover
> [ 2409.815861] usb 1-3: Manufacturer: Microsoft
> [ 2409.815864] usb 1-3: SerialNumber: 041396550454
> [ 2419.823679] hid-multitouch 0003:045E:07DC.0012: usb_submit_urb(ctrl) failed: -1
> [ 2419.823710] hid-multitouch 0003:045E:07DC.0012: timeout initializing reports
> [ 2419.824295] input: Microsoft Surface Type Cover Keyboard as /devices/pci0000:00/0000:00:14.0/usb1/1-3/1-3:1.0/0003:045E:07DC.0012/input/input179
> [ 2419.875399] input: Microsoft Surface Type Cover Consumer Control as /devices/pci0000:00/0000:00:14.0/usb1/1-3/1-3:1.0/0003:045E:07DC.0012/input/input181
> [ 2419.875837] input: Microsoft Surface Type Cover Touchpad as /devices/pci0000:00/0000:00:14.0/usb1/1-3/1-3:1.0/0003:045E:07DC.0012/input/input183
> [ 2419.876411] hid-multitouch 0003:045E:07DC.0012: input,hiddev0,hidraw4: USB HID v1.11 Keyboard [Microsoft Surface Type Cover] on usb-0000:00:14.0-3/input0
> [ 2419.876641] usb 1-3: USB disconnect, device number 16
> [ 2419.934821] hid-multitouch 0003:045E:07DC.0012: usb_submit_urb(ctrl) failed: -19
> [ 2420.191569] usb 1-3: new full-speed USB device number 17 using xhci_hcd
> [ 2420.356899] usb 1-3: New USB device found, idVendor=045e, idProduct=07dc
> [ 2420.356902] usb 1-3: New USB device strings: Mfr=1, Product=2, SerialNumber=3
> [ 2420.356903] usb 1-3: Product: Surface Type Cover
> [ 2420.356904] usb 1-3: Manufacturer: Microsoft
> [ 2420.356905] usb 1-3: SerialNumber: 041396550454
> [ 2430.361044] hid-multitouch 0003:045E:07DC.0013: usb_submit_urb(ctrl) failed: -1
> [ 2430.361072] hid-multitouch 0003:045E:07DC.0013: timeout initializing reports
> [ 2430.361967] input: Microsoft Surface Type Cover Keyboard as /devices/pci0000:00/0000:00:14.0/usb1/1-3/1-3:1.0/0003:045E:07DC.0013/input/input195
> [ 2430.414002] input: Microsoft Surface Type Cover Consumer Control as /devices/pci0000:00/0000:00:14.0/usb1/1-3/1-3:1.0/0003:045E:07DC.0013/input/input197
> [ 2430.414356] input: Microsoft Surface Type Cover Touchpad as /devices/pci0000:00/0000:00:14.0/usb1/1-3/1-3:1.0/0003:045E:07DC.0013/input/input199
> [ 2430.415114] hid-multitouch 0003:045E:07DC.0013: input,hiddev0,hidraw4: USB HID v1.11 Keyboard [Microsoft Surface Type Cover] on usb-0000:00:14.0-3/input0
> [ 2430.415365] usb 1-3: USB disconnect, device number 17
> [ 2430.457425] hid-multitouch 0003:045E:07DC.0013: usb_submit_urb(ctrl)
> failed: -19
> [ 2430.714979] usb 1-3: new full-speed USB device number 18 using xhci_hcd
> [ 2430.880521] usb 1-3: New USB device found, idVendor=045e, idProduct=07dc
> [ 2430.880528] usb 1-3: New USB device strings: Mfr=1, Product=2, SerialNumber=3
> [ 2430.880532] usb 1-3: Product: Surface Type Cover
> [ 2430.880534] usb 1-3: Manufacturer: Microsoft
> [ 2430.880537] usb 1-3: SerialNumber: 041396550454
> [ 2434.993319] input: Microsoft Surface Type Cover Keyboard as /devices/pci0000:00/0000:00:14.0/usb1/1-3/1-3:1.0/0003:045E:07DC.0014/input/input211
> [ 2435.045167] input: Microsoft Surface Type Cover Consumer Control as /devices/pci0000:00/0000:00:14.0/usb1/1-3/1-3:1.0/0003:045E:07DC.0014/input/input213
> [ 2435.045506] input: Microsoft Surface Type Cover Touchpad as /devices/pci0000:00/0000:00:14.0/usb1/1-3/1-3:1.0/0003:045E:07DC.0014/input/input215
> [ 2435.046174] hid-multitouch 0003:045E:07DC.0014: input,hiddev0,hidraw4: USB HID v1.11 Keyboard [Microsoft Surface Type Cover] on usb-0000:00:14.0-3/input0
> ---END---
> 
> Hopefully this is helpful. I've looked at the your patch, and - as
> you've mentioned - have no idea why this only occurs with your patch. 
> 

Looks like the same issue Andy is seeing on the Surface Book. So I think
my patch must have a bug where it resets the quirks set by usbhid. And
this is why we are screwed here.

Regarding your disconnect issue, what happens I think is that you
disconnect it before the timeout of usb_submit_urb(), and so the usb
layer is stuck trying to access the device and can't access the newly
plugged one. If you wait enough (initial timeout + new timeout -> 20
secs), then the device behaves properly again.

Cheers,
Benjamin
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] HID: multitouch: add support for Type Cover Pro 3

[Dennis Chen]

[-- Attachment #1.1: Type: text/plain, Size: 798 bytes --]

On Fri, 2016-06-24 at 09:14 +0200, Benjamin Tissoires wrote:
> Looks like the same issue Andy is seeing on the Surface Book. So I
> think my patch must have a bug where it resets the quirks set by
> usbhid. And this is why we are screwed here.
> 

Yes, I think that's likely. There doesn't seem to be an issue most of
the time, so it's not a huge deal. 

> Regarding your disconnect issue, what happens I think is that you
> disconnect it before the timeout of usb_submit_urb(), and so the usb
> layer is stuck trying to access the device and can't access the newly
> plugged one. If you wait enough (initial timeout + new timeout -> 20
> secs), then the device behaves properly again.
> 

That is the behavior I have noticed. Is there any potential fix?

Sincerely,
Dennis Chen

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5755 bytes --]

Re: [PATCH] HID: multitouch: add support for Type Cover Pro 3

[Benjamin Tissoires]

Hi,

On Jun 25 2016 or thereabouts, Dennis Chen wrote:
> On Fri, 2016-06-24 at 09:14 +0200, Benjamin Tissoires wrote:
> > Looks like the same issue Andy is seeing on the Surface Book. So I
> > think my patch must have a bug where it resets the quirks set by
> > usbhid. And this is why we are screwed here.
> > 
> 
> Yes, I think that's likely. There doesn't seem to be an issue most of
> the time, so it's not a huge deal. 

I spent a good amount of time trying to figure out where the bug was,
and I couldn't reproduce it either with uhid or even with usb_gadget.
Even KASan doesn't gives any wrong memory access, and I can't understand
why you get this faulty behavior.

So I must say, I am puzzled on why you end up calling
usbhid_init_reports() while the quirk HID_QUIRK_NO_INIT_REPORTS should
be in place.
Would you mind adding some printk() in hid-multitouch to dump the value
of hdev->quirks before and after calling hid_hw_start() in mt_probe()?

Also, ideally, if you could add a dump_stack() in
drivers/hid/usbhid/hid-core.c, right before leaving
usbhid_init_reports(), that would be awesome.

I would have prefer being able to understand myself what is going on,
but it looks like there is something different happening when I do not
have the device.

> 
> > Regarding your disconnect issue, what happens I think is that you
> > disconnect it before the timeout of usb_submit_urb(), and so the usb
> > layer is stuck trying to access the device and can't access the newly
> > plugged one. If you wait enough (initial timeout + new timeout -> 20
> > secs), then the device behaves properly again.
> > 
> 
> That is the behavior I have noticed. Is there any potential fix?
> 

The fix is to not fetch the buggy features that timeout. For that, we
usually set the flag HID_QUIRK_NO_INIT_REPORTS, which prevents it. But
in your case, the code says that the flag is set, while the code is
still called :(

Thanks,
Benjamin


--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] HID: multitouch: add support for Type Cover Pro 3

[Dennis Chen]

[-- Attachment #1.1: Type: text/plain, Size: 4878 bytes --]


On Fri, 2016-07-01 at 16:52 +0200, Benjamin Tissoires wrote:
> 
> I spent a good amount of time trying to figure out where the bug was,
> and I couldn't reproduce it either with uhid or even with usb_gadget.
> Even KASan doesn't gives any wrong memory access, and I can't
> understand why you get this faulty behavior.
> 
> So I must say, I am puzzled on why you end up calling
> usbhid_init_reports() while the quirk HID_QUIRK_NO_INIT_REPORTS
> should be in place.
> Would you mind adding some printk() in hid-multitouch to dump the
> value of hdev->quirks before and after calling hid_hw_start() in
> mt_probe()?
> 
> Also, ideally, if you could add a dump_stack() in
> drivers/hid/usbhid/hid-core.c, right before leaving
> usbhid_init_reports(), that would be awesome.
> 

Here's a dmesg dump. It seems the quirks aren't cleared.

[    3.787730] Pre-hid_hw_start hdev->quirks: -2147483328
[    3.787731] clocksource: Switched to clocksource tsc
[    4.217617] scsi 4:0:0:0: Direct-Access     Generic- USB3.0 CRW   -SD 1.00 PQ: 0 ANSI: 6
[    4.218413] sd 4:0:0:0: Attached scsi generic sg2 type 0
[    5.011924] Console: switching to colour frame buffer device 270x90
[    5.030415] i915 0000:00:02.0: fb0: inteldrmfb frame buffer device
[    5.471214] sd 4:0:0:0: [sdc] 31116288 512-byte logical blocks: (15.9 GB/14.8 GiB)
[    5.474542] sd 4:0:0:0: [sdc] Write Protect is off
[    5.474547] sd 4:0:0:0: [sdc] Mode Sense: 2f 00 00 00
[    5.477097] sd 4:0:0:0: [sdc] Write cache: disabled, read cache: enabled, doesn't support DPO or FUA
[    5.492349]  sdc: sdc1
[    5.494359] sd 4:0:0:0: [sdc] Attached SCSI removable disk
[    7.817402] CPU: 3 PID: 338 Comm: systemd-udevd Not tainted 4.7.0-rc7+ #7
[    7.817409] Hardware name: Microsoft Corporation Surface Pro 3/Surface Pro 3, BIOS 3.11.0850 04/07/2015
[    7.817413]  0000000000000286 000000007db35b48 ffff880145c839f8 ffffffff813d618f
[    7.817420]  0000000000000000 ffff880149404000 ffff880145c83a28 ffffffff81688b7e
[    7.817425]  ffff880145c68350 0000000000000002 ffff880149404000 ffff88003fb5ad08
[    7.817430] Call Trace:
[    7.817443]  [<ffffffff813d618f>] dump_stack+0x63/0x84
[    7.817453]  [<ffffffff81688b7e>] usbhid_init_reports+0xde/0x110
[    7.817459]  [<ffffffff816890da>] usbhid_start+0x52a/0x770
[    7.817465]  [<ffffffff81686fc0>] ? hid_retry_timeout+0x60/0x60
[    7.817471]  [<ffffffff81686710>] ? usbhid_restart_out_queue+0x110/0x110
[    7.817481]  [<ffffffffa00535f5>] mt_probe+0x1b5/0x263 [hid_multitouch]
[    7.817487]  [<ffffffff8167c707>] hid_device_probe+0xd7/0x150
[    7.817492]  [<ffffffff8151cbac>] driver_probe_device+0x22c/0x440
[    7.817496]  [<ffffffff8151ce91>] __driver_attach+0xd1/0xf0
[    7.817500]  [<ffffffff8151cdc0>] ? driver_probe_device+0x440/0x440
[    7.817508]  [<ffffffff8151a4dc>] bus_for_each_dev+0x6c/0xc0
[    7.817512]  [<ffffffff8151c29e>] driver_attach+0x1e/0x20
[    7.817517]  [<ffffffff8151bce3>] bus_add_driver+0x1c3/0x280
[    7.817522]  [<ffffffffa0011000>] ? 0xffffffffa0011000
[    7.817526]  [<ffffffff8151d7e0>] driver_register+0x60/0xe0
[    7.817529]  [<ffffffffa0011000>] ? 0xffffffffa0011000
[    7.817533]  [<ffffffff8167b543>] __hid_register_driver+0x53/0x90
[    7.817540]  [<ffffffffa001101e>] mt_driver_init+0x1e/0x1000 [hid_multitouch]
[    7.817547]  [<ffffffff81002190>] do_one_initcall+0x50/0x180
[    7.817554]  [<ffffffff811d9cba>] ? kvfree+0x2a/0x40
[    7.817561]  [<ffffffff81221099>] ? kfree+0x159/0x170
[    7.817567]  [<ffffffff8121fb82>] ? kmem_cache_alloc_trace+0x182/0x1d0
[    7.817574]  [<ffffffff811b6116>] ? do_init_module+0x27/0x1d8
[    7.817580]  [<ffffffff811b614e>] do_init_module+0x5f/0x1d8
[    7.817589]  [<ffffffff8112ecdc>] load_module+0x1fdc/0x27d0
[    7.817595]  [<ffffffff8112bbb0>] ? __symbol_put+0x60/0x60
[    7.817605]  [<ffffffff8124555b>] ? vfs_read+0x11b/0x130
[    7.817612]  [<ffffffff8112f746>] SYSC_finit_module+0xe6/0x120
[    7.817619]  [<ffffffff8112f79e>] SyS_finit_module+0xe/0x10
[    7.817623]  [<ffffffff81003d52>] do_syscall_64+0x62/0x110
[    7.817631]  [<ffffffff817e0ca1>] entry_SYSCALL64_slow_path+0x25/0x25
[    7.818076] input: Microsoft Surface Type Cover Keyboard as /devices/pci0000:00/0000:00:14.0/usb1/1-3/1-3:1.0/0003:045E:07DC.0001/input/input2
[    7.869886] input: Microsoft Surface Type Cover Consumer Control as /devices/pci0000:00/0000:00:14.0/usb1/1-3/1-3:1.0/0003:045E:07DC.0001/input/input4
[    7.870276] input: Microsoft Surface Type Cover Touchpad as /devices/pci0000:00/0000:00:14.0/usb1/1-3/1-3:1.0/0003:045E:07DC.0001/input/input6
[    7.870849] hid-multitouch 0003:045E:07DC.0001: input,hiddev0,hidraw0: USB HID v1.11 Keyboard [Microsoft Surface Type Cover] on usb-0000:00:14.0-3/input0
[    7.870857] Post-hid_hw_start hdev->quirks: -2147483328

Sincerely,
Dennis Chen

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5755 bytes --]

Re: [PATCH] HID: multitouch: add support for Type Cover Pro 3

[Benjamin Tissoires]

On Jul 23 2016 or thereabouts, Dennis Chen wrote:
> 
> On Fri, 2016-07-01 at 16:52 +0200, Benjamin Tissoires wrote:
> > 
> > I spent a good amount of time trying to figure out where the bug was,
> > and I couldn't reproduce it either with uhid or even with usb_gadget.
> > Even KASan doesn't gives any wrong memory access, and I can't
> > understand why you get this faulty behavior.
> > 
> > So I must say, I am puzzled on why you end up calling
> > usbhid_init_reports() while the quirk HID_QUIRK_NO_INIT_REPORTS
> > should be in place.
> > Would you mind adding some printk() in hid-multitouch to dump the
> > value of hdev->quirks before and after calling hid_hw_start() in
> > mt_probe()?
> > 
> > Also, ideally, if you could add a dump_stack() in
> > drivers/hid/usbhid/hid-core.c, right before leaving
> > usbhid_init_reports(), that would be awesome.
> > 
> 
> Here's a dmesg dump. It seems the quirks aren't cleared.

Thanks for the logs. They confirm the hid-core patch is not
interfering, but there is somethign weird:

> 
> [    3.787730] Pre-hid_hw_start hdev->quirks: -2147483328

This should hopefully translate to 0x80000140 which means:
  HID_QUIRK_NO_INPUT_SYNC | 
  HID_QUIRK_MULTI_INPUT |
  HID_QUIRK_NO_EMPTY_INPUT

The issue is that HID_QUIRK_NO_INIT_REPORTS is not set, and this should
be set by usbhid through the table hid_blacklist.
Did you also amend this table while applying the patches?

If not, I'd like to see the value of the quirks just at the beginning of
mt_probe to understand where we clear the quirk.

Cheers,
Benjamin

> [    3.787731] clocksource: Switched to clocksource tsc
> [    4.217617] scsi 4:0:0:0: Direct-Access     Generic- USB3.0 CRW   -SD 1.00 PQ: 0 ANSI: 6
> [    4.218413] sd 4:0:0:0: Attached scsi generic sg2 type 0
> [    5.011924] Console: switching to colour frame buffer device 270x90
> [    5.030415] i915 0000:00:02.0: fb0: inteldrmfb frame buffer device
> [    5.471214] sd 4:0:0:0: [sdc] 31116288 512-byte logical blocks: (15.9 GB/14.8 GiB)
> [    5.474542] sd 4:0:0:0: [sdc] Write Protect is off
> [    5.474547] sd 4:0:0:0: [sdc] Mode Sense: 2f 00 00 00
> [    5.477097] sd 4:0:0:0: [sdc] Write cache: disabled, read cache: enabled, doesn't support DPO or FUA
> [    5.492349]  sdc: sdc1
> [    5.494359] sd 4:0:0:0: [sdc] Attached SCSI removable disk
> [    7.817402] CPU: 3 PID: 338 Comm: systemd-udevd Not tainted 4.7.0-rc7+ #7
> [    7.817409] Hardware name: Microsoft Corporation Surface Pro 3/Surface Pro 3, BIOS 3.11.0850 04/07/2015
> [    7.817413]  0000000000000286 000000007db35b48 ffff880145c839f8 ffffffff813d618f
> [    7.817420]  0000000000000000 ffff880149404000 ffff880145c83a28 ffffffff81688b7e
> [    7.817425]  ffff880145c68350 0000000000000002 ffff880149404000 ffff88003fb5ad08
> [    7.817430] Call Trace:
> [    7.817443]  [<ffffffff813d618f>] dump_stack+0x63/0x84
> [    7.817453]  [<ffffffff81688b7e>] usbhid_init_reports+0xde/0x110
> [    7.817459]  [<ffffffff816890da>] usbhid_start+0x52a/0x770
> [    7.817465]  [<ffffffff81686fc0>] ? hid_retry_timeout+0x60/0x60
> [    7.817471]  [<ffffffff81686710>] ? usbhid_restart_out_queue+0x110/0x110
> [    7.817481]  [<ffffffffa00535f5>] mt_probe+0x1b5/0x263 [hid_multitouch]
> [    7.817487]  [<ffffffff8167c707>] hid_device_probe+0xd7/0x150
> [    7.817492]  [<ffffffff8151cbac>] driver_probe_device+0x22c/0x440
> [    7.817496]  [<ffffffff8151ce91>] __driver_attach+0xd1/0xf0
> [    7.817500]  [<ffffffff8151cdc0>] ? driver_probe_device+0x440/0x440
> [    7.817508]  [<ffffffff8151a4dc>] bus_for_each_dev+0x6c/0xc0
> [    7.817512]  [<ffffffff8151c29e>] driver_attach+0x1e/0x20
> [    7.817517]  [<ffffffff8151bce3>] bus_add_driver+0x1c3/0x280
> [    7.817522]  [<ffffffffa0011000>] ? 0xffffffffa0011000
> [    7.817526]  [<ffffffff8151d7e0>] driver_register+0x60/0xe0
> [    7.817529]  [<ffffffffa0011000>] ? 0xffffffffa0011000
> [    7.817533]  [<ffffffff8167b543>] __hid_register_driver+0x53/0x90
> [    7.817540]  [<ffffffffa001101e>] mt_driver_init+0x1e/0x1000 [hid_multitouch]
> [    7.817547]  [<ffffffff81002190>] do_one_initcall+0x50/0x180
> [    7.817554]  [<ffffffff811d9cba>] ? kvfree+0x2a/0x40
> [    7.817561]  [<ffffffff81221099>] ? kfree+0x159/0x170
> [    7.817567]  [<ffffffff8121fb82>] ? kmem_cache_alloc_trace+0x182/0x1d0
> [    7.817574]  [<ffffffff811b6116>] ? do_init_module+0x27/0x1d8
> [    7.817580]  [<ffffffff811b614e>] do_init_module+0x5f/0x1d8
> [    7.817589]  [<ffffffff8112ecdc>] load_module+0x1fdc/0x27d0
> [    7.817595]  [<ffffffff8112bbb0>] ? __symbol_put+0x60/0x60
> [    7.817605]  [<ffffffff8124555b>] ? vfs_read+0x11b/0x130
> [    7.817612]  [<ffffffff8112f746>] SYSC_finit_module+0xe6/0x120
> [    7.817619]  [<ffffffff8112f79e>] SyS_finit_module+0xe/0x10
> [    7.817623]  [<ffffffff81003d52>] do_syscall_64+0x62/0x110
> [    7.817631]  [<ffffffff817e0ca1>] entry_SYSCALL64_slow_path+0x25/0x25
> [    7.818076] input: Microsoft Surface Type Cover Keyboard as /devices/pci0000:00/0000:00:14.0/usb1/1-3/1-3:1.0/0003:045E:07DC.0001/input/input2
> [    7.869886] input: Microsoft Surface Type Cover Consumer Control as /devices/pci0000:00/0000:00:14.0/usb1/1-3/1-3:1.0/0003:045E:07DC.0001/input/input4
> [    7.870276] input: Microsoft Surface Type Cover Touchpad as /devices/pci0000:00/0000:00:14.0/usb1/1-3/1-3:1.0/0003:045E:07DC.0001/input/input6
> [    7.870849] hid-multitouch 0003:045E:07DC.0001: input,hiddev0,hidraw0: USB HID v1.11 Keyboard [Microsoft Surface Type Cover] on usb-0000:00:14.0-3/input0
> [    7.870857] Post-hid_hw_start hdev->quirks: -2147483328
> 
> Sincerely,
> Dennis Chen




--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] HID: multitouch: add support for Type Cover Pro 3

[Dennis Chen]

[-- Attachment #1.1: Type: text/plain, Size: 533 bytes --]


On Mon, 2016-07-25 at 12:16 +0200, Benjamin Tissoires wrote:
[SNIP]

> This should hopefully translate to 0x80000140 which means:
>   HID_QUIRK_NO_INPUT_SYNC | 
>   HID_QUIRK_MULTI_INPUT |
>   HID_QUIRK_NO_EMPTY_INPUT
> 
> The issue is that HID_QUIRK_NO_INIT_REPORTS is not set, and this
> should be set by usbhid through the table hid_blacklist.
> Did you also amend this table while applying the patches?

The table was amended in the same fashion as your second patch of the
series. 

Sincerely,
Dennis Chen

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5755 bytes --]