[PATCH v1 0/3] Initial work for Rust abstraction for HID device driver development

[PATCH v1 0/3] Initial work for Rust abstraction for HID device driver development

[Rahul Rameshbabu]

Hello again,

I have come back with a revision of my HID binding work based on feedback from
the maintainers and the community. Going with Benjamin Tissoires's feedback, I
have opted to implement support for a much simpler HID device as a starting
point for this work. I have gone ahead with making a Rust reference driver for
the Glorious PC Gaming Race Model O and O- mice. I chose these devices because
they were easily accessible for my development work. I have gone ahead with
naming this driver the "Glorious Rust" driver. My binding work now adopts the
core infrastructure Danilo Krummrich has provided for the Rust for Linux
community.

I have decided to move this series out of RFC status, since I feel the
implemented code is in a stable state. It could be used for other drivers as-is
with the caveat that those drivers are only performing report fixups. Hopefully,
a HID-BPF solution can be taken for production report fixup purposes. In
reality, I will need to bring more of the HID subsystem functionality to Rust to
promote implementations of more complex drivers, such as the USB Monitor Control
driver I am planning to implement (as expressed in my RFC series). I am hoping
to do a better job of documenting my progress involving this work on my blog
while I am working on each iteration/addition for anyone interested in following
along outside my mailing list posts.

Thanks,
Rahul Rameshbabu

Link: https://lore.kernel.org/rust-for-linux/20250313160220.6410-2-sergeantsagara@protonmail.com/
Link: https://binary-eater.github.io/tags/usb-monitor-control/

Rahul Rameshbabu (3):
  HID: core: Change hid_driver to use a const char* for name
  rust: core abstractions for HID drivers
  rust: hid: Glorious Gaming PC Race Model O and O- mice reference
    driver

 MAINTAINERS                      |  14 +
 drivers/hid/Kconfig              |  16 ++
 drivers/hid/Makefile             |   1 +
 drivers/hid/hid-glorious.c       |   2 +
 drivers/hid/hid_glorious_rust.rs |  62 +++++
 include/linux/hid.h              |   2 +-
 rust/bindings/bindings_helper.h  |   1 +
 rust/kernel/hid.rs               | 421 +++++++++++++++++++++++++++++++
 rust/kernel/lib.rs               |   2 +
 9 files changed, 520 insertions(+), 1 deletion(-)
 create mode 100644 drivers/hid/hid_glorious_rust.rs
 create mode 100644 rust/kernel/hid.rs


base-commit: 0303584766b7bdb6564c7e8f13e0b59b6ef44984
-- 
2.49.0

[PATCH v1 1/3] HID: core: Change hid_driver to use a const char* for name

[Rahul Rameshbabu]

name is never mutated by the core HID stack. Making name a const char*
simplifies passing the string from Rust to C. Otherwise, it becomes difficult to
pass a 'static lifetime CStr from Rust to a char*, rather than a const char*,
due to lack of guarantee that the underlying data of the CStr will not be
mutated by the C code.

Signed-off-by: Rahul Rameshbabu <sergeantsagara@protonmail.com>
---
 include/linux/hid.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/hid.h b/include/linux/hid.h
index 568a9d8c749b..d65c202783da 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -816,7 +816,7 @@ struct hid_usage_id {
  * zero from them.
  */
 struct hid_driver {
-	char *name;
+	const char *name;
 	const struct hid_device_id *id_table;
 
 	struct list_head dyn_list;
-- 
2.49.0

[PATCH v1 2/3] rust: core abstractions for HID drivers

[Rahul Rameshbabu]

These abstractions enable the development of HID drivers in Rust by binding with
the HID core C API. They provide Rust types that map to the equivalents in C. In
this initial draft, only hid_device and hid_device_id are provided direct Rust
type equivalents. hid_driver is specially wrapped with a custom Driver type. The
module_hid_driver! macro provides analogous functionality to its C equivalent.
Only the .report_fixup callback is binded to Rust so far.

Future work for these abstractions would include more bindings for common
HID-related types, such as hid_field, hid_report_enum, and hid_report as well as
more bus callbacks. Providing Rust equivalents to useful core HID functions will
also be necessary for HID driver development in Rust.

Signed-off-by: Rahul Rameshbabu <sergeantsagara@protonmail.com>
---

Notes:
    Some points I did not address from the last review cycle:
    
      * Enabling CONFIG_HID=m with the Rust bindings
        - This is a limitation due to the Makefile rules that currently exist,
          compiling all the Rust bindings into kernel.o, which gets linked into the
          core kernel image.
          From rust/Makefile
            $(obj)/kernel.o: private rustc_target_flags = --extern ffi --extern pin_init \
                --extern build_error --extern macros --extern bindings --extern uapi
            $(obj)/kernel.o: $(src)/kernel/lib.rs $(obj)/build_error.o $(obj)/pin_init.o \
                $(obj)/$(libmacros_name) $(obj)/bindings.o $(obj)/uapi.o FORCE
              +$(call if_changed_rule,rustc_library)
    
            ifdef CONFIG_JUMP_LABEL
            $(obj)/kernel.o: $(obj)/kernel/generated_arch_static_branch_asm.rs
            endif
        - I would be happy to work on support for module-level bindings, but this is
          not a problem unique only to the Rust HID binding work.
      * I did not look into autogenerating all the getter functions for various
        fields exported from the binded C structures.
        - I would be interested in hearing opinions from folks actively involved
          with Rust for Linux on this topic.
    
    Changelog:
    
      RFC->v1:
        * Use Danilo's core infrastructure
        * Account for HID device groups
        * Remove probe and remove callbacks
        * Implement report_fixup support
        * Properly comment code including SAFETY comments

 MAINTAINERS                     |   7 +
 drivers/hid/Kconfig             |   8 +
 rust/bindings/bindings_helper.h |   1 +
 rust/kernel/hid.rs              | 421 ++++++++++++++++++++++++++++++++
 rust/kernel/lib.rs              |   2 +
 5 files changed, 439 insertions(+)
 create mode 100644 rust/kernel/hid.rs

diff --git a/MAINTAINERS b/MAINTAINERS
index c3f7fbd0d67a..487750d9fd1e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10686,6 +10686,13 @@ F:	include/uapi/linux/hid*
 F:	samples/hid/
 F:	tools/testing/selftests/hid/
 
+HID CORE LAYER [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:	rust/kernel/hid.rs
+
 HID LOGITECH DRIVERS
 R:	Filipe Laíns <lains@riseup.net>
 L:	linux-input@vger.kernel.org
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 43859fc75747..5c8839ddc999 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -744,6 +744,14 @@ config HID_MEGAWORLD_FF
 	Say Y here if you have a Mega World based game controller and want
 	to have force feedback support for it.
 
+config RUST_HID_ABSTRACTIONS
+	bool "Rust HID abstractions support"
+	depends on RUST
+	depends on HID=y
+	help
+	Adds support needed for HID drivers written in Rust. It provides a
+	wrapper around the C hid core.
+
 config HID_REDRAGON
 	tristate "Redragon keyboards"
 	default !EXPERT
diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index 8cbb660e2ec2..1cbb119684ef 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -52,6 +52,7 @@
 #include <linux/file.h>
 #include <linux/firmware.h>
 #include <linux/fs.h>
+#include <linux/hid.h>
 #include <linux/jiffies.h>
 #include <linux/jump_label.h>
 #include <linux/mdio.h>
diff --git a/rust/kernel/hid.rs b/rust/kernel/hid.rs
new file mode 100644
index 000000000000..a2f8a3ad5549
--- /dev/null
+++ b/rust/kernel/hid.rs
@@ -0,0 +1,421 @@
+// SPDX-License-Identifier: GPL-2.0
+
+// Copyright (C) 2025 Rahul Rameshbabu <sergeantsagara@protonmail.com>
+
+//! Abstractions for the HID interface.
+//!
+//! C header: [`include/linux/hid.h`](srctree/include/linux/hid.h)
+
+use crate::{device, device_id::RawDeviceId, driver, error::*, prelude::*, types::Opaque};
+use core::marker::PhantomData;
+
+/// Indicates the item is static read-only.
+///
+/// Refer to [Device Class Definition for HID 1.11]
+/// Section 6.2.2.5 Input, Output, and Feature Items.
+///
+/// [Device Class Definition for HID 1.11]: https://www.usb.org/sites/default/files/hid1_11.pdf
+pub const MAIN_ITEM_CONSTANT: u8 = bindings::HID_MAIN_ITEM_CONSTANT as u8;
+/// Indicates the item represents data from a physical control.
+///
+/// Refer to [Device Class Definition for HID 1.11]
+/// Section 6.2.2.5 Input, Output, and Feature Items.
+///
+/// [Device Class Definition for HID 1.11]: https://www.usb.org/sites/default/files/hid1_11.pdf
+pub const MAIN_ITEM_VARIABLE: u8 = bindings::HID_MAIN_ITEM_VARIABLE as u8;
+/// Indicates the item should be treated as a relative change from the previous
+/// report.
+///
+/// Refer to [Device Class Definition for HID 1.11]
+/// Section 6.2.2.5 Input, Output, and Feature Items.
+///
+/// [Device Class Definition for HID 1.11]: https://www.usb.org/sites/default/files/hid1_11.pdf
+pub const MAIN_ITEM_RELATIVE: u8 = bindings::HID_MAIN_ITEM_RELATIVE as u8;
+/// Indicates the item should wrap around when reaching the extreme high or
+/// extreme low values.
+///
+/// Refer to [Device Class Definition for HID 1.11]
+/// Section 6.2.2.5 Input, Output, and Feature Items.
+///
+/// [Device Class Definition for HID 1.11]: https://www.usb.org/sites/default/files/hid1_11.pdf
+pub const MAIN_ITEM_WRAP: u8 = bindings::HID_MAIN_ITEM_WRAP as u8;
+/// Indicates the item should wrap around when reaching the extreme high or
+/// extreme low values.
+///
+/// Refer to [Device Class Definition for HID 1.11]
+/// Section 6.2.2.5 Input, Output, and Feature Items.
+///
+/// [Device Class Definition for HID 1.11]: https://www.usb.org/sites/default/files/hid1_11.pdf
+pub const MAIN_ITEM_NONLINEAR: u8 = bindings::HID_MAIN_ITEM_NONLINEAR as u8;
+/// Indicates whether the control has a preferred state it will physically
+/// return to without user intervention.
+///
+/// Refer to [Device Class Definition for HID 1.11]
+/// Section 6.2.2.5 Input, Output, and Feature Items.
+///
+/// [Device Class Definition for HID 1.11]: https://www.usb.org/sites/default/files/hid1_11.pdf
+pub const MAIN_ITEM_NO_PREFERRED: u8 = bindings::HID_MAIN_ITEM_NO_PREFERRED as u8;
+/// Indicates whether the control has a physical state where it will not send
+/// any reports.
+///
+/// Refer to [Device Class Definition for HID 1.11]
+/// Section 6.2.2.5 Input, Output, and Feature Items.
+///
+/// [Device Class Definition for HID 1.11]: https://www.usb.org/sites/default/files/hid1_11.pdf
+pub const MAIN_ITEM_NULL_STATE: u8 = bindings::HID_MAIN_ITEM_NULL_STATE as u8;
+/// Indicates whether the control requires host system logic to change state.
+///
+/// Refer to [Device Class Definition for HID 1.11]
+/// Section 6.2.2.5 Input, Output, and Feature Items.
+///
+/// [Device Class Definition for HID 1.11]: https://www.usb.org/sites/default/files/hid1_11.pdf
+pub const MAIN_ITEM_VOLATILE: u8 = bindings::HID_MAIN_ITEM_VOLATILE as u8;
+/// Indicates whether the item is fixed size or a variable buffer of bytes.
+///
+/// Refer to [Device Class Definition for HID 1.11]
+/// Section 6.2.2.5 Input, Output, and Feature Items.
+///
+/// [Device Class Definition for HID 1.11]: https://www.usb.org/sites/default/files/hid1_11.pdf
+pub const MAIN_ITEM_BUFFERED_BYTE: u8 = bindings::HID_MAIN_ITEM_BUFFERED_BYTE as u8;
+
+/// HID device groups are intended to help categories HID devices based on a set
+/// of common quirks and logic that they will require to function correctly.
+pub enum Group {
+    /// Indicates a generic device that should need no custom logic from the
+    /// core HID stack.
+    Generic = bindings::HID_GROUP_GENERIC as isize,
+    /// Maps multitouch devices to hid-multitouch instead of hid-generic.
+    Multitouch = bindings::HID_GROUP_MULTITOUCH as isize,
+    /// Used for autodetecing and mapping of HID sensor hubs to
+    /// hid-sensor-hub.
+    SensorHub = bindings::HID_GROUP_SENSOR_HUB as isize,
+    /// Used for autodetecing and mapping Win 8 multitouch devices to set the
+    /// needed quirks.
+    MultitouchWin8 = bindings::HID_GROUP_MULTITOUCH_WIN_8 as isize,
+
+    /// Used to distinguish Synpatics touchscreens from other products. The
+    /// touchscreens will be handled by hid-multitouch instead, while everything
+    /// else will be managed by hid-rmi.
+    RMI = bindings::HID_GROUP_RMI as isize,
+    /// Used for hid-core handling to automatically identify Wacom devices and
+    /// have them probed by hid-wacom.
+    Wacom = bindings::HID_GROUP_WACOM as isize,
+    /// Used by logitech-djreceiver and logitech-djdevice to autodetect if
+    /// devices paied to the DJ receivers are DJ devices and handle them with
+    /// the device driver.
+    LogitechDJDevice = bindings::HID_GROUP_LOGITECH_DJ_DEVICE as isize,
+    /// Since the Valve Steam Controller only has vendor-specific usages,
+    /// prevent hid-generic from parsing its reports since there would be
+    /// nothing hid-generic could do for the device.
+    Steam = bindings::HID_GROUP_STEAM as isize,
+    /// Used to differentiate 27 Mhz frequency Logitech DJ devices from other
+    /// Logitech DJ devices.
+    Logitech27MHzDevice = bindings::HID_GROUP_LOGITECH_27MHZ_DEVICE as isize,
+    /// Used for autodetecting and mapping Vivaldi devices to hid-vivaldi.
+    Vivaldi = bindings::HID_GROUP_VIVALDI as isize,
+}
+
+impl Group {
+    /// Internal function used to convert Group variants into u16
+    const fn into(self) -> u16 {
+        // variants assigned constants that can be represented as u16
+        self as u16
+    }
+}
+
+/// The HID device representation.
+///
+/// This structure represents the Rust abstraction for a C `struct hid_device`. The implementation
+/// abstracts the usage of an already existing C `struct hid_device` within Rust code that we get
+/// passed from the C side.
+///
+/// # Invariants
+///
+/// A [`Device`] instance represents a valid `struct hid_device` created by the C portion of the kernel.
+#[repr(transparent)]
+pub struct Device<Ctx: device::DeviceContext = device::Normal>(
+    Opaque<bindings::hid_device>,
+    PhantomData<Ctx>,
+);
+
+impl<Ctx: device::DeviceContext> Device<Ctx> {
+    fn as_raw(&self) -> *mut bindings::hid_device {
+        self.0.get()
+    }
+}
+
+impl Device {
+    /// Returns the HID transport bus ID.
+    pub fn bus(&self) -> u16 {
+        // SAFETY: `self.as_raw` is a valid pointer to a `struct hid_device`
+        unsafe { *self.as_raw() }.bus
+    }
+
+    /// Returns the HID report group.
+    pub fn group(&self) -> u16 {
+        // SAFETY: `self.as_raw` is a valid pointer to a `struct hid_device`
+        unsafe { *self.as_raw() }.group
+    }
+
+    /// Returns the HID vendor ID.
+    pub fn vendor(&self) -> u32 {
+        // SAFETY: `self.as_raw` is a valid pointer to a `struct hid_device`
+        unsafe { *self.as_raw() }.vendor
+    }
+
+    /// Returns the HID product ID.
+    pub fn product(&self) -> u32 {
+        // SAFETY: `self.as_raw` is a valid pointer to a `struct hid_device`
+        unsafe { *self.as_raw() }.product
+    }
+}
+
+/// Abstraction for the HID device ID structure ([`struct hid_device_id`]).
+#[repr(transparent)]
+#[derive(Clone, Copy)]
+pub struct DeviceId(bindings::hid_device_id);
+
+impl DeviceId {
+    /// Equivalent to C's `HID_USB_DEVICE` macro.
+    ///
+    /// Create a new `hid::DeviceId` from a group, vendor ID, and device ID
+    /// number.
+    pub const fn new_usb(group: Group, vendor: u32, product: u32) -> Self {
+        Self(bindings::hid_device_id {
+            bus: 0x3, /* BUS_USB */
+            group: group.into(),
+            vendor,
+            product,
+            driver_data: 0,
+        })
+    }
+
+    /// Returns the HID transport bus ID.
+    pub fn bus(&self) -> u16 {
+        self.0.bus
+    }
+
+    /// Returns the HID report group.
+    pub fn group(&self) -> u16 {
+        self.0.group
+    }
+
+    /// Returns the HID vendor ID.
+    pub fn vendor(&self) -> u32 {
+        self.0.vendor
+    }
+
+    /// Returns the HID product ID.
+    pub fn product(&self) -> u32 {
+        self.0.product
+    }
+}
+
+// SAFETY:
+// * `DeviceId` is a `#[repr(transparent)` wrapper of `hid_device_id` and does not add
+//   additional invariants, so it's safe to transmute to `RawType`.
+// * `DRIVER_DATA_OFFSET` is the offset to the `driver_data` field.
+unsafe impl RawDeviceId for DeviceId {
+    type RawType = bindings::hid_device_id;
+
+    const DRIVER_DATA_OFFSET: usize = core::mem::offset_of!(bindings::hid_device_id, driver_data);
+
+    fn index(&self) -> usize {
+        self.0.driver_data
+    }
+}
+
+/// IdTable type for HID
+pub type IdTable<T> = &'static dyn kernel::device_id::IdTable<DeviceId, T>;
+
+/// Create a HID `IdTable` with its alias for modpost.
+#[macro_export]
+macro_rules! hid_device_table {
+    ($table_name:ident, $module_table_name:ident, $id_info_type: ty, $table_data: expr) => {
+        const $table_name: $crate::device_id::IdArray<
+            $crate::hid::DeviceId,
+            $id_info_type,
+            { $table_data.len() },
+        > = $crate::device_id::IdArray::new($table_data);
+
+        $crate::module_device_table!("hid", $module_table_name, $table_name);
+    };
+}
+
+/// The HID driver trait.
+///
+/// # Example
+///
+///```
+/// use kernel::hid;
+///
+/// const USB_VENDOR_ID_VALVE: u32 = 0x28de;
+/// const USB_DEVICE_ID_STEAM_DECK: u32 = 0x1205;
+///
+/// struct MyDriver;
+///
+/// kernel::hid_device_table!(
+///     HID_TABLE,
+///     MODULE_HID_TABLE,
+///     <MyDriver as hid::Driver>::IdInfo,
+///     [(
+///         hid::DeviceId::new_usb(
+///             hid::Group::Steam,
+///             USB_VENDOR_ID_VALVE,
+///             USB_DEVICE_ID_STEAM_DECK,
+///         ),
+///         (),
+///     )]
+/// );
+///
+/// #[vtable]
+/// impl hid::Driver for MyDriver {
+///     type IdInfo = ();
+///     const ID_TABLE: hid::IdTable<Self::IdInfo> = &HID_TABLE;
+///
+///     /// This function is optional to implement.
+///     fn report_fixup<'a, 'b: 'a>(_hdev: &hid::Device, rdesc: &'b mut [u8]) -> &'a [u8] {
+///         // Perform some report descriptor fixup
+///         rdesc
+///     }
+/// }
+///```
+/// Drivers must implement this trait in order to get a HID driver registered.
+/// Please refer to the `Adapter` documentation for an example.
+#[vtable]
+pub trait Driver: Send {
+    /// The type holding information about each device id supported by the driver.
+    // TODO: Use `associated_type_defaults` once stabilized:
+    //
+    // ```
+    // type IdInfo: 'static = ();
+    // ```
+    type IdInfo: 'static;
+
+    /// The table of device ids supported by the driver.
+    const ID_TABLE: IdTable<Self::IdInfo>;
+
+    /// Called before report descriptor parsing. Can be used to mutate the
+    /// report descriptor before the core HID logic processes the descriptor.
+    /// Useful for problematic report descriptors that prevent HID devices from
+    /// functioning correctly.
+    ///
+    /// Optional to implement.
+    fn report_fixup<'a, 'b: 'a>(_hdev: &Device, _rdesc: &'b mut [u8]) -> &'a [u8] {
+        build_error!(VTABLE_DEFAULT_ERROR)
+    }
+}
+
+/// An adapter for the registration of HID drivers.
+pub struct Adapter<T: Driver>(T);
+
+// SAFETY: A call to `unregister` for a given instance of `RegType` is guaranteed to be valid if
+// a preceding call to `register` has been successful.
+unsafe impl<T: Driver + 'static> driver::RegistrationOps for Adapter<T> {
+    type RegType = bindings::hid_driver;
+
+    unsafe fn register(
+        hdrv: &Opaque<Self::RegType>,
+        name: &'static CStr,
+        module: &'static ThisModule,
+    ) -> Result {
+        // SAFETY: It's safe to set the fields of `struct hid_driver` on initialization.
+        let raw_hdrv = unsafe { &mut *hdrv.get() };
+
+        raw_hdrv.name = name.as_char_ptr();
+        raw_hdrv.id_table = T::ID_TABLE.as_ptr();
+        raw_hdrv.report_fixup = if T::HAS_REPORT_FIXUP {
+            Some(Self::report_fixup_callback)
+        } else {
+            None
+        };
+
+        // SAFETY: `hdrv` is guaranteed to be a valid `RegType`
+        to_result(unsafe {
+            bindings::__hid_register_driver(hdrv.get(), module.0, name.as_char_ptr())
+        })
+    }
+
+    unsafe fn unregister(hdrv: &Opaque<Self::RegType>) {
+        // SAFETY: `hdrv` is guaranteed to be a valid `RegType`
+        unsafe { bindings::hid_unregister_driver(hdrv.get()) }
+    }
+}
+
+impl<T: Driver + 'static> Adapter<T> {
+    extern "C" fn report_fixup_callback(
+        hdev: *mut bindings::hid_device,
+        buf: *mut u8,
+        size: *mut kernel::ffi::c_uint,
+    ) -> *const u8 {
+        // SAFETY: The HID subsystem only ever calls the report_fixup callback
+        // with a valid pointer to a `struct hid_device`.
+        //
+        // INVARIANT: `hdev` is valid for the duration of
+        // `report_fixup_callback()`.
+        let hdev = unsafe { &*hdev.cast::<Device>() };
+
+        // SAFETY: The HID subsystem only ever calls the report_fixup callback
+        // with a valid pointer to a `kernel::ffi::c_uint`.
+        //
+        // INVARIANT: `size` is valid for the duration of
+        // `report_fixup_callback()`.
+        let buf_len: usize = match unsafe { *size }.try_into() {
+            Ok(len) => len,
+            Err(e) => {
+                pr_err!(
+                    "Cannot fix report description due to length conversion failure: {}!\n",
+                    e
+                );
+
+                return buf;
+            }
+        };
+
+        // Build a mutable Rust slice from buf and size
+        //
+        // SAFETY: The HID subsystem only ever calls the report_fixup callback
+        // with a valid pointer to a `u8` buffer.
+        //
+        // INVARIANT: `buf` is valid for the duration of
+        // `report_fixup_callback()`.
+        let rdesc_slice = unsafe { core::slice::from_raw_parts_mut(buf, buf_len) };
+        let rdesc_slice = T::report_fixup(hdev, rdesc_slice);
+
+        match rdesc_slice.len().try_into() {
+            // SAFETY: The HID subsystem only ever calls the report_fixup
+            // callback with a valid pointer to a `kernel::ffi::c_uint`.
+            //
+            // INVARIANT: `size` is valid for the duration of
+            // `report_fixup_callback()`.
+            Ok(len) => unsafe { *size = len },
+            Err(e) => {
+                pr_err!("Fixed report description will not be used due to {}!\n", e);
+
+                return buf;
+            }
+        }
+
+        rdesc_slice.as_ptr()
+    }
+}
+
+/// Declares a kernel module that exposes a single HID driver.
+///
+/// # Example
+///
+///```ignore
+/// kernel::module_hid_driver! {
+///     type: MyDriver,
+///     name: "Module name",
+///     authors: ["Author name"],
+///     description: "Description",
+///     license: "GPL",
+/// }
+///```
+#[macro_export]
+macro_rules! module_hid_driver {
+    ($($f:tt)*) => {
+        $crate::module_driver!(<T>, $crate::hid::Adapter<T>, { $($f)* });
+    };
+}
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 6b4774b2b1c3..a50f7d099e8a 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -78,6 +78,8 @@
 #[cfg(CONFIG_RUST_FW_LOADER_ABSTRACTIONS)]
 pub mod firmware;
 pub mod fs;
