* [PATCH] HID: fix A4Tech horizontal scrolling
@ 2019-05-02 21:36 Błażej Szczygieł
2019-05-03 7:36 ` Benjamin Tissoires
0 siblings, 1 reply; 7+ messages in thread
From: Błażej Szczygieł @ 2019-05-02 21:36 UTC (permalink / raw)
Cc: igorkuo, Błażej Szczygieł,
Jiri Kosina, Benjamin Tissoires, linux-input, linux-kernel
Since recent high resolution scrolling changes the A4Tech driver must
check for the "REL_WHEEL_HI_RES" usage code.
Fixes: 2dc702c991e3774af9d7ce410eef410ca9e2357e (HID: input: use the
Resolution Multiplier for high-resolution scrolling)
Signed-off-by: Błażej Szczygieł <spaz16@wp.pl>
---
drivers/hid/hid-a4tech.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/hid/hid-a4tech.c b/drivers/hid/hid-a4tech.c
index 9428ea7cdf8a..fafb9fa558e7 100644
--- a/drivers/hid/hid-a4tech.c
+++ b/drivers/hid/hid-a4tech.c
@@ -38,7 +38,7 @@ static int a4_input_mapped(struct hid_device *hdev, struct hid_input *hi,
{
struct a4tech_sc *a4 = hid_get_drvdata(hdev);
- if (usage->type == EV_REL && usage->code == REL_WHEEL)
+ if (usage->type == EV_REL && usage->code == REL_WHEEL_HI_RES)
set_bit(REL_HWHEEL, *bit);
if ((a4->quirks & A4_2WHEEL_MOUSE_HACK_7) && usage->hid == 0x00090007)
@@ -60,7 +60,7 @@ static int a4_event(struct hid_device *hdev, struct hid_field *field,
input = field->hidinput->input;
if (a4->quirks & A4_2WHEEL_MOUSE_HACK_B8) {
- if (usage->type == EV_REL && usage->code == REL_WHEEL) {
+ if (usage->type == EV_REL && usage->code == REL_WHEEL_HI_RES) {
a4->delayed_value = value;
return 1;
}
@@ -77,7 +77,7 @@ static int a4_event(struct hid_device *hdev, struct hid_field *field,
return 1;
}
- if (usage->code == REL_WHEEL && a4->hw_wheel) {
+ if (usage->code == REL_WHEEL_HI_RES && a4->hw_wheel) {
input_event(input, usage->type, REL_HWHEEL, value);
return 1;
}
--
2.21.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] HID: fix A4Tech horizontal scrolling
2019-05-02 21:36 [PATCH] HID: fix A4Tech horizontal scrolling Błażej Szczygieł
@ 2019-05-03 7:36 ` Benjamin Tissoires
2019-05-03 9:22 ` Błażej Szczygieł
2019-05-03 9:36 ` Igor Kushnir
0 siblings, 2 replies; 7+ messages in thread
From: Benjamin Tissoires @ 2019-05-03 7:36 UTC (permalink / raw)
To: Błażej Szczygieł
Cc: igorkuo, Jiri Kosina, open list:HID CORE LAYER, lkml
Hi,
On Thu, May 2, 2019 at 11:37 PM Błażej Szczygieł <spaz16@wp.pl> wrote:
>
> Since recent high resolution scrolling changes the A4Tech driver must
> check for the "REL_WHEEL_HI_RES" usage code.
>
> Fixes: 2dc702c991e3774af9d7ce410eef410ca9e2357e (HID: input: use the
> Resolution Multiplier for high-resolution scrolling)
>
> Signed-off-by: Błażej Szczygieł <spaz16@wp.pl>
Thanks for the patch. I do not doubt this fixes the issues, but I
still wonder if we should not export REL_HWHEEL_HI_RES instead of
REL_HWHEEL events.
Also, I can not figure out how the events are processed by the kernel.
Could you attach a hid-recorder dump of the mouse wheels with
hid-recorder from https://gitlab.freedesktop.org/libevdev/hid-tools ?
This should give me a better view of the mouse, and I could also add
it to the regression tests I am running for each commit.
Cheers,
Benjamin
> ---
> drivers/hid/hid-a4tech.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/hid/hid-a4tech.c b/drivers/hid/hid-a4tech.c
> index 9428ea7cdf8a..fafb9fa558e7 100644
> --- a/drivers/hid/hid-a4tech.c
> +++ b/drivers/hid/hid-a4tech.c
> @@ -38,7 +38,7 @@ static int a4_input_mapped(struct hid_device *hdev, struct hid_input *hi,
> {
> struct a4tech_sc *a4 = hid_get_drvdata(hdev);
>
> - if (usage->type == EV_REL && usage->code == REL_WHEEL)
> + if (usage->type == EV_REL && usage->code == REL_WHEEL_HI_RES)
> set_bit(REL_HWHEEL, *bit);
>
> if ((a4->quirks & A4_2WHEEL_MOUSE_HACK_7) && usage->hid == 0x00090007)
> @@ -60,7 +60,7 @@ static int a4_event(struct hid_device *hdev, struct hid_field *field,
> input = field->hidinput->input;
>
> if (a4->quirks & A4_2WHEEL_MOUSE_HACK_B8) {
> - if (usage->type == EV_REL && usage->code == REL_WHEEL) {
> + if (usage->type == EV_REL && usage->code == REL_WHEEL_HI_RES) {
> a4->delayed_value = value;
> return 1;
> }
> @@ -77,7 +77,7 @@ static int a4_event(struct hid_device *hdev, struct hid_field *field,
> return 1;
> }
>
> - if (usage->code == REL_WHEEL && a4->hw_wheel) {
> + if (usage->code == REL_WHEEL_HI_RES && a4->hw_wheel) {
> input_event(input, usage->type, REL_HWHEEL, value);
> return 1;
> }
> --
> 2.21.0
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] HID: fix A4Tech horizontal scrolling
2019-05-03 7:36 ` Benjamin Tissoires
@ 2019-05-03 9:22 ` Błażej Szczygieł
2019-05-03 9:36 ` Igor Kushnir
1 sibling, 0 replies; 7+ messages in thread
From: Błażej Szczygieł @ 2019-05-03 9:22 UTC (permalink / raw)
To: Benjamin Tissoires; +Cc: igorkuo, Jiri Kosina, open list:HID CORE LAYER, lkml
Hi,
I used the hid-record tool and my results are here:
https://gitlab.com/snippets/1853568
Cheers,
Błażej
> Hi,
>
> On Thu, May 2, 2019 at 11:37 PM Błażej Szczygieł <spaz16@wp.pl> wrote:
>>
>> Since recent high resolution scrolling changes the A4Tech driver must
>> check for the "REL_WHEEL_HI_RES" usage code.
>>
>> Fixes: 2dc702c991e3774af9d7ce410eef410ca9e2357e (HID: input: use the
>> Resolution Multiplier for high-resolution scrolling)
>>
>> Signed-off-by: Błażej Szczygieł <spaz16@wp.pl>
>
> Thanks for the patch. I do not doubt this fixes the issues, but I
> still wonder if we should not export REL_HWHEEL_HI_RES instead of
> REL_HWHEEL events.
>
> Also, I can not figure out how the events are processed by the kernel.
> Could you attach a hid-recorder dump of the mouse wheels with
> hid-recorder from https://gitlab.freedesktop.org/libevdev/hid-tools ?
>
> This should give me a better view of the mouse, and I could also add
> it to the regression tests I am running for each commit.
>
> Cheers,
> Benjamin
>
>> ---
>> drivers/hid/hid-a4tech.c | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/hid/hid-a4tech.c b/drivers/hid/hid-a4tech.c
>> index 9428ea7cdf8a..fafb9fa558e7 100644
>> --- a/drivers/hid/hid-a4tech.c
>> +++ b/drivers/hid/hid-a4tech.c
>> @@ -38,7 +38,7 @@ static int a4_input_mapped(struct hid_device *hdev, struct hid_input *hi,
>> {
>> struct a4tech_sc *a4 = hid_get_drvdata(hdev);
>>
>> - if (usage->type == EV_REL && usage->code == REL_WHEEL)
>> + if (usage->type == EV_REL && usage->code == REL_WHEEL_HI_RES)
>> set_bit(REL_HWHEEL, *bit);
>>
>> if ((a4->quirks & A4_2WHEEL_MOUSE_HACK_7) && usage->hid == 0x00090007)
>> @@ -60,7 +60,7 @@ static int a4_event(struct hid_device *hdev, struct hid_field *field,
>> input = field->hidinput->input;
>>
>> if (a4->quirks & A4_2WHEEL_MOUSE_HACK_B8) {
>> - if (usage->type == EV_REL && usage->code == REL_WHEEL) {
>> + if (usage->type == EV_REL && usage->code == REL_WHEEL_HI_RES) {
>> a4->delayed_value = value;
>> return 1;
>> }
>> @@ -77,7 +77,7 @@ static int a4_event(struct hid_device *hdev, struct hid_field *field,
>> return 1;
>> }
>>
>> - if (usage->code == REL_WHEEL && a4->hw_wheel) {
>> + if (usage->code == REL_WHEEL_HI_RES && a4->hw_wheel) {
>> input_event(input, usage->type, REL_HWHEEL, value);
>> return 1;
>> }
>> --
>> 2.21.0
>>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] HID: fix A4Tech horizontal scrolling
2019-05-03 7:36 ` Benjamin Tissoires
2019-05-03 9:22 ` Błażej Szczygieł
@ 2019-05-03 9:36 ` Igor Kushnir
2019-05-03 11:59 ` Benjamin Tissoires
1 sibling, 1 reply; 7+ messages in thread
From: Igor Kushnir @ 2019-05-03 9:36 UTC (permalink / raw)
To: Benjamin Tissoires, Błażej Szczygieł
Cc: Jiri Kosina, open list:HID CORE LAYER, lkml
Hi Benjamin,
On 5/3/19 10:36 AM, Benjamin Tissoires wrote:
> Hi,
>
> On Thu, May 2, 2019 at 11:37 PM Błażej Szczygieł <spaz16@wp.pl> wrote:
>>
>> Since recent high resolution scrolling changes the A4Tech driver must
>> check for the "REL_WHEEL_HI_RES" usage code.
>>
>> Fixes: 2dc702c991e3774af9d7ce410eef410ca9e2357e (HID: input: use the
>> Resolution Multiplier for high-resolution scrolling)
>>
>> Signed-off-by: Błażej Szczygieł <spaz16@wp.pl>
>
> Thanks for the patch. I do not doubt this fixes the issues, but I
> still wonder if we should not export REL_HWHEEL_HI_RES instead of
> REL_HWHEEL events.
If you mean exporting REL_HWHEEL_HI_RES instead of REL_HWHEEL from
hid-a4tech.c, then it makes sense to me, though I do not know the code
well enough to be certain.
Błażej and I have discussed the bug and the patch here:
https://bugzilla.kernel.org/show_bug.cgi?id=203369
In summary: the patch fixes the bug for both our mice;
the documentation in input/event-codes.rst states that
REL_WHEEL, REL_HWHEEL "are legacy codes and REL_WHEEL_HI_RES and
REL_HWHEEL_HI_RES should be preferred where available."
> Also, I can not figure out how the events are processed by the kernel.
> Could you attach a hid-recorder dump of the mouse wheels with
> hid-recorder from https://gitlab.freedesktop.org/libevdev/hid-tools ?
>
> This should give me a better view of the mouse, and I could also add
> it to the regression tests I am running for each commit.
>
> Cheers,
> Benjamin
After launching hid-recorder for my A4Tech WOP-49Z mouse under kernel
5.0.10 patched with Błażej's patch I:
* scrolled the vertical wheel down ("Wheel: -1");
* scrolled the vertical wheel up ("Wheel: 1");
* scrolled the horizontal wheel "left" ("Wheel: -1");
* scrolled the horizontal wheel "right" ("Wheel: 1").
Note that the horizontal wheel is physically scrolled just like the
vertical one in this mouse (forward/back), so "left" and "right" are the
effects these scrollings make in applications when the kernel supports
the mouse properly.
$ sudo ./hid-recorder /dev/hidraw1
# A4Tech PS/2+USB Mouse
# 0x05, 0x01, // Usage Page (Generic Desktop) 0
# 0x09, 0x02, // Usage (Mouse) 2
# 0xa1, 0x01, // Collection (Application) 4
# 0x09, 0x01, // Usage (Pointer) 6
# 0xa1, 0x00, // Collection (Physical) 8
# 0x05, 0x09, // Usage Page (Button) 10
# 0x19, 0x01, // Usage Minimum (1) 12
# 0x29, 0x07, // Usage Maximum (7) 14
# 0x15, 0x00, // Logical Minimum (0) 16
# 0x25, 0x01, // Logical Maximum (1) 18
# 0x75, 0x01, // Report Size (1) 20
# 0x95, 0x07, // Report Count (7) 22
# 0x81, 0x02, // Input (Data,Var,Abs) 24
# 0x75, 0x01, // Report Size (1) 26
# 0x95, 0x01, // Report Count (1) 28
# 0x81, 0x01, // Input (Cnst,Arr,Abs) 30
# 0x05, 0x01, // Usage Page (Generic Desktop) 32
# 0x09, 0x30, // Usage (X) 34
# 0x09, 0x31, // Usage (Y) 36
# 0x09, 0x38, // Usage (Wheel) 38
# 0x15, 0x81, // Logical Minimum (-127) 40
# 0x25, 0x7f, // Logical Maximum (127) 42
# 0x75, 0x08, // Report Size (8) 44
# 0x95, 0x03, // Report Count (3) 46
# 0x81, 0x06, // Input (Data,Var,Rel) 48
# 0xc0, // End Collection 50
# 0xc0, // End Collection 51
#
R: 52 05 01 09 02 a1 01 09 01 a1 00 05 09 19 01 29 07 15 00 25 01 75 01
95 07 81 02 75 01 95 01 81 01 05 01 09 30 09 31 09 38 15 81 25 7f 75 08
95 03 81 06 c0 c0
N: A4Tech PS/2+USB Mouse
I: 3 09da 0006
# Button: 0 0 0 0 0 0 0 | # | X: 0 | Y: 0 | Wheel: -1
E: 000000.000000 4 00 00 00 ff
# Button: 0 0 0 0 0 0 0 | # | X: 0 | Y: 0 | Wheel: -1
E: 000000.071952 4 00 00 00 ff
# Button: 0 0 0 0 0 0 0 | # | X: 0 | Y: 0 | Wheel: -1
E: 000000.159957 4 00 00 00 ff
# Button: 0 0 0 0 0 0 0 | # | X: 0 | Y: 0 | Wheel: 1
E: 000002.912232 4 00 00 00 01
# Button: 0 0 0 0 0 0 0 | # | X: 0 | Y: 0 | Wheel: 1
E: 000002.952190 4 00 00 00 01
# Button: 0 0 0 0 0 0 0 | # | X: 0 | Y: 0 | Wheel: 1
E: 000004.512359 4 00 00 00 01
# Button: 0 0 0 0 0 0 0 | # | X: 0 | Y: 0 | Wheel: 1
E: 000004.584332 4 00 00 00 01
# Button: 0 0 0 0 0 0 1 | # | X: 0 | Y: 0 | Wheel: -1
E: 000007.528626 4 40 00 00 ff
# Button: 0 0 0 0 0 0 1 | # | X: 0 | Y: 0 | Wheel: -1
E: 000007.568577 4 40 00 00 ff
# Button: 0 0 0 0 0 0 1 | # | X: 0 | Y: 0 | Wheel: -1
E: 000008.256395 4 40 00 00 ff
# Button: 0 0 0 0 0 0 1 | # | X: 0 | Y: 0 | Wheel: -1
E: 000008.336669 4 40 00 00 ff
# Button: 0 0 0 0 0 0 1 | # | X: 0 | Y: 0 | Wheel: -1
E: 000008.400649 4 40 00 00 ff
# Button: 0 0 0 0 0 0 1 | # | X: 0 | Y: 0 | Wheel: 1
E: 000010.936908 4 40 00 00 01
# Button: 0 0 0 0 0 0 1 | # | X: 0 | Y: 0 | Wheel: 1
E: 000010.984864 4 40 00 00 01
# Button: 0 0 0 0 0 0 1 | # | X: 0 | Y: 0 | Wheel: 1
E: 000011.056897 4 40 00 00 01
# Button: 0 0 0 0 0 0 1 | # | X: 0 | Y: 0 | Wheel: 1
E: 000011.528936 4 40 00 00 01
# Button: 0 0 0 0 0 0 1 | # | X: 0 | Y: 0 | Wheel: 1
E: 000011.616923 4 40 00 00 01
Cheers,
Igor
>> ---
>> drivers/hid/hid-a4tech.c | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/hid/hid-a4tech.c b/drivers/hid/hid-a4tech.c
>> index 9428ea7cdf8a..fafb9fa558e7 100644
>> --- a/drivers/hid/hid-a4tech.c
>> +++ b/drivers/hid/hid-a4tech.c
>> @@ -38,7 +38,7 @@ static int a4_input_mapped(struct hid_device *hdev, struct hid_input *hi,
>> {
>> struct a4tech_sc *a4 = hid_get_drvdata(hdev);
>>
>> - if (usage->type == EV_REL && usage->code == REL_WHEEL)
>> + if (usage->type == EV_REL && usage->code == REL_WHEEL_HI_RES)
>> set_bit(REL_HWHEEL, *bit);
>>
>> if ((a4->quirks & A4_2WHEEL_MOUSE_HACK_7) && usage->hid == 0x00090007)
>> @@ -60,7 +60,7 @@ static int a4_event(struct hid_device *hdev, struct hid_field *field,
>> input = field->hidinput->input;
>>
>> if (a4->quirks & A4_2WHEEL_MOUSE_HACK_B8) {
>> - if (usage->type == EV_REL && usage->code == REL_WHEEL) {
>> + if (usage->type == EV_REL && usage->code == REL_WHEEL_HI_RES) {
>> a4->delayed_value = value;
>> return 1;
>> }
>> @@ -77,7 +77,7 @@ static int a4_event(struct hid_device *hdev, struct hid_field *field,
>> return 1;
>> }
>>
>> - if (usage->code == REL_WHEEL && a4->hw_wheel) {
>> + if (usage->code == REL_WHEEL_HI_RES && a4->hw_wheel) {
>> input_event(input, usage->type, REL_HWHEEL, value);
>> return 1;
>> }
>> --
>> 2.21.0
>>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] HID: fix A4Tech horizontal scrolling
2019-05-03 9:36 ` Igor Kushnir
@ 2019-05-03 11:59 ` Benjamin Tissoires
2019-05-07 5:01 ` Peter Hutterer
0 siblings, 1 reply; 7+ messages in thread
From: Benjamin Tissoires @ 2019-05-03 11:59 UTC (permalink / raw)
To: Igor Kushnir, Peter Hutterer
Cc: Błażej Szczygieł,
Jiri Kosina, open list:HID CORE LAYER, lkml
Hi,
On Fri, May 3, 2019 at 11:43 AM Igor Kushnir <igorkuo@gmail.com> wrote:
>
> Hi Benjamin,
>
> On 5/3/19 10:36 AM, Benjamin Tissoires wrote:
> > Hi,
> >
> > On Thu, May 2, 2019 at 11:37 PM Błażej Szczygieł <spaz16@wp.pl> wrote:
> >>
> >> Since recent high resolution scrolling changes the A4Tech driver must
> >> check for the "REL_WHEEL_HI_RES" usage code.
> >>
> >> Fixes: 2dc702c991e3774af9d7ce410eef410ca9e2357e (HID: input: use the
> >> Resolution Multiplier for high-resolution scrolling)
> >>
> >> Signed-off-by: Błażej Szczygieł <spaz16@wp.pl>
> >
> > Thanks for the patch. I do not doubt this fixes the issues, but I
> > still wonder if we should not export REL_HWHEEL_HI_RES instead of
> > REL_HWHEEL events.
>
>
> If you mean exporting REL_HWHEEL_HI_RES instead of REL_HWHEEL from
> hid-a4tech.c, then it makes sense to me, though I do not know the code
> well enough to be certain.
Yep, that's what I meant. I am worried that userspace doesn't know
well how to deal with a device that mixes the new and old REL_WHEEL
events.
>
> Błażej and I have discussed the bug and the patch here:
> https://bugzilla.kernel.org/show_bug.cgi?id=203369
Oh cool.
Then we should add: "Link:
https://bugzilla.kernel.org/show_bug.cgi?id=203369" in the commit
description.
Also, given that the patch will likely see a v2, te format of the
"Fixes" tag is not correct: see
https://www.kernel.org/doc/html/v4.20/process/submitting-patches.html#describe-your-changes
(I have been notified that I tend to not follow the rules here, so I
am trying to do better here :-P )
>
> In summary: the patch fixes the bug for both our mice;
> the documentation in input/event-codes.rst states that
> REL_WHEEL, REL_HWHEEL "are legacy codes and REL_WHEEL_HI_RES and
> REL_HWHEEL_HI_RES should be preferred where available."
>
> > Also, I can not figure out how the events are processed by the kernel.
> > Could you attach a hid-recorder dump of the mouse wheels with
> > hid-recorder from https://gitlab.freedesktop.org/libevdev/hid-tools ?
> >
> > This should give me a better view of the mouse, and I could also add
> > it to the regression tests I am running for each commit.
> >
> > Cheers,
> > Benjamin
>
> After launching hid-recorder for my A4Tech WOP-49Z mouse under kernel
> 5.0.10 patched with Błażej's patch I:
> * scrolled the vertical wheel down ("Wheel: -1");
> * scrolled the vertical wheel up ("Wheel: 1");
> * scrolled the horizontal wheel "left" ("Wheel: -1");
> * scrolled the horizontal wheel "right" ("Wheel: 1").
> Note that the horizontal wheel is physically scrolled just like the
> vertical one in this mouse (forward/back), so "left" and "right" are the
> effects these scrollings make in applications when the kernel supports
> the mouse properly.
>
> $ sudo ./hid-recorder /dev/hidraw1
> # A4Tech PS/2+USB Mouse
> # 0x05, 0x01, // Usage Page (Generic Desktop) 0
> # 0x09, 0x02, // Usage (Mouse) 2
> # 0xa1, 0x01, // Collection (Application) 4
> # 0x09, 0x01, // Usage (Pointer) 6
> # 0xa1, 0x00, // Collection (Physical) 8
> # 0x05, 0x09, // Usage Page (Button) 10
> # 0x19, 0x01, // Usage Minimum (1) 12
> # 0x29, 0x07, // Usage Maximum (7) 14
> # 0x15, 0x00, // Logical Minimum (0) 16
> # 0x25, 0x01, // Logical Maximum (1) 18
> # 0x75, 0x01, // Report Size (1) 20
> # 0x95, 0x07, // Report Count (7) 22
> # 0x81, 0x02, // Input (Data,Var,Abs) 24
> # 0x75, 0x01, // Report Size (1) 26
> # 0x95, 0x01, // Report Count (1) 28
> # 0x81, 0x01, // Input (Cnst,Arr,Abs) 30
> # 0x05, 0x01, // Usage Page (Generic Desktop) 32
> # 0x09, 0x30, // Usage (X) 34
> # 0x09, 0x31, // Usage (Y) 36
> # 0x09, 0x38, // Usage (Wheel) 38
> # 0x15, 0x81, // Logical Minimum (-127) 40
> # 0x25, 0x7f, // Logical Maximum (127) 42
> # 0x75, 0x08, // Report Size (8) 44
> # 0x95, 0x03, // Report Count (3) 46
> # 0x81, 0x06, // Input (Data,Var,Rel) 48
> # 0xc0, // End Collection 50
> # 0xc0, // End Collection 51
> #
> R: 52 05 01 09 02 a1 01 09 01 a1 00 05 09 19 01 29 07 15 00 25 01 75 01
> 95 07 81 02 75 01 95 01 81 01 05 01 09 30 09 31 09 38 15 81 25 7f 75 08
> 95 03 81 06 c0 c0
> N: A4Tech PS/2+USB Mouse
> I: 3 09da 0006
> # Button: 0 0 0 0 0 0 0 | # | X: 0 | Y: 0 | Wheel: -1
> E: 000000.000000 4 00 00 00 ff
> # Button: 0 0 0 0 0 0 0 | # | X: 0 | Y: 0 | Wheel: -1
> E: 000000.071952 4 00 00 00 ff
> # Button: 0 0 0 0 0 0 0 | # | X: 0 | Y: 0 | Wheel: -1
> E: 000000.159957 4 00 00 00 ff
> # Button: 0 0 0 0 0 0 0 | # | X: 0 | Y: 0 | Wheel: 1
> E: 000002.912232 4 00 00 00 01
> # Button: 0 0 0 0 0 0 0 | # | X: 0 | Y: 0 | Wheel: 1
> E: 000002.952190 4 00 00 00 01
> # Button: 0 0 0 0 0 0 0 | # | X: 0 | Y: 0 | Wheel: 1
> E: 000004.512359 4 00 00 00 01
> # Button: 0 0 0 0 0 0 0 | # | X: 0 | Y: 0 | Wheel: 1
> E: 000004.584332 4 00 00 00 01
> # Button: 0 0 0 0 0 0 1 | # | X: 0 | Y: 0 | Wheel: -1
> E: 000007.528626 4 40 00 00 ff
> # Button: 0 0 0 0 0 0 1 | # | X: 0 | Y: 0 | Wheel: -1
> E: 000007.568577 4 40 00 00 ff
> # Button: 0 0 0 0 0 0 1 | # | X: 0 | Y: 0 | Wheel: -1
> E: 000008.256395 4 40 00 00 ff
> # Button: 0 0 0 0 0 0 1 | # | X: 0 | Y: 0 | Wheel: -1
> E: 000008.336669 4 40 00 00 ff
> # Button: 0 0 0 0 0 0 1 | # | X: 0 | Y: 0 | Wheel: -1
> E: 000008.400649 4 40 00 00 ff
> # Button: 0 0 0 0 0 0 1 | # | X: 0 | Y: 0 | Wheel: 1
> E: 000010.936908 4 40 00 00 01
> # Button: 0 0 0 0 0 0 1 | # | X: 0 | Y: 0 | Wheel: 1
> E: 000010.984864 4 40 00 00 01
> # Button: 0 0 0 0 0 0 1 | # | X: 0 | Y: 0 | Wheel: 1
> E: 000011.056897 4 40 00 00 01
> # Button: 0 0 0 0 0 0 1 | # | X: 0 | Y: 0 | Wheel: 1
> E: 000011.528936 4 40 00 00 01
> # Button: 0 0 0 0 0 0 1 | # | X: 0 | Y: 0 | Wheel: 1
> E: 000011.616923 4 40 00 00 01
>
OK, thanks both of you for your logs, this is helpful.
So just in case I need to come back later, the horizontal wheel is
"just" the normal wheel plus a modifier in the report.
Anyway, ideally, can we have a v2 of the patch with the 2 changes
requested above in the commit message and the introduction of
REL_HWHEEL_HI_RES events in addition to REL_HWHEEL?
REL_HWHEEL_HI_RES should report `120*value` and we should also keep
the reporting of REL_WHEEL as it is currently.
Peter, I grepped in the hid code, and it seems hid-cypress.c is having
the exact same issue. Sigh.
Cheers,
Benjamin
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] HID: fix A4Tech horizontal scrolling
2019-05-03 11:59 ` Benjamin Tissoires
@ 2019-05-07 5:01 ` Peter Hutterer
2019-05-12 20:59 ` Błażej Szczygieł
0 siblings, 1 reply; 7+ messages in thread
From: Peter Hutterer @ 2019-05-07 5:01 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Igor Kushnir, Błażej Szczygieł,
Jiri Kosina, open list:HID CORE LAYER, lkml
On Fri, May 03, 2019 at 01:59:23PM +0200, Benjamin Tissoires wrote:
> Hi,
>
> On Fri, May 3, 2019 at 11:43 AM Igor Kushnir <igorkuo@gmail.com> wrote:
> >
> > Hi Benjamin,
> >
> > On 5/3/19 10:36 AM, Benjamin Tissoires wrote:
> > > Hi,
> > >
> > > On Thu, May 2, 2019 at 11:37 PM Błażej Szczygieł <spaz16@wp.pl> wrote:
> > >>
> > >> Since recent high resolution scrolling changes the A4Tech driver must
> > >> check for the "REL_WHEEL_HI_RES" usage code.
> > >>
> > >> Fixes: 2dc702c991e3774af9d7ce410eef410ca9e2357e (HID: input: use the
> > >> Resolution Multiplier for high-resolution scrolling)
> > >>
> > >> Signed-off-by: Błażej Szczygieł <spaz16@wp.pl>
> > >
> > > Thanks for the patch. I do not doubt this fixes the issues, but I
> > > still wonder if we should not export REL_HWHEEL_HI_RES instead of
> > > REL_HWHEEL events.
> >
> >
> > If you mean exporting REL_HWHEEL_HI_RES instead of REL_HWHEEL from
> > hid-a4tech.c, then it makes sense to me, though I do not know the code
> > well enough to be certain.
>
> Yep, that's what I meant. I am worried that userspace doesn't know
> well how to deal with a device that mixes the new and old REL_WHEEL
> events.
sorry, I'm not sure what you mean here. The new events are always mixed with
the old ones anyway, and both should be treated as separate event streams.
The kernel interface to userspace is fairly easy to deal with, it's the rest
that's a bit of mess.
[..]
> >
>
> OK, thanks both of you for your logs, this is helpful.
> So just in case I need to come back later, the horizontal wheel is
> "just" the normal wheel plus a modifier in the report.
>
> Anyway, ideally, can we have a v2 of the patch with the 2 changes
> requested above in the commit message and the introduction of
> REL_HWHEEL_HI_RES events in addition to REL_HWHEEL?
> REL_HWHEEL_HI_RES should report `120*value` and we should also keep
> the reporting of REL_WHEEL as it is currently.
>
> Peter, I grepped in the hid code, and it seems hid-cypress.c is having
> the exact same issue. Sigh.
yeah, I found that too when grepping through it. seems to be the only other
one though and we can use Błażej's patch as boilerplate once it's done.
Cheers,
Peter
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] HID: fix A4Tech horizontal scrolling
2019-05-07 5:01 ` Peter Hutterer
@ 2019-05-12 20:59 ` Błażej Szczygieł
0 siblings, 0 replies; 7+ messages in thread
From: Błażej Szczygieł @ 2019-05-12 20:59 UTC (permalink / raw)
To: Peter Hutterer
Cc: Benjamin Tissoires, Igor Kushnir, Jiri Kosina,
open list:HID CORE LAYER, lkml
Hi,
On 07.05.2019 at 07:01, Peter Hutterer wrote:
> On Fri, May 03, 2019 at 01:59:23PM +0200, Benjamin Tissoires wrote:
>> Hi,
>>
>> On Fri, May 3, 2019 at 11:43 AM Igor Kushnir <igorkuo@gmail.com> wrote:
>>>
>>> Hi Benjamin,
>>>
>>> On 5/3/19 10:36 AM, Benjamin Tissoires wrote:
>>>> Hi,
>>>>
>>>> On Thu, May 2, 2019 at 11:37 PM Błażej Szczygieł <spaz16@wp.pl> wrote:
>>>>>
>>>>> Since recent high resolution scrolling changes the A4Tech driver must
>>>>> check for the "REL_WHEEL_HI_RES" usage code.
>>>>>
>>>>> Fixes: 2dc702c991e3774af9d7ce410eef410ca9e2357e (HID: input: use the
>>>>> Resolution Multiplier for high-resolution scrolling)
>>>>>
>>>>> Signed-off-by: Błażej Szczygieł <spaz16@wp.pl>
>>>>
>>>> Thanks for the patch. I do not doubt this fixes the issues, but I
>>>> still wonder if we should not export REL_HWHEEL_HI_RES instead of
>>>> REL_HWHEEL events.
>>>
>>>
>>> If you mean exporting REL_HWHEEL_HI_RES instead of REL_HWHEEL from
>>> hid-a4tech.c, then it makes sense to me, though I do not know the code
>>> well enough to be certain.
>>
>> Yep, that's what I meant. I am worried that userspace doesn't know
>> well how to deal with a device that mixes the new and old REL_WHEEL
>> events.
>
> sorry, I'm not sure what you mean here. The new events are always mixed with
> the old ones anyway, and both should be treated as separate event streams.
> The kernel interface to userspace is fairly easy to deal with, it's the rest
> that's a bit of mess.
>
> [..]
>
>>>
>>
>> OK, thanks both of you for your logs, this is helpful.
>> So just in case I need to come back later, the horizontal wheel is
>> "just" the normal wheel plus a modifier in the report.
>>
>> Anyway, ideally, can we have a v2 of the patch with the 2 changes
>> requested above in the commit message and the introduction of
>> REL_HWHEEL_HI_RES events in addition to REL_HWHEEL?
>> REL_HWHEEL_HI_RES should report `120*value` and we should also keep
>> the reporting of REL_WHEEL as it is currently.
>>
>> Peter, I grepped in the hid code, and it seems hid-cypress.c is having
>> the exact same issue. Sigh.
>
> yeah, I found that too when grepping through it. seems to be the only other
> one though and we can use Błażej's patch as boilerplate once it's done.
Peter, I also found comparison of "usage->code ==" with "REL_HWHEEL"
and "REL_WHEEL" in hid-lenovo.c, hid-apple.c, hid-ezkey.c, hid-lg.c.
Unfortunatelly, I don't have such devices to test :(
Cheers,
Błażej.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-05-12 20:59 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-02 21:36 [PATCH] HID: fix A4Tech horizontal scrolling Błażej Szczygieł
2019-05-03 7:36 ` Benjamin Tissoires
2019-05-03 9:22 ` Błażej Szczygieł
2019-05-03 9:36 ` Igor Kushnir
2019-05-03 11:59 ` Benjamin Tissoires
2019-05-07 5:01 ` Peter Hutterer
2019-05-12 20:59 ` Błażej Szczygieł
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).