[PATCH v2] HID: input: fix a4tech horizontal wheel custom usage

[PATCH v2] HID: input: fix a4tech horizontal wheel custom usage

[Nicolas Saenz Julienne]

Some a4tech mice use the 'GenericDesktop.00b8' usage to inform whether
the previous wheel report was horizontal or vertical. Before
c01908a14bf73 ("HID: input: add mapping for "Toggle Display" key") this
usage was being mapped to 'Relative.Misc'. After the patch it's simply
ignored (usage->type == 0 & usage->code == 0). Which ultimately makes
hid-a4tech ignore the WHEEL/HWHEEL selection event, as it has no
usage->type.

We shouldn't rely on a mapping for that usage as it's nonstandard and
doesn't really map to an input event. So we bypass the mapping and make
sure the custom event handling properly handles both reports.

Fixes: c01908a14bf73 ("HID: input: add mapping for "Toggle Display" key")
Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
---

NOTE: I CC'd Wolfgang as he's the one who can test this.

Changes since v1:
  - new approach, moved fix into hid-a4tech

 drivers/hid/hid-a4tech.c | 30 +++++++++++++++++++++++++++---
 1 file changed, 27 insertions(+), 3 deletions(-)

diff --git a/drivers/hid/hid-a4tech.c b/drivers/hid/hid-a4tech.c
index 98bf694626f7..3a8c4a5971f7 100644
--- a/drivers/hid/hid-a4tech.c
+++ b/drivers/hid/hid-a4tech.c
@@ -23,12 +23,36 @@
 #define A4_2WHEEL_MOUSE_HACK_7	0x01
 #define A4_2WHEEL_MOUSE_HACK_B8	0x02
 
+#define A4_WHEEL_ORIENTATION	(HID_UP_GENDESK | 0x000000b8)
+
 struct a4tech_sc {
 	unsigned long quirks;
 	unsigned int hw_wheel;
 	__s32 delayed_value;
 };
 
+static int a4_input_mapping(struct hid_device *hdev, struct hid_input *hi,
+			    struct hid_field *field, struct hid_usage *usage,
+			    unsigned long **bit, int *max)
+{
+	struct a4tech_sc *a4 = hid_get_drvdata(hdev);
+
+	if (a4->quirks & A4_2WHEEL_MOUSE_HACK_B8 &&
+	    usage->hid == A4_WHEEL_ORIENTATION) {
+		/*
+		 * We do not want to have this usage mapped to anything as it's
+		 * nonstandard and doesn't really behave like an HID report.
+		 * It's only selecting the orientation (vertical/horizontal) of
+		 * the previous mouse wheel report. The input_events will be
+		 * generated once both reports are recorded in a4_event().
+		 */
+		return -1;
+	}
+
+	return 0;
+
+}
+
 static int a4_input_mapped(struct hid_device *hdev, struct hid_input *hi,
 		struct hid_field *field, struct hid_usage *usage,
 		unsigned long **bit, int *max)
