Re: [PATCH] HID: logitech-hidpp: fix hi-res scroll for Bolt-connected MX Master

[Alexey Zagorodnikov <xglooom@gmail.com>]

Hi, thanks for your patch. I've tested it with my MX Master 4 + Bolt.

I'm not seeing a regression from mainline hid++ enabling, as in Solaar 
high-res scrolling was disabled in my config.

But with hid++ for Bolt enabled by mainline and your patch fixing 
high-res scroll init, I finally got smooth high-res scrolling on MX 
Master 4 via a Bolt connection.

Tested-by: Alexey Zagorodnikov <xglooom@gmail.com>

On 9/4/26 08:48, Rafael Passos wrote:
> The new added support for Logitech HID++ caused this mouse to
> scroll too far for each tic when using the USB dongle (Bolt).
> Previously, this mouse was handled as hid_generic over the Bold
> connection, and logitech-hidpp when over Bluetooth.
>
> Cause:
> Over the Bolt receiver, the mouse reports  are forwarded over the generic
> interface 1, instead of the new HID++ child device. The generic hid does
> now know about the hi-res scrolling used by the HID++.
> In my tests, I discovered this mouse has a multiplier factor of 15. This
> multiplier is only handled by the logitech-hidpp driver. When piped to
> hid_generic, the "hi-res" value was piped to a "low-res" field, making
> the scroll unbearable.
>
> The fix routes the wheel using the HID++, so the ticks are scaled by the
> multiplier and correctly reported as a hi-res event.
>
> I also had to move the hidpp_is_bolt_child function up, to use it in the
> hidpp_connect_event function, where the gate for
> hidpp_initialize_hires_scroll function lives.
>
> Tested with:
> - Logitech MX Master 3S (mouse) via Bolt receiver and Bluetooth.
>
> Fixes: 022eb347ff3a ("HID: logitech: add Bolt receiver support for Logitech HID++ devices")
> Signed-off-by: Rafael Passos <rafael@rcpassos.me>
> ---
>
> Hi,
> I decided to send this patch as soon as I made it work, to try to make
> it into this fix cycle.
>
> I do not have other devices to test this with.
> It would *be great* to have other users testing this.
>
> Lastly, I decided to make all the changes in a single patch.
> But I can break it up if asked.
>
> Thanks,
> Rafael
>
>
>   drivers/hid/hid-logitech-hidpp.c | 47 ++++++++++++++++++++++----------
>   1 file changed, 32 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
> index 1504de32b1c84..960543bfd5ea1 100644
> --- a/drivers/hid/hid-logitech-hidpp.c
> +++ b/drivers/hid/hid-logitech-hidpp.c
> @@ -3900,6 +3900,19 @@ static int hidpp10_consumer_keys_raw_event(struct hidpp_device *hidpp,
>   	return 1;
>   }
>   
> +static bool hidpp_is_bolt_child(struct hid_device *hdev)
> +{
> +	struct device *parent = hdev->dev.parent;
> +	struct hid_device *receiver_hdev;
> +
> +	if (!parent)
> +		return false;
> +
> +	receiver_hdev = to_hid_device(parent);
> +	return receiver_hdev->vendor == USB_VENDOR_ID_LOGITECH &&
> +	       receiver_hdev->product == USB_DEVICE_ID_LOGITECH_BOLT_RECEIVER;
> +}
> +
>   /* -------------------------------------------------------------------------- */
>   /* High-resolution scroll wheels                                              */
>   /* -------------------------------------------------------------------------- */
> @@ -3910,7 +3923,9 @@ 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);
> +		bool use_hidpp = hidpp_is_bolt_child(hidpp->hid_dev);
> +
> +		ret = hidpp_hrw_set_wheel_mode(hidpp, false, true, use_hidpp);
>   		if (ret == 0)
>   			ret = hidpp_hrw_get_wheel_capability(hidpp, &multiplier);
>   	} else if (hidpp->capabilities & HIDPP_CAPABILITY_HIDPP20_HI_RES_SCROLL) {
> @@ -3998,6 +4013,19 @@ static int hidpp20_hires_wheel_raw_event(struct hidpp_device *hidpp,
>   		return 1;
>   	}
>   
> +	/* wheel movement event: 16-bit signed delta in HID++ ticks */
> +	if ((data[3] & 0xf0) == 0x00 && size >= 7 && hidpp->input &&
> +	    hidpp->vertical_wheel_counter.wheel_multiplier) {
> +		s16 delta = get_unaligned_be16(&data[5]);
> +
> +		if (delta) {
> +			hidpp_scroll_counter_handle_scroll(hidpp->input,
> +				&hidpp->vertical_wheel_counter, delta);
> +			input_sync(hidpp->input);
> +		}
> +		return 1;
> +	}
> +
>   	return 0;
>   }
>   
> @@ -4405,19 +4433,6 @@ static int hidpp_initialize_battery(struct hidpp_device *hidpp)
>   	return ret;
>   }
>   
> -static bool hidpp_is_bolt_child(struct hid_device *hdev)
> -{
> -	struct device *parent = hdev->dev.parent;
> -	struct hid_device *receiver_hdev;
> -
> -	if (!parent)
> -		return false;
> -
> -	receiver_hdev = to_hid_device(parent);
> -	return receiver_hdev->vendor == USB_VENDOR_ID_LOGITECH &&
> -	       receiver_hdev->product == USB_DEVICE_ID_LOGITECH_BOLT_RECEIVER;
> -}
> -
>   static int hidpp_bolt_init(struct hidpp_device *hidpp)
>   {
>   	struct hid_device *hdev = hidpp->hid_dev;
> @@ -4579,7 +4594,9 @@ static void hidpp_connect_event(struct work_struct *work)
>   	}
>   
>   	hidpp_initialize_battery(hidpp);
> -	if (!hid_is_usb(hidpp->hid_dev))
> +
> +	if (!hid_is_usb(hidpp->hid_dev) ||
> +	    hidpp_is_bolt_child(hidpp->hid_dev))
>   		hidpp_initialize_hires_scroll(hidpp);
>   
>   	/* forward current battery state */