Re: [PATCH] HID: fix A4Tech horizontal scrolling

[BÅaÅej SzczygieÅ]

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@xxxxxxxxx> 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@xxxxx> 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@xxxxx>
> > > >
> > > > 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.