HID: input: make sure the wheel high resolution multiplier is set

Some old mice have a tendency to not accept the high resolution multiplier.
They reply with a -EPIPE which was previously ignored.

Force the call to resolution multiplier to be synchronous and actually
check for the answer. If this fails, consider the mouse like a normal one.

Fixes: 2dc702c991e377 ("HID: input: use the Resolution Multiplier for
       high-resolution scrolling")
Link: https://bugzilla.redhat.com/show_bug.cgi?id=1700071
Reported-and-tested-by: James Feeney <james@nurealm.net>
Cc: stable@vger.kernel.org  # v5.0+
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 860e21ec6a492a35392f2b47146f5e0a811c1068..bff8186e1ea5aec651e8d936f530a10a3e0a5f1a 100644 (file)
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1624,7 +1624,7 @@ static struct hid_report *hid_get_report(struct hid_report_enum *report_enum,
  * Implement a generic .request() callback, using .raw_request()
  * DO NOT USE in hid drivers directly, but through hid_hw_request instead.
  */
-void __hid_request(struct hid_device *hid, struct hid_report *report,
+int __hid_request(struct hid_device *hid, struct hid_report *report,
                int reqtype)
 {
        char *buf;
@@ -1633,7 +1633,7 @@ void __hid_request(struct hid_device *hid, struct hid_report *report,
 
        buf = hid_alloc_report_buf(report, GFP_KERNEL);
        if (!buf)
-               return;
+               return -ENOMEM;
 
        len = hid_report_len(report);
 
@@ -1650,8 +1650,11 @@ void __hid_request(struct hid_device *hid, struct hid_report *report,
        if (reqtype == HID_REQ_GET_REPORT)
                hid_input_report(hid, report->type, buf, ret, 0);
 
+       ret = 0;
+
 out:
        kfree(buf);
+       return ret;
 }
 EXPORT_SYMBOL_GPL(__hid_request);
 

diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index 1fce0076e7dc470e94cf561bf8c55b78cd92c6f2..fadf76f0a386d2e99325fb8ccd7bf17c3ea28740 100644 (file)
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -1542,52 +1542,71 @@ static void hidinput_close(struct input_dev *dev)
        hid_hw_close(hid);
 }
 
-static void hidinput_change_resolution_multipliers(struct hid_device *hid)
+static bool __hidinput_change_resolution_multipliers(struct hid_device *hid,
+               struct hid_report *report, bool use_logical_max)
 {
-       struct hid_report_enum *rep_enum;
-       struct hid_report *rep;
        struct hid_usage *usage;
+       bool update_needed = false;
        int i, j;
 
-       rep_enum = &hid->report_enum[HID_FEATURE_REPORT];
-       list_for_each_entry(rep, &rep_enum->report_list, list) {
-               bool update_needed = false;
+       if (report->maxfield == 0)
+               return false;
 
-               if (rep->maxfield == 0)
-                       continue;
+       /*
+        * If we have more than one feature within this report we
+        * need to fill in the bits from the others before we can
+        * overwrite the ones for the Resolution Multiplier.
+        */
+       if (report->maxfield > 1) {
+               hid_hw_request(hid, report, HID_REQ_GET_REPORT);
+               hid_hw_wait(hid);
+       }
 
-               /*
-                * If we have more than one feature within this report we
-                * need to fill in the bits from the others before we can
-                * overwrite the ones for the Resolution Multiplier.
+       for (i = 0; i < report->maxfield; i++) {
+               __s32 value = use_logical_max ?
+                             report->field[i]->logical_maximum :
+                             report->field[i]->logical_minimum;
+
+               /* There is no good reason for a Resolution
+                * Multiplier to have a count other than 1.
+                * Ignore that case.
                 */
-               if (rep->maxfield > 1) {
-                       hid_hw_request(hid, rep, HID_REQ_GET_REPORT);
-                       hid_hw_wait(hid);
-               }
+               if (report->field[i]->report_count != 1)
+                       continue;
 
-               for (i = 0; i < rep->maxfield; i++) {
-                       __s32 logical_max = rep->field[i]->logical_maximum;
+               for (j = 0; j < report->field[i]->maxusage; j++) {
+                       usage = &report->field[i]->usage[j];
 
-                       /* There is no good reason for a Resolution
-                        * Multiplier to have a count other than 1.
-                        * Ignore that case.
-                        */
-                       if (rep->field[i]->report_count != 1)
+                       if (usage->hid != HID_GD_RESOLUTION_MULTIPLIER)
                                continue;
 
-                       for (j = 0; j < rep->field[i]->maxusage; j++) {
-                               usage = &rep->field[i]->usage[j];
+                       *report->field[i]->value = value;
+                       update_needed = true;
+               }
+       }
+
+       return update_needed;
+}
 
-                               if (usage->hid != HID_GD_RESOLUTION_MULTIPLIER)
-                                       continue;
+static void hidinput_change_resolution_multipliers(struct hid_device *hid)
+{
+       struct hid_report_enum *rep_enum;
+       struct hid_report *rep;
+       int ret;
 
-                               *rep->field[i]->value = logical_max;
-                               update_needed = true;
+       rep_enum = &hid->report_enum[HID_FEATURE_REPORT];
+       list_for_each_entry(rep, &rep_enum->report_list, list) {
+               bool update_needed = __hidinput_change_resolution_multipliers(hid,
+                                                                    rep, true);
+
+               if (update_needed) {
+                       ret = __hid_request(hid, rep, HID_REQ_SET_REPORT);
+                       if (ret) {
+                               __hidinput_change_resolution_multipliers(hid,
+                                                                   rep, false);
+                               return;
                        }
                }
-               if (update_needed)
-                       hid_hw_request(hid, rep, HID_REQ_SET_REPORT);
        }
 
        /* refresh our structs */

diff --git a/include/linux/hid.h b/include/linux/hid.h
index f9707d1dcb584a8bf9cac4d614fef4659bb00b12..2c69340d372dcd195d19cca4e968ce77c736acb1 100644 (file)
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -893,7 +893,7 @@ struct hid_field *hidinput_get_led_field(struct hid_device *hid);
 unsigned int hidinput_count_leds(struct hid_device *hid);
 __s32 hidinput_calc_abs_res(const struct hid_field *field, __u16 code);
 void hid_output_report(struct hid_report *report, __u8 *data);
-void __hid_request(struct hid_device *hid, struct hid_report *rep, int reqtype);
+int __hid_request(struct hid_device *hid, struct hid_report *rep, int reqtype);
 u8 *hid_alloc_report_buf(struct hid_report *report, gfp_t flags);
 struct hid_device *hid_allocate_device(void);
 struct hid_report *hid_register_report(struct hid_device *device,