[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20151007133409.GA25728@mail.corp.redhat.com>
Date: Wed, 7 Oct 2015 09:34:10 -0400
From: Benjamin Tissoires <benjamin.tissoires@...hat.com>
To: Mika Westerberg <mika.westerberg@...ux.intel.com>
Cc: Jiri Kosina <jikos@...nel.org>, linux-input@...r.kernel.org,
linux-kernel@...r.kernel.org,
Seth Forshee <seth.forshee@...onical.com>,
Andrew Duggan <aduggan@...aptics.com>
Subject: Re: [PATCH v2] HID: multitouch: Fetch feature reports on demand for
Win8 devices
[sorry for the late answer Jiri, I was on vacations last week]
On Oct 07 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 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 retrieval for Win8 devices.
> Instead we fetch reports as needed in mt_feature_mapping(). This prevents
> fetching reports which might cause problems with the device in question.
>
> Suggested-by: Benjamin Tissoires <benjamin.tissoires@...hat.com>
> Signed-off-by: Mika Westerberg <mika.westerberg@...ux.intel.com>
> ---
> Added check for MT_CLS_WIN_8 so that the fix is only done for real Win8
> devices.
I think we are good now. I would just like to have Seth or Andrew giving
a tested-by for Windows precision touchpads that can be found on the Dell
laptops.
If the buttonpad property is set correctly, this is:
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@...hat.com>
Cheers,
Benjamin
>
> drivers/hid/hid-multitouch.c | 45 +++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 44 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
> index 661b4fce1a5d..dd64461e5e85 100644
> --- a/drivers/hid/hid-multitouch.c
> +++ b/drivers/hid/hid-multitouch.c
> @@ -309,6 +309,41 @@ static struct attribute_group mt_attribute_group = {
> .attrs = sysfs_attrs
> };
>
> +static void mt_get_feature(struct hid_device *hdev, struct hid_report *report)
> +{
> + struct mt_device *td = hid_get_drvdata(hdev);
> + int ret, size = hid_report_len(report);
> + u8 *buf;
> +
> + /*
> + * Only fetch the feature report if initial reports are not already
> + * been retrieved. Currently this is only done for Windows 8 touch
> + * devices.
> + */
> + if (!(hdev->quirks & HID_QUIRK_NO_INIT_REPORTS))
> + return;
> + if (td->mtclass.name != MT_CLS_WIN_8)
> + return;
> +
> + buf = hid_alloc_report_buf(report, GFP_KERNEL);
> + if (!buf)
> + return;
> +
> + ret = hid_hw_raw_request(hdev, report->id, buf, size,
> + HID_FEATURE_REPORT, HID_REQ_GET_REPORT);
> + if (ret < 0) {
> + dev_warn(&hdev->dev, "failed to fetch feature %d\n",
> + report->id);
> + } else {
> + ret = hid_report_raw_event(hdev, HID_FEATURE_REPORT, buf,
> + size, 0);
> + if (ret)
> + dev_warn(&hdev->dev, "failed to report feature\n");
> + }
> +
> + kfree(buf);
> +}
> +
> static void mt_feature_mapping(struct hid_device *hdev,
> struct hid_field *field, struct hid_usage *usage)
> {
> @@ -327,6 +362,8 @@ static void mt_feature_mapping(struct hid_device *hdev,
>
> break;
> case HID_DG_CONTACTMAX:
> + mt_get_feature(hdev, field->report);
> +
> td->maxcontact_report_id = field->report->id;
> td->maxcontacts = field->value[0];
> if (!td->maxcontacts &&
> @@ -343,6 +380,7 @@ static void mt_feature_mapping(struct hid_device *hdev,
> break;
> }
>
> + mt_get_feature(hdev, field->report);
> if (field->value[usage->usage_index] == MT_BUTTONTYPE_CLICKPAD)
> td->is_buttonpad = true;
>
> @@ -1029,8 +1067,13 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
> * reports. Fortunately, the Win8 spec says that all touches
> * should be sent during each report, making the initialization
> * of input reports unnecessary.
> + *
> + * In addition some touchpads do not behave well if we read
> + * all feature reports from them. Instead we prevent
> + * initial report fetching and then selectively fetch each
> + * report we are interested in.
> */
> - 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.3
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists