Re: [PATCH] HID: fix A4Tech horizontal scrolling

[Peter Hutterer]

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.

Cheers,
   Peter