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@xxxxxxxxxx/ [2] https://lore.kernel.org/rust-for-linux/20250313022454.147118-1-dakr@xxxxxxxxxx/ > Signed-off-by: Rahul Rameshbabu <sergeantsagara@xxxxxxxxxxxxxx> > --- > 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