hidpp_probe() restarts IO after setting things up, if we get a connect
event just before hidpp_probe() stops all IO then hidpp_connect_event()
will see IO errors causing it to fail to setup the connected device.
Add a new io_mutex which hidpp_probe() locks while restarting IO and
which is also taken by hidpp_connect_event() to avoid these 2 things
from racing.
Hopefully this will help with the occasional connect errors leading to
e.g. missing battery monitoring.
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
---
drivers/hid/hid-logitech-hidpp.c | 35 ++++++++++++++++++++++----------
1 file changed, 24 insertions(+), 11 deletions(-)
diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index a209d51bd247..33f9cd98485a 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -181,6 +181,7 @@ struct hidpp_scroll_counter {
struct hidpp_device {
struct hid_device *hid_dev;
struct input_dev *input;
+ struct mutex io_mutex;
struct mutex send_mutex;
void *send_receive_buf;
char *name; /* will never be NULL and should not be freed */
@@ -4207,36 +4208,39 @@ static void hidpp_connect_event(struct hidpp_device *hidpp)
return;
}
+ /* Avoid probe() restarting IO */
+ mutex_lock(&hidpp->io_mutex);
+
if (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP) {
ret = wtp_connect(hdev, connected);
if (ret)
- return;
+ goto out_unlock;
} else if (hidpp->quirks & HIDPP_QUIRK_CLASS_M560) {
ret = m560_send_config_command(hdev, connected);
if (ret)
- return;
+ goto out_unlock;
} else if (hidpp->quirks & HIDPP_QUIRK_CLASS_K400) {
ret = k400_connect(hdev, connected);
if (ret)
- return;
+ goto out_unlock;
}
if (hidpp->quirks & HIDPP_QUIRK_HIDPP_WHEELS) {
ret = hidpp10_wheel_connect(hidpp);
if (ret)
- return;
+ goto out_unlock;
}
if (hidpp->quirks & HIDPP_QUIRK_HIDPP_EXTRA_MOUSE_BTNS) {
ret = hidpp10_extra_mouse_buttons_connect(hidpp);
if (ret)
- return;
+ goto out_unlock;
}
if (hidpp->quirks & HIDPP_QUIRK_HIDPP_CONSUMER_VENDOR_KEYS) {
ret = hidpp10_consumer_keys_connect(hidpp);
if (ret)
- return;
+ goto out_unlock;
}
/* the device is already connected, we can ask for its name and
@@ -4245,7 +4249,7 @@ static void hidpp_connect_event(struct hidpp_device *hidpp)
ret = hidpp_root_get_protocol_version(hidpp);
if (ret) {
hid_err(hdev, "Can not get the protocol version.\n");
- return;
+ goto out_unlock;
}
}
@@ -4256,7 +4260,7 @@ static void hidpp_connect_event(struct hidpp_device *hidpp)
"%s", name);
kfree(name);
if (!devm_name)
- return;
+ goto out_unlock;
hidpp->name = devm_name;
}
@@ -4291,12 +4295,12 @@ static void hidpp_connect_event(struct hidpp_device *hidpp)
if (!(hidpp->quirks & HIDPP_QUIRK_DELAYED_INIT) || hidpp->delayed_input)
/* if the input nodes are already created, we can stop now */
- return;
+ goto out_unlock;
input = hidpp_allocate_input(hdev);
if (!input) {
hid_err(hdev, "cannot allocate new input device: %d\n", ret);
- return;
+ goto out_unlock;
}
hidpp_populate_input(hidpp, input);
@@ -4304,10 +4308,12 @@ static void hidpp_connect_event(struct hidpp_device *hidpp)
ret = input_register_device(input);
if (ret) {
input_free_device(input);
- return;
+ goto out_unlock;
}
hidpp->delayed_input = input;
+out_unlock:
+ mutex_unlock(&hidpp->io_mutex);
}
static DEVICE_ATTR(builtin_power_supply, 0000, NULL, NULL);
@@ -4450,6 +4456,7 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
will_restart = true;
INIT_WORK(&hidpp->work, delayed_work_cb);
+ mutex_init(&hidpp->io_mutex);
mutex_init(&hidpp->send_mutex);
init_waitqueue_head(&hidpp->wait);
@@ -4519,6 +4526,9 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
flush_work(&hidpp->work);
if (will_restart) {
+ /* Avoid hidpp_connect_event() running while restarting */
+ mutex_lock(&hidpp->io_mutex);
+
/* Reset the HID node state */
hid_device_io_stop(hdev);
hid_hw_close(hdev);
@@ -4529,6 +4539,7 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
/* Now export the actual inputs and hidraw nodes to the world */
ret = hid_hw_start(hdev, connect_mask);
+ mutex_unlock(&hidpp->io_mutex);
if (ret) {
hid_err(hdev, "%s:hid_hw_start returned error\n", __func__);
goto hid_hw_start_fail;
@@ -4553,6 +4564,7 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
sysfs_remove_group(&hdev->dev.kobj, &ps_attribute_group);
cancel_work_sync(&hidpp->work);
mutex_destroy(&hidpp->send_mutex);
+ mutex_destroy(&hidpp->io_mutex);
return ret;
}
@@ -4568,6 +4580,7 @@ static void hidpp_remove(struct hid_device *hdev)
hid_hw_stop(hdev);
cancel_work_sync(&hidpp->work);
mutex_destroy(&hidpp->send_mutex);
+ mutex_destroy(&hidpp->io_mutex);
}
#define LDJ_DEVICE(product) \
--
2.41.0
