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

Roman Stingler posted 1 patch 18 hours ago
There is a newer version of this series
drivers/hid/hid-logitech-hidpp.c | 30 +++++++++++++++++++-----------
1 file changed, 19 insertions(+), 11 deletions(-)
[PATCH] HID: logitech-hidpp: do not overwrite the device's hi-res wheel mode
Posted by Roman Stingler 18 hours ago
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 and scale vertical_wheel_counter.wheel_multiplier to match rather
than forcing high resolution: a device left in high-resolution mode
still gets its multiplier, and one the user configured for low
resolution is left alone.

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/
Signed-off-by: Roman Stingler <roman.stingler@gmail.com>
---
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/

Tested on 7.3-rc3 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

With the mouse left in high-resolution mode, a full module reload leaves
it in high resolution and the multiplier is fetched as before. I checked
that the mode is honoured; I did not instrument events per detent,
though that path is unchanged.

 drivers/hid/hid-logitech-hidpp.c | 30 +++++++++++++++++++-----------
 1 file changed, 19 insertions(+), 11 deletions(-)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 493763a12518..338477bfcaeb 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;
 }
 
 /* -------------------------------------------------------------------------- */
@@ -3910,8 +3910,16 @@ static int hi_res_scroll_enable(struct hidpp_device *hidpp)
 	u8 multiplier = 1;
 
 	if (hidpp->capabilities & HIDPP_CAPABILITY_HIDPP20_HI_RES_WHEEL) {
-		ret = hidpp_hrw_set_wheel_mode(hidpp, false, true, false);
-		if (ret == 0)
+		u8 mode;
+
+		/*
+		 * The wheel mode is persistent state in the device, so read it
+		 * rather than overwriting it, and scale to match. A device
+		 * left in hi-res still gets the multiplier it needs; one the
+		 * user configured for low resolution is left alone.
+		 */
+		ret = hidpp_hrw_get_wheel_mode(hidpp, &mode);
+		if (ret == 0 && (mode & BIT(1)))
 			ret = hidpp_hrw_get_wheel_capability(hidpp, &multiplier);
 	} else if (hidpp->capabilities & HIDPP_CAPABILITY_HIDPP20_HI_RES_SCROLL) {
 		ret = hidpp_hrs_set_highres_scrolling_mode(hidpp, true,

base-commit: fe2ec83746e501645709761605c2464a44fd2929
-- 
2.55.0