Message-ID: <yq7byjaea3rfrfbttc6ngwnajiqvxl6xdhscyfmroraq2lr7bj@fvd4t2clnhz3>
Date: Mon, 25 Nov 2024 15:08:41 +0100
From: Benjamin Tissoires <bentiss@...nel.org>
To: Mike Galbraith <efault@....de>, 
	Linus Torvalds <torvalds@...ux-foundation.org>
Cc: Jiri Kosina <jikos@...nel.org>, linux-kernel@...r.kernel.org
Subject: Re: [GIT PULL] HID for 6.13

On Nov 24 2024, Mike Galbraith wrote:
> On Fri, 2024-11-22 at 12:13 -0800, Linus Torvalds wrote:
> > On Mon, 18 Nov 2024 at 13:16, Jiri Kosina <jikos@...nel.org> wrote:
> > >
> > > please pull from
> > >
> > >   git://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git tags/hid-for-linus-2024111801
> > >
> > > to receive HID subsystem queue for 6.13 merge window.
> >
> > Hmm. There's something odd going on here. My mouse scroll-wheel
> > stopped working (Logitech MX Anywhere 3), and this pull would look
> > like the prime suspect.
> >
> > The mouse otherwise works, so it's not that the mouse isn't found,
> > it's literally just the scroll-wheel functionality that doesn't work.
> >
> > Oddly enough, if I remove and re-insert the Logitech wireless dongle,
> > the scroll wheel works again. So it's not some kind of complete
> > breakage - but it also wasn't a one-time fluke thing, in that it
> > happened twice in a row when rebooting into a new kernel.
> >
> > Any ideas? Does this make anybody go "Hmm, maybe ..."
> 
> No, but my M215 had the same issue, it bisected to 6fd47effe92b, and
> revert via patch confirmed it.
> 

Thanks a lot for the bisect (and the logs in the followup message).

I was puzzled at first but after a few tests today I think I found out
the reason (I'll detail it below).

So I am currently contemplating 3 options:

1. just revert 6fd47effe92b (and e14e0eaeb040, the matching selftests
addition), quick and easy, and postpone the inclusion of a fixed
6fd47effe92b in 6.14

2. try to generically fix 6fd47effe92b (in hid-core), but it will likely
won't be done today as I'm just back from sick leave and have 3 weeks of
pending emails to process too and this requires some more brain than
just a "let's try this"

3. fix hid-logitech-hidpp as it assumes it can only have one input per
node attached to it (again, not easy as probably some testing is
required).

So, Linus, depending on how much annoying this bug is for you, feel free
to revert 6fd47effe92b right now in your tree or wait a few more days
for me to find an appropriate fix for 2. or 3.

---
Reason of this bug:
The idea of 6fd47effe92b, was to be able to call hid_bpf_rdesc_fixup()
once per reprobe of the device.
However, because the bpf filter can now change the quirk value, the call
had to be moved before the driver gets bound (which was previously
ensuring the unicity of the call).

The net effect is that now, in the case hid-generic gets loaded first and
then the specific driver gets loaded once the disk is available, the
value of ->quirks is not reset, but kept to the value that was set by
hid-generic (HID_QUIRK_INPUT_PER_APP).

Once hid-logitech-hidpp kicks in, that quirk is now set, which creates 2
inputs for the single mouse: one keyboard for fancy shortcuts, and one
mouse node.

However, hid-logitech-hidpp expects only one input node to be attached
(it stores it into hidpp->input), and when a wheel event is received,
because there is some processing with high-resolution wheel events, the
wheel event is injected into hidpp->input. And of course, when
HID_QUIRK_INPUT_PER_APP is set, hidpp->input gets the keyboard node,
which doesn't have wheel event type, and the events are ignored.
---

Clearly, hid-logitech-hidpp would require a fix too, but OTOH, it might
not be the only driver confused by an extra quirk being set depending of
if hid-generic ever handled the device or not.

So the more I think of it, the more the revert makes sense to me.

Please tell me if you want me to send the revert or if you'll just apply
it yourself.

Cheers,
Benjamin

Powered by blists - more mailing lists