[PATCH] HID: logitech-hidpp: sync wheel multiplier on wheel mode changes

The hid-logitech-hidpp driver enables high resolution scrolling on
device connect for capable HID++ 2.0 devices. Driver also reads the
wheel capability and caches the returned high resolution wheel scroll
multiplier, that is used for scroll scaling when handling wheel scroll
events.

Wheel mode can also be set externally through HID++ requests, which
can leave the cached multiplier stale and cause incorrect scroll
scaling. If external SetWheelMode HID++ request sets the mode to
low resolution, the cached multiplier is not updated accordingly. This
causes extremely slow scrolling since driver expects multiple wheel
scroll events per detent but is only getting one.

The fix listens for HID++ SetWheelMode request responses and updates
the wheel scroll multiplier based on the set high resolution scroll
mode. The fix has been tested with Logitech G502X lightspeed mouse.

Signed-off-by: Lauri Saurus <redacted>
---
 drivers/hid/hid-logitech-hidpp.c | 37 ++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index ccbf28869a96..e80c4aa6115b 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c

@@ -207,6 +207,9 @@ struct hidpp_device {
 
 	u8 wireless_feature_index;
 
+	int hires_wheel_multiplier;
+	u8 hires_wheel_feature_index;
+
 	bool connected_once;
 };
 

@@ -3710,6 +3713,7 @@ static int hi_res_scroll_enable(struct hidpp_device *hidpp)
 		multiplier = 1;
 	}
 
+	hidpp->hires_wheel_multiplier = multiplier;
 	hidpp->vertical_wheel_counter.wheel_multiplier = multiplier;
 	hid_dbg(hidpp->hid_dev, "wheel multiplier = %d\n", multiplier);
 	return 0;

@@ -3720,6 +3724,7 @@ static int hidpp_initialize_hires_scroll(struct hidpp_device *hidpp)
 	int ret;
 	unsigned long capabilities;
 
+	hidpp->hires_wheel_feature_index = 0xff;
 	capabilities = hidpp->capabilities;
 
 	if (hidpp->protocol_major >= 2) {

@@ -3729,6 +3734,7 @@ static int hidpp_initialize_hires_scroll(struct hidpp_device *hidpp)
 					     &feature_index);
 		if (!ret) {
 			hidpp->capabilities |= HIDPP_CAPABILITY_HIDPP20_HI_RES_WHEEL;
+			hidpp->hires_wheel_feature_index = feature_index;
 			hid_dbg(hidpp->hid_dev, "Detected HID++ 2.0 hi-res scroll wheel\n");
 			return 0;
 		}

@@ -3751,6 +3757,31 @@ static int hidpp_initialize_hires_scroll(struct hidpp_device *hidpp)
 	return 0;
 }
 
+static int hidpp20_hires_wheel_raw_event(struct hidpp_device *hidpp,
+						u8 *data, int size)
+{
+	if (hidpp->hires_wheel_feature_index == 0xff)
+		return 0;
+
+	if (size < 5)
+		return 0;
+
+	if (data[0] != REPORT_ID_HIDPP_LONG ||
+	    data[2] != hidpp->hires_wheel_feature_index)
+		return 0;
+
+	if ((data[3] & 0xf0) == CMD_HIRES_WHEEL_SET_WHEEL_MODE) {
+		u8 mode = data[4];
+		bool hires = (mode & 0x02) != 0;
+		int new_multiplier = (hires && hidpp->hires_wheel_multiplier > 0)
+			? hidpp->hires_wheel_multiplier : 1;
+		hidpp->vertical_wheel_counter.wheel_multiplier = new_multiplier;
+		return 1;
+	}
+
+	return 0;
+}
+
 /* -------------------------------------------------------------------------- */
 /* Generic HID++ devices                                                      */
 /* -------------------------------------------------------------------------- */

@@ -3947,6 +3978,12 @@ static int hidpp_raw_hidpp_event(struct hidpp_device *hidpp, u8 *data,
 			return ret;
 	}
 
+	if (hidpp->capabilities & HIDPP_CAPABILITY_HIDPP20_HI_RES_WHEEL) {
+		ret = hidpp20_hires_wheel_raw_event(hidpp, data, size);
+		if (ret != 0)
+			return ret;
+	}
+
 	return 0;
 }
 

-- 
2.54.0

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The logic incorrectly assumes that the device's response to the `CMD_HIRES_WHEEL_SET_WHEEL_MODE` command will echo the requested mode in its payload (`data[4]`), violating HID++ 2.0 protocol behavior.
- [Medium] The function `hidpp20_hires_wheel_raw_event` strictly checks for `REPORT_ID_HIDPP_LONG` (0x11), completely ignoring responses sent via Very Long (0x12) reports.
- [Low] Data race on `wheel_multiplier` and `hires_wheel_multiplier` due to unprotected concurrent accesses across interrupt and process contexts.
--

commit 83eeb25697adad15bd7ac874fb7119dc29516cf1
Author: Lauri Saurus [off-list ref]

HID: logitech-hidpp: sync wheel multiplier on wheel mode changes
    
The hid-logitech-hidpp driver enables high resolution scrolling on
device connect for capable HID++ 2.0 devices. Driver also reads the
wheel capability and caches the returned high resolution wheel scroll
multiplier, that is used for scroll scaling when handling wheel scroll
events.

[ ... ]

Does this code need to handle VERY_LONG reports as well?

