Skip to content

Commit 11ca032

Browse files
jwrdegoedeBenjamin Tissoires
authored andcommitted
HID: logitech-hidpp: Don't restart IO, instead defer hid_connect() only
Restarting IO causes 2 problems: 1. Some devices do not like IO being restarted this was addressed in commit 498ba20 ("HID: logitech-hidpp: Don't restart communication if not necessary"), but that change has issues of its own and needs to be reverted. 2. Restarting IO and specifically calling hid_device_io_stop() causes received packets to be missed, which may cause connect-events to get missed. Restarting IO was introduced in commit 91cf9a9 ("HID: logitech-hidpp: make .probe usbhid capable") to allow to retrieve the device's name and serial number and store these in hdev->name and hdev->uniq before connecting any hid subdrivers (hid-input, hidraw) exporting this info to userspace. But this does not require restarting IO, this merely requires deferring calling hid_connect(). Calling hid_hw_start() with a connect-mask of 0 makes it skip calling hid_connect(), so hidpp_probe() can simply call hid_connect() later without needing to restart IO. Remove the stop + restart of IO and instead just call hid_connect() later to avoid the issues caused by restarting IO. Now that IO is no longer stopped, hid_hw_close() must be called at the end of probe() to balance the hid_hw_open() done at the beginning probe(). This series has been tested on the following devices: Logitech Bluetooth Laser Travel Mouse (bluetooth, HID++ 1.0) Logitech M720 Triathlon (bluetooth, HID++ 4.5) Logitech M720 Triathlon (unifying, HID++ 4.5) Logitech K400 Pro (unifying, HID++ 4.1) Logitech K270 (eQUAD nano Lite, HID++ 2.0) Logitech M185 (eQUAD nano Lite, HID++ 4.5) Logitech LX501 keyboard (27 Mhz, HID++ builtin scroll-wheel, HID++ 1.0) Logitech M-RAZ105 mouse (27 Mhz, HID++ extra mouse buttons, HID++ 1.0) And by bentiss: Logitech Touchpad T650 (unifying) Logitech Touchpad T651 (bluetooth) Logitech MX Master 3B (BLE) Logitech G403 (plain USB / Gaming receiver) Fixes: 498ba20 ("HID: logitech-hidpp: Don't restart communication if not necessary") Suggested-by: Benjamin Tissoires <[email protected]> Signed-off-by: Hans de Goede <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Benjamin Tissoires <[email protected]>
1 parent bab19d1 commit 11ca032

File tree

1 file changed

+12
-10
lines changed

1 file changed

+12
-10
lines changed

drivers/hid/hid-logitech-hidpp.c

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4460,8 +4460,10 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
44604460
hdev->name);
44614461

44624462
/*
4463-
* Plain USB connections need to actually call start and open
4464-
* on the transport driver to allow incoming data.
4463+
* First call hid_hw_start(hdev, 0) to allow IO without connecting any
4464+
* hid subdrivers (hid-input, hidraw). This allows retrieving the dev's
4465+
* name and serial number and store these in hdev->name and hdev->uniq,
4466+
* before the hid-input and hidraw drivers expose these to userspace.
44654467
*/
44664468
ret = hid_hw_start(hdev, will_restart ? 0 : connect_mask);
44674469
if (ret) {
@@ -4519,19 +4521,14 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
45194521
flush_work(&hidpp->work);
45204522

45214523
if (will_restart) {
4522-
/* Reset the HID node state */
4523-
hid_device_io_stop(hdev);
4524-
hid_hw_close(hdev);
4525-
hid_hw_stop(hdev);
4526-
45274524
if (hidpp->quirks & HIDPP_QUIRK_DELAYED_INIT)
45284525
connect_mask &= ~HID_CONNECT_HIDINPUT;
45294526

45304527
/* Now export the actual inputs and hidraw nodes to the world */
4531-
ret = hid_hw_start(hdev, connect_mask);
4528+
ret = hid_connect(hdev, connect_mask);
45324529
if (ret) {
4533-
hid_err(hdev, "%s:hid_hw_start returned error\n", __func__);
4534-
goto hid_hw_start_fail;
4530+
hid_err(hdev, "%s:hid_connect returned error %d\n", __func__, ret);
4531+
goto hid_hw_init_fail;
45354532
}
45364533
}
45374534

@@ -4543,6 +4540,11 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
45434540
ret);
45444541
}
45454542

4543+
/*
4544+
* This relies on logi_dj_ll_close() being a no-op so that DJ connection
4545+
* events will still be received.
4546+
*/
4547+
hid_hw_close(hdev);
45464548
return ret;
45474549

45484550
hid_hw_init_fail:

0 commit comments

Comments
 (0)