Re: [PATCH] hid-logitech-hidpp: holdable_thumb_buttons parameter for Logitech MX Anywhere 3

[Bastien Nocera <hadess@hadess.net>]

On Mon, 2022-10-03 at 00:36 +0200, Tim Schumacher wrote:
> On the Logitech MX Anywhere 3 the thumb buttons only activate on
> release.
> This is because the mouse also uses those buttons as modifiers to
> enable
> horizontal scrolling with the mouse wheel. This patch adds the
> holdable_thumb_buttons module parameter. Users who don't care about
> horizontal scrolling and want properly functioning thumb buttons can
> set
> this parameter. If it's set we use the feature 0x1b04 (special keys
> and
> mouse buttons) to divert thumb button events and handle them in
> software.
> 
> Signed-off-by: Tim Schumacher <tim@tschumacher.net>
> ---
> 
> Hi,
> 
> this is my first kernel patch, please be nice :)
> The reason this can't be done in user space with a tool like xmodmap
> (exception would be a user space driver) is that without making use
> of
> the 0x1b04 feature the mouse sends nothing whatsoever when the thumb
> buttons are pressed down so we can't handle those events. I
> discovered
> this because the thumb buttons felt unresponsive on my mouse and it's
> been bugging me since.
> 
> I'm not sure if other Logitech devices could benefit from this patch,
> something in the spirit of "thumb buttons only activate on release"
> has
> been said about the MX Master series mice on the Arch wiki but I'm
> not
> sure if this is true or if the patch would work as-is. Hope this is
> not
> too specific to get into the kernel.
> 
> <snip>
> +       { /* Mouse Logitech MX Anywhere 3 */
> +         LDJ_DEVICE(0x4090), .driver_data =
> HIDPP_QUIRK_CLASS_MX_ANYWHERE_3 },

As discussed in recent hid-logitech-hidpp related discussions, new
settings should be gated by asking the hardware whether it supports a
particular feature, rather than hardcoding device IDs.

See:

https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git/commit/?h=for-6.1/logitech&id=908d325e1665b2781085580070554cbbe5fc3c89

As a sidenote, make sure to run your patches through checkpatch.pl, it
should warn you that C++ style comments aren't allowed.

You should also not use magic numbers (0x53/0x56) and define them as
constants instead.

>         { /* Keyboard logitech K400 */
>           LDJ_DEVICE(0x4024),
>           .driver_data = HIDPP_QUIRK_CLASS_K400 },

Looking forward to v2!