Re: [PATCH v3 14/16] rust: of: add `of::DeviceId` abstraction

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Oct 22, 2024 at 11:33 PM Danilo Krummrich <dakr@xxxxxxxxxx> 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>
>  #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.
> +//!
> +//! C header: [`include/linux/of_*.h`](srctree/include/linux/of_*.h)
> +
> +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`.

Your #[repr(transparent)] marker is missing.

> +// * `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 {

Since you make a copy of `compatible`, you don't need 'static.

> +        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;
> +        }
> +
> +        Self(of)
> +    }
> +
> +    /// The compatible string of the embedded `struct bindings::of_device_id` as `&CStr`.
> +    pub fn compatible<'a>(&self) -> &'a CStr {

This should probably be:

pub fn compatible(&self) -> &CStr {

Right now, the returned CStr is not tied to self at all, even though
it points inside `self`.

Alice





[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux