* [PATCH] HID: multitouch: Do not fetch initial feature reports for Win8 devices
@ 2015-09-24 10:12 Mika Westerberg
2015-09-25 1:16 ` Benjamin Tissoires
0 siblings, 1 reply; 3+ messages in thread
From: Mika Westerberg @ 2015-09-24 10:12 UTC (permalink / raw)
To: Jiri Kosina
Cc: Benjamin Tissoires, Mika Westerberg, linux-input, linux-kernel
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(-)
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) {
--
2.5.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] HID: multitouch: Do not fetch initial feature reports for Win8 devices
2015-09-24 10:12 [PATCH] HID: multitouch: Do not fetch initial feature reports for Win8 devices Mika Westerberg
@ 2015-09-25 1:16 ` Benjamin Tissoires
2015-09-25 14:23 ` Mika Westerberg
0 siblings, 1 reply; 3+ messages in thread
From: Benjamin Tissoires @ 2015-09-25 1:16 UTC (permalink / raw)
To: Mika Westerberg; +Cc: Jiri Kosina, linux-input, linux-kernel
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
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] HID: multitouch: Do not fetch initial feature reports for Win8 devices
2015-09-25 1:16 ` Benjamin Tissoires
@ 2015-09-25 14:23 ` Mika Westerberg
0 siblings, 0 replies; 3+ messages in thread
From: Mika Westerberg @ 2015-09-25 14:23 UTC (permalink / raw)
To: Benjamin Tissoires; +Cc: Jiri Kosina, linux-input, linux-kernel
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.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2015-09-25 14:25 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-09-24 10:12 [PATCH] HID: multitouch: Do not fetch initial feature reports for Win8 devices Mika Westerberg
2015-09-25 1:16 ` Benjamin Tissoires
2015-09-25 14:23 ` Mika Westerberg
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox