| Message ID | 1466196443.21223.1.camel@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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,
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
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
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
Sincerely, Dennis Chen
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
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
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
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
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
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
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
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
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
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,
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(+) -- 2.5.5