Skip to content

Commit 55bf703

Browse files
jwrdegoedeBenjamin Tissoires
authored andcommitted
HID: logitech-hidpp: Revert "Don't restart communication if not necessary"
Commit 91cf9a9 ("HID: logitech-hidpp: make .probe usbhid capable") makes hidpp_probe() first call hid_hw_start(hdev, 0) to allow IO without connecting any hid subdrivers (hid-input, hidraw). This is done to allow to retrieve the device's name and serial number and store these in hdev->name and hdev->uniq. Then later on IO was stopped and started again with hid_hw_start(hdev, HID_CONNECT_DEFAULT) connecting hid-input and hidraw after the name and serial number have been setup. Commit 498ba20 ("HID: logitech-hidpp: Don't restart communication if not necessary") changed the probe() code to only do the start with a 0 connect-mask + restart later for unifying devices. But for non unifying devices hdev->name and hdev->uniq are updated too. So this change re-introduces the problem for which the start with a 0 connect-mask + restart later behavior was introduced. The previous patch in this series changes the unifying path to instead of restarting IO only call hid_connect() later. This avoids possible issues with restarting IO seen on non unifying devices. Revert the change to limit the restart behavior to unifying devices to fix hdev->name changing after userspace facing devices have already been registered. 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") 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 11ca032 commit 55bf703

File tree

1 file changed

+8
-16
lines changed

1 file changed

+8
-16
lines changed

drivers/hid/hid-logitech-hidpp.c

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -4394,7 +4394,6 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
43944394
bool connected;
43954395
unsigned int connect_mask = HID_CONNECT_DEFAULT;
43964396
struct hidpp_ff_private_data data;
4397-
bool will_restart = false;
43984397

43994398
/* report_fixup needs drvdata to be set before we call hid_parse */
44004399
hidpp = devm_kzalloc(&hdev->dev, sizeof(*hidpp), GFP_KERNEL);
@@ -4445,10 +4444,6 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
44454444
return ret;
44464445
}
44474446

4448-
if (hidpp->quirks & HIDPP_QUIRK_DELAYED_INIT ||
4449-
hidpp->quirks & HIDPP_QUIRK_UNIFYING)
4450-
will_restart = true;
4451-
44524447
INIT_WORK(&hidpp->work, delayed_work_cb);
44534448
mutex_init(&hidpp->send_mutex);
44544449
init_waitqueue_head(&hidpp->wait);
@@ -4465,7 +4460,7 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
44654460
* name and serial number and store these in hdev->name and hdev->uniq,
44664461
* before the hid-input and hidraw drivers expose these to userspace.
44674462
*/
4468-
ret = hid_hw_start(hdev, will_restart ? 0 : connect_mask);
4463+
ret = hid_hw_start(hdev, 0);
44694464
if (ret) {
44704465
hid_err(hdev, "hw start failed\n");
44714466
goto hid_hw_start_fail;
@@ -4504,7 +4499,6 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
45044499
hidpp->wireless_feature_index = 0;
45054500
else if (ret)
45064501
goto hid_hw_init_fail;
4507-
ret = 0;
45084502
}
45094503

45104504
if (connected && (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP)) {
@@ -4520,16 +4514,14 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
45204514
schedule_work(&hidpp->work);
45214515
flush_work(&hidpp->work);
45224516

4523-
if (will_restart) {
4524-
if (hidpp->quirks & HIDPP_QUIRK_DELAYED_INIT)
4525-
connect_mask &= ~HID_CONNECT_HIDINPUT;
4517+
if (hidpp->quirks & HIDPP_QUIRK_DELAYED_INIT)
4518+
connect_mask &= ~HID_CONNECT_HIDINPUT;
45264519

4527-
/* Now export the actual inputs and hidraw nodes to the world */
4528-
ret = hid_connect(hdev, connect_mask);
4529-
if (ret) {
4530-
hid_err(hdev, "%s:hid_connect returned error %d\n", __func__, ret);
4531-
goto hid_hw_init_fail;
4532-
}
4520+
/* Now export the actual inputs and hidraw nodes to the world */
4521+
ret = hid_connect(hdev, connect_mask);
4522+
if (ret) {
4523+
hid_err(hdev, "%s:hid_connect returned error %d\n", __func__, ret);
4524+
goto hid_hw_init_fail;
45334525
}
45344526

45354527
if (hidpp->quirks & HIDPP_QUIRK_CLASS_G920) {

0 commit comments

Comments
 (0)