2019-06-17 21:21:40

by Greg Kroah-Hartman

[permalink] [raw]
Subject: [PATCH 5.1 003/115] HID: input: make sure the wheel high resolution multiplier is set

From: Benjamin Tissoires <[email protected]>

commit d43c17ead879ba7c076dc2f5fd80cd76047c9ff4 upstream.

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 <[email protected]>
Cc: [email protected] # v5.0+
Signed-off-by: Benjamin Tissoires <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>

---
drivers/hid/hid-core.c | 7 ++--
drivers/hid/hid-input.c | 81 +++++++++++++++++++++++++++++-------------------
include/linux/hid.h | 2 -
3 files changed, 56 insertions(+), 34 deletions(-)

--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1636,7 +1636,7 @@ static struct hid_report *hid_get_report
* 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;
@@ -1645,7 +1645,7 @@ void __hid_request(struct hid_device *hi

buf = hid_alloc_report_buf(report, GFP_KERNEL);
if (!buf)
- return;
+ return -ENOMEM;

len = hid_report_len(report);

@@ -1662,8 +1662,11 @@ void __hid_request(struct hid_device *hi
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);

--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -1557,52 +1557,71 @@ static void hidinput_close(struct input_
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;
+}
+
+static void hidinput_change_resolution_multipliers(struct hid_device *hid)
+{
+ struct hid_report_enum *rep_enum;
+ struct hid_report *rep;
+ int ret;

- if (usage->hid != HID_GD_RESOLUTION_MULTIPLIER)
- continue;
+ 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);

- *rep->field[i]->value = logical_max;
- update_needed = 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 */
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -894,7 +894,7 @@ struct hid_field *hidinput_get_led_field
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,



2019-06-18 17:23:24

by James Feeney

[permalink] [raw]
Subject: Re: [PATCH 5.1 003/115] HID: input: make sure the wheel high resolution multiplier is set

Uhm - could someone please "clue me in" here?

When I look into:

'move all the pending queues back to their "real" places'
https://git.kernel.org/pub/scm/linux/kernel/git/stable/stable-queue.git/commit/?id=c5da0df8985ac2f29ffdaba77bae201121bc0e10

I can find both the "d43c17ead879ba7c076dc2f5fd80cd76047c9ff4" patch, "HID: input: make sure the wheel high resolution multiplier is set" and the "39b3c3a5fbc5d744114e497d35bf0c12f798c134" patch, "HID: input: fix assignment of .value".

I take this to mean that these patches are "in the stable-queue". But then, these patches are not "in the kernel".

So then, how do these patches go from being "in the stable-queue" to being "in the kernel"?

To the "uninitiated" and "naive", as I am, to outward appearances, the patches are "just sitting there". How do the patches get selected for inclusion into the "next" kernel revision?

Thanks
James

2019-06-18 17:50:12

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 5.1 003/115] HID: input: make sure the wheel high resolution multiplier is set

On Tue, Jun 18, 2019 at 11:22:55AM -0600, James Feeney wrote:
> Uhm - could someone please "clue me in" here?
>
> When I look into:
>
> 'move all the pending queues back to their "real" places'
> https://git.kernel.org/pub/scm/linux/kernel/git/stable/stable-queue.git/commit/?id=c5da0df8985ac2f29ffdaba77bae201121bc0e10

No need to worry about that patch, that was done because my scripts
normally assume specific directory locations of the patch queues, and I
had to do a kernel release that did not include the existing pending
patches.

> I can find both the "d43c17ead879ba7c076dc2f5fd80cd76047c9ff4" patch, "HID: input: make sure the wheel high resolution multiplier is set" and the "39b3c3a5fbc5d744114e497d35bf0c12f798c134" patch, "HID: input: fix assignment of .value".
>
> I take this to mean that these patches are "in the stable-queue". But then, these patches are not "in the kernel".

Yes.

> So then, how do these patches go from being "in the stable-queue" to being "in the kernel"?

I apply them when I do the release in a few hours/days.

> To the "uninitiated" and "naive", as I am, to outward appearances, the
> patches are "just sitting there". How do the patches get selected for
> inclusion into the "next" kernel revision?

I already selected them, sent emails saying they were selected and to
what specific branches they were selected to. Then when the -rc
releases happen so that people can do one final round of testing and
object if I messed anything up, they get sent out again (which you
responded to here.)

If all goes well, when the "deadling" passes (usually 2 days +-2 days
depending on stuff), I'll do a realease and apply the patches "for real"
to the different kernel branches and cut a release.

Then I start all over again...

I understand that seeing a git tree of patches in a quilt series is odd,
but it is very powerful and works very very well for what we do here.

Does that help?

greg k-h