* Re: [PATCH] Partially revert "HID: generic: create one input report per application type"
From: Benjamin Tissoires @ 2018-09-04 13:33 UTC (permalink / raw)
To: Jiri Kosina, Dmitry Torokhov; +Cc: open list:HID CORE LAYER, lkml, 3.8+
In-Reply-To: <CAO-hwJKxAt95WTrNT6-SfejEpxZvv5aAgoF25SjTxuTmti=ZUQ@mail.gmail.com>
On Tue, Sep 4, 2018 at 1:55 PM Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
>
> On Fri, Aug 31, 2018 at 11:36 AM Benjamin Tissoires
> <benjamin.tissoires@redhat.com> wrote:
> >
> > This partially reverts commit f07b3c1da92db108662f99417a212fc1eddc44d1.
> >
> > It looks like some mice are not correctly treated by
> > HID_QUIRK_INPUT_PER_APP. Those mice have the following
> > report descriptor:
> >
> > 0x05, 0x01, // Usage Page (Generic Desktop) 0
> > 0x09, 0x02, // Usage (Mouse) 2
> > 0xa1, 0x01, // Collection (Application) 4
> > 0x85, 0x01, // Report ID (1) 6
> > 0x09, 0x01, // Usage (Pointer) 8
> > 0xa1, 0x00, // Collection (Physical) 10
> > 0x95, 0x05, // Report Count (5) 12
> > 0x75, 0x01, // Report Size (1) 14
> > 0x05, 0x09, // Usage Page (Button) 16
> > 0x19, 0x01, // Usage Minimum (1) 18
> > 0x29, 0x05, // Usage Maximum (5) 20
> > 0x15, 0x00, // Logical Minimum (0) 22
> > 0x25, 0x01, // Logical Maximum (1) 24
> > 0x81, 0x02, // Input (Data,Var,Abs) 26
> > ...
> > 0xc0, // End Collection 57
> > 0x85, 0x02, // Report ID (2) 58
> > 0x09, 0x01, // Usage (Consumer Control) 60
> > 0xa1, 0x00, // Collection (Physical) 62
> > 0x75, 0x0c, // Report Size (12) 64
> > 0x95, 0x02, // Report Count (2) 66
> > 0x05, 0x01, // Usage Page (Generic Desktop) 68
> > 0x09, 0x30, // Usage (X) 70
> > 0x09, 0x31, // Usage (Y) 72
> > 0x16, 0x01, 0xf8, // Logical Minimum (-2047) 74
> > 0x26, 0xff, 0x07, // Logical Maximum (2047) 77
> > 0x81, 0x06, // Input (Data,Var,Rel) 80
> > 0xc0, // End Collection 82
> > 0xc0, // End Collection 83
> > ...
> >
> > Both the cursor position and the buttons are located in the
> > same application collection (Mouse) and the kernel should
> > only create one input device for those.
> >
> > However, for an undetermined reason, the kernel splits the
> > device in 2, making systemd not tagging the second mouse
> > with the coordinates only as a mouse. And then userspace
> > ignores it which leads to a mouse where only the buttons
> > are working.
> >
> > Until the quirk gets properly fixed, we should probably
> > revert applying it to all of the generic devices and
> > re-enable it when the root reason has been found.
>
> Jiri,
>
> I actually just got the proper fix today. I think it would be better
> to directly take the fix instead of the revert and a revert of the
> revert later.
>
> I just need to make sure the tests are correctly handled and I should
> be able to submit the patch today.
Patch now submitted: https://patchwork.kernel.org/patch/10587369/
Cheers,
Benjamin
^ permalink raw reply
* [PATCH 4/4] HID: multitouch: simplify the application retrieval
From: Benjamin Tissoires @ 2018-09-04 13:31 UTC (permalink / raw)
To: Jiri Kosina, Dmitry Torokhov
Cc: Benjamin Tissoires, linux-input, linux-kernel
In-Reply-To: <20180904133115.5111-1-benjamin.tissoires@redhat.com>
Now that the application is simply stored in struct hid_input, we can
overwrite it in mt_input_mapping() for the faulty egalax and have a
simpler suffix processing in mt_input_configured()
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
drivers/hid/hid-multitouch.c | 72 ++++++++++++++++++++------------------------
1 file changed, 32 insertions(+), 40 deletions(-)
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index da954f3f4da7..f7c6de2b6730 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -1319,6 +1319,13 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
return mt_touch_input_mapping(hdev, hi, field, usage, bit, max,
application);
+ /*
+ * some egalax touchscreens have "application == DG_TOUCHSCREEN"
+ * for the stylus. Overwrite the hid_input application
+ */
+ if (field->physical == HID_DG_STYLUS)
+ hi->application = HID_DG_STYLUS;
+
/* let hid-core decide for the others */
return 0;
}
@@ -1507,14 +1514,12 @@ static int mt_input_configured(struct hid_device *hdev, struct hid_input *hi)
struct mt_device *td = hid_get_drvdata(hdev);
char *name;
const char *suffix = NULL;
- unsigned int application = 0;
struct mt_report_data *rdata;
struct mt_application *mt_application = NULL;
struct hid_report *report;
int ret;
list_for_each_entry(report, &hi->reports, hidinput_list) {
- application = report->application;
rdata = mt_find_report_data(td, report);
if (!rdata) {
hid_err(hdev, "failed to allocate data for report\n");
@@ -1529,46 +1534,33 @@ static int mt_input_configured(struct hid_device *hdev, struct hid_input *hi)
if (ret)
return ret;
}
-
- /*
- * some egalax touchscreens have "application == DG_TOUCHSCREEN"
- * for the stylus. Check this first, and then rely on
- * the application field.
- */
- if (report->field[0]->physical == HID_DG_STYLUS) {
- suffix = "Pen";
- /* force BTN_STYLUS to allow tablet matching in udev */
- __set_bit(BTN_STYLUS, hi->input->keybit);
- }
}
- if (!suffix) {
- switch (application) {
- case HID_GD_KEYBOARD:
- case HID_GD_KEYPAD:
- case HID_GD_MOUSE:
- case HID_DG_TOUCHPAD:
- case HID_GD_SYSTEM_CONTROL:
- case HID_CP_CONSUMER_CONTROL:
- case HID_GD_WIRELESS_RADIO_CTLS:
- case HID_GD_SYSTEM_MULTIAXIS:
- /* already handled by hid core */
- break;
- case HID_DG_TOUCHSCREEN:
- /* we do not set suffix = "Touchscreen" */
- hi->input->name = hdev->name;
- break;
- case HID_DG_STYLUS:
- /* force BTN_STYLUS to allow tablet matching in udev */
- __set_bit(BTN_STYLUS, hi->input->keybit);
- break;
- case HID_VD_ASUS_CUSTOM_MEDIA_KEYS:
- suffix = "Custom Media Keys";
- break;
- default:
- suffix = "UNKNOWN";
- break;
- }
+ switch (hi->application) {
+ case HID_GD_KEYBOARD:
+ case HID_GD_KEYPAD:
+ case HID_GD_MOUSE:
+ case HID_DG_TOUCHPAD:
+ case HID_GD_SYSTEM_CONTROL:
+ case HID_CP_CONSUMER_CONTROL:
+ case HID_GD_WIRELESS_RADIO_CTLS:
+ case HID_GD_SYSTEM_MULTIAXIS:
+ /* already handled by hid core */
+ break;
+ case HID_DG_TOUCHSCREEN:
+ /* we do not set suffix = "Touchscreen" */
+ hi->input->name = hdev->name;
+ break;
+ case HID_DG_STYLUS:
+ /* force BTN_STYLUS to allow tablet matching in udev */
+ __set_bit(BTN_STYLUS, hi->input->keybit);
+ break;
+ case HID_VD_ASUS_CUSTOM_MEDIA_KEYS:
+ suffix = "Custom Media Keys";
+ break;
+ default:
+ suffix = "UNKNOWN";
+ break;
}
if (suffix) {
--
2.14.3
^ permalink raw reply related
* [PATCH 3/4] HID: core: fix grouping by application
From: Benjamin Tissoires @ 2018-09-04 13:31 UTC (permalink / raw)
To: Jiri Kosina, Dmitry Torokhov
Cc: Benjamin Tissoires, linux-input, linux-kernel, stable
In-Reply-To: <20180904133115.5111-1-benjamin.tissoires@redhat.com>
commit f07b3c1da92d ("HID: generic: create one input report per
application type") was effectively the same as MULTI_INPUT:
hidinput->report was never set, so hidinput_match_application()
always returned null.
Fix that by testing against the real application.
Note that this breaks some old eGalax touchscreens that expect MULTI_INPUT
instead of HID_QUIRK_INPUT_PER_APP. Enable this quirk for backward
compatibility on all non-Win8 touchscreens.
link: https://bugzilla.kernel.org/show_bug.cgi?id=200847
link: https://bugzilla.kernel.org/show_bug.cgi?id=200849
link: https://bugs.archlinux.org/task/59699
link: https://github.com/NixOS/nixpkgs/issues/45165
Cc: stable@vger.kernel.org # v4.18+
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
This replaces https://patchwork.kernel.org/patch/10583471/
A proper fix is better than a revert.
drivers/hid/hid-input.c | 4 ++--
drivers/hid/hid-multitouch.c | 3 +++
include/linux/hid.h | 1 +
3 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index 1e9ba8f7a16b..907b08e50a9b 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -1588,6 +1588,7 @@ static struct hid_input *hidinput_allocate(struct hid_device *hid,
input_dev->dev.parent = &hid->dev;
hidinput->input = input_dev;
+ hidinput->application = application;
list_add_tail(&hidinput->list, &hid->inputs);
INIT_LIST_HEAD(&hidinput->reports);
@@ -1683,8 +1684,7 @@ static struct hid_input *hidinput_match_application(struct hid_report *report)
struct hid_input *hidinput;
list_for_each_entry(hidinput, &hid->inputs, list) {
- if (hidinput->report &&
- hidinput->report->application == report->application)
+ if (hidinput->application == report->application)
return hidinput;
}
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 88da991ef256..da954f3f4da7 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -1697,6 +1697,9 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
*/
hdev->quirks |= HID_QUIRK_INPUT_PER_APP;
+ if (id->group != HID_GROUP_MULTITOUCH_WIN_8)
+ hdev->quirks |= HID_QUIRK_MULTI_INPUT;
+
timer_setup(&td->release_timer, mt_expired_timeout, 0);
ret = hid_parse(hdev);
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 834e6461a690..d44a78362942 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -526,6 +526,7 @@ struct hid_input {
const char *name;
bool registered;
struct list_head reports; /* the list of reports */
+ unsigned int application; /* application usage for this input */
};
enum hid_type {
--
2.14.3
^ permalink raw reply related
* [PATCH 2/4] HID: input: do not append a suffix if the name already has it
From: Benjamin Tissoires @ 2018-09-04 13:31 UTC (permalink / raw)
To: Jiri Kosina, Dmitry Torokhov
Cc: Benjamin Tissoires, linux-input, linux-kernel
In-Reply-To: <20180904133115.5111-1-benjamin.tissoires@redhat.com>
Or it creates some weird input names like:
"MI Dongle MI Wireless Mouse Mouse"
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
drivers/hid/hid-input.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index ac201817a2dd..1e9ba8f7a16b 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -1516,6 +1516,7 @@ static struct hid_input *hidinput_allocate(struct hid_device *hid,
struct hid_input *hidinput = kzalloc(sizeof(*hidinput), GFP_KERNEL);
struct input_dev *input_dev = input_allocate_device();
const char *suffix = NULL;
+ size_t suffix_len, name_len;
if (!hidinput || !input_dev)
goto fail;
@@ -1559,10 +1560,15 @@ static struct hid_input *hidinput_allocate(struct hid_device *hid,
}
if (suffix) {
- hidinput->name = kasprintf(GFP_KERNEL, "%s %s",
- hid->name, suffix);
- if (!hidinput->name)
- goto fail;
+ name_len = strlen(hid->name);
+ suffix_len = strlen(suffix);
+ if ((name_len < suffix_len) ||
+ strcmp(hid->name + name_len - suffix_len, suffix)) {
+ hidinput->name = kasprintf(GFP_KERNEL, "%s %s",
+ hid->name, suffix);
+ if (!hidinput->name)
+ goto fail;
+ }
}
input_set_drvdata(input_dev, hid);
--
2.14.3
^ permalink raw reply related
* [PATCH 1/4] HID: multitouch: fix Elan panels with 2 input modes declaration
From: Benjamin Tissoires @ 2018-09-04 13:31 UTC (permalink / raw)
To: Jiri Kosina, Dmitry Torokhov
Cc: Benjamin Tissoires, linux-input, linux-kernel, stable
In-Reply-To: <20180904133115.5111-1-benjamin.tissoires@redhat.com>
When implementing commit 7f81c8db5489 ("HID: multitouch: simplify
the settings of the various features"), I wrongly removed a test
that made sure we never try to set the second InputMode feature
to something else than 0.
This broke badly some recent Elan panels that now forget to send the
click button in some area of the touchpad.
Fixes 7f81c8db5489
Link: https://bugzilla.kernel.org/show_bug.cgi?id=200899
Cc: stable@vger.kernel.org # v4.18+
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
drivers/hid/hid-multitouch.c | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 40fbb7c52723..88da991ef256 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -1375,7 +1375,8 @@ static bool mt_need_to_apply_feature(struct hid_device *hdev,
struct hid_usage *usage,
enum latency_mode latency,
bool surface_switch,
- bool button_switch)
+ bool button_switch,
+ bool *inputmode_found)
{
struct mt_device *td = hid_get_drvdata(hdev);
struct mt_class *cls = &td->mtclass;
@@ -1387,6 +1388,14 @@ static bool mt_need_to_apply_feature(struct hid_device *hdev,
switch (usage->hid) {
case HID_DG_INPUTMODE:
+ /*
+ * Some elan panels wrongly declare 2 input mode features,
+ * and silently ignore when we set the value in the second
+ * field. Skip the second feature and hope for the best.
+ */
+ if (*inputmode_found)
+ return false;
+
if (cls->quirks & MT_QUIRK_FORCE_GET_FEATURE) {
report_len = hid_report_len(report);
buf = hid_alloc_report_buf(report, GFP_KERNEL);
@@ -1402,6 +1411,7 @@ static bool mt_need_to_apply_feature(struct hid_device *hdev,
}
field->value[index] = td->inputmode_value;
+ *inputmode_found = true;
return true;
case HID_DG_CONTACTMAX:
@@ -1439,6 +1449,7 @@ static void mt_set_modes(struct hid_device *hdev, enum latency_mode latency,
struct hid_usage *usage;
int i, j;
bool update_report;
+ bool inputmode_found = false;
rep_enum = &hdev->report_enum[HID_FEATURE_REPORT];
list_for_each_entry(rep, &rep_enum->report_list, list) {
@@ -1457,7 +1468,8 @@ static void mt_set_modes(struct hid_device *hdev, enum latency_mode latency,
usage,
latency,
surface_switch,
- button_switch))
+ button_switch,
+ &inputmode_found))
update_report = true;
}
}
--
2.14.3
^ permalink raw reply related
* [PATCH 0/4] HID fixes
From: Benjamin Tissoires @ 2018-09-04 13:31 UTC (permalink / raw)
To: Jiri Kosina, Dmitry Torokhov
Cc: Benjamin Tissoires, linux-input, linux-kernel
Hi Jiri,
there is no real link between those 4 commit but the fact that I wrote
them today ;)
2 patches should at least be scheduled for v4.19: 1/4 and 3/4
Both are stable fixes for mistakes I made in v4.18.
Patch 2 and 4 are just nice to have, so v4.20 should be fine.
Cheers,
Benjamin
Benjamin Tissoires (4):
HID: multitouch: fix Elan panels with 2 input modes declaration
HID: input: do not append a suffix if the name already has it
HID: core: fix grouping by application
HID: multitouch: simplify the application retrieval
drivers/hid/hid-input.c | 18 ++++++---
drivers/hid/hid-multitouch.c | 91 ++++++++++++++++++++++++--------------------
include/linux/hid.h | 1 +
3 files changed, 62 insertions(+), 48 deletions(-)
--
2.14.3
^ permalink raw reply
* Re: [PATCH] Partially revert "HID: generic: create one input report per application type"
From: Benjamin Tissoires @ 2018-09-04 11:55 UTC (permalink / raw)
To: Jiri Kosina, Dmitry Torokhov; +Cc: open list:HID CORE LAYER, lkml, 3.8+
In-Reply-To: <20180831093604.5915-1-benjamin.tissoires@redhat.com>
On Fri, Aug 31, 2018 at 11:36 AM Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
>
> This partially reverts commit f07b3c1da92db108662f99417a212fc1eddc44d1.
>
> It looks like some mice are not correctly treated by
> HID_QUIRK_INPUT_PER_APP. Those mice have the following
> report descriptor:
>
> 0x05, 0x01, // Usage Page (Generic Desktop) 0
> 0x09, 0x02, // Usage (Mouse) 2
> 0xa1, 0x01, // Collection (Application) 4
> 0x85, 0x01, // Report ID (1) 6
> 0x09, 0x01, // Usage (Pointer) 8
> 0xa1, 0x00, // Collection (Physical) 10
> 0x95, 0x05, // Report Count (5) 12
> 0x75, 0x01, // Report Size (1) 14
> 0x05, 0x09, // Usage Page (Button) 16
> 0x19, 0x01, // Usage Minimum (1) 18
> 0x29, 0x05, // Usage Maximum (5) 20
> 0x15, 0x00, // Logical Minimum (0) 22
> 0x25, 0x01, // Logical Maximum (1) 24
> 0x81, 0x02, // Input (Data,Var,Abs) 26
> ...
> 0xc0, // End Collection 57
> 0x85, 0x02, // Report ID (2) 58
> 0x09, 0x01, // Usage (Consumer Control) 60
> 0xa1, 0x00, // Collection (Physical) 62
> 0x75, 0x0c, // Report Size (12) 64
> 0x95, 0x02, // Report Count (2) 66
> 0x05, 0x01, // Usage Page (Generic Desktop) 68
> 0x09, 0x30, // Usage (X) 70
> 0x09, 0x31, // Usage (Y) 72
> 0x16, 0x01, 0xf8, // Logical Minimum (-2047) 74
> 0x26, 0xff, 0x07, // Logical Maximum (2047) 77
> 0x81, 0x06, // Input (Data,Var,Rel) 80
> 0xc0, // End Collection 82
> 0xc0, // End Collection 83
> ...
>
> Both the cursor position and the buttons are located in the
> same application collection (Mouse) and the kernel should
> only create one input device for those.
>
> However, for an undetermined reason, the kernel splits the
> device in 2, making systemd not tagging the second mouse
> with the coordinates only as a mouse. And then userspace
> ignores it which leads to a mouse where only the buttons
> are working.
>
> Until the quirk gets properly fixed, we should probably
> revert applying it to all of the generic devices and
> re-enable it when the root reason has been found.
Jiri,
I actually just got the proper fix today. I think it would be better
to directly take the fix instead of the revert and a revert of the
revert later.
I just need to make sure the tests are correctly handled and I should
be able to submit the patch today.
Cheers,
Benjamin
>
> link: https://bugzilla.kernel.org/show_bug.cgi?id=200847
> link: https://bugzilla.kernel.org/show_bug.cgi?id=200849
> link: https://bugs.archlinux.org/task/59699
> link: https://github.com/NixOS/nixpkgs/issues/45165
>
> Cc: stable@vger.kernel.org # v4.18+
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> ---
> drivers/hid/hid-generic.c | 15 ---------------
> 1 file changed, 15 deletions(-)
>
> diff --git a/drivers/hid/hid-generic.c b/drivers/hid/hid-generic.c
> index 3b6eccbc2519..c25b4718de44 100644
> --- a/drivers/hid/hid-generic.c
> +++ b/drivers/hid/hid-generic.c
> @@ -56,20 +56,6 @@ static bool hid_generic_match(struct hid_device *hdev,
> return true;
> }
>
> -static int hid_generic_probe(struct hid_device *hdev,
> - const struct hid_device_id *id)
> -{
> - int ret;
> -
> - hdev->quirks |= HID_QUIRK_INPUT_PER_APP;
> -
> - ret = hid_parse(hdev);
> - if (ret)
> - return ret;
> -
> - return hid_hw_start(hdev, HID_CONNECT_DEFAULT);
> -}
> -
> static const struct hid_device_id hid_table[] = {
> { HID_DEVICE(HID_BUS_ANY, HID_GROUP_ANY, HID_ANY_ID, HID_ANY_ID) },
> { }
> @@ -80,7 +66,6 @@ static struct hid_driver hid_generic = {
> .name = "hid-generic",
> .id_table = hid_table,
> .match = hid_generic_match,
> - .probe = hid_generic_probe,
> };
> module_hid_driver(hid_generic);
>
> --
> 2.14.3
>
^ permalink raw reply
* Re: [PATCH] Partially revert "HID: generic: create one input report per application type"
From: Benjamin Tissoires @ 2018-09-04 9:53 UTC (permalink / raw)
To: Terry.Junge
Cc: Jiri Kosina, Dmitry Torokhov, open list:HID CORE LAYER, lkml,
3.8+
In-Reply-To: <SN6PR02MB4864D83F3B3FD94CB4741732ED0F0@SN6PR02MB4864.namprd02.prod.outlook.com>
Hi Terry,
On Fri, Aug 31, 2018 at 7:48 PM Junge, Terry
<Terry.Junge@plantronics.com> wrote:
>
> For what it's worth the Report Descriptor is a little questionable
> which could be causing the collection to split in two
>
> >-----Original Message-----
> >From: linux-input-owner@vger.kernel.org [mailto:linux-input-
> >owner@vger.kernel.org] On Behalf Of Benjamin Tissoires
> >Sent: Friday, August 31, 2018 2:36 AM
> >To: Jiri Kosina <jikos@kernel.org>; Dmitry Torokhov
> ><dmitry.torokhov@gmail.com>
> >Cc: linux-input@vger.kernel.org; linux-kernel@vger.kernel.org; Benjamin
> >Tissoires <benjamin.tissoires@redhat.com>; stable@vger.kernel.org
> >Subject: [PATCH] Partially revert "HID: generic: create one input report per
> >application type"
> >
> >This partially reverts commit f07b3c1da92db108662f99417a212fc1eddc44d1.
> >
> >It looks like some mice are not correctly treated by
> >HID_QUIRK_INPUT_PER_APP. Those mice have the following
> >report descriptor:
> >
> >0x05, 0x01, // Usage Page (Generic Desktop) 0
> >0x09, 0x02, // Usage (Mouse) 2
> >0xa1, 0x01, // Collection (Application) 4
> >0x85, 0x01, // Report ID (1) 6
> >0x09, 0x01, // Usage (Pointer) 8
>
> This physical collection is associated with Generic Desktop:Pointer (0x0001:0x0001)
>
> >0xa1, 0x00, // Collection (Physical) 10
> >0x95, 0x05, // Report Count (5) 12
> >0x75, 0x01, // Report Size (1) 14
> >0x05, 0x09, // Usage Page (Button) 16
>
> We are now in the Button page
>
> >0x19, 0x01, // Usage Minimum (1) 18
> >0x29, 0x05, // Usage Maximum (5) 20
> >0x15, 0x00, // Logical Minimum (0) 22
> >0x25, 0x01, // Logical Maximum (1) 24
> >0x81, 0x02, // Input (Data,Var,Abs) 26
> >...
> >0xc0, // End Collection 57
> >0x85, 0x02, // Report ID (2) 58
> >0x09, 0x01, // Usage (Consumer Control) 60
>
> This physical collection is associated with Button:Button 1 (0x0009:0x0001)
> not Generic Desktop:Pointer (0x0001:0x0001)
>
> >0xa1, 0x00, // Collection (Physical) 62
> >0x75, 0x0c, // Report Size (12) 64
> >0x95, 0x02, // Report Count (2) 66
> >0x05, 0x01, // Usage Page (Generic Desktop) 68
>
> Now we're back in the Generic Desktop page
You are missing one bit in the HID specification. The report
descriptor is both a stack and a machine with states. The 'usage page'
can be considered as a 'register' that needs to be associated to the
'usage' to provide the final usage.
For example, Usage Page 0x01 (Generic Desktop) plus Usage 0x30 gives
you the "X" usage.
While Usage Page 0x0d (digitizer) plus the same Usage 0x30 gives you
the "Tip Pressure".
So here, there is nothing wrong.
The stack part concerns the collections. You open a collection type
(physical, or application), and you can close it.
So here, the problem is that the 2 reports with Ids 1 and 2 are part
of the same application collection "mouse" and should be tied to the
same input device (at least that is how I thought of the code).
Unfortunately, the bug makes that these 2 reports each have an input
node, and this is where things start to mess up.
Cheers,
Benjamin
>
> >0x09, 0x30, // Usage (X) 70
> >0x09, 0x31, // Usage (Y) 72
> >0x16, 0x01, 0xf8, // Logical Minimum (-2047) 74
> >0x26, 0xff, 0x07, // Logical Maximum (2047) 77
> >0x81, 0x06, // Input (Data,Var,Rel) 80
> >0xc0, // End Collection 82
> >0xc0, // End Collection 83
> >...
> >
^ permalink raw reply
* Re: [PATCH v2 1/5] Add the `REL_WHEEL_HI_RES` event code
From: Peter Hutterer @ 2018-09-04 6:28 UTC (permalink / raw)
To: Harry Cutts
Cc: linux-input, LKML, Jiri Kosina, Dmitry Torokhov,
Benjamin Tissoires, linux-doc, Jonathan Corbet
In-Reply-To: <20180830215622.47550-2-hcutts@chromium.org>
On Thu, Aug 30, 2018 at 02:56:18PM -0700, Harry Cutts wrote:
> This event code represents scroll reports from high-resolution wheels,
> and will be used by future patches in this series. See the linux-input
> "Reporting high-resolution scroll events" thread [0] for more details.
>
> [0]: https://www.spinics.net/lists/linux-input/msg57380.html
>
> Signed-off-by: Harry Cutts <hcutts@chromium.org>
looks good to me, thanks. For the archives, the libinput issue filed for
this is here: https://gitlab.freedesktop.org/libinput/libinput/issues/130
Cheers,
Peter
> ---
>
> Changes in v2: None
>
> Documentation/input/event-codes.rst | 11 ++++++++++-
> include/uapi/linux/input-event-codes.h | 1 +
> 2 files changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/input/event-codes.rst b/Documentation/input/event-codes.rst
> index a8c0873beb95..cef220c176a4 100644
> --- a/Documentation/input/event-codes.rst
> +++ b/Documentation/input/event-codes.rst
> @@ -190,7 +190,16 @@ A few EV_REL codes have special meanings:
> * REL_WHEEL, REL_HWHEEL:
>
> - These codes are used for vertical and horizontal scroll wheels,
> - respectively.
> + respectively. The value is the number of "notches" moved on the wheel, the
> + physical size of which varies by device. For high-resolution wheels (which
> + report multiple events for each notch of movement, or do not have notches)
> + this may be an approximation based on the high-resolution scroll events.
> +
> +* REL_WHEEL_HI_RES:
> +
> + - If a vertical scroll wheel supports high-resolution scrolling, this code
> + will be emitted in addition to REL_WHEEL. The value is the (approximate)
> + distance travelled by the user's finger, in microns.
>
> EV_ABS
> ------
> diff --git a/include/uapi/linux/input-event-codes.h b/include/uapi/linux/input-event-codes.h
> index 53fbae27b280..dad8d3890a3a 100644
> --- a/include/uapi/linux/input-event-codes.h
> +++ b/include/uapi/linux/input-event-codes.h
> @@ -708,6 +708,7 @@
> #define REL_DIAL 0x07
> #define REL_WHEEL 0x08
> #define REL_MISC 0x09
> +#define REL_WHEEL_HI_RES 0x0a
> #define REL_MAX 0x0f
> #define REL_CNT (REL_MAX+1)
>
> --
> 2.19.0.rc0.228.g281dcd1b4d0-goog
>
^ permalink raw reply
* KASAN: use-after-free Read in string
From: syzbot @ 2018-09-03 21:51 UTC (permalink / raw)
To: dmitry.torokhov, linux-input, linux-kernel, rydberg,
syzkaller-bugs
Hello,
syzbot found the following crash on:
HEAD commit: 58c3f14f86c9 Merge tag 'riscv-for-linus-4.19-rc2' of git:/..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=15506dc1400000
kernel config: https://syzkaller.appspot.com/x/.config?x=531a917630d2a492
dashboard link: https://syzkaller.appspot.com/bug?extid=f648cfb7e0b52bf7ae32
compiler: gcc (GCC) 8.0.1 20180413 (experimental)
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=1178aa46400000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=11735e0a400000
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+f648cfb7e0b52bf7ae32@syzkaller.appspotmail.com
RDX: 0000000000000001 RSI: 0000000000005502 RDI: 0000000000000004
RBP: 00000000006cf018 R08: 0000000000000001 R09: 0000000000000038
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000006
R13: ffffffffffffffff R14: 0000000000000000 R15: 0000000000000000
==================================================================
BUG: KASAN: use-after-free in string+0x2c6/0x2d0 lib/vsprintf.c:604
Read of size 1 at addr ffff8801d79c5080 by task syz-executor215/4685
CPU: 1 PID: 4685 Comm: syz-executor215 Not tainted 4.19.0-rc1+ #215
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x1c9/0x2b4 lib/dump_stack.c:113
print_address_description+0x6c/0x20b mm/kasan/report.c:256
kasan_report_error mm/kasan/report.c:354 [inline]
kasan_report.cold.7+0x242/0x30d mm/kasan/report.c:412
__asan_report_load1_noabort+0x14/0x20 mm/kasan/report.c:430
string+0x2c6/0x2d0 lib/vsprintf.c:604
vsnprintf+0x48f/0x1b60 lib/vsprintf.c:2293
add_uevent_var+0x140/0x2f0 lib/kobject_uevent.c:659
input_dev_uevent+0x15d/0x860 drivers/input/input.c:1594
dev_uevent+0x340/0x7e0 drivers/base/core.c:1008
kobject_uevent_env+0x45b/0x1110 lib/kobject_uevent.c:547
kobject_uevent+0x1f/0x30 lib/kobject_uevent.c:636
kobject_cleanup lib/kobject.c:649 [inline]
kobject_release lib/kobject.c:691 [inline]
kref_put include/linux/kref.h:70 [inline]
kobject_put+0x1fb/0x280 lib/kobject.c:708
put_device+0x20/0x30 drivers/base/core.c:2024
input_put_device include/linux/input.h:349 [inline]
evdev_free+0x4f/0x70 drivers/input/evdev.c:369
device_release+0x7e/0x210 drivers/base/core.c:891
kobject_cleanup lib/kobject.c:662 [inline]
kobject_release lib/kobject.c:691 [inline]
kref_put include/linux/kref.h:70 [inline]
kobject_put+0x194/0x280 lib/kobject.c:708
cdev_default_release+0x3e/0x50 fs/char_dev.c:607
kobject_cleanup lib/kobject.c:662 [inline]
kobject_release lib/kobject.c:691 [inline]
kref_put include/linux/kref.h:70 [inline]
kobject_put+0x194/0x280 lib/kobject.c:708
cdev_put.part.0+0x36/0x50 fs/char_dev.c:368
cdev_put+0x1f/0x30 fs/char_dev.c:366
__fput+0x8ae/0xa40 fs/file_table.c:281
____fput+0x15/0x20 fs/file_table.c:309
task_work_run+0x1e8/0x2a0 kernel/task_work.c:113
exit_task_work include/linux/task_work.h:22 [inline]
do_exit+0x1ae4/0x26e0 kernel/exit.c:867
do_group_exit+0x177/0x440 kernel/exit.c:970
__do_sys_exit_group kernel/exit.c:981 [inline]
__se_sys_exit_group kernel/exit.c:979 [inline]
__x64_sys_exit_group+0x3e/0x50 kernel/exit.c:979
do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x443a68
Code: Bad RIP value.
RSP: 002b:00007fff70261298 EFLAGS: 00000246 ORIG_RAX: 00000000000000e7
RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 0000000000443a68
RDX: 0000000000000000 RSI: 000000000000003c RDI: 0000000000000000
RBP: 00000000004c39c8 R08: 00000000000000e7 R09: ffffffffffffffd0
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000001
R13: 00000000006d5180 R14: 0000000000000000 R15: 0000000000000000
Allocated by task 4685:
save_stack+0x43/0xd0 mm/kasan/kasan.c:448
set_track mm/kasan/kasan.c:460 [inline]
kasan_kmalloc+0xc4/0xe0 mm/kasan/kasan.c:553
__do_kmalloc mm/slab.c:3718 [inline]
__kmalloc_track_caller+0x14a/0x720 mm/slab.c:3733
kstrndup+0x5f/0xe0 mm/util.c:98
uinput_setup_device_legacy drivers/input/misc/uinput.c:549 [inline]
uinput_write+0x2fb/0x1120 drivers/input/misc/uinput.c:621
__vfs_write+0x117/0x9d0 fs/read_write.c:485
vfs_write+0x1fc/0x560 fs/read_write.c:549
ksys_write+0x101/0x260 fs/read_write.c:598
__do_sys_write fs/read_write.c:610 [inline]
__se_sys_write fs/read_write.c:607 [inline]
__x64_sys_write+0x73/0xb0 fs/read_write.c:607
do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x49/0xbe
Freed by task 4685:
save_stack+0x43/0xd0 mm/kasan/kasan.c:448
set_track mm/kasan/kasan.c:460 [inline]
__kasan_slab_free+0x11a/0x170 mm/kasan/kasan.c:521
kasan_slab_free+0xe/0x10 mm/kasan/kasan.c:528
__cache_free mm/slab.c:3498 [inline]
kfree+0xd9/0x210 mm/slab.c:3813
uinput_destroy_device+0xf8/0x250 drivers/input/misc/uinput.c:310
uinput_ioctl_handler.isra.10+0x1439/0x2540 drivers/input/misc/uinput.c:880
uinput_ioctl+0x4c/0x60 drivers/input/misc/uinput.c:1047
vfs_ioctl fs/ioctl.c:46 [inline]
file_ioctl fs/ioctl.c:501 [inline]
do_vfs_ioctl+0x1de/0x1720 fs/ioctl.c:685
ksys_ioctl+0xa9/0xd0 fs/ioctl.c:702
__do_sys_ioctl fs/ioctl.c:709 [inline]
__se_sys_ioctl fs/ioctl.c:707 [inline]
__x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:707
do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x49/0xbe
The buggy address belongs to the object at ffff8801d79c5080
which belongs to the cache kmalloc-32 of size 32
The buggy address is located 0 bytes inside of
32-byte region [ffff8801d79c5080, ffff8801d79c50a0)
The buggy address belongs to the page:
page:ffffea00075e7140 count:1 mapcount:0 mapping:ffff8801dac001c0
index:0xffff8801d79c5fc1
flags: 0x2fffc0000000100(slab)
raw: 02fffc0000000100 ffffea00075e0908 ffffea00075e7408 ffff8801dac001c0
raw: ffff8801d79c5fc1 ffff8801d79c5000 000000010000003f 0000000000000000
page dumped because: kasan: bad access detected
Memory state around the buggy address:
ffff8801d79c4f80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
ffff8801d79c5000: 00 00 fc fc fc fc fc fc 00 00 00 fc fc fc fc fc
> ffff8801d79c5080: fb fb fb fb fc fc fc fc 07 fc fc fc fc fc fc fc
^
ffff8801d79c5100: fb fb fb fb fc fc fc fc fb fb fb fb fc fc fc fc
ffff8801d79c5180: fb fb fb fb fc fc fc fc fb fb fb fb fc fc fc fc
==================================================================
---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with
syzbot.
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches
^ permalink raw reply
* Re: [PATCH] HID: i2c-hid: Don't reset device upon system resume
From: Kai-Heng Feng @ 2018-09-03 15:54 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Hans de Goede, AceLan Kao, Aaron Ma, linux-input,
Linux Kernel Mailing List
In-Reply-To: <CAO-hwJKAiXfvkBKrTDCSZXMFGp0Ygkhq+G2sgT+Qazm7RGx05Q@mail.gmail.com>
Hi Benjamin,
I forgot to include mailing list, first time -in-reply-to user :)
at 17:39, Benjamin Tissoires <benjamin.tissoires@redhat.com> wrote:
> On Mon, Sep 3, 2018 at 11:09 AM Kai-Heng Feng
> <kai.heng.feng@canonical.com> wrote:
>> Raydium touchscreen triggers interrupt storm after system-wide suspend:
>> [ 179.085033] i2c_hid i2c-CUST0000:00: i2c_hid_get_input: incomplete
>> report (58/65535)
>>
>> According to Raydium, Windows driver does not reset the device after
>> system resume.
>>
>> The HID over I2C spec does specify a reset should be used at
>> intialization, but it doesn't specify if reset is required for system
>> suspend.
>
> Hmm, that would explain a lot, thanks for digging into this.
>
> Jiri, I think this patch should partially replace
> https://patchwork.kernel.org/patch/10583481/
>
> Kai-heng, if you do not need to re-request the HID descriptors, I
> think it should be good to squash these together to:
> - remove I2C_HID_QUIRK_RESEND_REPORT_DESCR
> - remove the re-request the HID descriptors on resume
> - merge with this patch
There's still one user uses I2C_HID_QUIRK_RESEND_REPORT_DESCR,
USB_DEVICE_ID_SIS10FB_TOUCH.
So I'd like to have an Ack from Hans first.
I'll send a new patch with your suggestion afterward.
>
>
>
>> Tested this patch on other i2c-hid touchpanels I have and those
>> touchpanels do work after S3 without doing reset. If any regression
>> happens to other touchpanel vendors, we can use quirk for Raydium
>> devices.
>>
>> Cc: AceLan Kao <acelan.kao@canonical.com>
>> Cc: Aaron Ma <aaron.ma@canonical.com>
>> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
>> ---
>>
>> Benjamin, Hans,
>>
>> I guess some vendors interpret spec this way, so hardware reset is an
>> unexpected request for their devices.
>> Can you give this patch a try?
>
> The only device I have that you should not is the Surface 3. And I am
> not so sure it goes into S3 fluently (it seems to, but I am not 100%
> sure). If you have tested on a Dell XPS 9360, I think there is not
> much more I could test.
At least for the Raydium touchscreen, the issue can be observed under both
S2Idle and S3.
So I guess Surface 3 is still suitable for test if it can do S2Idle.
I've test this on XPS 9370 and Precision 5520, the patch doesn't break them.
Kai-Heng
>
> Cheers,
> Benjamin
>
>> drivers/hid/i2c-hid/i2c-hid.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
>> index 57126f6837bb..7567034a009f 100644
>> --- a/drivers/hid/i2c-hid/i2c-hid.c
>> +++ b/drivers/hid/i2c-hid/i2c-hid.c
>> @@ -1237,7 +1237,7 @@ static int i2c_hid_resume(struct device *dev)
>> pm_runtime_enable(dev);
>>
>> enable_irq(client->irq);
>> - ret = i2c_hid_hwreset(client);
>> + ret = i2c_hid_set_power(client, I2C_HID_PWR_ON);
>> if (ret)
>> return ret;
>>
>> --
>> 2.17.1
^ permalink raw reply
* Re: [PATCH v2 1/5] Add the `REL_WHEEL_HI_RES` event code
From: Jiri Kosina @ 2018-09-03 13:43 UTC (permalink / raw)
To: Harry Cutts, Dmitry Torokhov
Cc: linux-input, LKML, Jiri Kosina, Benjamin Tissoires, linux-doc,
Jonathan Corbet
In-Reply-To: <20180830215622.47550-2-hcutts@chromium.org>
On Thu, 30 Aug 2018, Harry Cutts wrote:
> This event code represents scroll reports from high-resolution wheels,
> and will be used by future patches in this series. See the linux-input
> "Reporting high-resolution scroll events" thread [0] for more details.
>
> [0]: https://www.spinics.net/lists/linux-input/msg57380.html
>
> Signed-off-by: Harry Cutts <hcutts@chromium.org>
> ---
>
> Changes in v2: None
>
> Documentation/input/event-codes.rst | 11 ++++++++++-
> include/uapi/linux/input-event-codes.h | 1 +
> 2 files changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/input/event-codes.rst b/Documentation/input/event-codes.rst
> index a8c0873beb95..cef220c176a4 100644
> --- a/Documentation/input/event-codes.rst
> +++ b/Documentation/input/event-codes.rst
> @@ -190,7 +190,16 @@ A few EV_REL codes have special meanings:
> * REL_WHEEL, REL_HWHEEL:
>
> - These codes are used for vertical and horizontal scroll wheels,
> - respectively.
> + respectively. The value is the number of "notches" moved on the wheel, the
> + physical size of which varies by device. For high-resolution wheels (which
> + report multiple events for each notch of movement, or do not have notches)
> + this may be an approximation based on the high-resolution scroll events.
> +
> +* REL_WHEEL_HI_RES:
> +
> + - If a vertical scroll wheel supports high-resolution scrolling, this code
> + will be emitted in addition to REL_WHEEL. The value is the (approximate)
> + distance travelled by the user's finger, in microns.
>
> EV_ABS
> ------
> diff --git a/include/uapi/linux/input-event-codes.h b/include/uapi/linux/input-event-codes.h
> index 53fbae27b280..dad8d3890a3a 100644
> --- a/include/uapi/linux/input-event-codes.h
> +++ b/include/uapi/linux/input-event-codes.h
> @@ -708,6 +708,7 @@
> #define REL_DIAL 0x07
> #define REL_WHEEL 0x08
> #define REL_MISC 0x09
> +#define REL_WHEEL_HI_RES 0x0a
> #define REL_MAX 0x0f
> #define REL_CNT (REL_MAX+1)
Dmitry, is this OK with you?
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH] HID: i2c-hid: enforce I2C_HID_QUIRK_RESEND_REPORT_DESCR
From: Jiri Kosina @ 2018-09-03 13:13 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Hans de Goede, open list:HID CORE LAYER, lkml, kai.heng.feng
In-Reply-To: <CAO-hwJKABBJExKciVt5R-8bjBtUe+uSc0Z_ikc35jbf1zrNpog@mail.gmail.com>
On Mon, 3 Sep 2018, Benjamin Tissoires wrote:
> > > The spec states that the HID devices should allow
> > > the host to query the HID descriptor at any time.
> > >
> > > Some devices require the HID descriptor to be set
> > > on resume, or they will fail.
> > >
> > > Instead of having a growing list of blacklisted devices
> > > make the call part of the general resume process
> > > so we can remove this list.
> > >
> > > Tested on a Dell XPS 9360 and a Surface 3.
> > >
> > > link: https://bugzilla.redhat.com/show_bug.cgi?id=1622695
> > >
> > > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> >
> > I agree we should just do this everywhere:
> >
> > Acked-by: Hans de Goede <hdegoede@redhat.com>
>
> Jiri, please hold with this patch, Canonical might have found a better
> solution. Simply not resetting the device after S3 might be the trick.
Thanks for the heads up.
In case that's the case, please also send a reference to the subsititute
patch submission into this thread, so that archives to pick it up :)
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH] HID: i2c-hid: enforce I2C_HID_QUIRK_RESEND_REPORT_DESCR
From: Benjamin Tissoires @ 2018-09-03 9:41 UTC (permalink / raw)
To: Jiri Kosina; +Cc: Hans de Goede, open list:HID CORE LAYER, lkml, kai.heng.feng
In-Reply-To: <4687e76a-d9a8-4a20-194b-526bb345cf73@redhat.com>
On Fri, Aug 31, 2018 at 12:04 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi,
>
> On 31-08-18 11:54, Benjamin Tissoires wrote:
> > The spec states that the HID devices should allow
> > the host to query the HID descriptor at any time.
> >
> > Some devices require the HID descriptor to be set
> > on resume, or they will fail.
> >
> > Instead of having a growing list of blacklisted devices
> > make the call part of the general resume process
> > so we can remove this list.
> >
> > Tested on a Dell XPS 9360 and a Surface 3.
> >
> > link: https://bugzilla.redhat.com/show_bug.cgi?id=1622695
> >
> > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
>
> I agree we should just do this everywhere:
>
> Acked-by: Hans de Goede <hdegoede@redhat.com>
Jiri, please hold with this patch, Canonical might have found a better
solution. Simply not resetting the device after S3 might be the trick.
Cheers,
Benjamin
>
> Regards,
>
> Hans
>
> > ---
> > drivers/hid/i2c-hid/i2c-hid.c | 16 ++++------------
> > 1 file changed, 4 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
> > index 57126f6837bb..17ec57c62a0b 100644
> > --- a/drivers/hid/i2c-hid/i2c-hid.c
> > +++ b/drivers/hid/i2c-hid/i2c-hid.c
> > @@ -47,7 +47,7 @@
> > /* quirks to control the device */
> > #define I2C_HID_QUIRK_SET_PWR_WAKEUP_DEV BIT(0)
> > #define I2C_HID_QUIRK_NO_IRQ_AFTER_RESET BIT(1)
> > -#define I2C_HID_QUIRK_RESEND_REPORT_DESCR BIT(2)
> > +/* reserved: I2C_HID_QUIRK_RESEND_REPORT_DESCR BIT(2) */
> >
> > /* flags */
> > #define I2C_HID_STARTED 0
> > @@ -170,12 +170,6 @@ static const struct i2c_hid_quirks {
> > I2C_HID_QUIRK_SET_PWR_WAKEUP_DEV },
> > { I2C_VENDOR_ID_HANTICK, I2C_PRODUCT_ID_HANTICK_5288,
> > I2C_HID_QUIRK_NO_IRQ_AFTER_RESET },
> > - { I2C_VENDOR_ID_RAYD, I2C_PRODUCT_ID_RAYD_3118,
> > - I2C_HID_QUIRK_RESEND_REPORT_DESCR },
> > - { USB_VENDOR_ID_SIS_TOUCH, USB_DEVICE_ID_SIS10FB_TOUCH,
> > - I2C_HID_QUIRK_RESEND_REPORT_DESCR },
> > - { I2C_VENDOR_ID_RAYD, I2C_PRODUCT_ID_RAYD_4B33,
> > - I2C_HID_QUIRK_RESEND_REPORT_DESCR },
> > { 0, 0 }
> > };
> >
> > @@ -1245,11 +1239,9 @@ static int i2c_hid_resume(struct device *dev)
> > * after resume, after this it will be back normal.
> > * otherwise it issues too many incomplete reports.
> > */
> > - if (ihid->quirks & I2C_HID_QUIRK_RESEND_REPORT_DESCR) {
> > - ret = i2c_hid_command(client, &hid_report_descr_cmd, NULL, 0);
> > - if (ret)
> > - return ret;
> > - }
> > + ret = i2c_hid_command(client, &hid_report_descr_cmd, NULL, 0);
> > + if (ret)
> > + return ret;
> >
> > if (hid->driver && hid->driver->reset_resume) {
> > ret = hid->driver->reset_resume(hid);
> >
^ permalink raw reply
* Re: [PATCH v2 0/5] Add support for high-resolution scrolling on Logitech mice
From: Benjamin Tissoires @ 2018-09-03 8:15 UTC (permalink / raw)
To: hcutts
Cc: open list:HID CORE LAYER, lkml, jiri.kosina, Dmitry Torokhov,
linux-doc, corbet, Jiri Kosina
In-Reply-To: <20180830215622.47550-1-hcutts@chromium.org>
Hi Harry,
On Thu, Aug 30, 2018 at 11:56 PM Harry Cutts <hcutts@chromium.org> wrote:
>
> Hi everyone,
>
> This is v2 of the patch set adding support for high-resolution scroll
> wheels on Logitech mice. See the linux-input "Reporting high-resolution
> scroll events" thread [0] for previous discussion of the evdev changes.
> I would love to hear your feedback.
Series looks good. I just tested it on a regular Logitech mouse and a
MX Master with high-res scrolling and everything works as expected.
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Cheers,
Benjamin
>
> Thanks,
>
> Harry Cutts
> Chrome OS Touch/Input team
>
> [0]: https://www.spinics.net/lists/linux-input/msg57380.html
>
>
> Changes in v2:
> * Changed the REL_WHEEL_HI_RES code to use micrometres (a.k.a. microns)
> as its units, instead of 256ths of a millimetre.
> * Removed support for mice connecting over Bluetooth, due to a bug where
> the mouse's high-res mode and the driver's settings get out of sync
> when the mouse is power cycled.
> * Moved the creation of the HID++ 1.0 function and the refactor to use
> the LDJ_DEVICE macro into separate patches.
> * Added a couple of explanatory comments to the Logitech driver.
>
> Harry Cutts (5):
> Add the `REL_WHEEL_HI_RES` event code
> Create a utility class for counting scroll events
> Add function to enable HID++ 1.0 "scrolling acceleration"
> Enable high-resolution scrolling on Logitech mice
> Use LDJ_DEVICE macro for existing Logitech mice
>
> Documentation/input/event-codes.rst | 11 +-
> drivers/hid/hid-input.c | 45 ++++
> drivers/hid/hid-logitech-hidpp.c | 311 ++++++++++++++++++++++---
> include/linux/hid.h | 28 +++
> include/uapi/linux/input-event-codes.h | 1 +
> 5 files changed, 368 insertions(+), 28 deletions(-)
>
> --
> 2.19.0.rc0.228.g281dcd1b4d0-goog
>
^ permalink raw reply
* [PATCH] hid: hid-core: Fix a sleep-in-atomic-context bug in __hid_request()
From: Jia-Ju Bai @ 2018-09-01 12:16 UTC (permalink / raw)
To: jikos, benjamin.tissoires; +Cc: linux-input, linux-kernel, Jia-Ju Bai
The driver may sleep with holding a spinlock.
The function call paths (from bottom to top) in Linux-4.16 are:
[FUNC] hid_alloc_report_buf(GFP_KERNEL)
drivers/hid/hid-core.c, 1435:
hid_alloc_report_buf in __hid_request
./include/linux/hid.h, 1023:
__hid_request in hid_hw_request
drivers/hid/hid-picolcd_core.c, 111:
hid_hw_request in picolcd_send_and_wait
drivers/hid/hid-picolcd_core.c, 100:
_raw_spin_lock_irqsave in picolcd_send_and_wait
[FUNC] hid_alloc_report_buf(GFP_KERNEL)
drivers/hid/hid-core.c, 1435:
hid_alloc_report_buf in __hid_request
./include/linux/hid.h, 1023:
__hid_request in hid_hw_request
drivers/hid/hid-picolcd_core.c, 245:
hid_hw_request in picolcd_reset
drivers/hid/hid-picolcd_core.c, 235:
_raw_spin_lock_irqsave in picolcd_reset
[FUNC] hid_alloc_report_buf(GFP_KERNEL)
drivers/hid/hid-core.c, 1435:
hid_alloc_report_buf in __hid_request
./include/linux/hid.h, 1023:
__hid_request in hid_hw_request
drivers/hid/hid-picolcd_fb.c, 215:
hid_hw_request in picolcd_fb_reset
drivers/hid/hid-picolcd_fb.c, 206:
_raw_spin_lock_irqsave in picolcd_fb_reset
[FUNC] hid_alloc_report_buf(GFP_KERNEL)
drivers/hid/hid-core.c, 1435:
hid_alloc_report_buf in __hid_request
./include/linux/hid.h, 1023:
__hid_request in hid_hw_request
drivers/hid/hid-lg4ff.c, 465:
hid_hw_request in lg4ff_play
drivers/hid/hid-lg4ff.c, 441:
_raw_spin_lock_irqsave in lg4ff_play
To fix this bug, GFP_KERNEL is replaced with GFP_ATOMIC.
This bug is found by my static analysis tool DSAC.
Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
---
drivers/hid/hid-core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 3942ee61bd1c..c886af00c8c9 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1442,7 +1442,7 @@ void __hid_request(struct hid_device *hid, struct hid_report *report,
int ret;
u32 len;
- buf = hid_alloc_report_buf(report, GFP_KERNEL);
+ buf = hid_alloc_report_buf(report, GFP_ATOMIC);
if (!buf)
return;
--
2.17.0
^ permalink raw reply related
* RE: [PATCH] Partially revert "HID: generic: create one input report per application type"
From: Junge, Terry @ 2018-08-31 17:47 UTC (permalink / raw)
To: Benjamin Tissoires, Jiri Kosina, Dmitry Torokhov
Cc: linux-input@vger.kernel.org, linux-kernel@vger.kernel.org,
stable@vger.kernel.org
In-Reply-To: <20180831093604.5915-1-benjamin.tissoires@redhat.com>
For what it's worth the Report Descriptor is a little questionable
which could be causing the collection to split in two
>-----Original Message-----
>From: linux-input-owner@vger.kernel.org [mailto:linux-input-
>owner@vger.kernel.org] On Behalf Of Benjamin Tissoires
>Sent: Friday, August 31, 2018 2:36 AM
>To: Jiri Kosina <jikos@kernel.org>; Dmitry Torokhov
><dmitry.torokhov@gmail.com>
>Cc: linux-input@vger.kernel.org; linux-kernel@vger.kernel.org; Benjamin
>Tissoires <benjamin.tissoires@redhat.com>; stable@vger.kernel.org
>Subject: [PATCH] Partially revert "HID: generic: create one input report per
>application type"
>
>This partially reverts commit f07b3c1da92db108662f99417a212fc1eddc44d1.
>
>It looks like some mice are not correctly treated by
>HID_QUIRK_INPUT_PER_APP. Those mice have the following
>report descriptor:
>
>0x05, 0x01, // Usage Page (Generic Desktop) 0
>0x09, 0x02, // Usage (Mouse) 2
>0xa1, 0x01, // Collection (Application) 4
>0x85, 0x01, // Report ID (1) 6
>0x09, 0x01, // Usage (Pointer) 8
This physical collection is associated with Generic Desktop:Pointer (0x0001:0x0001)
>0xa1, 0x00, // Collection (Physical) 10
>0x95, 0x05, // Report Count (5) 12
>0x75, 0x01, // Report Size (1) 14
>0x05, 0x09, // Usage Page (Button) 16
We are now in the Button page
>0x19, 0x01, // Usage Minimum (1) 18
>0x29, 0x05, // Usage Maximum (5) 20
>0x15, 0x00, // Logical Minimum (0) 22
>0x25, 0x01, // Logical Maximum (1) 24
>0x81, 0x02, // Input (Data,Var,Abs) 26
>...
>0xc0, // End Collection 57
>0x85, 0x02, // Report ID (2) 58
>0x09, 0x01, // Usage (Consumer Control) 60
This physical collection is associated with Button:Button 1 (0x0009:0x0001)
not Generic Desktop:Pointer (0x0001:0x0001)
>0xa1, 0x00, // Collection (Physical) 62
>0x75, 0x0c, // Report Size (12) 64
>0x95, 0x02, // Report Count (2) 66
>0x05, 0x01, // Usage Page (Generic Desktop) 68
Now we're back in the Generic Desktop page
>0x09, 0x30, // Usage (X) 70
>0x09, 0x31, // Usage (Y) 72
>0x16, 0x01, 0xf8, // Logical Minimum (-2047) 74
>0x26, 0xff, 0x07, // Logical Maximum (2047) 77
>0x81, 0x06, // Input (Data,Var,Rel) 80
>0xc0, // End Collection 82
>0xc0, // End Collection 83
>...
>
>Both the cursor position and the buttons are located in the
>same application collection (Mouse) and the kernel should
>only create one input device for those.
>
>However, for an undetermined reason, the kernel splits the
>device in 2, making systemd not tagging the second mouse
>with the coordinates only as a mouse. And then userspace
>ignores it which leads to a mouse where only the buttons
>are working.
>
>Until the quirk gets properly fixed, we should probably
>revert applying it to all of the generic devices and
>re-enable it when the root reason has been found.
>
>link: https://bugzilla.kernel.org/show_bug.cgi?id=200847
>link: https://bugzilla.kernel.org/show_bug.cgi?id=200849
>link: https://bugs.archlinux.org/task/59699
>link: https://github.com/NixOS/nixpkgs/issues/45165
>
>Cc: stable@vger.kernel.org # v4.18+
>Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
>---
> drivers/hid/hid-generic.c | 15 ---------------
> 1 file changed, 15 deletions(-)
>
>diff --git a/drivers/hid/hid-generic.c b/drivers/hid/hid-generic.c
>index 3b6eccbc2519..c25b4718de44 100644
>--- a/drivers/hid/hid-generic.c
>+++ b/drivers/hid/hid-generic.c
>@@ -56,20 +56,6 @@ static bool hid_generic_match(struct hid_device *hdev,
> return true;
> }
>
>-static int hid_generic_probe(struct hid_device *hdev,
>- const struct hid_device_id *id)
>-{
>- int ret;
>-
>- hdev->quirks |= HID_QUIRK_INPUT_PER_APP;
>-
>- ret = hid_parse(hdev);
>- if (ret)
>- return ret;
>-
>- return hid_hw_start(hdev, HID_CONNECT_DEFAULT);
>-}
>-
> static const struct hid_device_id hid_table[] = {
> { HID_DEVICE(HID_BUS_ANY, HID_GROUP_ANY, HID_ANY_ID,
>HID_ANY_ID) },
> { }
>@@ -80,7 +66,6 @@ static struct hid_driver hid_generic = {
> .name = "hid-generic",
> .id_table = hid_table,
> .match = hid_generic_match,
>- .probe = hid_generic_probe,
> };
> module_hid_driver(hid_generic);
>
>--
>2.14.3
>
>________________________________
>NOTE: THIS EMAIL DID NOT COME FROM A PLANTRONICS ASSOCIATE.
>If the sender claims to be a Plantronics associate it may be fraudulent and
>spoofing the associates address. Please report suspicious emails or phishing
>attempts to:
>phishalarm@Plantronics.com<mailto:phishalarm@Plantronics.com>.
^ permalink raw reply
* Re: [PATCH] HID: i2c-hid: enforce I2C_HID_QUIRK_RESEND_REPORT_DESCR
From: Hans de Goede @ 2018-08-31 10:04 UTC (permalink / raw)
To: Benjamin Tissoires, Jiri Kosina; +Cc: linux-input, linux-kernel
In-Reply-To: <20180831095417.8541-1-benjamin.tissoires@redhat.com>
Hi,
On 31-08-18 11:54, Benjamin Tissoires wrote:
> The spec states that the HID devices should allow
> the host to query the HID descriptor at any time.
>
> Some devices require the HID descriptor to be set
> on resume, or they will fail.
>
> Instead of having a growing list of blacklisted devices
> make the call part of the general resume process
> so we can remove this list.
>
> Tested on a Dell XPS 9360 and a Surface 3.
>
> link: https://bugzilla.redhat.com/show_bug.cgi?id=1622695
>
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
I agree we should just do this everywhere:
Acked-by: Hans de Goede <hdegoede@redhat.com>
Regards,
Hans
> ---
> drivers/hid/i2c-hid/i2c-hid.c | 16 ++++------------
> 1 file changed, 4 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
> index 57126f6837bb..17ec57c62a0b 100644
> --- a/drivers/hid/i2c-hid/i2c-hid.c
> +++ b/drivers/hid/i2c-hid/i2c-hid.c
> @@ -47,7 +47,7 @@
> /* quirks to control the device */
> #define I2C_HID_QUIRK_SET_PWR_WAKEUP_DEV BIT(0)
> #define I2C_HID_QUIRK_NO_IRQ_AFTER_RESET BIT(1)
> -#define I2C_HID_QUIRK_RESEND_REPORT_DESCR BIT(2)
> +/* reserved: I2C_HID_QUIRK_RESEND_REPORT_DESCR BIT(2) */
>
> /* flags */
> #define I2C_HID_STARTED 0
> @@ -170,12 +170,6 @@ static const struct i2c_hid_quirks {
> I2C_HID_QUIRK_SET_PWR_WAKEUP_DEV },
> { I2C_VENDOR_ID_HANTICK, I2C_PRODUCT_ID_HANTICK_5288,
> I2C_HID_QUIRK_NO_IRQ_AFTER_RESET },
> - { I2C_VENDOR_ID_RAYD, I2C_PRODUCT_ID_RAYD_3118,
> - I2C_HID_QUIRK_RESEND_REPORT_DESCR },
> - { USB_VENDOR_ID_SIS_TOUCH, USB_DEVICE_ID_SIS10FB_TOUCH,
> - I2C_HID_QUIRK_RESEND_REPORT_DESCR },
> - { I2C_VENDOR_ID_RAYD, I2C_PRODUCT_ID_RAYD_4B33,
> - I2C_HID_QUIRK_RESEND_REPORT_DESCR },
> { 0, 0 }
> };
>
> @@ -1245,11 +1239,9 @@ static int i2c_hid_resume(struct device *dev)
> * after resume, after this it will be back normal.
> * otherwise it issues too many incomplete reports.
> */
> - if (ihid->quirks & I2C_HID_QUIRK_RESEND_REPORT_DESCR) {
> - ret = i2c_hid_command(client, &hid_report_descr_cmd, NULL, 0);
> - if (ret)
> - return ret;
> - }
> + ret = i2c_hid_command(client, &hid_report_descr_cmd, NULL, 0);
> + if (ret)
> + return ret;
>
> if (hid->driver && hid->driver->reset_resume) {
> ret = hid->driver->reset_resume(hid);
>
^ permalink raw reply
* [PATCH] HID: i2c-hid: enforce I2C_HID_QUIRK_RESEND_REPORT_DESCR
From: Benjamin Tissoires @ 2018-08-31 9:54 UTC (permalink / raw)
To: Jiri Kosina, Hans de Goede; +Cc: linux-input, linux-kernel, Benjamin Tissoires
The spec states that the HID devices should allow
the host to query the HID descriptor at any time.
Some devices require the HID descriptor to be set
on resume, or they will fail.
Instead of having a growing list of blacklisted devices
make the call part of the general resume process
so we can remove this list.
Tested on a Dell XPS 9360 and a Surface 3.
link: https://bugzilla.redhat.com/show_bug.cgi?id=1622695
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
drivers/hid/i2c-hid/i2c-hid.c | 16 ++++------------
1 file changed, 4 insertions(+), 12 deletions(-)
diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
index 57126f6837bb..17ec57c62a0b 100644
--- a/drivers/hid/i2c-hid/i2c-hid.c
+++ b/drivers/hid/i2c-hid/i2c-hid.c
@@ -47,7 +47,7 @@
/* quirks to control the device */
#define I2C_HID_QUIRK_SET_PWR_WAKEUP_DEV BIT(0)
#define I2C_HID_QUIRK_NO_IRQ_AFTER_RESET BIT(1)
-#define I2C_HID_QUIRK_RESEND_REPORT_DESCR BIT(2)
+/* reserved: I2C_HID_QUIRK_RESEND_REPORT_DESCR BIT(2) */
/* flags */
#define I2C_HID_STARTED 0
@@ -170,12 +170,6 @@ static const struct i2c_hid_quirks {
I2C_HID_QUIRK_SET_PWR_WAKEUP_DEV },
{ I2C_VENDOR_ID_HANTICK, I2C_PRODUCT_ID_HANTICK_5288,
I2C_HID_QUIRK_NO_IRQ_AFTER_RESET },
- { I2C_VENDOR_ID_RAYD, I2C_PRODUCT_ID_RAYD_3118,
- I2C_HID_QUIRK_RESEND_REPORT_DESCR },
- { USB_VENDOR_ID_SIS_TOUCH, USB_DEVICE_ID_SIS10FB_TOUCH,
- I2C_HID_QUIRK_RESEND_REPORT_DESCR },
- { I2C_VENDOR_ID_RAYD, I2C_PRODUCT_ID_RAYD_4B33,
- I2C_HID_QUIRK_RESEND_REPORT_DESCR },
{ 0, 0 }
};
@@ -1245,11 +1239,9 @@ static int i2c_hid_resume(struct device *dev)
* after resume, after this it will be back normal.
* otherwise it issues too many incomplete reports.
*/
- if (ihid->quirks & I2C_HID_QUIRK_RESEND_REPORT_DESCR) {
- ret = i2c_hid_command(client, &hid_report_descr_cmd, NULL, 0);
- if (ret)
- return ret;
- }
+ ret = i2c_hid_command(client, &hid_report_descr_cmd, NULL, 0);
+ if (ret)
+ return ret;
if (hid->driver && hid->driver->reset_resume) {
ret = hid->driver->reset_resume(hid);
--
2.14.3
^ permalink raw reply related
* [PATCH] Partially revert "HID: generic: create one input report per application type"
From: Benjamin Tissoires @ 2018-08-31 9:36 UTC (permalink / raw)
To: Jiri Kosina, Dmitry Torokhov
Cc: linux-input, linux-kernel, Benjamin Tissoires, stable
This partially reverts commit f07b3c1da92db108662f99417a212fc1eddc44d1.
It looks like some mice are not correctly treated by
HID_QUIRK_INPUT_PER_APP. Those mice have the following
report descriptor:
0x05, 0x01, // Usage Page (Generic Desktop) 0
0x09, 0x02, // Usage (Mouse) 2
0xa1, 0x01, // Collection (Application) 4
0x85, 0x01, // Report ID (1) 6
0x09, 0x01, // Usage (Pointer) 8
0xa1, 0x00, // Collection (Physical) 10
0x95, 0x05, // Report Count (5) 12
0x75, 0x01, // Report Size (1) 14
0x05, 0x09, // Usage Page (Button) 16
0x19, 0x01, // Usage Minimum (1) 18
0x29, 0x05, // Usage Maximum (5) 20
0x15, 0x00, // Logical Minimum (0) 22
0x25, 0x01, // Logical Maximum (1) 24
0x81, 0x02, // Input (Data,Var,Abs) 26
...
0xc0, // End Collection 57
0x85, 0x02, // Report ID (2) 58
0x09, 0x01, // Usage (Consumer Control) 60
0xa1, 0x00, // Collection (Physical) 62
0x75, 0x0c, // Report Size (12) 64
0x95, 0x02, // Report Count (2) 66
0x05, 0x01, // Usage Page (Generic Desktop) 68
0x09, 0x30, // Usage (X) 70
0x09, 0x31, // Usage (Y) 72
0x16, 0x01, 0xf8, // Logical Minimum (-2047) 74
0x26, 0xff, 0x07, // Logical Maximum (2047) 77
0x81, 0x06, // Input (Data,Var,Rel) 80
0xc0, // End Collection 82
0xc0, // End Collection 83
...
Both the cursor position and the buttons are located in the
same application collection (Mouse) and the kernel should
only create one input device for those.
However, for an undetermined reason, the kernel splits the
device in 2, making systemd not tagging the second mouse
with the coordinates only as a mouse. And then userspace
ignores it which leads to a mouse where only the buttons
are working.
Until the quirk gets properly fixed, we should probably
revert applying it to all of the generic devices and
re-enable it when the root reason has been found.
link: https://bugzilla.kernel.org/show_bug.cgi?id=200847
link: https://bugzilla.kernel.org/show_bug.cgi?id=200849
link: https://bugs.archlinux.org/task/59699
link: https://github.com/NixOS/nixpkgs/issues/45165
Cc: stable@vger.kernel.org # v4.18+
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
drivers/hid/hid-generic.c | 15 ---------------
1 file changed, 15 deletions(-)
diff --git a/drivers/hid/hid-generic.c b/drivers/hid/hid-generic.c
index 3b6eccbc2519..c25b4718de44 100644
--- a/drivers/hid/hid-generic.c
+++ b/drivers/hid/hid-generic.c
@@ -56,20 +56,6 @@ static bool hid_generic_match(struct hid_device *hdev,
return true;
}
-static int hid_generic_probe(struct hid_device *hdev,
- const struct hid_device_id *id)
-{
- int ret;
-
- hdev->quirks |= HID_QUIRK_INPUT_PER_APP;
-
- ret = hid_parse(hdev);
- if (ret)
- return ret;
-
- return hid_hw_start(hdev, HID_CONNECT_DEFAULT);
-}
-
static const struct hid_device_id hid_table[] = {
{ HID_DEVICE(HID_BUS_ANY, HID_GROUP_ANY, HID_ANY_ID, HID_ANY_ID) },
{ }
@@ -80,7 +66,6 @@ static struct hid_driver hid_generic = {
.name = "hid-generic",
.id_table = hid_table,
.match = hid_generic_match,
- .probe = hid_generic_probe,
};
module_hid_driver(hid_generic);
--
2.14.3
^ permalink raw reply related
* Re: [PATCH 3/3] Enable high-resolution scrolling on Logitech mice
From: Nestor Lopez Casado @ 2018-08-31 9:14 UTC (permalink / raw)
To: hcutts
Cc: Benjamin Tissoires, open list:HID CORE LAYER, linux-kernel,
Dmitry Torokhov, jikos
In-Reply-To: <CAE7qMrrBdZSg1P+JTr9SA5a1ui-zdAmsg6pE-B1N5La_1WwvHw@mail.gmail.com>
On Fri, Aug 31, 2018 at 11:11 AM Nestor Lopez Casado
<nlopezcasad@logitech.com> wrote:
>
>
>
> On Thu, Aug 30, 2018 at 10:38 PM Harry Cutts <hcutts@chromium.org> wrote:
>>
>> Hi Benjamin,
>>
>> On Thu, 30 Aug 2018 at 00:18, Benjamin Tissoires
>> <benjamin.tissoires@redhat.com> wrote:
>> >> On Thu, Aug 30, 2018 at 1:06 AM Harry Cutts <hcutts@chromium.org> wrote:
>> > > > The conversion input_report_rel(... REL_WHEEL,...) to
>> > > > hid_scroll_counter_handle_scroll() should be dealt in a separate
>> > > > patch.
>> > >
>> > > OK, I'll do that in v2, but might I ask why? I don't see how this
>> > > particular hunk is special.
>> >
>> > I tend to consider that existing code rewrite need to be in their
>> > special commit, especially if the change isn't supposed to change the
>> > behaviour. This is my personal taste in case something goes wrong and
>> > (in this case) a m560 suddenly starts complaining about an issue with
>> > this mouse.
>> > I wouldn't mind too much if you rather keep the
>> > hid_scroll_counter_handle_scroll() introduction to this commit though.
>>
>> Yes, I see the reasoning for that, but this hunk is pretty tied to the
>> main change in that scrolling on the M560 would be 8x too fast without
>> it. I'll keep it in the same one, if you don't mind.
>>
>> > > [snip]
>> > > Yes, it seems to work fine without it (at least for the MX Master 2S).
>> > > Unfortunately, while testing this I encountered a bug with high-res
>> > > scrolling over Bluetooth. (It seems that if you turn off the MX Master
>> > > 2S while it's connected over Bluetooth, we don't detect that and
>> > > remove the input device, meaning that when it reconnects the driver
>> > > thinks it's in high-res mode but the mouse is in low-res.) I'm
>> > > investigating, but in the meantime I'll remove the Bluetooth support
>> > > from v2 and add it back in later.
>> >
>> > As far as I can see, the MX Master 2S is connected over BLE. Bluez
>> > keeps the uhid node opened (and thus the existing bluetooth HID
>> > device) to be able to reconnect faster.
>> > I would suppose you should get notified in the connect event of a
>> > reconnection, but it doesn't seem to be the case.
>> >
>> > Nestor, is there any event emitted by the mouse when it gets
>> > reconnected over BLE or is that a bluez issue?
>>
>> Ah, interesting. The MX Master 2S is indeed using BLE, and testing
>> with another Logitech BLE mouse (the M585) also leaves the input
>> device around. I think this is something Logitech-specific, though, as
>> the Microsoft Surface Precision mouse (also BLE) does have its input
>> device removed when it turns off. I notice that btmon does show
>> "device disconnected" and "device connected" events when I turn the
>> M585 on and off; maybe we need to plumb those through to the driver.
>> We've decided to delay Bluetooth support for high-res scrolling until
>> the Chrome OS Bluetooth stack is more stable.
-----Ooops, no html now...
This might be related to how the device disconnects from the host.
Sometimes a disconnection comes from the device not responding anymore
and it is the BT supervision timeout that kicks in (host side). In
some cases (hw support required on the device side) a device will send
a disconnect request when switched off. Maybe these different
disconnect flavors are the root cause behind the input device being
removed or not.
--nestor
>>
>> Harry Cutts
>> Chrome OS Touch/Input team
^ permalink raw reply
* [PATCH v2 5/5] Use LDJ_DEVICE macro for existing Logitech mice
From: Harry Cutts @ 2018-08-30 21:56 UTC (permalink / raw)
To: linux-input, LKML
Cc: Jiri Kosina, Dmitry Torokhov, Benjamin Tissoires, Harry Cutts,
Jiri Kosina
In-Reply-To: <20180830215622.47550-1-hcutts@chromium.org>
Signed-off-by: Harry Cutts <hcutts@chromium.org>
---
Changes in v2:
* Changed the REL_WHEEL_HI_RES code to use micrometres (a.k.a. microns)
as its units, instead of 256ths of a millimetre.
* Removed support for mice connecting over Bluetooth, due to a bug where
the mouse's high-res mode and the driver's settings get out of sync
when the mouse is power cycled.
* Moved the creation of the HID++ 1.0 function and the refactor to use
the LDJ_DEVICE macro into separate patches.
* Added a couple of explanatory comments to the Logitech driver.
drivers/hid/hid-logitech-hidpp.c | 15 +++++----------
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index fd6a8c325fa0..5f0c080059c6 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -3314,13 +3314,11 @@ static void hidpp_remove(struct hid_device *hdev)
static const struct hid_device_id hidpp_devices[] = {
{ /* wireless touchpad */
- HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE,
- USB_VENDOR_ID_LOGITECH, 0x4011),
+ LDJ_DEVICE(0x4011),
.driver_data = HIDPP_QUIRK_CLASS_WTP | HIDPP_QUIRK_DELAYED_INIT |
HIDPP_QUIRK_WTP_PHYSICAL_BUTTONS },
{ /* wireless touchpad T650 */
- HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE,
- USB_VENDOR_ID_LOGITECH, 0x4101),
+ LDJ_DEVICE(0x4101),
.driver_data = HIDPP_QUIRK_CLASS_WTP | HIDPP_QUIRK_DELAYED_INIT },
{ /* wireless touchpad T651 */
HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH,
@@ -3360,16 +3358,13 @@ static const struct hid_device_id hidpp_devices[] = {
{ /* Mouse Logitech Performance MX */
LDJ_DEVICE(0x101a), .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_1P0 },
{ /* Keyboard logitech K400 */
- HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE,
- USB_VENDOR_ID_LOGITECH, 0x4024),
+ LDJ_DEVICE(0x4024),
.driver_data = HIDPP_QUIRK_CLASS_K400 },
{ /* Solar Keyboard Logitech K750 */
- HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE,
- USB_VENDOR_ID_LOGITECH, 0x4002),
+ LDJ_DEVICE(0x4002),
.driver_data = HIDPP_QUIRK_CLASS_K750 },
- { HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE,
- USB_VENDOR_ID_LOGITECH, HID_ANY_ID)},
+ { LDJ_DEVICE(HID_ANY_ID) },
{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_G920_WHEEL),
.driver_data = HIDPP_QUIRK_CLASS_G920 | HIDPP_QUIRK_FORCE_OUTPUT_REPORTS},
--
2.19.0.rc0.228.g281dcd1b4d0-goog
^ permalink raw reply related
* [PATCH v2 4/5] Enable high-resolution scrolling on Logitech mice
From: Harry Cutts @ 2018-08-30 21:56 UTC (permalink / raw)
To: linux-input, LKML
Cc: Jiri Kosina, Dmitry Torokhov, Benjamin Tissoires, Harry Cutts,
Jiri Kosina
In-Reply-To: <20180830215622.47550-1-hcutts@chromium.org>
There are three features used by various Logitech mice for
high-resolution scrolling: the scrolling acceleration bit in HID++ 1.0,
and the x2120 and x2121 features in HID++ 2.0 and above. This patch
supports all three, and uses the multiplier reported by the mouse for
the HID++ 2.0+ features.
The full list of product IDs of mice which support high-resolution
scrolling was provided by Logitech, but the patch was tested using the
following mice (using the Unifying receiver):
* HID++ 1.0: Anywhere MX, Performance MX
* x2120: M560
* x2121: MX Anywhere 2, MX Master 2S
Signed-off-by: Harry Cutts <hcutts@chromium.org>
---
Changes in v2: None
drivers/hid/hid-logitech-hidpp.c | 249 ++++++++++++++++++++++++++++++-
1 file changed, 245 insertions(+), 4 deletions(-)
diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 7f8218f6ff56..fd6a8c325fa0 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -64,6 +64,14 @@ MODULE_PARM_DESC(disable_tap_to_click,
#define HIDPP_QUIRK_NO_HIDINPUT BIT(23)
#define HIDPP_QUIRK_FORCE_OUTPUT_REPORTS BIT(24)
#define HIDPP_QUIRK_UNIFYING BIT(25)
+#define HIDPP_QUIRK_HI_RES_SCROLL_1P0 BIT(26)
+#define HIDPP_QUIRK_HI_RES_SCROLL_X2120 BIT(27)
+#define HIDPP_QUIRK_HI_RES_SCROLL_X2121 BIT(28)
+
+/* Convenience constant to check for any high-res support. */
+#define HIDPP_QUIRK_HI_RES_SCROLL (HIDPP_QUIRK_HI_RES_SCROLL_1P0 | \
+ HIDPP_QUIRK_HI_RES_SCROLL_X2120 | \
+ HIDPP_QUIRK_HI_RES_SCROLL_X2121)
#define HIDPP_QUIRK_DELAYED_INIT HIDPP_QUIRK_NO_HIDINPUT
@@ -149,6 +157,7 @@ struct hidpp_device {
unsigned long capabilities;
struct hidpp_battery battery;
+ struct hid_scroll_counter vertical_wheel_counter;
};
/* HID++ 1.0 error codes */
@@ -1157,6 +1166,101 @@ static int hidpp_battery_get_property(struct power_supply *psy,
return ret;
}
+/* -------------------------------------------------------------------------- */
+/* 0x2120: Hi-resolution scrolling */
+/* -------------------------------------------------------------------------- */
+
+#define HIDPP_PAGE_HI_RESOLUTION_SCROLLING 0x2120
+
+#define CMD_HI_RESOLUTION_SCROLLING_SET_HIGHRES_SCROLLING_MODE 0x10
+
+static int hidpp_hrs_set_highres_scrolling_mode(struct hidpp_device *hidpp,
+ bool enabled, u8 *multiplier)
+{
+ u8 feature_index;
+ u8 feature_type;
+ int ret;
+ u8 params[1];
+ struct hidpp_report response;
+
+ ret = hidpp_root_get_feature(hidpp,
+ HIDPP_PAGE_HI_RESOLUTION_SCROLLING,
+ &feature_index,
+ &feature_type);
+ if (ret)
+ return ret;
+
+ params[0] = enabled ? BIT(0) : 0;
+ ret = hidpp_send_fap_command_sync(hidpp, feature_index,
+ CMD_HI_RESOLUTION_SCROLLING_SET_HIGHRES_SCROLLING_MODE,
+ params, sizeof(params), &response);
+ if (ret)
+ return ret;
+ *multiplier = response.fap.params[1];
+ return 0;
+}
+
+/* -------------------------------------------------------------------------- */
+/* 0x2121: HiRes Wheel */
+/* -------------------------------------------------------------------------- */
+
+#define HIDPP_PAGE_HIRES_WHEEL 0x2121
+
+#define CMD_HIRES_WHEEL_GET_WHEEL_CAPABILITY 0x00
+#define CMD_HIRES_WHEEL_SET_WHEEL_MODE 0x20
+
+static int hidpp_hrw_get_wheel_capability(struct hidpp_device *hidpp,
+ u8 *multiplier)
+{
+ u8 feature_index;
+ u8 feature_type;
+ int ret;
+ struct hidpp_report response;
+
+ ret = hidpp_root_get_feature(hidpp, HIDPP_PAGE_HIRES_WHEEL,
+ &feature_index, &feature_type);
+ if (ret)
+ goto return_default;
+
+ ret = hidpp_send_fap_command_sync(hidpp, feature_index,
+ CMD_HIRES_WHEEL_GET_WHEEL_CAPABILITY,
+ NULL, 0, &response);
+ if (ret)
+ goto return_default;
+
+ *multiplier = response.fap.params[0];
+ return 0;
+return_default:
+ *multiplier = 8;
+ hid_warn(hidpp->hid_dev,
+ "Couldn't get wheel multiplier (error %d), assuming %d.\n",
+ ret, *multiplier);
+ return ret;
+}
+
+static int hidpp_hrw_set_wheel_mode(struct hidpp_device *hidpp, bool invert,
+ bool high_resolution, bool use_hidpp)
+{
+ u8 feature_index;
+ u8 feature_type;
+ int ret;
+ u8 params[1];
+ struct hidpp_report response;
+
+ ret = hidpp_root_get_feature(hidpp, HIDPP_PAGE_HIRES_WHEEL,
+ &feature_index, &feature_type);
+ if (ret)
+ return ret;
+
+ params[0] = (invert ? BIT(2) : 0) |
+ (high_resolution ? BIT(1) : 0) |
+ (use_hidpp ? BIT(0) : 0);
+
+ return hidpp_send_fap_command_sync(hidpp, feature_index,
+ CMD_HIRES_WHEEL_SET_WHEEL_MODE,
+ params, sizeof(params), &response);
+}
+
/* -------------------------------------------------------------------------- */
/* 0x4301: Solar Keyboard */
/* -------------------------------------------------------------------------- */
@@ -2420,7 +2524,8 @@ static int m560_raw_event(struct hid_device *hdev, u8 *data, int size)
input_report_rel(mydata->input, REL_Y, v);
v = hid_snto32(data[6], 8);
- input_report_rel(mydata->input, REL_WHEEL, v);
+ hid_scroll_counter_handle_scroll(
+ &hidpp->vertical_wheel_counter, v);
input_sync(mydata->input);
}
@@ -2548,6 +2653,73 @@ static int g920_get_config(struct hidpp_device *hidpp)
return 0;
}
+/* -------------------------------------------------------------------------- */
+/* High-resolution scroll wheels */
+/* -------------------------------------------------------------------------- */
+
+/**
+ * struct hi_res_scroll_info - Stores info on a device's high-res scroll wheel.
+ * @product_id: the HID product ID of the device being described.
+ * @microns_per_hi_res_unit: the distance moved by the user's finger for each
+ * high-resolution unit reported by the device, in
+ * 256ths of a millimetre.
+ */
+struct hi_res_scroll_info {
+ __u32 product_id;
+ int microns_per_hi_res_unit;
+};
+
+static struct hi_res_scroll_info hi_res_scroll_devices[] = {
+ { /* Anywhere MX */
+ .product_id = 0x1017, .microns_per_hi_res_unit = 445 },
+ { /* Performance MX */
+ .product_id = 0x101a, .microns_per_hi_res_unit = 406 },
+ { /* M560 */
+ .product_id = 0x402d, .microns_per_hi_res_unit = 435 },
+ { /* MX Master 2S */
+ .product_id = 0x4069, .microns_per_hi_res_unit = 406 },
+};
+
+static int hi_res_scroll_look_up_microns(__u32 product_id)
+{
+ int i;
+ int num_devices = sizeof(hi_res_scroll_devices)
+ / sizeof(hi_res_scroll_devices[0]);
+ for (i = 0; i < num_devices; i++) {
+ if (hi_res_scroll_devices[i].product_id == product_id)
+ return hi_res_scroll_devices[i].microns_per_hi_res_unit;
+ }
+ /* We don't have a value for this device, so use a sensible default. */
+ return 406;
+}
+
+static int hi_res_scroll_enable(struct hidpp_device *hidpp)
+{
+ int ret;
+ u8 multiplier;
+
+ if (hidpp->quirks & HIDPP_QUIRK_HI_RES_SCROLL_X2121) {
+ ret = hidpp_hrw_set_wheel_mode(hidpp, false, true, false);
+ hidpp_hrw_get_wheel_capability(hidpp, &multiplier);
+ } else if (hidpp->quirks & HIDPP_QUIRK_HI_RES_SCROLL_X2120) {
+ ret = hidpp_hrs_set_highres_scrolling_mode(hidpp, true,
+ &multiplier);
+ } else /* if (hidpp->quirks & HIDPP_QUIRK_HI_RES_SCROLL_1P0) */ {
+ ret = hidpp10_enable_scrolling_acceleration(hidpp);
+ multiplier = 8;
+ }
+ if (ret)
+ return ret;
+
+ hidpp->vertical_wheel_counter.resolution_multiplier = multiplier;
+ hidpp->vertical_wheel_counter.microns_per_hi_res_unit =
+ hi_res_scroll_look_up_microns(hidpp->hid_dev->product);
+ hid_info(hidpp->hid_dev, "multiplier = %d, microns = %d\n",
+ multiplier,
+ hidpp->vertical_wheel_counter.microns_per_hi_res_unit);
+ return 0;
+}
+
/* -------------------------------------------------------------------------- */
/* Generic HID++ devices */
/* -------------------------------------------------------------------------- */
@@ -2593,6 +2765,11 @@ static void hidpp_populate_input(struct hidpp_device *hidpp,
wtp_populate_input(hidpp, input, origin_is_hid_core);
else if (hidpp->quirks & HIDPP_QUIRK_CLASS_M560)
m560_populate_input(hidpp, input, origin_is_hid_core);
+
+ if (hidpp->quirks & HIDPP_QUIRK_HI_RES_SCROLL) {
+ input_set_capability(input, EV_REL, REL_WHEEL_HI_RES);
+ hidpp->vertical_wheel_counter.dev = input;
+ }
}
static int hidpp_input_configured(struct hid_device *hdev,
@@ -2711,6 +2888,27 @@ static int hidpp_raw_event(struct hid_device *hdev, struct hid_report *report,
return 0;
}
+static int hidpp_event(struct hid_device *hdev, struct hid_field *field,
+ struct hid_usage *usage, __s32 value)
+{
+ /* This function will only be called for scroll events, due to the
+ * restriction imposed in hidpp_usages.
+ */
+ struct hidpp_device *hidpp = hid_get_drvdata(hdev);
+ struct hid_scroll_counter *counter = &hidpp->vertical_wheel_counter;
+ /* A scroll event may occur before the multiplier has been retrieved or
+ * the input device set, or high-res scroll enabling may fail. In such
+ * cases we must return early (falling back to default behaviour) to
+ * avoid a crash in hid_scroll_counter_handle_scroll.
+ */
+ if (!(hidpp->quirks & HIDPP_QUIRK_HI_RES_SCROLL) || value == 0
+ || counter->dev == NULL || counter->resolution_multiplier == 0)
+ return 0;
+
+ hid_scroll_counter_handle_scroll(counter, value);
+ return 1;
+}
+
static int hidpp_initialize_battery(struct hidpp_device *hidpp)
{
static atomic_t battery_no = ATOMIC_INIT(0);
@@ -2922,6 +3120,9 @@ static void hidpp_connect_event(struct hidpp_device *hidpp)
if (hidpp->battery.ps)
power_supply_changed(hidpp->battery.ps);
+ if (hidpp->quirks & HIDPP_QUIRK_HI_RES_SCROLL)
+ hi_res_scroll_enable(hidpp);
+
if (!(hidpp->quirks & HIDPP_QUIRK_NO_HIDINPUT) || hidpp->delayed_input)
/* if the input nodes are already created, we can stop now */
return;
@@ -3107,6 +3308,10 @@ static void hidpp_remove(struct hid_device *hdev)
mutex_destroy(&hidpp->send_mutex);
}
+#define LDJ_DEVICE(product) \
+ HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE, \
+ USB_VENDOR_ID_LOGITECH, (product))
+
static const struct hid_device_id hidpp_devices[] = {
{ /* wireless touchpad */
HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE,
@@ -3121,10 +3326,39 @@ static const struct hid_device_id hidpp_devices[] = {
HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH,
USB_DEVICE_ID_LOGITECH_T651),
.driver_data = HIDPP_QUIRK_CLASS_WTP },
+ { /* Mouse Logitech Anywhere MX */
+ LDJ_DEVICE(0x1017), .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_1P0 },
+ { /* Mouse Logitech Cube */
+ LDJ_DEVICE(0x4010), .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_X2120 },
+ { /* Mouse Logitech M335 */
+ LDJ_DEVICE(0x4050), .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_X2121 },
+ { /* Mouse Logitech M515 */
+ LDJ_DEVICE(0x4007), .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_X2120 },
{ /* Mouse logitech M560 */
- HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE,
- USB_VENDOR_ID_LOGITECH, 0x402d),
- .driver_data = HIDPP_QUIRK_DELAYED_INIT | HIDPP_QUIRK_CLASS_M560 },
+ LDJ_DEVICE(0x402d),
+ .driver_data = HIDPP_QUIRK_DELAYED_INIT | HIDPP_QUIRK_CLASS_M560
+ | HIDPP_QUIRK_HI_RES_SCROLL_X2120 },
+ { /* Mouse Logitech M705 (firmware RQM17) */
+ LDJ_DEVICE(0x101b), .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_1P0 },
+ { /* Mouse Logitech M705 (firmware RQM67) */
+ LDJ_DEVICE(0x406d), .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_X2121 },
+ { /* Mouse Logitech M720 */
+ LDJ_DEVICE(0x405e), .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_X2121 },
+ { /* Mouse Logitech MX Anywhere 2 */
+ LDJ_DEVICE(0x404a), .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_X2121 },
+ { LDJ_DEVICE(0xb013), .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_X2121 },
+ { LDJ_DEVICE(0xb018), .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_X2121 },
+ { LDJ_DEVICE(0xb01f), .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_X2121 },
+ { /* Mouse Logitech MX Anywhere 2S */
+ LDJ_DEVICE(0x406a), .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_X2121 },
+ { /* Mouse Logitech MX Master */
+ LDJ_DEVICE(0x4041), .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_X2121 },
+ { LDJ_DEVICE(0x4060), .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_X2121 },
+ { LDJ_DEVICE(0x4071), .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_X2121 },
+ { /* Mouse Logitech MX Master 2S */
+ LDJ_DEVICE(0x4069), .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_X2121 },
+ { /* Mouse Logitech Performance MX */
+ LDJ_DEVICE(0x101a), .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_1P0 },
{ /* Keyboard logitech K400 */
HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE,
USB_VENDOR_ID_LOGITECH, 0x4024),
@@ -3144,12 +3378,19 @@ static const struct hid_device_id hidpp_devices[] = {
MODULE_DEVICE_TABLE(hid, hidpp_devices);
+static const struct hid_usage_id hidpp_usages[] = {
+ { HID_GD_WHEEL, EV_REL, REL_WHEEL },
+ { HID_ANY_ID - 1, HID_ANY_ID - 1, HID_ANY_ID - 1}
+};
+
static struct hid_driver hidpp_driver = {
.name = "logitech-hidpp-device",
.id_table = hidpp_devices,
.probe = hidpp_probe,
.remove = hidpp_remove,
.raw_event = hidpp_raw_event,
+ .usage_table = hidpp_usages,
+ .event = hidpp_event,
.input_configured = hidpp_input_configured,
.input_mapping = hidpp_input_mapping,
.input_mapped = hidpp_input_mapped,
--
2.19.0.rc0.228.g281dcd1b4d0-goog
^ permalink raw reply related
* [PATCH v2 3/5] Add function to enable HID++ 1.0 "scrolling acceleration"
From: Harry Cutts @ 2018-08-30 21:56 UTC (permalink / raw)
To: linux-input, LKML
Cc: Jiri Kosina, Dmitry Torokhov, Benjamin Tissoires, Harry Cutts,
Jiri Kosina
In-Reply-To: <20180830215622.47550-1-hcutts@chromium.org>
"Scrolling acceleration" is a bit of a misnomer: it doesn't deal with
acceleration at all. However, that's the name used in Logitech's spec,
so I used it here.
Signed-off-by: Harry Cutts <hcutts@chromium.org>
---
Changes in v2: None
drivers/hid/hid-logitech-hidpp.c | 47 +++++++++++++++++++++++---------
1 file changed, 34 insertions(+), 13 deletions(-)
diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 19cc980eebce..7f8218f6ff56 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -400,32 +400,53 @@ static void hidpp_prefix_name(char **name, int name_length)
#define HIDPP_SET_LONG_REGISTER 0x82
#define HIDPP_GET_LONG_REGISTER 0x83
-#define HIDPP_REG_GENERAL 0x00
-
-static int hidpp10_enable_battery_reporting(struct hidpp_device *hidpp_dev)
+/**
+ * hidpp10_set_register_bit() - Sets a single bit in a HID++ 1.0 register.
+ * @hidpp_dev: the device to set the register on.
+ * @register_address: the address of the register to modify.
+ * @byte: the byte of the register to modify. Should be less than 3.
+ * Return: 0 if successful, otherwise a negative error code.
+ */
+static int hidpp10_set_register_bit(struct hidpp_device *hidpp_dev,
+ u8 register_address, u8 byte, u8 bit)
{
struct hidpp_report response;
int ret;
u8 params[3] = { 0 };
ret = hidpp_send_rap_command_sync(hidpp_dev,
- REPORT_ID_HIDPP_SHORT,
- HIDPP_GET_REGISTER,
- HIDPP_REG_GENERAL,
- NULL, 0, &response);
+ REPORT_ID_HIDPP_SHORT,
+ HIDPP_GET_REGISTER,
+ register_address,
+ NULL, 0, &response);
if (ret)
return ret;
memcpy(params, response.rap.params, 3);
- /* Set the battery bit */
- params[0] |= BIT(4);
+ params[byte] |= BIT(bit);
return hidpp_send_rap_command_sync(hidpp_dev,
- REPORT_ID_HIDPP_SHORT,
- HIDPP_SET_REGISTER,
- HIDPP_REG_GENERAL,
- params, 3, &response);
+ REPORT_ID_HIDPP_SHORT,
+ HIDPP_SET_REGISTER,
+ register_address,
+ params, 3, &response);
+}
+
+
+#define HIDPP_REG_GENERAL 0x00
+
+static int hidpp10_enable_battery_reporting(struct hidpp_device *hidpp_dev)
+{
+ return hidpp10_set_register_bit(hidpp_dev, HIDPP_REG_GENERAL, 0, 4);
+}
+
+#define HIDPP_REG_FEATURES 0x01
+
+/* On HID++ 1.0 devices, high-res scroll was called "scrolling acceleration". */
+static int hidpp10_enable_scrolling_acceleration(struct hidpp_device *hidpp_dev)
+{
+ return hidpp10_set_register_bit(hidpp_dev, HIDPP_REG_FEATURES, 0, 6);
}
#define HIDPP_REG_BATTERY_STATUS 0x07
--
2.19.0.rc0.228.g281dcd1b4d0-goog
^ permalink raw reply related
* [PATCH v2 2/5] Create a utility class for counting scroll events
From: Harry Cutts @ 2018-08-30 21:56 UTC (permalink / raw)
To: linux-input, LKML
Cc: Jiri Kosina, Dmitry Torokhov, Benjamin Tissoires, Harry Cutts,
Jiri Kosina
In-Reply-To: <20180830215622.47550-1-hcutts@chromium.org>
To avoid code duplication, this class counts high-resolution scroll
movements and emits the legacy low-resolution events when appropriate.
Drivers should be able to create one instance for each scroll wheel that
they need to handle.
Signed-off-by: Harry Cutts <hcutts@chromium.org>
---
Changes in v2: None
drivers/hid/hid-input.c | 45 +++++++++++++++++++++++++++++++++++++++++
include/linux/hid.h | 28 +++++++++++++++++++++++++
2 files changed, 73 insertions(+)
diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index 4e94ea3e280a..6e84e7b9afcb 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -1826,3 +1826,48 @@ void hidinput_disconnect(struct hid_device *hid)
}
EXPORT_SYMBOL_GPL(hidinput_disconnect);
+/**
+ * hid_scroll_counter_handle_scroll() - Send high- and low-resolution scroll
+ * events given a high-resolution wheel
+ * movement.
+ * @counter: a hid_scroll_counter struct describing the wheel.
+ * @hi_res_value: the movement of the wheel, in the mouse's high-resolution
+ * units.
+ *
+ * Given a high-resolution movement, this function converts the movement into
+ * microns and emits high-resolution scroll events for the input device. It also
+ * uses the multiplier from &struct hid_scroll_counter to emit low-resolution
+ * scroll events when appropriate for backwards-compatibility with userspace
+ * input libraries.
+ */
+void hid_scroll_counter_handle_scroll(struct hid_scroll_counter *counter,
+ int hi_res_value)
+{
+ int low_res_scroll_amount;
+ /* Some wheels will rest 7/8ths of a notch from the previous notch
+ * after slow movement, so we want the threshold for low-res events to
+ * be in the middle of the notches (e.g. after 4/8ths) as opposed to on
+ * the notches themselves (8/8ths).
+ */
+ int threshold = counter->resolution_multiplier / 2;
+
+ input_report_rel(counter->dev, REL_WHEEL_HI_RES,
+ hi_res_value * counter->microns_per_hi_res_unit);
+
+ counter->remainder += hi_res_value;
+ if (abs(counter->remainder) >= threshold) {
+ /* Add (or subtract) 1 because we want to trigger when the wheel
+ * is half-way to the next notch (i.e. scroll 1 notch after a
+ * 1/2 notch movement, 2 notches after a 1 1/2 notch movement,
+ * etc.).
+ */
+ low_res_scroll_amount =
+ counter->remainder / counter->resolution_multiplier
+ + (hi_res_value > 0 ? 1 : -1);
+ input_report_rel(counter->dev, REL_WHEEL,
+ low_res_scroll_amount);
+ counter->remainder -=
+ low_res_scroll_amount * counter->resolution_multiplier;
+ }
+}
+EXPORT_SYMBOL_GPL(hid_scroll_counter_handle_scroll);
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 834e6461a690..037e37b0b0e6 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -1138,6 +1138,34 @@ static inline u32 hid_report_len(struct hid_report *report)
int hid_report_raw_event(struct hid_device *hid, int type, u8 *data, u32 size,
int interrupt);
+
+/**
+ * struct hid_scroll_counter - Utility class for processing high-resolution
+ * scroll events.
+ * @dev: the input device for which events should be reported.
+ * @microns_per_hi_res_unit: the amount moved by the user's finger for each
+ * high-resolution unit reported by the mouse, in
+ * microns.
+ * @resolution_multiplier: the wheel's resolution in high-resolution mode as a
+ * multiple of its lower resolution. For example, if
+ * moving the wheel by one "notch" would result in a
+ * value of 1 in low-resolution mode but 8 in
+ * high-resolution, the multiplier is 8.
+ * @remainder: counts the number of high-resolution units moved since the last
+ * low-resolution event (REL_WHEEL or REL_HWHEEL) was sent. Should
+ * only be used by class methods.
+ */
+struct hid_scroll_counter {
+ struct input_dev *dev;
+ int microns_per_hi_res_unit;
+ int resolution_multiplier;
+
+ int remainder;
+};
+
+void hid_scroll_counter_handle_scroll(struct hid_scroll_counter *counter,
+ int hi_res_value);
+
/* HID quirks API */
unsigned long hid_lookup_quirk(const struct hid_device *hdev);
int hid_quirks_init(char **quirks_param, __u16 bus, int count);
--
2.19.0.rc0.228.g281dcd1b4d0-goog
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox