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

[Danilo Krummrich <dakr@kernel.org>]

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