@@ -52,8 +76,7 @@ static int a4_event(struct hid_device *hdev, struct hid_field *field,
 	struct a4tech_sc *a4 = hid_get_drvdata(hdev);
 	struct input_dev *input;
 
-	if (!(hdev->claimed & HID_CLAIMED_INPUT) || !field->hidinput ||
-			!usage->type)
+	if (!(hdev->claimed & HID_CLAIMED_INPUT) || !field->hidinput)
 		return 0;
 
 	input = field->hidinput->input;
@@ -64,7 +87,7 @@ static int a4_event(struct hid_device *hdev, struct hid_field *field,
 			return 1;
 		}
 
-		if (usage->hid == 0x000100b8) {
+		if (usage->hid == A4_WHEEL_ORIENTATION) {
 			input_event(input, EV_REL, value ? REL_HWHEEL :
 					REL_WHEEL, a4->delayed_value);
 			input_event(input, EV_REL, value ? REL_HWHEEL_HI_RES :
@@ -131,6 +154,7 @@ MODULE_DEVICE_TABLE(hid, a4_devices);
 static struct hid_driver a4_driver = {
 	.name = "a4tech",
 	.id_table = a4_devices,
+	.input_mapping = a4_input_mapping,
 	.input_mapped = a4_input_mapped,
 	.event = a4_event,
 	.probe = a4_probe,
-- 
2.21.0

Re: [PATCH v2] HID: input: fix a4tech horizontal wheel custom usage

[Benjamin Tissoires]

On Tue, Jun 11, 2019 at 2:13 PM Nicolas Saenz Julienne
<nsaenzjulienne@suse.de> wrote:
>
> Some a4tech mice use the 'GenericDesktop.00b8' usage to inform whether
> the previous wheel report was horizontal or vertical. Before
> c01908a14bf73 ("HID: input: add mapping for "Toggle Display" key") this
> usage was being mapped to 'Relative.Misc'. After the patch it's simply
> ignored (usage->type == 0 & usage->code == 0). Which ultimately makes
> hid-a4tech ignore the WHEEL/HWHEEL selection event, as it has no
> usage->type.
>
> We shouldn't rely on a mapping for that usage as it's nonstandard and
> doesn't really map to an input event. So we bypass the mapping and make
> sure the custom event handling properly handles both reports.
>
> Fixes: c01908a14bf73 ("HID: input: add mapping for "Toggle Display" key")
> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> ---

Yep, I like this one much better.

>
> NOTE: I CC'd Wolfgang as he's the one who can test this.

I'll wait for Wolfram to confirm that the patch works before pushing then.

>
> Changes since v1:
>   - new approach, moved fix into hid-a4tech
>
>  drivers/hid/hid-a4tech.c | 30 +++++++++++++++++++++++++++---
>  1 file changed, 27 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/hid/hid-a4tech.c b/drivers/hid/hid-a4tech.c
> index 98bf694626f7..3a8c4a5971f7 100644
> --- a/drivers/hid/hid-a4tech.c
> +++ b/drivers/hid/hid-a4tech.c
> @@ -23,12 +23,36 @@
>  #define A4_2WHEEL_MOUSE_HACK_7 0x01
>  #define A4_2WHEEL_MOUSE_HACK_B8        0x02
>
> +#define A4_WHEEL_ORIENTATION   (HID_UP_GENDESK | 0x000000b8)
> +
>  struct a4tech_sc {
>         unsigned long quirks;
>         unsigned int hw_wheel;
>         __s32 delayed_value;
>  };
>
> +static int a4_input_mapping(struct hid_device *hdev, struct hid_input *hi,
> +                           struct hid_field *field, struct hid_usage *usage,
> +                           unsigned long **bit, int *max)
> +{
> +       struct a4tech_sc *a4 = hid_get_drvdata(hdev);
> +
> +       if (a4->quirks & A4_2WHEEL_MOUSE_HACK_B8 &&
> +           usage->hid == A4_WHEEL_ORIENTATION) {
> +               /*
> +                * We do not want to have this usage mapped to anything as it's
> +                * nonstandard and doesn't really behave like an HID report.
> +                * It's only selecting the orientation (vertical/horizontal) of
> +                * the previous mouse wheel report. The input_events will be
> +                * generated once both reports are recorded in a4_event().
> +                */
> +               return -1;

You seem to be extra precocious here. This is basically what the
current hid-input.c mapping does. But I think it is for the best,
given that this time you are locking the behavior of this reserved
usage.

Cheers,
Benjamin

> +       }
> +
> +       return 0;
> +
> +}
> +
>  static int a4_input_mapped(struct hid_device *hdev, struct hid_input *hi,
>                 struct hid_field *field, struct hid_usage *usage,
>                 unsigned long **bit, int *max)
> @@ -52,8 +76,7 @@ static int a4_event(struct hid_device *hdev, struct hid_field *field,
>         struct a4tech_sc *a4 = hid_get_drvdata(hdev);
>         struct input_dev *input;
>
> -       if (!(hdev->claimed & HID_CLAIMED_INPUT) || !field->hidinput ||
> -                       !usage->type)
> +       if (!(hdev->claimed & HID_CLAIMED_INPUT) || !field->hidinput)
>                 return 0;
>
>         input = field->hidinput->input;
> @@ -64,7 +87,7 @@ static int a4_event(struct hid_device *hdev, struct hid_field *field,
>                         return 1;
>                 }
>
> -               if (usage->hid == 0x000100b8) {
> +               if (usage->hid == A4_WHEEL_ORIENTATION) {
>                         input_event(input, EV_REL, value ? REL_HWHEEL :
>                                         REL_WHEEL, a4->delayed_value);
>                         input_event(input, EV_REL, value ? REL_HWHEEL_HI_RES :
> @@ -131,6 +154,7 @@ MODULE_DEVICE_TABLE(hid, a4_devices);
>  static struct hid_driver a4_driver = {
>         .name = "a4tech",
>         .id_table = a4_devices,
> +       .input_mapping = a4_input_mapping,
>         .input_mapped = a4_input_mapped,
>         .event = a4_event,
>         .probe = a4_probe,
> --
> 2.21.0
>

Re: [PATCH v2] HID: input: fix a4tech horizontal wheel custom usage

[Wolfgang Bauer]

On Tuesday, 11. Juni 2019, 16:42:37 Benjamin Tissoires wrote:
> On Tue, Jun 11, 2019 at 2:13 PM Nicolas Saenz Julienne
> 
> <nsaenzjulienne@suse.de> wrote:
> > NOTE: I CC'd Wolfgang as he's the one who can test this.
> 
> I'll wait for Wolfram to confirm that the patch works before pushing then.

My name is Wolfgang, not Wolfram... ;-)
But never mind.

I tested the patch meanwhile on top of kernel 5.2.rc4, where the mouse wheel 
actually worked.
As the patch didn't apply cleanly (it's obviously based upon 
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=abf82e8f7e9af40a49e3d905187c662a43c96c8f , called "HID-
a4tech-fix-horizontal-scrolling.patch" below), I added that patch as well.

My results:
kernel 5.2.rc4 works
kernel 5.2.rc4 + HID-a4tech-fix-horizontal-scrolling.patch is broken
kernel 5.2.rc4 + HID-a4tech-fix-horizontal-scrolling.patch +
HID-input-fix-a4tech-horizontal-wheel-custom-usage.patch (i.e. this patch) 
works again

kernel 5.2.rc4 + HID-input-fix-a4tech-horizontal-wheel-custom-usage.patch 
works as well.

So AFAICT this patch seems to be fine.

For completeness, this is my mouse as listed by lsusb:
Bus 003 Device 002: ID 09da:000a A4Tech Co., Ltd. Optical Mouse Opto 510D / 
OP-620D

Kind Regards,
Wolfgang

Re: [PATCH v2] HID: input: fix a4tech horizontal wheel custom usage

[Wolfgang Bauer]

I tested linux-next (20190612) meanwhile as well.
My mouse wheel doesn't work with that kernel, this patch fixes it.

Kind Regards,
Wolfgang

Re: [PATCH v2] HID: input: fix a4tech horizontal wheel custom usage

[Benjamin Tissoires]

Hi Wolfgang,

On Thu, Jun 13, 2019 at 1:49 PM Wolfgang Bauer <wbauer@tmo.at> wrote:
>
> On Tuesday, 11. Juni 2019, 16:42:37 Benjamin Tissoires wrote:
> > On Tue, Jun 11, 2019 at 2:13 PM Nicolas Saenz Julienne
> >
> > <nsaenzjulienne@suse.de> wrote:
> > > NOTE: I CC'd Wolfgang as he's the one who can test this.
> >
> > I'll wait for Wolfram to confirm that the patch works before pushing then.
>
> My name is Wolfgang, not Wolfram... ;-)

ouch, sorry for that (I am more used to talk to the I2C maintainer apparently)

> But never mind.
>
> I tested the patch meanwhile on top of kernel 5.2.rc4, where the mouse wheel
> actually worked.

Actually, I am a little bit lost here.

The patch mentions a fix of c01908a14bf73, which is in 5.1 final.
So if your mouse works in 5.2.rc4, I am not sure how
HID-a4tech-fix-horizontal-scrolling.patch could break it.

Could you be slightly more specific in what "works" and what doesn't?

Do we have the report descriptors available somewhere?
And if not, could you run hid-recorder from
https://gitlab.freedesktop.org/libevdev/hid-tools and attach the logs
when you move the horizontal wheel?

Cheers,
Benjamin

> As the patch didn't apply cleanly (it's obviously based upon
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=abf82e8f7e9af40a49e3d905187c662a43c96c8f , called "HID-
> a4tech-fix-horizontal-scrolling.patch" below), I added that patch as well.
>
> My results:
> kernel 5.2.rc4 works
> kernel 5.2.rc4 + HID-a4tech-fix-horizontal-scrolling.patch is broken
> kernel 5.2.rc4 + HID-a4tech-fix-horizontal-scrolling.patch +
> HID-input-fix-a4tech-horizontal-wheel-custom-usage.patch (i.e. this patch)
> works again
>
> kernel 5.2.rc4 + HID-input-fix-a4tech-horizontal-wheel-custom-usage.patch
> works as well.
>
> So AFAICT this patch seems to be fine.
>
> For completeness, this is my mouse as listed by lsusb:
> Bus 003 Device 002: ID 09da:000a A4Tech Co., Ltd. Optical Mouse Opto 510D /
> OP-620D
>
> Kind Regards,
> Wolfgang
>

Re: [PATCH v2] HID: input: fix a4tech horizontal wheel custom usage

[Nicolas Saenz Julienne]

[-- Attachment #1: Type: text/plain, Size: 5224 bytes --]

On Fri, 2019-06-14 at 15:36 +0200, Benjamin Tissoires wrote:
> Hi Wolfgang,
> 
> On Thu, Jun 13, 2019 at 1:49 PM Wolfgang Bauer <wbauer@tmo.at> wrote:
> > On Tuesday, 11. Juni 2019, 16:42:37 Benjamin Tissoires wrote:
> > > On Tue, Jun 11, 2019 at 2:13 PM Nicolas Saenz Julienne
> > > 
> > > <nsaenzjulienne@suse.de> wrote:
> > > > NOTE: I CC'd Wolfgang as he's the one who can test this.
> > > 
> > > I'll wait for Wolfram to confirm that the patch works before pushing then.
> > 
> > My name is Wolfgang, not Wolfram... ;-)
> 
> ouch, sorry for that (I am more used to talk to the I2C maintainer apparently)
> 
> > But never mind.
> > 
> > I tested the patch meanwhile on top of kernel 5.2.rc4, where the mouse wheel
> > actually worked.
> 
> Actually, I am a little bit lost here.
>
> The patch mentions a fix of c01908a14bf73, which is in 5.1 final.
> So if your mouse works in 5.2.rc4, I am not sure how
> HID-a4tech-fix-horizontal-scrolling.patch could break it.
> 
> Could you be slightly more specific in what "works" and what doesn't?

Hi Benjamin,

First of all here's the descriptor:
0x05, 0x01, /*  Usage Page (Desktop),               */
0x09, 0x02, /*  Usage (Mouse),                      */
0xA1, 0x01, /*  Collection (Application),           */
0x09, 0x01, /*      Usage (Pointer),                */
0xA1, 0x00, /*      Collection (Physical),          */
0x05, 0x09, /*          Usage Page (Button),        */
0x19, 0x01, /*          Usage Minimum (01h),        */
0x29, 0x08, /*          Usage Maximum (08h),        */
0x15, 0x00, /*          Logical Minimum (0),        */
0x25, 0x01, /*          Logical Maximum (1),        */
0x75, 0x01, /*          Report Size (1),            */
0x95, 0x08, /*          Report Count (8),           */
0x81, 0x02, /*          Input (Variable),           */
0x05, 0x01, /*          Usage Page (Desktop),       */
0x09, 0x30, /*          Usage (X),                  */
0x09, 0x31, /*          Usage (Y),                  */
0x09, 0x38, /*          Usage (Wheel),              */
0x09, 0xB8, /*          Usage (B8h),                */
0x15, 0x81, /*          Logical Minimum (-127),     */
0x25, 0x7F, /*          Logical Maximum (127),      */
0x75, 0x08, /*          Report Size (8),            */
0x95, 0x04, /*          Report Count (4),           */
0x81, 0x06, /*          Input (Variable, Relative), */
0xC0,       /*      End Collection,                 */
0xC0        /*  End Collection


Sorry for the confusion, I'll try to explain the situation:

In v5.2-rc4 without "HID-a4tech-fix-horizontal-scrolling.patch" the vertical
wheel works out of luck as it's mapped to REL_WHEEL_HIGH_RES, which hid-a4tech
ignores and lets hid-input process, the horizontal wheel is broken. On top of
that Usage(0xB8) is also ignored by both hid-a4tech and hid-input as it isn't
mapped to anything.

There are two distinct bugs here:
  - High resolution wheel processing in hid-a4tech not being implemented,
    breaking horizontal wheels.
  - hid-a4tech not taking care of Usage(0xB8) correctly as it depended on it
    being mapped to "Rel.Misc". That behaviour changed in v5.1 with "HID:
    input: add mapping for "Toggle Display" key".

Once high resolution wheel reports are fixed and handled in hid-a4tech's custom
event, the mouse breaks as it's the processing of Usage(0xB8) that triggers the
input_events, which is being ignored.

You'll probably ask how come we didn't see this when
"HID-a4tech-fix-horizontal-scrolling.patch" was merged. It's due to the fact it
was tested on an older kernel, v5.0.15, that didn't contain "HID: input: add
mapping for "Toggle Display" key"[1].

So that's why I added that specific fix tag. For LTS kernels, it is possible
that "Toggle Display" support was back-ported but not the high resolution
wheels support.

Hope it made things more clear.
Regards,
Nicolas

[1] 
https://lkml.kernel.org/lkml/nycvar.YFH.7.76.1906010028440.1962@cbobk.fhfr.pm/T/

> 
> Do we have the report descriptors available somewhere?
> And if not, could you run hid-recorder from
> https://gitlab.freedesktop.org/libevdev/hid-tools and attach the logs
> when you move the horizontal wheel?
> 
> Cheers,
> Benjamin
> 
> > As the patch didn't apply cleanly (it's obviously based upon
> > 
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=abf82e8f7e9af40a49e3d905187c662a43c96c8f
, called
> > "HID-
> > a4tech-fix-horizontal-scrolling.patch" below), I added that patch as well.
> > 
> > My results:
> > kernel 5.2.rc4 works
> > kernel 5.2.rc4 + HID-a4tech-fix-horizontal-scrolling.patch is broken
> > kernel 5.2.rc4 + HID-a4tech-fix-horizontal-scrolling.patch +
> > HID-input-fix-a4tech-horizontal-wheel-custom-usage.patch (i.e. this patch)
> > works again
> > 
> > kernel 5.2.rc4 + HID-input-fix-a4tech-horizontal-wheel-custom-usage.patch
> > works as well.
> > 
> > So AFAICT this patch seems to be fine.
> > 
> > For completeness, this is my mouse as listed by lsusb:
> > Bus 003 Device 002: ID 09da:000a A4Tech Co., Ltd. Optical Mouse Opto 510D /
> > OP-620D
> > 
> > Kind Regards,
> > Wolfgang
> > 


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2] HID: input: fix a4tech horizontal wheel custom usage

[Nicolas Saenz Julienne]

[-- Attachment #1: Type: text/plain, Size: 4206 bytes --]

On Fri, 2019-06-14 at 16:25 +0200, Nicolas Saenz Julienne wrote:
> On Fri, 2019-06-14 at 15:36 +0200, Benjamin Tissoires wrote:
> > Hi Wolfgang,
> > 
> > On Thu, Jun 13, 2019 at 1:49 PM Wolfgang Bauer <wbauer@tmo.at> wrote:
> > > On Tuesday, 11. Juni 2019, 16:42:37 Benjamin Tissoires wrote:
> > > > On Tue, Jun 11, 2019 at 2:13 PM Nicolas Saenz Julienne
> > > > 
> > > > <nsaenzjulienne@suse.de> wrote:
> > > > > NOTE: I CC'd Wolfgang as he's the one who can test this.
> > > > 
> > > > I'll wait for Wolfram to confirm that the patch works before pushing
> > > > then.
> > > 
> > > My name is Wolfgang, not Wolfram... ;-)
> > 
> > ouch, sorry for that (I am more used to talk to the I2C maintainer
> > apparently)
> > 
> > > But never mind.
> > > 
> > > I tested the patch meanwhile on top of kernel 5.2.rc4, where the mouse
> > > wheel
> > > actually worked.
> > 
> > Actually, I am a little bit lost here.
> > 
> > The patch mentions a fix of c01908a14bf73, which is in 5.1 final.
> > So if your mouse works in 5.2.rc4, I am not sure how
> > HID-a4tech-fix-horizontal-scrolling.patch could break it.
> > 
> > Could you be slightly more specific in what "works" and what doesn't?
> 
> Hi Benjamin,
> 
> First of all here's the descriptor:
> 0x05, 0x01, /*  Usage Page (Desktop),               */
> 0x09, 0x02, /*  Usage (Mouse),                      */
> 0xA1, 0x01, /*  Collection (Application),           */
> 0x09, 0x01, /*      Usage (Pointer),                */
> 0xA1, 0x00, /*      Collection (Physical),          */
> 0x05, 0x09, /*          Usage Page (Button),        */
> 0x19, 0x01, /*          Usage Minimum (01h),        */
> 0x29, 0x08, /*          Usage Maximum (08h),        */
> 0x15, 0x00, /*          Logical Minimum (0),        */
> 0x25, 0x01, /*          Logical Maximum (1),        */
> 0x75, 0x01, /*          Report Size (1),            */
> 0x95, 0x08, /*          Report Count (8),           */
> 0x81, 0x02, /*          Input (Variable),           */
> 0x05, 0x01, /*          Usage Page (Desktop),       */
> 0x09, 0x30, /*          Usage (X),                  */
> 0x09, 0x31, /*          Usage (Y),                  */
> 0x09, 0x38, /*          Usage (Wheel),              */
> 0x09, 0xB8, /*          Usage (B8h),                */
> 0x15, 0x81, /*          Logical Minimum (-127),     */
> 0x25, 0x7F, /*          Logical Maximum (127),      */
> 0x75, 0x08, /*          Report Size (8),            */
> 0x95, 0x04, /*          Report Count (4),           */
> 0x81, 0x06, /*          Input (Variable, Relative), */
> 0xC0,       /*      End Collection,                 */
> 0xC0        /*  End Collection
> 
> 
> Sorry for the confusion, I'll try to explain the situation:
> 
> In v5.2-rc4 without "HID-a4tech-fix-horizontal-scrolling.patch" the vertical
> wheel works out of luck as it's mapped to REL_WHEEL_HIGH_RES, which hid-a4tech
> ignores and lets hid-input process, the horizontal wheel is broken. On top of
> that Usage(0xB8) is also ignored by both hid-a4tech and hid-input as it isn't
> mapped to anything.
> 
> There are two distinct bugs here:
>   - High resolution wheel processing in hid-a4tech not being implemented,
>     breaking horizontal wheels.
>   - hid-a4tech not taking care of Usage(0xB8) correctly as it depended on it
>     being mapped to "Rel.Misc". That behaviour changed in v5.1 with "HID:
>     input: add mapping for "Toggle Display" key".
> 
> Once high resolution wheel reports are fixed and handled in hid-a4tech's
> custom
> event, the mouse breaks as it's the processing of Usage(0xB8) that triggers
> the
> input_events, which is being ignored.
> 
> You'll probably ask how come we didn't see this when
> "HID-a4tech-fix-horizontal-scrolling.patch" was merged. It's due to the fact
> it
> was tested on an older kernel, v5.0.15, that didn't contain "HID: input: add
> mapping for "Toggle Display" key"[1].
> 
> So that's why I added that specific fix tag. For LTS kernels, it is possible
> that "Toggle Display" support was back-ported but not the high resolution
> wheels support.

Hi,
Any thoughts on this?

Regards,
Nicolas


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2] HID: input: fix a4tech horizontal wheel custom usage

[Jeremy Cline]

On Fri, Jun 14, 2019 at 03:36:03PM +0200, Benjamin Tissoires wrote:
> Hi Wolfgang,
> 
> On Thu, Jun 13, 2019 at 1:49 PM Wolfgang Bauer <wbauer@tmo.at> wrote:
> >
> > On Tuesday, 11. Juni 2019, 16:42:37 Benjamin Tissoires wrote:
> > > On Tue, Jun 11, 2019 at 2:13 PM Nicolas Saenz Julienne
> > >
> > > <nsaenzjulienne@suse.de> wrote:
> > > > NOTE: I CC'd Wolfgang as he's the one who can test this.
> > >
> > > I'll wait for Wolfram to confirm that the patch works before pushing then.
> >
> > My name is Wolfgang, not Wolfram... ;-)
> 
> ouch, sorry for that (I am more used to talk to the I2C maintainer apparently)
> 
> > But never mind.
> >
> > I tested the patch meanwhile on top of kernel 5.2.rc4, where the mouse wheel
> > actually worked.
> 
> Actually, I am a little bit lost here.
> 
> The patch mentions a fix of c01908a14bf73, which is in 5.1 final.
> So if your mouse works in 5.2.rc4, I am not sure how
> HID-a4tech-fix-horizontal-scrolling.patch could break it.
> 
> Could you be slightly more specific in what "works" and what doesn't?
> 

For what it's worth, at least one user has hit this issue in Fedora
starting on stable kernel v5.1.17[0] which has commit abf82e8f7e9a
("HID: a4tech: fix horizontal scrolling"). Applying this patch on top of
v5.1.18 fixed the issue for them.

[0] https://bugzilla.redhat.com/show_bug.cgi?id=1730762


Regards,
Jeremy

Re: [PATCH v2] HID: input: fix a4tech horizontal wheel custom usage

[Nicolas Saenz Julienne]

[-- Attachment #1: Type: text/plain, Size: 1042 bytes --]

On Tue, 2019-06-11 at 14:13 +0200, Nicolas Saenz Julienne wrote:
> Some a4tech mice use the 'GenericDesktop.00b8' usage to inform whether
> the previous wheel report was horizontal or vertical. Before
> c01908a14bf73 ("HID: input: add mapping for "Toggle Display" key") this
> usage was being mapped to 'Relative.Misc'. After the patch it's simply
> ignored (usage->type == 0 & usage->code == 0). Which ultimately makes
> hid-a4tech ignore the WHEEL/HWHEEL selection event, as it has no
> usage->type.
> 
> We shouldn't rely on a mapping for that usage as it's nonstandard and
> doesn't really map to an input event. So we bypass the mapping and make
> sure the custom event handling properly handles both reports.
> 
> Fixes: c01908a14bf73 ("HID: input: add mapping for "Toggle Display" key")
> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> ---

It would be nice for this patch not to get lost. It fixes issues both repoted
on opensuse and fedora.

I can resubmit it if needed.

Regards,
Nicolas


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2] HID: input: fix a4tech horizontal wheel custom usage

[Jiri Kosina]

On Thu, 1 Aug 2019, Nicolas Saenz Julienne wrote:

> > Some a4tech mice use the 'GenericDesktop.00b8' usage to inform whether
> > the previous wheel report was horizontal or vertical. Before
> > c01908a14bf73 ("HID: input: add mapping for "Toggle Display" key") this
> > usage was being mapped to 'Relative.Misc'. After the patch it's simply
> > ignored (usage->type == 0 & usage->code == 0). Which ultimately makes
> > hid-a4tech ignore the WHEEL/HWHEEL selection event, as it has no
> > usage->type.
> > 
> > We shouldn't rely on a mapping for that usage as it's nonstandard and
> > doesn't really map to an input event. So we bypass the mapping and make
> > sure the custom event handling properly handles both reports.
> > 
> > Fixes: c01908a14bf73 ("HID: input: add mapping for "Toggle Display" key")
> > Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> > ---
> 
> It would be nice for this patch not to get lost. It fixes issues both repoted
> on opensuse and fedora.

Sorry for the delay. I've now queued the patch. Thanks for fixing this,

-- 
Jiri Kosina
SUSE Labs

Re: Re: [PATCH v2] HID: input: fix a4tech horizontal wheel custom usage

[Heorhi Valakhanovich]

On 8/5/19 3:38 PM, Jiri Kosina wrote:
> On Thu, 1 Aug 2019, Nicolas Saenz Julienne wrote:
> 
>>> Some a4tech mice use the 'GenericDesktop.00b8' usage to inform whether
>>> the previous wheel report was horizontal or vertical. Before
>>> c01908a14bf73 ("HID: input: add mapping for "Toggle Display" key") this
>>> usage was being mapped to 'Relative.Misc'. After the patch it's simply
>>> ignored (usage->type == 0 & usage->code == 0). Which ultimately makes
>>> hid-a4tech ignore the WHEEL/HWHEEL selection event, as it has no
>>> usage->type.
>>>
>>> We shouldn't rely on a mapping for that usage as it's nonstandard and
>>> doesn't really map to an input event. So we bypass the mapping and make
>>> sure the custom event handling properly handles both reports.
>>>
>>> Fixes: c01908a14bf73 ("HID: input: add mapping for "Toggle Display" key")
>>> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
>>> ---
>>
>> It would be nice for this patch not to get lost. It fixes issues both repoted
>> on opensuse and fedora.
> 
> Sorry for the delay. I've now queued the patch. Thanks for fixing this,
> 

Any plans to apply this patch on top of 5.2.* and 5.1.* ?