DORMANTno replies

[PATCH v2] HID: logitech-hidpp: do not overwrite the device's hi-res wheel mode

From: Roman Stingler <hidden>
Date: 2026-09-23 19:01:45
Also in: lkml, regressions
Subsystem: hid core layer, hid logitech drivers, hid++ logitech drivers, the rest · Maintainers: Jiri Kosina, Benjamin Tissoires, Linus Torvalds

hi_res_scroll_enable() unconditionally puts HID++ 2.0 devices supporting
the HiRes Wheel feature (0x2121) into high-resolution mode on every
connect event.

On at least the MX Master 4 that mode is persistent state in the device.
With hid-logitech-hidpp unloaded, a mode set from userspace survives
switching the mouse off and on again. Writing it at connect therefore
destroys a setting the user configured, and does so on every probe --
cold boot, receiver replug or module reload -- so userspace cannot
reliably keep it either: it gets no indication that the mode it set has
been changed underneath it.

This became visible when Bolt receivers gained support in 7.3. Before
that these devices were driven by hid-generic, hid-logitech-hidpp never
bound to them, and nothing in the kernel wrote the setting.

0x2121 exposes getWheelMode alongside setWheelMode. Read the current
mode instead of forcing high resolution, and scale by the multiplier
only while the wheel is actually in high-resolution mode.

The multiplier itself is queried regardless of the current mode, because
hidpp20_hires_wheel_raw_event() uses the cached hires_wheel_multiplier
when userspace later switches the wheel to high resolution; leaving it
at 1 would make each sub-detent tick report a full detent.

Note this changes behaviour for devices sitting at a low-resolution
factory default -- the kernel will no longer switch those to
high-resolution scrolling.

Link: https://lore.kernel.org/all/20260920094508.39682-1-roman.stingler@gmail.com/ (local)
Signed-off-by: Roman Stingler <redacted>
---
v2: always query the wheel multiplier, not only when the device is
    already in high-resolution mode. In v1 a device probed in low
    resolution cached hires_wheel_multiplier = 1, so a later switch to
    high resolution from userspace scaled every sub-detent tick as a
    full detent. Caught by Sashiko AI review on v1.
v1: https://lore.kernel.org/all/20260923181203.422097-1-roman.stingler@gmail.com/ (local)

Lovekesh Solanki proposed an alternative in the report thread which
remembers the last mode seen from userspace. That fixes suspend/resume
but not the probe cases above, and he suggested sending this instead:
https://lore.kernel.org/all/arJs7GjCP3r4IaM-@eggarch/ (local)

Tested with an MX Master 4 (WPID B042) behind a Bolt receiver, built as
a module and loaded at boot:

                        stock   this patch
  suspend/resume          no        yes
  module reload           no        yes
  cold boot               no        yes

The v2 change cannot be exercised behind Bolt: there the wheel reports
arrive on the receiver's generic mouse interface rather than through
hid-logitech-hidpp, so the driver's multiplier is not in the path (that
is the separate unscaled scrolling problem being addressed elsewhere on
the list). I tested it over Bluetooth instead, where hid-logitech-hidpp
receives the wheel reports itself:

  - wheel set to low resolution, link dropped to force a fresh probe;
    with dynamic debug on, the driver logs
      wheel multiplier = 15, hi-res = 0
    (v1 would have cached a multiplier of 1 here)
  - switched to high resolution from userspace, with no further
    hi_res_scroll_enable() call logged afterwards, so scaling comes
    only from the cached multiplier
  - one physical detent then reports REL_WHEEL_HI_RES totalling 120
    and a single REL_WHEEL, i.e. correctly scaled

The same probe also left the low-resolution setting untouched over
Bluetooth, as it does behind Bolt.

 drivers/hid/hid-logitech-hidpp.c | 33 ++++++++++++++++++++------------
 1 file changed, 21 insertions(+), 12 deletions(-)
diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 493763a12518..53b344c9b741 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -2044,6 +2044,7 @@ static int hidpp_hrs_set_highres_scrolling_mode(struct hidpp_device *hidpp,
 #define HIDPP_PAGE_HIRES_WHEEL		0x2121
 
 #define CMD_HIRES_WHEEL_GET_WHEEL_CAPABILITY	0x00
+#define CMD_HIRES_WHEEL_GET_WHEEL_MODE		0x10
 #define CMD_HIRES_WHEEL_SET_WHEEL_MODE		0x20
 
 static int hidpp_hrw_get_wheel_capability(struct hidpp_device *hidpp,
@@ -2072,12 +2073,10 @@ static int hidpp_hrw_get_wheel_capability(struct hidpp_device *hidpp,
 	return ret;
 }
 
-static int hidpp_hrw_set_wheel_mode(struct hidpp_device *hidpp, bool invert,
-	bool high_resolution, bool use_hidpp)
+static int hidpp_hrw_get_wheel_mode(struct hidpp_device *hidpp, u8 *mode)
 {
 	u8 feature_index;
 	int ret;
-	u8 params[1];
 	struct hidpp_report response;
 
 	ret = hidpp_root_get_feature(hidpp, HIDPP_PAGE_HIRES_WHEEL,
@@ -2085,13 +2084,14 @@ static int hidpp_hrw_set_wheel_mode(struct hidpp_device *hidpp, bool invert,
 	if (ret)
 		return ret;
 
-	params[0] = (invert          ? BIT(2) : 0) |
-		    (high_resolution ? BIT(1) : 0) |
-		    (use_hidpp       ? BIT(0) : 0);
+	ret = hidpp_send_fap_command_sync(hidpp, feature_index,
+					  CMD_HIRES_WHEEL_GET_WHEEL_MODE,
+					  NULL, 0, &response);
+	if (ret)
+		return ret;
 
-	return hidpp_send_fap_command_sync(hidpp, feature_index,
-					   CMD_HIRES_WHEEL_SET_WHEEL_MODE,
-					   params, sizeof(params), &response);
+	*mode = response.fap.params[0];
+	return 0;
 }
 
 /* -------------------------------------------------------------------------- */
@@ -3908,9 +3908,16 @@ static int hi_res_scroll_enable(struct hidpp_device *hidpp)
 {
 	int ret;
 	u8 multiplier = 1;
+	u8 mode = BIT(1);	/* the other paths always end up in hi-res */
 
 	if (hidpp->capabilities & HIDPP_CAPABILITY_HIDPP20_HI_RES_WHEEL) {
-		ret = hidpp_hrw_set_wheel_mode(hidpp, false, true, false);
+		/*
+		 * The wheel mode is persistent state in the device, so read it
+		 * rather than overwriting it. The multiplier is fetched either
+		 * way, so that it is already known if userspace switches the
+		 * wheel to high resolution later on.
+		 */
+		ret = hidpp_hrw_get_wheel_mode(hidpp, &mode);
 		if (ret == 0)
 			ret = hidpp_hrw_get_wheel_capability(hidpp, &multiplier);
 	} else if (hidpp->capabilities & HIDPP_CAPABILITY_HIDPP20_HI_RES_SCROLL) {
@@ -3933,8 +3940,10 @@ static int hi_res_scroll_enable(struct hidpp_device *hidpp)
 	}
 
 	hidpp->hires_wheel_multiplier = multiplier;
-	hidpp->vertical_wheel_counter.wheel_multiplier = multiplier;
-	hid_dbg(hidpp->hid_dev, "wheel multiplier = %d\n", multiplier);
+	hidpp->vertical_wheel_counter.wheel_multiplier =
+		(mode & BIT(1)) ? multiplier : 1;
+	hid_dbg(hidpp->hid_dev, "wheel multiplier = %d, hi-res = %d\n",
+		multiplier, !!(mode & BIT(1)));
 	return 0;
 }
 
base-commit: fe2ec83746e501645709761605c2464a44fd2929
-- 
2.55.0
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help