Re: [PATCH RFC 1/3] rust: core abstractions for HID drivers

Danilo Krummrich
2025-03-13T19:25:29Z

On Thu, Mar 13, 2025 at 04:03:35PM +0000, Rahul Rameshbabu wrote:
> 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.

For a lot of things in this patch we have common infrastructure, please see
rust/kernel/{device.rs, driver.rs, device_id.rs}. I think you should make use of
the common infrastructure that solves the corresponding problems already.

It provides generic infrastructure for handling device IDs, a generalized
Registration type, based on InPlaceModule with a common module_driver!
implementation for busses to implement their corresponding module macro, etc.

There are two busses upstream, which are based on this infrastructure:
rust/kernel/{pci.rs, platform.rs}.

There is a patch series that improves soundness of those two bus abstractions
[1], which should be taken into consideration too. Even though your
implementation isn't prone to the addressed issue, it would be good to align
things accordingly.

There is a third bus abstraction (auxiliary) on the mailing list [2], which
already implements the mentioned improvements, which you can use as canonical
example too.

[1] https://lore.kernel.org/rust-for-linux/20250313021550.133041-1-dakr@kernel.org/
[2] https://lore.kernel.org/rust-for-linux/20250313022454.147118-1-dakr@kernel.org/

> 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