[PATCH v2 09/14] HID: logitech-hidpp: Move the connected check to after restarting IO

[Hans de Goede <hdegoede@redhat.com>]

While restarting IO incoming packets get disabled by calling
hid_device_io_stop() and they do not get re-enabled again until
the hid-core enables the when probe() is done.

This leaves a window where connect events may get lost, causing
hidpp to not be aware that a device has (dis)connected.

To fix this fully restart IO using the new hidpp_connect_and_start()
helper and move the connected check to after restarting IO.

This requires calling hid_hw_close() at the end of probe() to balance
the open() now done on restart.

Reported-by: Benjamin Tissoires <bentiss@kernel.org>
Closes: https://lore.kernel.org/linux-input/zjiang3fdy4o7r3daupwpnx6zesmeeerldpx5fno2adzialpre@cdp7tq4araww/
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/hid/hid-logitech-hidpp.c | 33 ++++++++++++++++++++------------
 1 file changed, 21 insertions(+), 12 deletions(-)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 45b371e7b9ee..ff834f905eda 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -4482,8 +4482,10 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
 			 hdev->name);
 
 	/*
-	 * Plain USB connections need to actually call start and open
-	 * on the transport driver to allow incoming data.
+	 * First call hid_hw_start(hdev, 0) to allow IO without connecting any
+	 * hid subdrivers (hid-input, hidraw). This allows retrieving the dev's
+	 * name and serial number and store these in hdev->name and hdev->uniq,
+	 * before the hid-input and hidraw drivers expose these to userspace.
 	 */
 	ret = hidpp_connect_and_start(hidpp, 0);
 	if (ret)
@@ -4495,18 +4497,12 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	else
 		hidpp_non_unifying_init(hidpp);
 
-	connected = hidpp_root_get_protocol_version(hidpp) == 0;
-	atomic_set(&hidpp->connected, connected);
-
 	if (hidpp->quirks & HIDPP_QUIRK_CLASS_G920) {
 		ret = g920_get_config(hidpp, &data);
 		if (ret)
 			goto hid_hw_init_fail;
 	}
 
-	schedule_work(&hidpp->work);
-	flush_work(&hidpp->work);
-
 	/* Reset the HID node state */
 	hid_device_io_stop(hdev);
 	hid_hw_close(hdev);
@@ -4516,11 +4512,19 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
 		connect_mask &= ~HID_CONNECT_HIDINPUT;
 
 	/* Now export the actual inputs and hidraw nodes to the world */
-	ret = hid_hw_start(hdev, connect_mask);
-	if (ret) {
-		hid_err(hdev, "%s:hid_hw_start returned error\n", __func__);
+	ret = hidpp_connect_and_start(hidpp, connect_mask);
+	if (ret)
 		goto hid_hw_start_fail;
-	}
+
+	/*
+	 * Now that incoming packets are enabled and will not be disabled
+	 * again (which may cause missing packets) check the connected state
+	 * of the device.
+	 */
+	connected = hidpp_root_get_protocol_version(hidpp) == 0;
+	atomic_set(&hidpp->connected, connected);
+	schedule_work(&hidpp->work);
+	flush_work(&hidpp->work);
 
 	if (hidpp->quirks & HIDPP_QUIRK_CLASS_G920) {
 		ret = hidpp_ff_init(hidpp, &data);
@@ -4530,6 +4534,11 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
 				 ret);
 	}
 
+	/*
+	 * This relies on logi_dj_ll_close() being a no-op so that DJ connection
+	 * events will still be received.
+	 */
+	hid_hw_close(hdev);
 	return ret;
 
 hid_hw_init_fail:
-- 
2.41.0