On Wed, Dec 7, 2022 at 3:24 PM Bastien Nocera <hadess@xxxxxxxxxx> wrote: > > On Wed, 2022-12-07 at 13:43 +0100, Bastien Nocera wrote: > > On Wed, 2022-12-07 at 11:19 +0100, Jiri Kosina wrote: > > > On Wed, 7 Dec 2022, Benjamin Tissoires wrote: > > > > > > > Agree, but OTOH, Rafael, your mouse is not brand new AFAICT, so I > > > > am > > > > worried that you won't be the only one complaining we just killed > > > > their > > > > mouse. So I think the even wiser solution would be to delay (and > > > > so > > > > revert in 6.1 or 6.2) the 2 patches that enable hid++ on all > > > > logitech > > > > mice (8544c812e43ab7bdf40458411b83987b8cba924d and > > > > 532223c8ac57605a10e46dc0ab23dcf01c9acb43). > > > > > > If we were not at -rc8 timeframe, I'd be in favor to coming up with > > > proper > > > fix still for 6.1. But as things currently are, let's just revert > > > those > > > and reschedule them with proper fix for 6.2+. > > > > Has anyone seen any other reports? > > > > Because, honestly, seeing work that adds support for dozens of > > devices > > getting tossed out at the last minute based on a single report with > > no > > opportunity to fix the problem is really frustrating. > > FWIW, I went out to buy a Logitech device that uses Bluetooth Classic, > the only one I could find in 2 different shops among dozens of Logitech > devices, tested it, and it worked correctly. Again, I understand the frustration. But the problem is not so much that we might or might not ever need another entry in that list. The problem is that some devices were supported previously (not in a fancy way), and now we have a chance to just disable those devices. Of course, we could say "just rmmod hid-logitech-hidpp". I have already been through that as well, and then you fight for 10 years on some forums where everybody says that if you have an issue with your touchscreen, just disable <insert any driver here> when the particular touchscreen is *not* using that driver at all. Anyway, let me write down my thoughts since yesterday: 1. Rafael already realized that the ->match() function was not working outside any other driver than hid-generic (and this was the design at the time) 2. We have an issue in hid-logitech-hidpp where during probe calling hidpp_root_get_protocol_version() returns an error, when userspace tools are working fine for the exact same command 3. IMO, the way hid-logitech-hidpp probe function is behaving is not resilient enough to be able to have a generic catch-all, because there is a non-zero chance the probe returns -ENODEV (see all the exit paths that return -ENODEV in probe). To solve 1, it needs a little bit of tinkering and Rafael already sent a v1 for that. IMO we should refine it, but that's an already ongoing process To solve 2, Bastien already mentioned one piece of the puzzle (the error code not being correctly reported and the signification changed between HID++ 1.0 and 2.0). But I am still yet to understand why there is a difference between userspace call of the function, and kernel space. To solve 3, I initially started to work on a simple, more resilient probe in hid-logitech-hidpp. I thought that we could regroup all device initialization we do in a hidpp_preinit() call, and if that call fails, revert to the generic hid processing. But then, looking at the bigger picture, it would make sense to not do that exactly. Instead of returning 0 and handling the device through hid-logitech-hidpp, maybe we should actually return -ENODEV, and have a fallback mechanism in hid-core that says "it seems I have tried all possible drivers, all of them failed, let's force hid-generic for this one". And as I type those lines, how about the cases when we actually want to disable a USB interface from HID because it is legitimate to do so? I'll need to think about this a little bit more. To be able to reintroduce the bluetooth catch-all, I think we need to solve 1 and 3. 2 would be nice to understand but is not preventing the series from being merged back. Cheers, Benjamin
