Re: [PATCH v1 3/3] rust: hid: Glorious Gaming PC Race Model O and O- mice reference driver

[Peter Hutterer <peter.hutterer@who-t.net>]

On Sun, Jun 29, 2025 at 04:51:22AM +0000, Rahul Rameshbabu wrote:
> Demonstrate how to perform a report fixup from a Rust HID driver. The mice
> specify the const flag incorrectly in the consumer input report descriptor,
> which leads to inputs being ignored. Correctly patch the report descriptor for
> the Model O and O- mice.
> 
> Portions of the HID report post-fixup:
> device 0:0
> ...
> 0x81, 0x06,                    //  Input (Data,Var,Rel)               84
> ...
> 0x81, 0x06,                    //  Input (Data,Var,Rel)               112
> ...
> 0x81, 0x06,                    //  Input (Data,Var,Rel)               140
> 
> Signed-off-by: Rahul Rameshbabu <sergeantsagara@protonmail.com>
> ---
>  MAINTAINERS                      |  7 ++++
>  drivers/hid/Kconfig              |  8 +++++
>  drivers/hid/Makefile             |  1 +
>  drivers/hid/hid-glorious.c       |  2 ++
>  drivers/hid/hid_glorious_rust.rs | 62 ++++++++++++++++++++++++++++++++
>  5 files changed, 80 insertions(+)
>  create mode 100644 drivers/hid/hid_glorious_rust.rs
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 487750d9fd1e..80849f76c6c3 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -10200,6 +10200,13 @@ L:	platform-driver-x86@vger.kernel.org
>  S:	Maintained
>  F:	drivers/platform/x86/gigabyte-wmi.c
>  
> +GLORIOUS RUST DRIVER [RUST]
> +M:	Rahul Rameshbabu <sergeantsagara@protonmail.com>
> +L:	linux-input@vger.kernel.org
> +S:	Maintained
> +T:	git git://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git
> +F:	drivers/hid/hid_glorious_rust.rs
> +
>  GNSS SUBSYSTEM
>  M:	Johan Hovold <johan@kernel.org>
>  S:	Maintained
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> index 5c8839ddc999..3592a4de113f 100644
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -406,6 +406,14 @@ config HID_GLORIOUS
>  	  Support for Glorious PC Gaming Race mice such as
>  	  the Glorious Model O, O- and D.
>  
> +config HID_GLORIOUS_RUST
> +	tristate "Glorious O and O- mice Rust reference driver"
> +	depends on USB_HID
> +	depends on RUST_HID_ABSTRACTIONS
> +	help
> +	  Support for Glorious PC Gaming Race O and O- mice
> +	  in Rust
> +
>  config HID_HOLTEK
>  	tristate "Holtek HID devices"
>  	depends on USB_HID
> diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
> index 10ae5dedbd84..bd86b3db5d88 100644
> --- a/drivers/hid/Makefile
> +++ b/drivers/hid/Makefile
> @@ -55,6 +55,7 @@ obj-$(CONFIG_HID_FT260)		+= hid-ft260.o
>  obj-$(CONFIG_HID_GEMBIRD)	+= hid-gembird.o
>  obj-$(CONFIG_HID_GFRM)		+= hid-gfrm.o
>  obj-$(CONFIG_HID_GLORIOUS)  += hid-glorious.o
> +obj-$(CONFIG_HID_GLORIOUS_RUST)	+= hid_glorious_rust.o
>  obj-$(CONFIG_HID_VIVALDI_COMMON) += hid-vivaldi-common.o
>  obj-$(CONFIG_HID_GOODIX_SPI)	+= hid-goodix-spi.o
>  obj-$(CONFIG_HID_GOOGLE_HAMMER)	+= hid-google-hammer.o
> diff --git a/drivers/hid/hid-glorious.c b/drivers/hid/hid-glorious.c
> index 5bbd81248053..d7362852c20f 100644
> --- a/drivers/hid/hid-glorious.c
> +++ b/drivers/hid/hid-glorious.c
> @@ -76,8 +76,10 @@ static int glorious_probe(struct hid_device *hdev,
>  }
>  
>  static const struct hid_device_id glorious_devices[] = {
> +#if !IS_ENABLED(CONFIG_HID_GLORIOUS_RUST)
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_SINOWEALTH,
>  		USB_DEVICE_ID_GLORIOUS_MODEL_O) },
> +#endif
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_SINOWEALTH,
>  		USB_DEVICE_ID_GLORIOUS_MODEL_D) },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_LAVIEW,
> diff --git a/drivers/hid/hid_glorious_rust.rs b/drivers/hid/hid_glorious_rust.rs
> new file mode 100644
> index 000000000000..de602ae33b2d
> --- /dev/null
> +++ b/drivers/hid/hid_glorious_rust.rs
> @@ -0,0 +1,62 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +// Copyright (C) 2025 Rahul Rameshbabu <sergeantsagara@protonmail.com>
> +
> +//! Rust reference HID driver for Glorious Model O and O- mice
> +
> +use kernel::{self, hid, prelude::*};
> +
> +const USB_VENDOR_ID_SINOWEALTH: u32 = 0x258a;
> +const USB_DEVICE_ID_GLORIOUS_MODEL_O: u32 = 0x0036;
> +
> +struct GloriousRust;
> +
> +kernel::hid_device_table!(
> +    HID_TABLE,
> +    MODULE_HID_TABLE,
> +    <GloriousRust as hid::Driver>::IdInfo,
> +    [(
> +        hid::DeviceId::new_usb(
> +            hid::Group::Generic,
> +            USB_VENDOR_ID_SINOWEALTH,
> +            USB_DEVICE_ID_GLORIOUS_MODEL_O,
> +        ),
> +        (),
> +    )]
> +);
> +
> +#[vtable]
> +impl hid::Driver for GloriousRust {
> +    type IdInfo = ();
> +    const ID_TABLE: hid::IdTable<Self::IdInfo> = &HID_TABLE;
> +
> +    /// Glorious Model O and O- specify the const flag in the consumer input
> +    /// report descriptor, which leads to inputs being ignored. Fix this by
> +    /// patching the descriptor.
> +    fn report_fixup<'a, 'b: 'a>(_hdev: &hid::Device, rdesc: &'b mut [u8]) -> &'a [u8] {
> +        if rdesc.len() == 213
> +            && rdesc[84] == 129
> +            && rdesc[112] == 129
> +            && rdesc[140] == 129
> +            && rdesc[85] == 3
> +            && rdesc[113] == 3
> +            && rdesc[141] == 3