+#[cfg(CONFIG_RUST_HID_ABSTRACTIONS)]
+pub mod hid;
 pub mod init;
 pub mod io;
 pub mod ioctl;
-- 
2.49.0

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

[Rahul Rameshbabu]

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
+        {
+            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

Re: [PATCH v1 2/3] rust: core abstractions for HID drivers

[Danilo Krummrich]

(Cc: +Jiri)

On Sun, Jun 29, 2025 at 04:51:15AM +0000, Rahul Rameshbabu wrote:
> diff --git a/MAINTAINERS b/MAINTAINERS
> index c3f7fbd0d67a..487750d9fd1e 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -10686,6 +10686,13 @@ F:	include/uapi/linux/hid*
>  F:	samples/hid/
>  F:	tools/testing/selftests/hid/
>  
> +HID CORE LAYER [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:	rust/kernel/hid.rs

I assume this is agreed with the HID CORE LAYER maintainers?

There are multiple possible options, for instance:

  1) Maintain the Rust abstractions as part of the existing MAINTAINERS entry.
     Optionally, the author can be added as another maintainer or reviewer.

  2) Add a separate MAINTAINERS entry; patches still go through the same
     subsystem tree.

  3) Add a separate MAINTAINERS entry; patches don't go through the subsystem
     tree (e.g. because the subsystem maintainers don't want to deal with it).

I usually recommend (1), but that's of course up to you and the HID maintainers.

Here it seems you want to go with option (2). Given that I recommend to add the
HID maintainers as reviewers (if they like) to the new MAINTAINERS entry, so
they can easily follow on what's going on.

> +/// The HID driver trait.
> +///
> +/// # Example
> +///
> +///```
> +/// use kernel::hid;
> +///
> +/// const USB_VENDOR_ID_VALVE: u32 = 0x28de;
> +/// const USB_DEVICE_ID_STEAM_DECK: u32 = 0x1205;

I think we should make the existing constants from drivers/hid/hid-ids.h
available, rather than defining them again.

<snip>

> +pub trait Driver: Send {
> +    /// The type holding information about each device id supported by the driver.
> +    // TODO: Use `associated_type_defaults` once stabilized:
> +    //
> +    // ```
> +    // type IdInfo: 'static = ();
> +    // ```
> +    type IdInfo: 'static;
> +
> +    /// The table of device ids supported by the driver.
> +    const ID_TABLE: IdTable<Self::IdInfo>;
> +
> +    /// Called before report descriptor parsing. Can be used to mutate the
> +    /// report descriptor before the core HID logic processes the descriptor.
> +    /// Useful for problematic report descriptors that prevent HID devices from
> +    /// functioning correctly.
> +    ///
> +    /// Optional to implement.
> +    fn report_fixup<'a, 'b: 'a>(_hdev: &Device, _rdesc: &'b mut [u8]) -> &'a [u8] {

report_fixup() is a bus device callback and seems to be synchronized against
other bus device callbacks, hence the device argument should be
&Device<device::Core>.

> +        build_error!(VTABLE_DEFAULT_ERROR)
> +    }
> +}
> +
> +/// An adapter for the registration of HID drivers.
> +pub struct Adapter<T: Driver>(T);
> +
> +// SAFETY: A call to `unregister` for a given instance of `RegType` is guaranteed to be valid if
> +// a preceding call to `register` has been successful.
> +unsafe impl<T: Driver + 'static> driver::RegistrationOps for Adapter<T> {
> +    type RegType = bindings::hid_driver;
> +
> +    unsafe fn register(
> +        hdrv: &Opaque<Self::RegType>,
> +        name: &'static CStr,
> +        module: &'static ThisModule,
> +    ) -> Result {
> +        // SAFETY: It's safe to set the fields of `struct hid_driver` on initialization.
> +        let raw_hdrv = unsafe { &mut *hdrv.get() };

I don't think you can guarantee that all fields of `hdrv` are properly
initialized to create this mutable reference. I think you should do what PCI and
platform does instead.

> +impl<T: Driver + 'static> Adapter<T> {
> +    extern "C" fn report_fixup_callback(
> +        hdev: *mut bindings::hid_device,
> +        buf: *mut u8,
> +        size: *mut kernel::ffi::c_uint,
> +    ) -> *const u8 {
> +        // SAFETY: The HID subsystem only ever calls the report_fixup callback
> +        // with a valid pointer to a `struct hid_device`.
> +        //
> +        // INVARIANT: `hdev` is valid for the duration of
> +        // `report_fixup_callback()`.
> +        let hdev = unsafe { &*hdev.cast::<Device>() };

::<Device<device::Core>

> +
> +        // SAFETY: The HID subsystem only ever calls the report_fixup callback
> +        // with a valid pointer to a `kernel::ffi::c_uint`.
> +        //
> +        // INVARIANT: `size` is valid for the duration of
> +        // `report_fixup_callback()`.
> +        let buf_len: usize = match unsafe { *size }.try_into() {
> +            Ok(len) => len,
> +            Err(e) => {
> +                pr_err!(
> +                    "Cannot fix report description due to length conversion failure: {}!\n",
> +                    e
> +                );

You have a valid device at this point, please use it to print with dev_err!().

> +
> +                return buf;
> +            }
> +        };
> +
> +        // Build a mutable Rust slice from buf and size
> +        //
> +        // SAFETY: The HID subsystem only ever calls the report_fixup callback
> +        // with a valid pointer to a `u8` buffer.
> +        //
> +        // INVARIANT: `buf` is valid for the duration of
> +        // `report_fixup_callback()`.
> +        let rdesc_slice = unsafe { core::slice::from_raw_parts_mut(buf, buf_len) };
> +        let rdesc_slice = T::report_fixup(hdev, rdesc_slice);
> +
> +        match rdesc_slice.len().try_into() {
> +            // SAFETY: The HID subsystem only ever calls the report_fixup
> +            // callback with a valid pointer to a `kernel::ffi::c_uint`.
> +            //
> +            // INVARIANT: `size` is valid for the duration of
> +            // `report_fixup_callback()`.
> +            Ok(len) => unsafe { *size = len },
> +            Err(e) => {
> +                pr_err!("Fixed report description will not be used due to {}!\n", e);

Same here.

> +
> +                return buf;
> +            }
> +        }
> +
> +        rdesc_slice.as_ptr()
> +    }
> +}

You also need to call the macros that generate the device context Deref
hierarchy and ARef<Device> conversion for you, i.e.

	// SAFETY: `Device` is a transparent wrapper of a type that doesn't depend on `Device`'s generic
	// argument.
	kernel::impl_device_context_deref!(unsafe { Device });
	kernel::impl_device_context_into_aref!(Device);

You probably don't need that latter, given that the only bus callback you
implement is report_fixup() and you don't implement AlwaysRefCounted for Device
yet.

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

[Danilo Krummrich]

On Sun, Jun 29, 2025 at 04:51:22AM +0000, Rahul Rameshbabu wrote:
> +const USB_VENDOR_ID_SINOWEALTH: u32 = 0x258a;
> +const USB_DEVICE_ID_GLORIOUS_MODEL_O: u32 = 0x0036;

As mentioned in the other patch, I think we shouldn't duplicate them, but use
the ones from drivers/hid/hid-ids.h.

> +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
> +        {
> +            pr_info!("patching Glorious Model O consumer control report descriptor\n");

Please use dev_dbg!() instead.

> +
> +            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
> 
> 

Re: [PATCH v1 2/3] rust: core abstractions for HID drivers

[Jiri Kosina]

On Sun, 29 Jun 2025, Danilo Krummrich wrote:

> (Cc: +Jiri)

Thanks.

> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index c3f7fbd0d67a..487750d9fd1e 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -10686,6 +10686,13 @@ F:	include/uapi/linux/hid*
> >  F:	samples/hid/
> >  F:	tools/testing/selftests/hid/
> >  
> > +HID CORE LAYER [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:	rust/kernel/hid.rs
> 
> I assume this is agreed with the HID CORE LAYER maintainers?
> 
> There are multiple possible options, for instance:
> 
>   1) Maintain the Rust abstractions as part of the existing MAINTAINERS entry.
>      Optionally, the author can be added as another maintainer or reviewer.
> 
>   2) Add a separate MAINTAINERS entry; patches still go through the same
>      subsystem tree.
> 
>   3) Add a separate MAINTAINERS entry; patches don't go through the subsystem
>      tree (e.g. because the subsystem maintainers don't want to deal with it).

I can't speak for Benjamin, but as far as I am concerned, I'd personally 
prefer option (3).

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH v1 2/3] rust: core abstractions for HID drivers

