On Tue, Oct 22, 2024 at 11:31:51PM +0200, Danilo Krummrich wrote: > `of::DeviceId` is an abstraction around `struct of_device_id`. > > This is used by subsequent patches, in particular the platform bus > abstractions, to create OF device ID tables. > > Signed-off-by: Danilo Krummrich <dakr@xxxxxxxxxx> > --- > MAINTAINERS | 1 + > rust/bindings/bindings_helper.h | 1 + > rust/kernel/lib.rs | 1 + > rust/kernel/of.rs | 63 +++++++++++++++++++++++++++++++++ > 4 files changed, 66 insertions(+) > create mode 100644 rust/kernel/of.rs > > diff --git a/MAINTAINERS b/MAINTAINERS > index d9c512a3e72b..87eb9a7869eb 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -17340,6 +17340,7 @@ T: git git://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git > F: Documentation/ABI/testing/sysfs-firmware-ofw > F: drivers/of/ > F: include/linux/of*.h > +F: rust/kernel/of.rs > F: scripts/dtc/ > F: tools/testing/selftests/dt/ > K: of_overlay_notifier_ > diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h > index cd4edd6496ae..312f03cbdce9 100644 > --- a/rust/bindings/bindings_helper.h > +++ b/rust/bindings/bindings_helper.h > @@ -15,6 +15,7 @@ > #include <linux/firmware.h> > #include <linux/jiffies.h> > #include <linux/mdio.h> > +#include <linux/of_device.h> Technically, you don't need this for *this* patch. You need mod_devicetable.h for of_device_id. Best to not rely on implicit includes. I've tried removing it and it still built, so I guess there is another implicit include somewhere... > #include <linux/pci.h> > #include <linux/phy.h> > #include <linux/refcount.h> > diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs > index 3ec690eb6d43..5946f59f1688 100644 > --- a/rust/kernel/lib.rs > +++ b/rust/kernel/lib.rs > @@ -51,6 +51,7 @@ > pub mod list; > #[cfg(CONFIG_NET)] > pub mod net; > +pub mod of; > pub mod page; > pub mod prelude; > pub mod print; > diff --git a/rust/kernel/of.rs b/rust/kernel/of.rs > new file mode 100644 > index 000000000000..a37629997974 > --- /dev/null > +++ b/rust/kernel/of.rs > @@ -0,0 +1,63 @@ > +// SPDX-License-Identifier: GPL-2.0 > + > +//! Open Firmware abstractions. s/Open Firmware/Devicetree/ Or keep both that prior versions of this code had. Most of DT/OF today is not OpenFirmware. > +//! > +//! C header: [`include/linux/of_*.h`](srctree/include/linux/of_*.h) I haven't quite figured out how this gets used. I guess just a link in documentation? I somewhat doubt this file is going to handle all DT abstractions. That might become quite long. Most of of_address.h and of_irq.h I actively don't want to see Rust bindings for because they are mainly used by higher level interfaces (e.g. platform dev resources). There's a slew of "don't add new users" APIs which I need to document. Also, the main header is of.h which wasn't included here. As of now, only the mod_devicetable.h header is used by this file, so I think you should just put it until that changes. Maybe there would be some savings if all of mod_devicetable.h was handled by 1 rust file? > + > +use crate::{bindings, device_id::RawDeviceId, prelude::*}; > + > +/// An open firmware device id. > +#[derive(Clone, Copy)] > +pub struct DeviceId(bindings::of_device_id); > + > +// SAFETY: > +// * `DeviceId` is a `#[repr(transparent)` wrapper of `struct of_device_id` and does not add > +// additional invariants, so it's safe to transmute to `RawType`. > +// * `DRIVER_DATA_OFFSET` is the offset to the `data` field. > +unsafe impl RawDeviceId for DeviceId { > + type RawType = bindings::of_device_id; > + > + const DRIVER_DATA_OFFSET: usize = core::mem::offset_of!(bindings::of_device_id, data); > + > + fn index(&self) -> usize { > + self.0.data as _ > + } > +} > + > +impl DeviceId { > + /// Create a new device id from an OF 'compatible' string. > + pub const fn new(compatible: &'static CStr) -> Self { > + let src = compatible.as_bytes_with_nul(); > + // Replace with `bindings::of_device_id::default()` once stabilized for `const`. > + // SAFETY: FFI type is valid to be zero-initialized. > + let mut of: bindings::of_device_id = unsafe { core::mem::zeroed() }; > + > + let mut i = 0; > + while i < src.len() { > + of.compatible[i] = src[i] as _; > + i += 1; > + } AFAICT, this loop will go away when C char maps to u8. Perhaps a note to that extent or commented code implementing that. > + > + Self(of) > + } > + > + /// The compatible string of the embedded `struct bindings::of_device_id` as `&CStr`. > + pub fn compatible<'a>(&self) -> &'a CStr { > + // SAFETY: `self.compatible` is a valid `char` pointer. > + unsafe { CStr::from_char_ptr(self.0.compatible.as_ptr()) } > + } I don't think we need this. The usage model is checking does a node's compatible string(s) match a compatible in the table. Most of the time we don't even need that. We just need the match data. Rob