[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