[Benjamin Tissoires]

On Jul 03 2025, Jiri Kosina wrote:
> On Sun, 29 Jun 2025, Danilo Krummrich wrote:
> 
> > (Cc: +Jiri)
> 
> Thanks.
> 
> > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > index c3f7fbd0d67a..487750d9fd1e 100644
> > > --- a/MAINTAINERS
> > > +++ b/MAINTAINERS
> > > @@ -10686,6 +10686,13 @@ F:	include/uapi/linux/hid*
> > >  F:	samples/hid/
> > >  F:	tools/testing/selftests/hid/
> > >  
> > > +HID CORE LAYER [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:	rust/kernel/hid.rs
> > 
> > I assume this is agreed with the HID CORE LAYER maintainers?
> > 
> > There are multiple possible options, for instance:
> > 
> >   1) Maintain the Rust abstractions as part of the existing MAINTAINERS entry.
> >      Optionally, the author can be added as another maintainer or reviewer.
> > 
> >   2) Add a separate MAINTAINERS entry; patches still go through the same
> >      subsystem tree.
> > 
> >   3) Add a separate MAINTAINERS entry; patches don't go through the subsystem
> >      tree (e.g. because the subsystem maintainers don't want to deal with it).
> 
> I can't speak for Benjamin, but as far as I am concerned, I'd personally 
> prefer option (3).

I understand Jiri's concerns, but I'm slighlty worried (3) will end up
also having the leaf drivers in a separate tree, which means that 2
trees will fight for the same resource.

First patch of this series is a first example where some changes are
needed in the HID bus for making rust life's easier.

Personally, I'd like to have a say and an eye on the rust abstractions
and most of it, on the leaf drivers. I can't say I'll proactively
review/merge things (these past few months have shown that I merely
manage to follow things happening ATM), but at least I can keep an eye
and shout if something is wrong.

OTOH, the HID tree (the core part) is a low change tree now, so maybe a
separate tree can be handled correctly without too much troubles.

Danilo, if a separate tree is chosen, is the common practice to directly
send the PR to Linus or does it need to got through the subsystem tree
first (like bpf with net for example).

Cheers,
Benjamin

Re: [PATCH v1 2/3] rust: core abstractions for HID drivers

[Danilo Krummrich]

On 7/3/25 10:01 AM, Benjamin Tissoires wrote:
> On Jul 03 2025, Jiri Kosina wrote:
>> On Sun, 29 Jun 2025, Danilo Krummrich wrote:
>>
>>> (Cc: +Jiri)
>>
>> Thanks.
>>
>>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>>> index c3f7fbd0d67a..487750d9fd1e 100644
>>>> --- a/MAINTAINERS
>>>> +++ b/MAINTAINERS
>>>> @@ -10686,6 +10686,13 @@ F:	include/uapi/linux/hid*
>>>>   F:	samples/hid/
>>>>   F:	tools/testing/selftests/hid/
>>>>   
>>>> +HID CORE LAYER [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:	rust/kernel/hid.rs
>>>
>>> I assume this is agreed with the HID CORE LAYER maintainers?
>>>
>>> There are multiple possible options, for instance:
>>>
>>>    1) Maintain the Rust abstractions as part of the existing MAINTAINERS entry.
>>>       Optionally, the author can be added as another maintainer or reviewer.
>>>
>>>    2) Add a separate MAINTAINERS entry; patches still go through the same
>>>       subsystem tree.
>>>
>>>    3) Add a separate MAINTAINERS entry; patches don't go through the subsystem
>>>       tree (e.g. because the subsystem maintainers don't want to deal with it).
>>
>> I can't speak for Benjamin, but as far as I am concerned, I'd personally
>> prefer option (3).
> 
> I understand Jiri's concerns, but I'm slighlty worried (3) will end up
> also having the leaf drivers in a separate tree, which means that 2
> trees will fight for the same resource.
> 
> First patch of this series is a first example where some changes are
> needed in the HID bus for making rust life's easier.
> 
> Personally, I'd like to have a say and an eye on the rust abstractions
> and most of it, on the leaf drivers. I can't say I'll proactively
> review/merge things (these past few months have shown that I merely
> manage to follow things happening ATM), but at least I can keep an eye
> and shout if something is wrong.
> 
> OTOH, the HID tree (the core part) is a low change tree now, so maybe a
> separate tree can be handled correctly without too much troubles.
> 
> Danilo, if a separate tree is chosen, is the common practice to directly
> send the PR to Linus or does it need to got through the subsystem tree
> first (like bpf with net for example).

We usually fall back to the global Rust tree, which is ultimately maintained by
Miguel, so it's his call in the end.

However, IMHO this is more like a "last resort" in case any other approaches 
within the same subsystem are not desired.

 From what I can read above, it feels to me as if having a separate tree
(maintained by Rahul) with him sending pull requests to you guys seems like a
good option. What do you think about that?

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

[Peter Hutterer]

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
> 
> 
> 

Re: [PATCH v1 2/3] rust: core abstractions for HID drivers

[Rahul Rameshbabu]

On Thu, 03 Jul, 2025 10:20:19 +0200 "Danilo Krummrich" <dakr@kernel.org> wrote:
> On 7/3/25 10:01 AM, Benjamin Tissoires wrote:
>> On Jul 03 2025, Jiri Kosina wrote:
>>> On Sun, 29 Jun 2025, Danilo Krummrich wrote:
>>>
>>>> (Cc: +Jiri)
>>>
>>> Thanks.
>>>
>>>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>>>> index c3f7fbd0d67a..487750d9fd1e 100644
>>>>> --- a/MAINTAINERS
>>>>> +++ b/MAINTAINERS
>>>>> @@ -10686,6 +10686,13 @@ F:	include/uapi/linux/hid*
>>>>>   F:	samples/hid/
>>>>>   F:	tools/testing/selftests/hid/
>>>>>
>>>>> +HID CORE LAYER [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:	rust/kernel/hid.rs
>>>>
>>>> I assume this is agreed with the HID CORE LAYER maintainers?
>>>>
>>>> There are multiple possible options, for instance:
>>>>
>>>>    1) Maintain the Rust abstractions as part of the existing MAINTAINERS entry.
>>>>       Optionally, the author can be added as another maintainer or reviewer.
>>>>
>>>>    2) Add a separate MAINTAINERS entry; patches still go through the same
>>>>       subsystem tree.
>>>>
>>>>    3) Add a separate MAINTAINERS entry; patches don't go through the subsystem
>>>>       tree (e.g. because the subsystem maintainers don't want to deal with it).
>>>
>>> I can't speak for Benjamin, but as far as I am concerned, I'd personally
>>> prefer option (3).
>>
>> I understand Jiri's concerns, but I'm slighlty worried (3) will end up
>> also having the leaf drivers in a separate tree, which means that 2
>> trees will fight for the same resource.
>>
>> First patch of this series is a first example where some changes are
>> needed in the HID bus for making rust life's easier.
>>
>> Personally, I'd like to have a say and an eye on the rust abstractions
>> and most of it, on the leaf drivers. I can't say I'll proactively
>> review/merge things (these past few months have shown that I merely
>> manage to follow things happening ATM), but at least I can keep an eye
>> and shout if something is wrong.
>>
>> OTOH, the HID tree (the core part) is a low change tree now, so maybe a
>> separate tree can be handled correctly without too much troubles.
>>
>> Danilo, if a separate tree is chosen, is the common practice to directly
>> send the PR to Linus or does it need to got through the subsystem tree
>> first (like bpf with net for example).
>
> We usually fall back to the global Rust tree, which is ultimately maintained by
> Miguel, so it's his call in the end.
>
> However, IMHO this is more like a "last resort" in case any other approaches
> within the same subsystem are not desired.
>
>  From what I can read above, it feels to me as if having a separate tree
> (maintained by Rahul) with him sending pull requests to you guys seems like a
> good option. What do you think about that?

First wanted to say thanks to everyone who responded to this series.
I am personally open to any of the above.

From what I can tell, Jiri and Benjamin would prefer pull
requests/discussion of the Rust for Linux bindings not contaminate the
hid git tree or linux-input mailing list. Patches will instead be
submitted through the rust-for-linux mailing list and pull requests
would be sent through the Rust for Linux tree. I can make Benjamin a
reviewer for this effort, so he will be explicitly CCed to take a look
at how the bindings are taking shape. I can make Jiri a reviewer if he
would like to participate (I was assuming maybe not based on the desire
for option 3 listed above). Maybe with this model, I have a
rust-for-linux-hid tree where I send pull requests to the rust-for-linux
tree?

On the other hand, Danilo would prefer something similar to the above
with the caveat of pull requests going through the hid tree and patches
through linux-input mailing list.

I am open to either personally.

I will be sending a v2 hopefully soon based on Danilo's and Peter's
feedback.

In parallel, I am thinking about how to design a Drop trait for handling
HID device unplug instead of needing a C style remove callback. This
will be crucial for implementing sophisticated leaf drivers. The driver
implemented here purely does report fixup. In my ideal world, all pure
report fixup work would be done in HID-BPF space and only sophisticated
drivers would be implemented as a Rust leaf driver. I have some separate
scratch work playing around HID-BPF that I have not had time to clean
up. Right now, it's just samples for doing things like key remapping on
a keyboard. I am envisioning an Alsa UCM like model for HID-BPF for
dealing with report fixups.

-- 
Thanks,

Rahul Rameshbabu

Re: [PATCH v1 2/3] rust: core abstractions for HID drivers

[Miguel Ojeda]

On Sat, Jul 5, 2025 at 9:31 AM Rahul Rameshbabu
<sergeantsagara@protonmail.com> wrote:
>
> From what I can tell,

I think Benjamin wants to keep an eye on things but may not have much
time to take care of patches and so on.

So it sounds like having a separate `MAINTAINERS` subentry under them
is best, with you applying patches and then sending PRs to them.

That way, they remain in control, they can coordinate any changes that
involve both the C and the Rust side if any, and so on; while they
don't need to handle directly Rust patches themselves, fixing issues
with the Rust side, etc., thus removing work from their side; i.e. you
would be doing that since you want to drive and use those Rust
abstractions.

That arrangement is great and what other subsystems have done (most of
the entries with "... [RUST]"). It also helps more people
involved/trained in the overall subsystem too (should they want more
reviewers/maintainers), and also allows you to become a maintainer
yourself, learning to send PRs, etc. (which you may welcome -- if you
already took patches, sent PRs etc., then please excuse me! I only did
a quick search so I may have missed it.)

We try to reserve the global Rust tree fallback for cases where there
is really no other way, e.g. when a key subsystem doesn't want
anything to do with Rust at all and thus would block many others
otherwise.

Thanks!

Cheers,
Miguel

Re: [PATCH v1 2/3] rust: core abstractions for HID drivers

[Danilo Krummrich]

On Sat, Jul 05, 2025 at 07:31:21AM +0000, Rahul Rameshbabu wrote:
> In parallel, I am thinking about how to design a Drop trait for handling
> HID device unplug instead of needing a C style remove callback. This
> will be crucial for implementing sophisticated leaf drivers.

Please have a look at how other bus abstractions are implemented, e.g. platform,
pci, auxiliary.

They all have a probe() function in the Driver trait, which returns a
Pin<KBox<Self>>, i.e. the drivers private data.

This ForeignOwnable is then stored in the private data pointer of the C struct
device, which I'm adding generic accessors for in [1].

On driver remove() this ForeignOwnable is extracted back into a Pin<KBox<Self>>
and dropped.

This means there's already Driver::drop() which is called on device unplug.

However, this is not sufficient, because Driver::drop() only gives you a &mut
self, i.e. a reference to Pin<KBox<Self>>, but not a Core context reference to
the Device, i.e. &Device<Core>. Hence you need a separate Driver::unbind()
callback. Please see [2] for that.

[1] https://lore.kernel.org/all/20250621195118.124245-3-dakr@kernel.org/
[2] https://lore.kernel.org/all/20250621195118.124245-7-dakr@kernel.org/

Re: [PATCH v1 0/3] Initial work for Rust abstraction for HID device driver development

[Aditya Garg]

On 29/06/25 10:21 am, Rahul Rameshbabu wrote:
> Hello again,
> 
> I have come back with a revision of my HID binding work based on feedback from
> the maintainers and the community. Going with Benjamin Tissoires's feedback, I
> have opted to implement support for a much simpler HID device as a starting
> point for this work. I have gone ahead with making a Rust reference driver for
> the Glorious PC Gaming Race Model O and O- mice. I chose these devices because
> they were easily accessible for my development work. I have gone ahead with
> naming this driver the "Glorious Rust" driver. My binding work now adopts the
> core infrastructure Danilo Krummrich has provided for the Rust for Linux
> community.
> 
> I have decided to move this series out of RFC status, since I feel the
> implemented code is in a stable state. It could be used for other drivers as-is
> with the caveat that those drivers are only performing report fixups. Hopefully,
> a HID-BPF solution can be taken for production report fixup purposes. In
> reality, I will need to bring more of the HID subsystem functionality to Rust to
> promote implementations of more complex drivers, such as the USB Monitor Control
> driver I am planning to implement (as expressed in my RFC series). I am hoping
> to do a better job of documenting my progress involving this work on my blog
> while I am working on each iteration/addition for anyone interested in following
> along outside my mailing list posts.

Interesting. Reminds me of the days trying to rewrite hid-apple in rust, but found
no rust bindings.

Re: [PATCH v1 2/3] rust: core abstractions for HID drivers

[Rahul Rameshbabu]

On Sat, 05 Jul, 2025 12:41:02 +0200 "Miguel Ojeda" <miguel.ojeda.sandonis@gmail.com> wrote:
> On Sat, Jul 5, 2025 at 9:31 AM Rahul Rameshbabu
> <sergeantsagara@protonmail.com> wrote:
>>
>> From what I can tell,
>
> I think Benjamin wants to keep an eye on things but may not have much
> time to take care of patches and so on.
>
> So it sounds like having a separate `MAINTAINERS` subentry under them
> is best, with you applying patches and then sending PRs to them.
>
> That way, they remain in control, they can coordinate any changes that
> involve both the C and the Rust side if any, and so on; while they
> don't need to handle directly Rust patches themselves, fixing issues
> with the Rust side, etc., thus removing work from their side; i.e. you
> would be doing that since you want to drive and use those Rust
> abstractions.
>
> That arrangement is great and what other subsystems have done (most of
> the entries with "... [RUST]"). It also helps more people
> involved/trained in the overall subsystem too (should they want more
> reviewers/maintainers), and also allows you to become a maintainer
> yourself, learning to send PRs, etc. (which you may welcome -- if you
> already took patches, sent PRs etc., then please excuse me! I only did
> a quick search so I may have missed it.)

Thanks Miguel. I would really enjoy the opportunity. Like you noticed, I
do not have this experience on the mailing list. I also want to give
Benjamin and Jiri time to consider these options. Let me get my v2
patches sent out in the meantime.

Thanks,
Rahul Rameshbabu

>
> We try to reserve the global Rust tree fallback for cases where there
> is really no other way, e.g. when a key subsystem doesn't want
> anything to do with Rust at all and thus would block many others
> otherwise.
>
> Thanks!
>
> Cheers,
> Miguel