[PATCH v1 0/2] HID: Revert catchall handling of Bluetooth device in hid-logitech-hidpp

[PATCH v1 0/2] HID: Revert catchall handling of Bluetooth device in hid-logitech-hidpp

[Benjamin Tissoires]

We are basically too late in the 6.1 cycle to be able to do anything
else. Let's revert these 2 patches as we are in a situation where we
would break too many users.

We will reintroduce them during the next cycle with proper fixes in the
driver.

Cheers,
Benjamin

Link: https://lore.kernel.org/linux-input/CAJZ5v0g-_o4AqMgNwihCb0jrwrcJZfRrX=jv8aH54WNKO7QB8A@mail.gmail.com/

Benjamin Tissoires (2):
  Revert "HID: logitech-hidpp: Remove special-casing of Bluetooth
    devices"
  Revert "HID: logitech-hidpp: Enable HID++ for all the Logitech
    Bluetooth devices"

 drivers/hid/hid-logitech-hidpp.c | 28 +++++++++-------------------
 1 file changed, 9 insertions(+), 19 deletions(-)

-- 
2.38.1

[PATCH v1 1/2] Revert "HID: logitech-hidpp: Remove special-casing of Bluetooth devices"

[Benjamin Tissoires]

This reverts commit 8544c812e43ab7bdf40458411b83987b8cba924d.

We need to revert commit 532223c8ac57 ("HID: logitech-hidpp: Enable HID++
for all the Logitech Bluetooth devices") because that commit might make
hid-logitech-hidpp bind on mice that are not well enough supported by
hid-logitech-hidpp, and the end result is that the probe of those mice
is now returning -ENODEV, leaving the end user with a dead mouse.

Given that commit 8544c812e43a ("HID: logitech-hidpp: Remove special-casing
of Bluetooth devices") is a direct dependency of 532223c8ac57, revert it
too.

Note that this also adapt according to commit 908d325e1665 ("HID:
logitech-hidpp: Detect hi-res scrolling support") to re-add support of
the devices that were removed from that commit too.

I have locally an MX Master and I tested this device with that revert,
ensuring we still have high-res scrolling.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/hid/hid-logitech-hidpp.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 9afbc68bf063..f11890f65c06 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -4367,6 +4367,15 @@ static const struct hid_device_id hidpp_devices[] = {
 	{ /* MX5500 keyboard over Bluetooth */
 	  HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb30b),
 	  .driver_data = HIDPP_QUIRK_HIDPP_CONSUMER_VENDOR_KEYS },
+	{ /* M-RCQ142 V470 Cordless Laser Mouse over Bluetooth */
+	  HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb008) },
+	{ /* MX Master mouse over Bluetooth */
+	  HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb012) },
+	{ /* MX Ergo trackball over Bluetooth */
+	  HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb01d) },
+	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb01e) },
+	{ /* MX Master 3 mouse over Bluetooth */
+	  HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb023) },
 
 	{ /* And try to enable HID++ for all the Logitech Bluetooth devices */
 	  HID_DEVICE(BUS_BLUETOOTH, HID_GROUP_ANY, USB_VENDOR_ID_LOGITECH, HID_ANY_ID) },
-- 
2.38.1

[PATCH v1 2/2] Revert "HID: logitech-hidpp: Enable HID++ for all the Logitech Bluetooth devices"

[Benjamin Tissoires]

This reverts commit 532223c8ac57605a10e46dc0ab23dcf01c9acb43.

As reported in [0], hid-logitech-hidpp now binds on all bluetooth mice,
but there are corner cases where hid-logitech-hidpp just gives up on
the mouse. This leads the end user with a dead mouse.

Given that we are at -rc8, we are definitively too late to find a proper
fix. We already identified 2 issues less than 24 hours after the bug
report. One in that ->match() was never designed to be used anywhere else
than in hid-generic, and the other that hid-logitech-hidpp has corner
cases where it gives up on devices it is not supposed to.

So we have no choice but postpone this patch to the next kernel release.

[0] https://lore.kernel.org/linux-input/CAJZ5v0g-_o4AqMgNwihCb0jrwrcJZfRrX=jv8aH54WNKO7QB8A@mail.gmail.com/

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/hid/hid-logitech-hidpp.c | 19 -------------------
 1 file changed, 19 deletions(-)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index f11890f65c06..c6e4a96e882e 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -4269,21 +4269,6 @@ static void hidpp_remove(struct hid_device *hdev)
 	mutex_destroy(&hidpp->send_mutex);
 }
 
-static const struct hid_device_id unhandled_hidpp_devices[] = {
-	/* Logitech Harmony Adapter for PS3, handled in hid-sony */
-	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_HARMONY_PS3) },
-	/* Handled in hid-generic */
-	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_DINOVO_EDGE_KBD) },
-	{}
-};
-
-static bool hidpp_match(struct hid_device *hdev,
-			bool ignore_special_driver)
-{
-	/* Refuse to handle devices handled by other HID drivers */
-	return !hid_match_id(hdev, unhandled_hidpp_devices);
-}
-
 #define LDJ_DEVICE(product) \
 	HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE, \
 		   USB_VENDOR_ID_LOGITECH, (product))
@@ -4376,9 +4361,6 @@ static const struct hid_device_id hidpp_devices[] = {
 	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb01e) },
 	{ /* MX Master 3 mouse over Bluetooth */
 	  HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb023) },
