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 --- drivers/hid/hid-core.c | 7 +++- drivers/hid/hid-input.c | 81 +++++++++++++++++++++++++---------------- include/linux/hid.h | 2 +- 3 files changed, 56 insertions(+), 34 deletions(-)
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c index 76464aabd20c..92387fc0bf18 100644 --- a/drivers/hid/hid-core.c +++ b/drivers/hid/hid-core.c @@ -1637,7 +1637,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; @@ -1646,7 +1646,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);
@@ -1663,8 +1663,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 1fce0076e7dc..fadf76f0a386 100644 --- 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 ac0c70b4ce10..5a24ebfb5b47 100644 --- a/include/linux/hid.h +++ b/include/linux/hid.h @@ -894,7 +894,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,
The value field is actually an array of .maxfield. We should assign the correct number to the correct usage.
Not that we never encounter a device that requires this ATM, but better have the proper code path.
Fixes: 2dc702c991e377 ("HID: input: use the Resolution Multiplier for high-resolution scrolling") Cc: stable@vger.kernel.org # v5.0+ Signed-off-by: Benjamin Tissoires benjamin.tissoires@redhat.com --- drivers/hid/hid-input.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c index fadf76f0a386..6dd0294e1133 100644 --- a/drivers/hid/hid-input.c +++ b/drivers/hid/hid-input.c @@ -1580,7 +1580,7 @@ static bool __hidinput_change_resolution_multipliers(struct hid_device *hid, if (usage->hid != HID_GD_RESOLUTION_MULTIPLIER) continue;
- *report->field[i]->value = value; + report->field[i]->value[j] = value; update_needed = true; } }
On Tue, Apr 23, 2019 at 5:46 PM Benjamin Tissoires benjamin.tissoires@redhat.com wrote:
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
Given that this basically breaks a basic functionality of many Microsoft mice, I went ahead and applied this series to for-5.1/upstream-fixes
Cheers, Benjamin
drivers/hid/hid-core.c | 7 +++- drivers/hid/hid-input.c | 81 +++++++++++++++++++++++++---------------- include/linux/hid.h | 2 +- 3 files changed, 56 insertions(+), 34 deletions(-)
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c index 76464aabd20c..92387fc0bf18 100644 --- a/drivers/hid/hid-core.c +++ b/drivers/hid/hid-core.c @@ -1637,7 +1637,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; @@ -1646,7 +1646,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);@@ -1663,8 +1663,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 1fce0076e7dc..fadf76f0a386 100644 --- 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 ac0c70b4ce10..5a24ebfb5b47 100644 --- a/include/linux/hid.h +++ b/include/linux/hid.h @@ -894,7 +894,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, -- 2.19.2
Hey Benjamin
On 4/24/19 7:30 AM, Benjamin Tissoires wrote:
Given that this basically breaks a basic functionality of many Microsoft mice, I went ahead and applied this series to for-5.1/upstream-fixes
Is there some reason that both patches should not be applied immediately, to the 5.0 series?
Or - likely I am uninformed - are the 5.1 patches applied as a set separate from the 5.0 series revisions?
Thanks James
On Wed, Apr 24, 2019 at 5:42 PM James Feeney james@nurealm.net wrote:
Hey Benjamin
On 4/24/19 7:30 AM, Benjamin Tissoires wrote:
Given that this basically breaks a basic functionality of many Microsoft mice, I went ahead and applied this series to for-5.1/upstream-fixes
Is there some reason that both patches should not be applied immediately, to the 5.0 series?
Or - likely I am uninformed - are the 5.1 patches applied as a set separate from the 5.0 series revisions?
For a patch to be picked up by stable, it first needs to go in Linus' tree. Currently we are working on 5.1, so any stable patches need to go in 5.1 first. Then, once they hit Linus' tree, the stable team will pick them and backport them in the appropriate stable tree.
But distributions can backport them as they wish, so you can make a request to your distribution to include them ASAP. They are officially upstream, though yet to be sent to Linus.
Cheers, Benjamin
Hey Everyone
On 4/24/19 10:41 AM, Benjamin Tissoires wrote:
For a patch to be picked up by stable, it first needs to go in Linus' tree. Currently we are working on 5.1, so any stable patches need to go in 5.1 first. Then, once they hit Linus' tree, the stable team will pick them and backport them in the appropriate stable tree.
Hmm - so, I just booted linux 5.1.9, and this patch set is *still* missing from the kernel.
Is there anything that we can do about this?
James
On Fri, Jun 14, 2019 at 04:09:35PM -0600, James Feeney wrote:
Hey Everyone
On 4/24/19 10:41 AM, Benjamin Tissoires wrote:
For a patch to be picked up by stable, it first needs to go in Linus' tree. Currently we are working on 5.1, so any stable patches need to go in 5.1 first. Then, once they hit Linus' tree, the stable team will pick them and backport them in the appropriate stable tree.
Hmm - so, I just booted linux 5.1.9, and this patch set is *still* missing from the kernel.
Is there anything that we can do about this?
What is the git commit id of the patch in Linus's tree?
As I said before, it can not be backported until it shows up there first.
thanks,
greg k-h
Den 15-06-2019 kl. 08:50, skrev Greg KH:
On Fri, Jun 14, 2019 at 04:09:35PM -0600, James Feeney wrote:
Hey Everyone
On 4/24/19 10:41 AM, Benjamin Tissoires wrote:
For a patch to be picked up by stable, it first needs to go in Linus' tree. Currently we are working on 5.1, so any stable patches need to go in 5.1 first. Then, once they hit Linus' tree, the stable team will pick them and backport them in the appropriate stable tree.
Hmm - so, I just booted linux 5.1.9, and this patch set is *still* missing from the kernel.
Is there anything that we can do about this?
What is the git commit id of the patch in Linus's tree?
As I said before, it can not be backported until it shows up there first.
That would be: d43c17ead879ba7c076dc2f5fd80cd76047c9ff4
and
39b3c3a5fbc5d744114e497d35bf0c12f798c134
-- Thomas
On Sat, Jun 15, 2019 at 12:03:04PM +0300, Thomas Backlund wrote:
Den 15-06-2019 kl. 08:50, skrev Greg KH:
On Fri, Jun 14, 2019 at 04:09:35PM -0600, James Feeney wrote:
Hey Everyone
On 4/24/19 10:41 AM, Benjamin Tissoires wrote:
For a patch to be picked up by stable, it first needs to go in Linus' tree. Currently we are working on 5.1, so any stable patches need to go in 5.1 first. Then, once they hit Linus' tree, the stable team will pick them and backport them in the appropriate stable tree.
Hmm - so, I just booted linux 5.1.9, and this patch set is *still* missing from the kernel.
Is there anything that we can do about this?
What is the git commit id of the patch in Linus's tree?
As I said before, it can not be backported until it shows up there first.
That would be: d43c17ead879ba7c076dc2f5fd80cd76047c9ff4
and
39b3c3a5fbc5d744114e497d35bf0c12f798c134
Thanks, now queued up.
greg k-h
linux-stable-mirror@lists.linaro.org
-
Benjamin Tissoires -
Greg KH -
James Feeney -
Thomas Backlund