| Message ID | 1443089571-16561-1-git-send-email-mika.westerberg@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Jiri Kosina |
| Headers | show |
Hi Mika, On Sep 24 2015 or thereabouts, Mika Westerberg wrote: > Some newer Intel Skylake based Dell laptops with Win8 precision touchpad > fail when initial feature reports are fetched from it. Below is an That is unfortunate. > example output with some additional debug included: > > i2c_hid i2c-DLL0704:01: Fetching the HID descriptor > i2c_hid i2c-DLL0704:01: __i2c_hid_command: cmd=20 00 > i2c_hid i2c-DLL0704:01: HID Descriptor: 1e 00 00 01 99 02 21 00 24 ... > ... > i2c_hid i2c-DLL0704:01: i2c_hid_get_report > i2c_hid i2c-DLL0704:01: __i2c_hid_command: cmd=22 00 38 02 23 00 > i2c_hid i2c-DLL0704:01: report (len=4): 04 00 08 05 > i2c_hid i2c-DLL0704:01: report id 13 > i2c_hid i2c-DLL0704:01: i2c_hid_get_report > i2c_hid i2c-DLL0704:01: __i2c_hid_command: cmd=22 00 3d 02 23 00 > i2c_hid i2c-DLL0704:01: failed to retrieve report from device. > i2c_hid i2c-DLL0704:01: report id 7 > i2c_hid i2c-DLL0704:01: i2c_hid_get_report > i2c_hid i2c-DLL0704:01: __i2c_hid_command: cmd=22 00 37 02 23 00 > i2c_hid i2c-DLL0704:01: report (len=259): 03 01 07 fc 28 fe 84 40 ... > i2c_hid i2c-DLL0704:01: report id 4 > i2c_hid i2c-DLL0704:01: i2c_hid_get_report > i2c_hid i2c-DLL0704:01: __i2c_hid_command: cmd=22 00 34 02 23 00 > > We manage to fetch few reports but then the touchpad dies: > > i2c_designware i2c_designware.1: i2c_dw_handle_tx_abort: lost arbitration > i2c_hid i2c-DLL0704:01: failed to retrieve report from device. > > it eventually pulls the whole I2C bus low: > > i2c_designware i2c_designware.1: controller timed out > i2c_hid i2c-DLL0704:01: failed to set a report to device. > > Fix this by preventing initial feature report fetch for Win8 devices. It > should not be needed anyway according to [1] because the two "GET" > feature reports for these devices are about device capabilities and Win8 > certification status which are not related to input events. > > [1] https://msdn.microsoft.com/en-us/library/windows/hardware/dn467314(v=vs.85).aspx > > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> > --- > drivers/hid/hid-multitouch.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c > index 426b2f1a3450..1911f7698511 100644 > --- a/drivers/hid/hid-multitouch.c > +++ b/drivers/hid/hid-multitouch.c > @@ -1020,14 +1020,16 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id) > /* > * Handle special quirks for Windows 8 certified devices. > */ > - if (id->group == HID_GROUP_MULTITOUCH_WIN_8) > + if (id->group == HID_GROUP_MULTITOUCH_WIN_8) { > /* > * Some multitouch screens do not like to be polled for input > - * reports. Fortunately, the Win8 spec says that all touches > - * should be sent during each report, making the initialization > - * of input reports unnecessary. > + * and feature reports. Fortunately, the Win8 spec says that all > + * touches should be sent during each report, making the > + * initialization of input reports unnecessary. > */ > hdev->quirks |= HID_QUIRK_NO_INIT_INPUT_REPORTS; > + hdev->quirks |= HID_QUIRK_NO_INIT_REPORTS; Problem is that with this quirk, event the features we are interested in (and that the device should support or Microsoft would complain) are not pre-read. I think we should set HID_QUIRK_NO_INIT_REPORTS (and remove HID_QUIRK_NO_INIT_INPUT_REPORTS BTW), and set up a quirk to actually retrieve the value of the feature in mt_feature_mapping. This way, we would be sure to only probe for features that are supported (contact count is the most important one). Cheers, Benjamin > + } > > td = devm_kzalloc(&hdev->dev, sizeof(struct mt_device), GFP_KERNEL); > if (!td) { > -- > 2.5.1 > -- 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 Thu, Sep 24, 2015 at 09:16:01PM -0400, Benjamin Tissoires wrote: > > diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c > > index 426b2f1a3450..1911f7698511 100644 > > --- a/drivers/hid/hid-multitouch.c > > +++ b/drivers/hid/hid-multitouch.c > > @@ -1020,14 +1020,16 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id) > > /* > > * Handle special quirks for Windows 8 certified devices. > > */ > > - if (id->group == HID_GROUP_MULTITOUCH_WIN_8) > > + if (id->group == HID_GROUP_MULTITOUCH_WIN_8) { > > /* > > * Some multitouch screens do not like to be polled for input > > - * reports. Fortunately, the Win8 spec says that all touches > > - * should be sent during each report, making the initialization > > - * of input reports unnecessary. > > + * and feature reports. Fortunately, the Win8 spec says that all > > + * touches should be sent during each report, making the > > + * initialization of input reports unnecessary. > > */ > > hdev->quirks |= HID_QUIRK_NO_INIT_INPUT_REPORTS; > > + hdev->quirks |= HID_QUIRK_NO_INIT_REPORTS; > > Problem is that with this quirk, event the features we are interested in > (and that the device should support or Microsoft would complain) are not > pre-read. I think we should set HID_QUIRK_NO_INIT_REPORTS (and remove > HID_QUIRK_NO_INIT_INPUT_REPORTS BTW), and set up a quirk to actually > retrieve the value of the feature in mt_feature_mapping. This way, we > would be sure to only probe for features that are supported (contact > count is the most important one). Good point. I didn't realize that we actually use those features -- somehow I missed the whole mt_feature_mapping() function :-/ I'll try to implement what you suggested instead. Thanks. -- 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
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c index 426b2f1a3450..1911f7698511 100644 --- a/drivers/hid/hid-multitouch.c +++ b/drivers/hid/hid-multitouch.c @@ -1020,14 +1020,16 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id) /* * Handle special quirks for Windows 8 certified devices. */ - if (id->group == HID_GROUP_MULTITOUCH_WIN_8) + if (id->group == HID_GROUP_MULTITOUCH_WIN_8) { /* * Some multitouch screens do not like to be polled for input - * reports. Fortunately, the Win8 spec says that all touches - * should be sent during each report, making the initialization - * of input reports unnecessary. + * and feature reports. Fortunately, the Win8 spec says that all + * touches should be sent during each report, making the + * initialization of input reports unnecessary. */ hdev->quirks |= HID_QUIRK_NO_INIT_INPUT_REPORTS; + hdev->quirks |= HID_QUIRK_NO_INIT_REPORTS; + } td = devm_kzalloc(&hdev->dev, sizeof(struct mt_device), GFP_KERNEL); if (!td) {
Some newer Intel Skylake based Dell laptops with Win8 precision touchpad fail when initial feature reports are fetched from it. Below is an example output with some additional debug included: i2c_hid i2c-DLL0704:01: Fetching the HID descriptor i2c_hid i2c-DLL0704:01: __i2c_hid_command: cmd=20 00 i2c_hid i2c-DLL0704:01: HID Descriptor: 1e 00 00 01 99 02 21 00 24 ... ... i2c_hid i2c-DLL0704:01: i2c_hid_get_report i2c_hid i2c-DLL0704:01: __i2c_hid_command: cmd=22 00 38 02 23 00 i2c_hid i2c-DLL0704:01: report (len=4): 04 00 08 05 i2c_hid i2c-DLL0704:01: report id 13 i2c_hid i2c-DLL0704:01: i2c_hid_get_report i2c_hid i2c-DLL0704:01: __i2c_hid_command: cmd=22 00 3d 02 23 00 i2c_hid i2c-DLL0704:01: failed to retrieve report from device. i2c_hid i2c-DLL0704:01: report id 7 i2c_hid i2c-DLL0704:01: i2c_hid_get_report i2c_hid i2c-DLL0704:01: __i2c_hid_command: cmd=22 00 37 02 23 00 i2c_hid i2c-DLL0704:01: report (len=259): 03 01 07 fc 28 fe 84 40 ... i2c_hid i2c-DLL0704:01: report id 4 i2c_hid i2c-DLL0704:01: i2c_hid_get_report i2c_hid i2c-DLL0704:01: __i2c_hid_command: cmd=22 00 34 02 23 00 We manage to fetch few reports but then the touchpad dies: i2c_designware i2c_designware.1: i2c_dw_handle_tx_abort: lost arbitration i2c_hid i2c-DLL0704:01: failed to retrieve report from device. it eventually pulls the whole I2C bus low: i2c_designware i2c_designware.1: controller timed out i2c_hid i2c-DLL0704:01: failed to set a report to device. Fix this by preventing initial feature report fetch for Win8 devices. It should not be needed anyway according to [1] because the two "GET" feature reports for these devices are about device capabilities and Win8 certification status which are not related to input events. [1] https://msdn.microsoft.com/en-us/library/windows/hardware/dn467314(v=vs.85).aspx Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> --- drivers/hid/hid-multitouch.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)