-
-	{ /* And try to enable HID++ for all the Logitech Bluetooth devices */
-	  HID_DEVICE(BUS_BLUETOOTH, HID_GROUP_ANY, USB_VENDOR_ID_LOGITECH, HID_ANY_ID) },
 	{}
 };
 
@@ -4392,7 +4374,6 @@ static const struct hid_usage_id hidpp_usages[] = {
 static struct hid_driver hidpp_driver = {
 	.name = "logitech-hidpp-device",
 	.id_table = hidpp_devices,
-	.match = hidpp_match,
 	.report_fixup = hidpp_report_fixup,
 	.probe = hidpp_probe,
 	.remove = hidpp_remove,
-- 
2.38.1

Re: [PATCH v1 0/2] HID: Revert catchall handling of Bluetooth device in hid-logitech-hidpp

[Jiri Kosina]

On Wed, 7 Dec 2022, Benjamin Tissoires wrote:

> We are basically too late in the 6.1 cycle to be able to do anything
> else. Let's revert these 2 patches as we are in a situation where we
> would break too many users.
> 
> We will reintroduce them during the next cycle with proper fixes in the
> driver.

Rafael,

it would be nice to get

	Reported-by:
	Tested-by:

for these reverts if possible.

Thanks!

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH v1 0/2] HID: Revert catchall handling of Bluetooth device in hid-logitech-hidpp

[Thorsten Leemhuis]

On 07.12.22 17:57, Jiri Kosina wrote:
> On Wed, 7 Dec 2022, Benjamin Tissoires wrote:
> 
>> We are basically too late in the 6.1 cycle to be able to do anything
>> else. Let's revert these 2 patches as we are in a situation where we
>> would break too many users.
>>
>> We will reintroduce them during the next cycle with proper fixes in the
>> driver.
> 
> it would be nice to get
> 
> 	Reported-by:

Nitpicking: and as for nearly every Reported-by it would be nice to have
an accompanying Link: tag pointing to the report, which makes my
regression tracking life a whole lot easier *and* is nice for future
code archaeologist, as it makes it easier to find Rafael's great
analysis of the cause -- which is why Linus want to see these tags, too
[1] (and why our docs tell people to add them[2]).

Ciao, Thorsten

[1] for details, see:
https://lore.kernel.org/all/CAHk-=wjMmSZzMJ3Xnskdg4+GGz=5p5p+GSYyFBTh0f-DgvdBWg@mail.gmail.com/
https://lore.kernel.org/all/CAHk-=wgs38ZrfPvy=nOwVkVzjpM3VFU1zobP37Fwd_h9iAD5JQ@mail.gmail.com/
https://lore.kernel.org/all/CAHk-=wjxzafG-=J8oT30s7upn4RhBs6TX-uVFZ5rME+L5_DoJA@mail.gmail.com/

[2] see Documentation/process/submitting-patches.rst
(http://docs.kernel.org/process/submitting-patches.html) and
Documentation/process/5.Posting.rst
(https://docs.kernel.org/process/5.Posting.html)

Re: [PATCH v1 0/2] HID: Revert catchall handling of Bluetooth device in hid-logitech-hidpp

[Rafael J. Wysocki]

On Wednesday, December 7, 2022 5:57:47 PM CET Jiri Kosina wrote:
> On Wed, 7 Dec 2022, Benjamin Tissoires wrote:
> 
> > We are basically too late in the 6.1 cycle to be able to do anything
> > else. Let's revert these 2 patches as we are in a situation where we
> > would break too many users.
> > 
> > We will reintroduce them during the next cycle with proper fixes in the
> > driver.
> 
> Rafael,
> 
> it would be nice to get
> 
> 	Reported-by:
> 	Tested-by:
> 
> for these reverts if possible.

I had been offline for the previous 3 days, sorry.

I'll test 6.1 and let you know.

Re: [PATCH v1 0/2] HID: Revert catchall handling of Bluetooth device in hid-logitech-hidpp

[Rafael J. Wysocki]

On Monday, December 12, 2022 6:03:07 PM CET Rafael J. Wysocki wrote:
> On Wednesday, December 7, 2022 5:57:47 PM CET Jiri Kosina wrote:
> > On Wed, 7 Dec 2022, Benjamin Tissoires wrote:
> > 
> > > We are basically too late in the 6.1 cycle to be able to do anything
> > > else. Let's revert these 2 patches as we are in a situation where we
> > > would break too many users.
> > > 
> > > We will reintroduce them during the next cycle with proper fixes in the
> > > driver.
> > 
> > Rafael,
> > 
> > it would be nice to get
> > 
> > 	Reported-by:
> > 	Tested-by:
> > 
> > for these reverts if possible.
> 
> I had been offline for the previous 3 days, sorry.
> 
> I'll test 6.1 and let you know.

Sorry for the delay.

I have tested final 6.1 on the affected machine and I can confirm that it works well.

Thanks!

Re: [PATCH v1 0/2] HID: Revert catchall handling of Bluetooth device in hid-logitech-hidpp

[Jiri Kosina]

On Wed, 7 Dec 2022, Benjamin Tissoires wrote:

> We are basically too late in the 6.1 cycle to be able to do anything
> else. Let's revert these 2 patches as we are in a situation where we
> would break too many users.
> 
> We will reintroduce them during the next cycle with proper fixes in the
> driver.

I have now queued this in for-6.1/upstream-fixes and will urgently send it 
to Linus so that we don't have the regression in 6.1 final.

-- 
Jiri Kosina
SUSE Labs