| Message ID | 20250530123815.1766726-1-igor.korotin.linux@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | rust: acpi: add `acpi::DeviceId` abstraction | expand |
> Alternatively, if you want to upstream this dependency already you can send the > following patches: > > - this acpi::DeviceId abstraction > - the glue code for the generic adapter trait in rust/kernel/driver.rs > - use this glue code in the platform abstraction > - add acpi support to the platform sample driver > > This way we can already validate that the code works correctly. All this is > required anyways if the I2C device you write a driver for is on the platform > bus. A few questions if I may: 1. I committed to 4 different files: `acpi.rs`, `driver.rs`, `platform.rs`, platform rust sample driver. Should I commit all of this as one commit or split each part to a separate commit and send it as a patch sequence? 2. From author's point of view, as Danilo noticed, `acpi table` abstraction code is in general just copy-paste from `of table` abstraction code. How should I explicitly mark that fact? Thanks Igor
On Tue, Jun 03, 2025 at 01:55:47PM +0100, Igor Korotin wrote: > > Alternatively, if you want to upstream this dependency already you can send the > > following patches: > > > > - this acpi::DeviceId abstraction > > - the glue code for the generic adapter trait in rust/kernel/driver.rs > > - use this glue code in the platform abstraction > > - add acpi support to the platform sample driver > > > > This way we can already validate that the code works correctly. All this is > > required anyways if the I2C device you write a driver for is on the platform > > bus. > > A few questions if I may: > 1. I committed to 4 different files: `acpi.rs`, `driver.rs`, > `platform.rs`, platform rust sample driver. > Should I commit all of this as one commit or split each part to a > separate commit and send it as a patch sequence? Every entry of my list above should be a separate commit. It might happen that writing the glue code for the generic adapter trait in rust/kernel/driver.rs breaks the build in the platform abstraction, then you have to fix it up in the same commit, i.e. we never break the build. Please also see [1]. > 2. From author's point of view, as Danilo noticed, `acpi table` > abstraction code is in general just copy-paste from `of table` > abstraction code. How should I explicitly mark that fact? You don't need to do anything specific here. You authored the commit, even though it's based on existing code. If you want you can add a note in the commit message that your case is based on the OF table abstraction. But AFAIC, you don't have to. [1] https://docs.kernel.org/process/submitting-patches.html#separate-your-changes
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() }; + // TODO: Use `clone_from_slice` once the corresponding types do match. + let mut i = 0; + while i < src.len() { + acpi.id[i] = src[i] as _; + 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; pub mod page; #[cfg(CONFIG_I2C)] pub mod i2c;
`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@gmail.com> --- MAINTAINERS | 1 + rust/kernel/acpi.rs | 63 +++++++++++++++++++++++++++++++++++++++++++++ rust/kernel/lib.rs | 1 + 3 files changed, 65 insertions(+) create mode 100644 rust/kernel/acpi.rs