[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aDnD_Bb3l6GiI_8K@cassiopeiae>
Date: Fri, 30 May 2025 16:43:08 +0200
From: Danilo Krummrich <dakr@...nel.org>
To: Igor Korotin <igor.korotin.linux@...il.com>
Cc: "Rafael J . Wysocki" <rafael@...nel.org>, Len Brown <lenb@...nel.org>,
Miguel Ojeda <ojeda@...nel.org>,
Alex Gaynor <alex.gaynor@...il.com>,
Boqun Feng <boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>,
Björn Roy Baron <bjorn3_gh@...tonmail.com>,
Benno Lossin <benno.lossin@...ton.me>,
Andreas Hindborg <a.hindborg@...nel.org>,
Alice Ryhl <aliceryhl@...gle.com>, Trevor Gross <tmgross@...ch.edu>,
linux-acpi@...r.kernel.org, rust-for-linux@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH] rust: acpi: add `acpi::DeviceId` abstraction
On Fri, May 30, 2025 at 01:38:06PM +0100, Igor Korotin wrote:
> `acpi::DeviceId` is an abstraction around `struct acpi_device_id`.
>
> This is used by subsequent patches, in particular the i2c driver
> abstractions, to create ACPI device ID tables.
>
> Signed-off-by: Igor Korotin <igor.korotin.linux@...il.com>
As Greg mentioned it would be nice to see the subsequent patches.
Also, for this to be useful you need to implement the generic glue code in
rust/kernel/driver.rs in the generic Adapter trait.
You also may want to connect it to the platform bus abstraction, just like the
OF bindings are, since I assume that's the bus you care about?
> ---
> MAINTAINERS | 1 +
> rust/kernel/acpi.rs | 63 +++++++++++++++++++++++++++++++++++++++++++++
> rust/kernel/lib.rs | 1 +
> 3 files changed, 65 insertions(+)
> create mode 100644 rust/kernel/acpi.rs
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index b659fb27ab63..5f8dfae08454 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -302,6 +302,7 @@ F: include/linux/acpi.h
> F: include/linux/fwnode.h
> F: include/linux/fw_table.h
> F: lib/fw_table.c
> +F: rust/kernel/acpi.rs
> F: tools/power/acpi/
>
> ACPI APEI
> diff --git a/rust/kernel/acpi.rs b/rust/kernel/acpi.rs
> new file mode 100644
> index 000000000000..bbd38910736c
> --- /dev/null
> +++ b/rust/kernel/acpi.rs
> @@ -0,0 +1,63 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Advanced Configuration and Power Interface abstractions.
> +
> +use crate::{bindings, device_id::RawDeviceId, prelude::*};
> +
> +/// IdTable type for ACPI drivers.
> +pub type IdTable<T> = &'static dyn kernel::device_id::IdTable<DeviceId, T>;
> +
> +/// An ACPI device id.
> +#[repr(transparent)]
> +#[derive(Clone, Copy)]
> +pub struct DeviceId(bindings::acpi_device_id);
> +
> +// SAFETY:
> +// * `DeviceId` is a `#[repr(transparent)` wrapper of `struct acpi_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::acpi_device_id;
> +
> + const DRIVER_DATA_OFFSET: usize = core::mem::offset_of!(bindings::acpi_device_id, driver_data);
> +
> + fn index(&self) -> usize {
> + self.0.driver_data as _
> + }
> +}
> +
> +impl DeviceId {
> + const ACPI_ID_LEN: usize = 16;
> +
> + /// Create a new device id from an ACPI 'id' string.
> + pub const fn new(id: &'static CStr) -> Self {
> + assert!(id.len() <= Self::ACPI_ID_LEN, "ID exceeds 16 bytes");
> + let src = id.as_bytes_with_nul();
> + // Replace with `bindings::acpi_device_id::default()` once stabilized for `const`.
> + // SAFETY: FFI type is valid to be zero-initialized.
> + let mut acpi: bindings::acpi_device_id = unsafe { core::mem::zeroed() };
The patch did not land yet, but for this there's the following improvement
upcoming:
https://lore.kernel.org/rust-for-linux/20250523145125.523275-13-lossin@kernel.org/
> + // TODO: Use `clone_from_slice` once the corresponding types do match.
clone_from_slice() can't be used from const context, this comment is wrong in
of.rs (where you probably copied it from).
> + let mut i = 0;
> + while i < src.len() {
> + acpi.id[i] = src[i] as _;
You can drop this cast, it shouldn't be needed anymore.
> + i += 1;
> + }
> +
> + Self(acpi)
> + }
> +}
> +
> +/// Create an ACPI `IdTable` with an "alias" for modpost.
> +#[macro_export]
> +macro_rules! acpi_device_table {
> + ($table_name:ident, $module_table_name:ident, $id_info_type: ty, $table_data: expr) => {
> + const $table_name: $crate::device_id::IdArray<
> + $crate::acpi::DeviceId,
> + $id_info_type,
> + { $table_data.len() },
> + > = $crate::device_id::IdArray::new($table_data);
> +
> + $crate::module_device_table!("acpi", $module_table_name, $table_name);
> + };
> +}
> +
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index 15c5f72976cd..05f1d3870bf7 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -78,6 +78,7 @@
> #[cfg(CONFIG_NET)]
> pub mod net;
> pub mod of;
> +pub mod acpi;
As mentioned by Greg, please keep this ordered.
Powered by blists - more mailing lists