Userspace tools or newer devices might transmit FAP commands using
REPORT_ID_HIDPP_VERY_LONG (0x12). If the request is a VERY_LONG report, the
response will also be VERY_LONG, and this check would drop it, failing to
update the multiplier.

Does the HID++ 2.0 protocol guarantee that SET command responses will echo
the requested mode in the payload?

In the FAP protocol, responses to SET commands often return an empty
(zero-filled) payload. If the device returns a zero payload, data[4] would
be 0x00. The code would then evaluate hires = false and forcefully set the
multiplier to 1, even if high-resolution mode was successfully requested.

Is there a potential data race on wheel_multiplier and
hires_wheel_multiplier here?

Looking at hi_res_scroll_enable(), it writes to these variables from
process context via the workqueue. Concurrently, this event handler reads
and writes to them from the HID raw event delivery path in interrupt context.
There does not appear to be any synchronization protecting these shared
variables across contexts.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260518192649.245691-1-saurla@saurla.com?part=1

quoted

+static int hidpp20_hires_wheel_raw_event(struct hidpp_device *hidpp,
+						u8 *data, int size)
+{
+	if (hidpp->hires_wheel_feature_index == 0xff)
+		return 0;
+
+	if (size < 5)
+		return 0;
+
+	if (data[0] != REPORT_ID_HIDPP_LONG ||
+	    data[2] != hidpp->hires_wheel_feature_index)
+		return 0;

Does this code need to handle VERY_LONG reports as well?

Userspace tools or newer devices might transmit FAP commands using
REPORT_ID_HIDPP_VERY_LONG (0x12). If the request is a VERY_LONG report, the
response will also be VERY_LONG, and this check would drop it, failing to
update the multiplier.

I agree, it could be possible some hosts could send the command as VERY_LONG report for newer devices which support that. I will change this to support VERY_LONG reports also.

quoted

+
+	if ((data[3] & 0xf0) == CMD_HIRES_WHEEL_SET_WHEEL_MODE) {
+		u8 mode = data[4];
+		bool hires = (mode & 0x02) != 0;

Does the HID++ 2.0 protocol guarantee that SET command responses will echo
the requested mode in the payload?

In the FAP protocol, responses to SET commands often return an empty
(zero-filled) payload. If the device returns a zero payload, data[4] would
be 0x00. The code would then evaluate hires = false and forcefully set the
multiplier to 1, even if high-resolution mode was successfully requested.

As per the "Protocol HID++2.0 essential features" section in https://lekensteyn.nl/files/logitech/logitech_hidpp_2.0_specification_draft_2012-06-04.pdf:
"
All functions must respect the following rule:
  All parameters in a request must always be repeated in the response:
    Any parameter that is fully supported must be repeated "as is".
    Any parameter that is only "partially supported", must be returned as supported.

The following examples should help to better understand these requirement:
  A read command where an address (i.e., memory address, register address, etc.) is given as parameter must return the address and the data in the response (and not just the address).
  A write command which sets a collection of bits or bit fields, should return the same value where all unsupported bits and bit fields have been masked to their default values (usually 0).
  A command such as open-lock, erase-memory, etc. should return its parameters unchanged.

Note that there is no requirement to implement partial support. Each feature designer is free to decide if "partially correct" parameters
should return and error or be "partially supported." However, if partial support is implemented, then the parameters must be returned
as supported.
"

Based on the aforementioned specification, all parameters must be echoed in the response. If the SetWheelMode would not support setting hires parameter (partial support), then hires parameter would be returned as default value 0 (low res) in the response. In that case, it is correct to set the multiplier to 1, since setting hires was not supported.

Unless there are verified cases of HID++ capable mouses behaving as described in the review comment, or knowledge of newer version of the specification to have changed aforementioned rule, I would assume all devices to work according to the specification.

quoted

+		int new_multiplier = (hires && hidpp->hires_wheel_multiplier > 0)
+			? hidpp->hires_wheel_multiplier : 1;
+		hidpp->vertical_wheel_counter.wheel_multiplier = new_multiplier;

Is there a potential data race on wheel_multiplier and
hires_wheel_multiplier here?

Looking at hi_res_scroll_enable(), it writes to these variables from
process context via the workqueue. Concurrently, this event handler reads
and writes to them from the HID raw event delivery path in interrupt context.
There does not appear to be any synchronization protecting these shared
variables across contexts.

quoted

+		return 1;
+	}
+
+	return 0;
+}

I agree that in theory there is a possible race condition. However, wheel_multiplier was already written to from hi_res_scroll_enable, and read from the HID event handler before this patch. Not sure if this patch should address this issue or not.

On Mon, 18 May 2026, Lauri Saurus wrote:

The hid-logitech-hidpp driver enables high resolution scrolling on
device connect for capable HID++ 2.0 devices. Driver also reads the
wheel capability and caches the returned high resolution wheel scroll
multiplier, that is used for scroll scaling when handling wheel scroll
events.

Wheel mode can also be set externally through HID++ requests, which
can leave the cached multiplier stale and cause incorrect scroll
scaling. If external SetWheelMode HID++ request sets the mode to
low resolution, the cached multiplier is not updated accordingly. This
causes extremely slow scrolling since driver expects multiple wheel
scroll events per detent but is only getting one.

The fix listens for HID++ SetWheelMode request responses and updates
the wheel scroll multiplier based on the set high resolution scroll
mode. The fix has been tested with Logitech G502X lightspeed mouse.

Signed-off-by: Lauri Saurus <redacted>

Applied, thank you.

-- 
Jiri Kosina
SUSE Labs