* [PATCH] Partially revert "HID: generic: create one input report per application type"
@ 2018-08-31 9:36 Benjamin Tissoires
2018-08-31 17:47 ` Junge, Terry
2018-09-04 11:55 ` Benjamin Tissoires
0 siblings, 2 replies; 5+ messages in thread
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 [flat|nested] 5+ messages in thread
* RE: [PATCH] Partially revert "HID: generic: create one input report per application type"
2018-08-31 9:36 [PATCH] Partially revert "HID: generic: create one input report per application type" Benjamin Tissoires
@ 2018-08-31 17:47 ` Junge, Terry
2018-09-04 9:53 ` Benjamin Tissoires
2018-09-04 11:55 ` Benjamin Tissoires
1 sibling, 1 reply; 5+ messages in thread
From: Junge, Terry @ 2018-08-31 17:47 UTC (permalink / raw)
To: Benjamin Tissoires, Jiri Kosina, Dmitry Torokhov
Cc: linux-input, linux-kernel, stable
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 [flat|nested] 5+ messages in thread
* Re: [PATCH] Partially revert "HID: generic: create one input report per application type"
2018-08-31 17:47 ` Junge, Terry
@ 2018-09-04 9:53 ` Benjamin Tissoires
0 siblings, 0 replies; 5+ messages in thread
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+
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 [flat|nested] 5+ messages in thread
* Re: [PATCH] Partially revert "HID: generic: create one input report per application type"
2018-08-31 9:36 [PATCH] Partially revert "HID: generic: create one input report per application type" Benjamin Tissoires
2018-08-31 17:47 ` Junge, Terry
@ 2018-09-04 11:55 ` Benjamin Tissoires
2018-09-04 13:33 ` Benjamin Tissoires
1 sibling, 1 reply; 5+ messages in thread
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+
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 [flat|nested] 5+ messages in thread
* Re: [PATCH] Partially revert "HID: generic: create one input report per application type"
2018-09-04 11:55 ` Benjamin Tissoires
@ 2018-09-04 13:33 ` Benjamin Tissoires
0 siblings, 0 replies; 5+ messages in thread
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+
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 [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-09-04 13:33 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-31 9:36 [PATCH] Partially revert "HID: generic: create one input report per application type" Benjamin Tissoires
2018-08-31 17:47 ` Junge, Terry
2018-09-04 9:53 ` Benjamin Tissoires
2018-09-04 11:55 ` Benjamin Tissoires
2018-09-04 13:33 ` Benjamin Tissoires
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).