fwiw, all tools that I've used in the past print hex for the hid
report descriptor items (including the excerpt in your commit message)
so IMO using hex here would be more useful.

Also at least in my head the items are logical groups, so this comparison
should arguably be:

> +            && (rdesc[84] == 129 && rdesc[85] == 3)

with extra () to hopefully stop rustfmt from breaking it up.

(pls ignore me if your approach is already a common pattern in the kernel)

Cheers,
  Peter

> +        {
> +            pr_info!("patching Glorious Model O consumer control report descriptor\n");
> +
> +            rdesc[85] = hid::MAIN_ITEM_VARIABLE | hid::MAIN_ITEM_RELATIVE;
> +            rdesc[113] = hid::MAIN_ITEM_VARIABLE | hid::MAIN_ITEM_RELATIVE;
> +            rdesc[141] = hid::MAIN_ITEM_VARIABLE | hid::MAIN_ITEM_RELATIVE;
> +        }
> +
> +        rdesc
> +    }
> +}
> +
> +kernel::module_hid_driver! {
> +    type: GloriousRust,
> +    name: "GloriousRust",
> +    authors: ["Rahul Rameshbabu <sergeantsagara@protonmail.com>"],
> +    description: "Rust reference HID driver for Glorious Model O and O- mice",
> +    license: "GPL",
> +}
> -- 
> 2.49.0
> 
> 
>