@@ -194,7 +194,6 @@ struct hidpp_device {
struct work_struct work;
struct kfifo delayed_work_fifo;
- atomic_t connected;
struct input_dev *delayed_input;
unsigned long quirks;
@@ -3893,8 +3892,6 @@ static int hidpp_raw_hidpp_event(struct hidpp_device *hidpp, u8 *data,
}
if (unlikely(hidpp_report_is_connect_event(hidpp, report))) {
- atomic_set(&hidpp->connected,
- !(report->rap.params[0] & (1 << 6)));
if (schedule_work(&hidpp->work) == 0)
dbg_hid("%s: connect event already queued\n", __func__);
return 1;
@@ -4189,12 +4186,14 @@ static struct input_dev *hidpp_allocate_input(struct hid_device *hdev)
static void hidpp_connect_event(struct hidpp_device *hidpp)
{
struct hid_device *hdev = hidpp->hid_dev;
- int ret = 0;
- bool connected = atomic_read(&hidpp->connected);
struct input_dev *input;
char *name, *devm_name;
+ int ret;
- if (!connected) {
+ /* Get device version to check if it is connected */
+ ret = hidpp_root_get_protocol_version(hidpp);
+ if (ret) {
+ hid_info(hidpp->hid_dev, "Disconnected\n");
if (hidpp->battery.ps) {
hidpp->battery.online = false;
hidpp->battery.status = POWER_SUPPLY_STATUS_UNKNOWN;
@@ -4236,16 +4235,6 @@ static void hidpp_connect_event(struct hidpp_device *hidpp)
return;
}
- /* the device is already connected, we can ask for its name and
- * protocol */
- if (!hidpp->protocol_major) {
- ret = hidpp_root_get_protocol_version(hidpp);
- if (ret) {
- hid_err(hdev, "Can not get the protocol version.\n");
- return;
- }
- }
-
if (hidpp->protocol_major >= 2) {
u8 feature_index;
@@ -4395,7 +4384,6 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
{
struct hidpp_device *hidpp;
int ret;
- bool connected;
unsigned int connect_mask = HID_CONNECT_DEFAULT;
/* report_fixup needs drvdata to be set before we call hid_parse */
@@ -4485,9 +4473,6 @@ 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);
-
schedule_work(&hidpp->work);
flush_work(&hidpp->work);
There is a connect event race in hidpp_probe() in these 2 lines: connected = hidpp_root_get_protocol_version(hidpp) == 0; atomic_set(&hidpp->connected, connected); Specifically the following can happen: 1. This line from hidpp_probe() is executed: connected = hidpp_root_get_protocol_version(hidpp) == 0; and sets connected to false; 2. A connect-event packet is received and does: atomic_set(&hidpp->connected, true); 3. The next line from hidpp_probe() is executed: atomic_set(&hidpp->connected, connected); and sets the atomic_t back to 0 again. 4. hidpp_connect_event() runs and sees the connected device as disconnected because of this. To fix this make hidpp_connect_event() query the connection status of the device itself instead of having it rely on possibly stale data cached in struct hidpp_device. 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) Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- drivers/hid/hid-logitech-hidpp.c | 25 +++++-------------------- 1 file changed, 5 insertions(+), 20 deletions(-)