linux-input.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Rahul Rameshbabu <sergeantsagara@protonmail.com>
To: linux-kernel@vger.kernel.org, rust-for-linux@vger.kernel.org,
	linux-input@vger.kernel.org, dri-devel@lists.freedesktop.org
Cc: "Jiri Kosina" <jikos@kernel.org>,
	"Benjamin Tissoires" <bentiss@kernel.org>,
	"Miguel Ojeda" <ojeda@kernel.org>,
	"Alex Gaynor" <alex.gaynor@gmail.com>,
	"Boqun Feng" <boqun.feng@gmail.com>,
	"Gary Guo" <gary@garyguo.net>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Benno Lossin" <benno.lossin@proton.me>,
	"Andreas Hindborg" <a.hindborg@kernel.org>,
	"Alice Ryhl" <aliceryhl@google.com>,
	"Trevor Gross" <tmgross@umich.edu>,
	"Danilo Krummrich" <dakr@kernel.org>,
	"Rahul Rameshbabu" <sergeantsagara@protonmail.com>
Subject: [PATCH RFC 1/3] rust: core abstractions for HID drivers
Date: Thu, 13 Mar 2025 16:03:35 +0000	[thread overview]
Message-ID: <20250313160220.6410-4-sergeantsagara@protonmail.com> (raw)
In-Reply-To: <20250313160220.6410-2-sergeantsagara@protonmail.com>

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.

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

Some concerns with this initial draft
  - The need for a DeviceId and DeviceIdShallow type.
    + DeviceIdShallow is used to guarantee the safety requirement for the
      Sync trait.
  - The create_hid_driver call in the module_hid_driver! macro does not use
    Pin semantics for passing the ID_TABLE. I could not get Pin semantics
    to work in a const fn. I get a feeling this might be safe but need help
    reviewing this.

Signed-off-by: Rahul Rameshbabu <sergeantsagara@protonmail.com>
---
 drivers/hid/Kconfig             |   8 ++
 rust/bindings/bindings_helper.h |   1 +
 rust/kernel/hid.rs              | 245 ++++++++++++++++++++++++++++++++
 rust/kernel/lib.rs              |   2 +
 4 files changed, 256 insertions(+)
 create mode 100644 rust/kernel/hid.rs

diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index b53eb569bd49..e085964c7ffc 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -714,6 +714,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 55354e4dec14..e2e95afe9f4a 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -16,6 +16,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..f13476b49e7d
--- /dev/null
+++ b/rust/kernel/hid.rs
@@ -0,0 +1,245 @@
+// SPDX-License-Identifier: GPL-2.0
+
+// Copyright (C) 2025 Rahul Rameshbabu <sergeantsagara@protonmail.com>
+
+use crate::{error::*, prelude::*, types::Opaque};
+use core::marker::PhantomData;
+
+#[repr(transparent)]
+pub struct Device(Opaque<bindings::hid_device>);
+
+impl Device {
+    unsafe fn from_ptr<'a>(ptr: *mut bindings::hid_device) -> &'a mut Self {
+        let ptr = ptr.cast::<Self>();
+
+        unsafe { &mut *ptr }
+    }
+
+    pub fn vendor(&self) -> u32 {
+        let hdev = self.0.get();
+
+        unsafe { (*hdev).vendor }
+    }
+
+    pub fn product(&self) -> u32 {
+        let hdev = self.0.get();
+
+        unsafe { (*hdev).product }
+    }
+}
+
+#[repr(transparent)]
+pub struct DeviceIdShallow(Opaque<bindings::hid_device_id>);
+
+// SAFETY: `DeviceIdShallow` doesn't expose any &self method to access internal data, so it's safe to
+// share `&DriverVTable` across execution context boundaries.
+unsafe impl Sync for DeviceIdShallow {}
+
+impl DeviceIdShallow {
+    pub const fn new() -> Self {
+        DeviceIdShallow(Opaque::new(bindings::hid_device_id {
+            // SAFETY: The rest is zeroed out to initialize `struct hid_device_id`,
+            // sets `Option<&F>` to be `None`.
+            ..unsafe { ::core::mem::MaybeUninit::<bindings::hid_device_id>::zeroed().assume_init() }
+        }))
+    }
+
+    pub const fn new_usb(vendor: u32, product: u32) -> Self {
+        DeviceIdShallow(Opaque::new(bindings::hid_device_id {
+            bus: 0x3, /* BUS_USB */
+            vendor: vendor,
+            product: product,
+            // SAFETY: The rest is zeroed out to initialize `struct hid_device_id`,
+            // sets `Option<&F>` to be `None`.
+            ..unsafe { ::core::mem::MaybeUninit::<bindings::hid_device_id>::zeroed().assume_init() }
+        }))
+    }
+
+    const unsafe fn as_ptr(&self) -> *const bindings::hid_device_id {
+        self.0.get()
+    }
+}
+
+#[repr(transparent)]
+pub struct DeviceId(Opaque<bindings::hid_device_id>);
+
+impl DeviceId {
+    unsafe fn from_ptr<'a>(ptr: *mut bindings::hid_device_id) -> &'a mut Self {
+        let ptr = ptr.cast::<Self>();
+
+        unsafe { &mut *ptr }
+    }
+
+    unsafe fn from_const_ptr<'a>(ptr: *const bindings::hid_device_id) -> &'a Self {
+        let ptr = ptr.cast::<Self>();
+
+        unsafe { &(*ptr) }
+    }
+
+    pub fn vendor(&self) -> u32 {
+        let hdev_id = self.0.get();
+
+        unsafe { (*hdev_id).vendor }
+    }
+
+    pub fn product(&self) -> u32 {
+        let hdev_id = self.0.get();
+
+        unsafe { (*hdev_id).product }
+    }
+}
+
+/*
+#[repr(transparent)]
+pub struct Field(Opaque<bindings::hid_field>);
+
+#[repr(transparent)]
+pub struct ReportEnum(Opaque<bindings::hid_report_enum>);
+
+#[repr(transparent)]
+pub struct Report(Opaque<bindings::hid_report>);
+*/
+
+#[vtable]
+pub trait Driver {
+    fn probe(_dev: &mut Device, _id: &DeviceId) -> Result {
+        build_error!(VTABLE_DEFAULT_ERROR)
+    }
+
+    fn remove(_dev: &mut Device) {
+    }
+}
+
+struct Adapter<T: Driver> {
+    _p: PhantomData<T>,
+}
+
+impl<T: Driver> Adapter<T> {
+    unsafe extern "C" fn probe_callback(
+        hdev: *mut bindings::hid_device,
+        hdev_id: *const bindings::hid_device_id,
+    ) -> crate::ffi::c_int {
+        from_result(|| {
+            let dev = unsafe { Device::from_ptr(hdev) };
+            let dev_id = unsafe { DeviceId::from_const_ptr(hdev_id) };
+            T::probe(dev, dev_id)?;
+            Ok(0)
+        })
+    }
+
+    unsafe extern "C" fn remove_callback(hdev: *mut bindings::hid_device) {
+        let dev = unsafe { Device::from_ptr(hdev) };
+        T::remove(dev);
+    }
+}
+
+#[repr(transparent)]
+pub struct DriverVTable(Opaque<bindings::hid_driver>);
+
+// SAFETY: `DriverVTable` doesn't expose any &self method to access internal data, so it's safe to
+// share `&DriverVTable` across execution context boundaries.
+unsafe impl Sync for DriverVTable {}
+
+pub const fn create_hid_driver<T: Driver>(
+    name: &'static CStr,
+    id_table: &'static DeviceIdShallow,
+) -> DriverVTable {
+    DriverVTable(Opaque::new(bindings::hid_driver {
+        name: name.as_char_ptr().cast_mut(),
+        id_table: unsafe { id_table.as_ptr() },
+        probe: if T::HAS_PROBE {
+            Some(Adapter::<T>::probe_callback)
+        } else {
+            None
+        },
+        remove: if T::HAS_REMOVE {
+            Some(Adapter::<T>::remove_callback)
+        } else {
+            None
+        },
+        // SAFETY: The rest is zeroed out to initialize `struct hid_driver`,
+        // sets `Option<&F>` to be `None`.
+        ..unsafe { core::mem::MaybeUninit::<bindings::hid_driver>::zeroed().assume_init() }
+    }))
+}
+
+pub struct Registration {
+    driver: Pin<&'static mut DriverVTable>,
+}
+
+unsafe impl Send for Registration {}
+
+impl Registration {
+    pub fn register(
+        module: &'static crate::ThisModule,
+        driver: Pin<&'static mut DriverVTable>,
+        name: &'static CStr,
+    ) -> Result<Self> {
+        to_result(unsafe {
+            bindings::__hid_register_driver(driver.0.get(), module.0, name.as_char_ptr())
+        })?;
+
+        Ok(Registration { driver })
+    }
+}
+
+impl Drop for Registration {
+    fn drop(&mut self) {
+        unsafe {
+            bindings::hid_unregister_driver(self.driver.0.get())
+        };
+    }
+}
+
+#[macro_export]
+macro_rules! usb_device {
+    (vendor: $vendor:expr, product: $product:expr $(,)?) => {
+        $crate::hid::DeviceIdShallow::new_usb($vendor, $product)
+    }
+}
+
+#[macro_export]
+macro_rules! module_hid_driver {
+    (@replace_expr $_t:tt $sub:expr) => {$sub};
+
+    (@count_devices $($x:expr),*) => {
+        0usize $(+ $crate::module_hid_driver!(@replace_expr $x 1usize))*
+    };
+
+    (driver: $driver:ident, id_table: [$($dev_id:expr),+ $(,)?], name: $name:tt, $($f:tt)*) => {
+        struct Module {
+            _reg: $crate::hid::Registration,
+        }
+
+        $crate::prelude::module! {
+            type: Module,
+            name: $name,
+            $($f)*
+        }
+
+        const _: () = {
+            static NAME: &$crate::str::CStr = $crate::c_str!($name);
+
+            static ID_TABLE: [$crate::hid::DeviceIdShallow;
+                $crate::module_hid_driver!(@count_devices $($dev_id),+) + 1] = [
+                $($dev_id),+,
+                $crate::hid::DeviceIdShallow::new(),
+            ];
+
+            static mut DRIVER: $crate::hid::DriverVTable =
+                $crate::hid::create_hid_driver::<$driver>(NAME, unsafe { &ID_TABLE[0] });
+
+            impl $crate::Module for Module {
+                fn init(module: &'static $crate::ThisModule) -> Result<Self> {
+                    let driver = unsafe { &mut DRIVER };
+                    let mut reg = $crate::hid::Registration::register(
+                        module,
+                        ::core::pin::Pin::static_mut(driver),
+                        NAME,
+                    )?;
+                    Ok(Module { _reg: reg })
+                }
+            }
+        };
+    }
+}
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 496ed32b0911..51b8c2689264 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -49,6 +49,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.47.2



  reply	other threads:[~2025-03-13 16:03 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-03-13 16:02 [PATCH RFC 0/3] Initial work for Rust abstraction for HID device driver development Rahul Rameshbabu
2025-03-13 16:03 ` Rahul Rameshbabu [this message]
2025-03-13 16:54   ` [PATCH RFC 1/3] rust: core abstractions for HID drivers Benjamin Tissoires
2025-03-13 19:25   ` Danilo Krummrich
2025-03-16  2:07     ` Rahul Rameshbabu
2025-03-13 16:03 ` [PATCH RFC 2/3] rust: hid: USB Monitor Control Class driver Rahul Rameshbabu
2025-03-13 16:58   ` Benjamin Tissoires
2025-03-14 14:41     ` Miguel Ojeda
2025-03-16  2:20       ` Rahul Rameshbabu
2025-03-13 16:04 ` [PATCH RFC 3/3] rust: hid: demo the core abstractions for probe and remove Rahul Rameshbabu
2025-03-13 17:05   ` Benjamin Tissoires
2025-03-16  4:20     ` Rahul Rameshbabu
2025-03-16 10:02       ` Daniel Brooks
2025-03-13 16:31 ` [PATCH RFC 0/3] Initial work for Rust abstraction for HID device driver development Benjamin Tissoires
2025-03-15 23:07   ` Rahul Rameshbabu
2025-03-13 18:04 ` Benno Lossin
  -- strict thread matches above, loose matches on Subject: below --
2025-03-16  1:09 [PATCH RFC 1/3] rust: core abstractions for HID drivers Rahul Rameshbabu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250313160220.6410-4-sergeantsagara@protonmail.com \
    --to=sergeantsagara@protonmail.com \
    --cc=a.hindborg@kernel.org \
    --cc=alex.gaynor@gmail.com \
    --cc=aliceryhl@google.com \
    --cc=benno.lossin@proton.me \
    --cc=bentiss@kernel.org \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun.feng@gmail.com \
    --cc=dakr@kernel.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=gary@garyguo.net \
    --cc=jikos@kernel.org \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=ojeda@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=tmgross@umich.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).