linux-acpi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/5] rust: Add ACPI match table support for Rust drivers
@ 2025-06-10 14:52 Igor Korotin
  2025-06-10 15:00 ` [PATCH v4 1/5] rust: acpi: add `acpi::DeviceId` abstraction Igor Korotin
                   ` (4 more replies)
  0 siblings, 5 replies; 8+ messages in thread
From: Igor Korotin @ 2025-06-10 14:52 UTC (permalink / raw)
  To: ojeda, alex.gaynor, rafael, gregkh, linux-kernel, rust-for-linux,
	linux-acpi
  Cc: boqun.feng, gary, bjorn3_gh, benno.lossin, a.hindborg, aliceryhl,
	tmgross, dakr, lenb, wedsonaf, viresh.kumar, alex.hung,
	dingxiangfei2009

This patch series introduces support for ACPI match tables in Rust 
drivers.

Currently, Rust abstractions support only Open Firmware (OF) device 
matching. This series extends the driver model to support ACPI-based 
matching, enabling Rust drivers to bind to ACPI-described devices.

Changes include:
  - A new `acpi::DeviceId` abstraction for working with 
   `struct acpi_device_id`.
  - Updates to the core `Adapter` trait and `platform::Driver` to support
    optional ACPI ID tables.
  - A sample implementation in the Rust platform driver, demonstrating 
    multi-bus matching.

This is especially useful for writing drivers that work across platforms 
using both OF and ACPI.

Tested using QEMU with a custom SSDT that creates an ACPI device matching
the sample Rust platform driver.

Igor Korotin (5):
  rust: acpi: add `acpi::DeviceId` abstraction
  rust: driver: Add ACPI id table support to Adapter trait
  rust: driver: Consolidate `Adapter` methods using `#[cfg]`
  rust: platform: Add ACPI match table support to `Driver` trait
  samples: rust: add ACPI match table example to platform driver

Changelog
---------
v4:
 - Fixed code example for `trait Adapter` in platform.rs 
 - Fixed driver implementation example in rust_driver_platform.rs and moved
   it to `trait Adapter` in platform.rs per Danilo Krummrich's suggestion.
 - Consolidated `Adapter::of_id_info` and `Adapter::acpi_id_info` methods using
   `#[cfg]` per Benno Lossin's suggestion.
 - Link to v3: https://lore.kernel.org/rust-for-linux/20250606170341.3880941-1-igor.korotin.linux@gmail.com/
v3:
 - Removed fwnode type check in `Adapter::id_info` per Greg's and Danilo's
   comments
 - Removed `is_of_node` rust helper, due to unnecessity. 
 - Fixed example code in `rust_driver_platform.rs` per Danilo's comment
 - Added an instruction of testing ACPI using QEMU with a custom SSDT
 - Fixed minor code formatting issues.
 - Link to v2: https://lore.kernel.org/rust-for-linux/20250605161956.3658374-1-igor.korotin.linux@gmail.com/
v2:
 - Removed misleading comment in `acpi::DeviceID` implementation. 
 - Removed unnecessary casting in `acpi::DeviceID::new`.
 - Moved `pub mod acpi` to correct alphabetical position in `rust/kernel/lib.rs`.
 - Link to v1: https://lore.kernel.org/rust-for-linux/20250530123815.1766726-1-igor.korotin.linux@gmail.com/

 MAINTAINERS                          |  1 +
 rust/bindings/bindings_helper.h      |  1 +
 rust/kernel/acpi.rs                  | 61 ++++++++++++++++++++++
 rust/kernel/driver.rs                | 77 ++++++++++++++++++++--------
 rust/kernel/lib.rs                   |  1 +
 rust/kernel/platform.rs              | 52 ++++++++++++++++++-
 samples/rust/rust_driver_platform.rs | 71 ++++++++++++++++++++++++-
 7 files changed, 241 insertions(+), 23 deletions(-)
 create mode 100644 rust/kernel/acpi.rs


base-commit: 19272b37aa4f83ca52bdf9c16d5d81bdd1354494
-- 
2.43.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v4 1/5] rust: acpi: add `acpi::DeviceId` abstraction
  2025-06-10 14:52 [PATCH v4 0/5] rust: Add ACPI match table support for Rust drivers Igor Korotin
@ 2025-06-10 15:00 ` Igor Korotin
  2025-06-10 15:03 ` [PATCH v4 2/5] rust: driver: Add ACPI id table support to Adapter trait Igor Korotin
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 8+ messages in thread
From: Igor Korotin @ 2025-06-10 15:00 UTC (permalink / raw)
  To: ojeda, alex.gaynor, rafael, gregkh, linux-kernel, rust-for-linux,
	linux-acpi
  Cc: boqun.feng, gary, bjorn3_gh, benno.lossin, a.hindborg, aliceryhl,
	tmgross, dakr, lenb, wedsonaf, viresh.kumar, alex.hung,
	dingxiangfei2009

`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 | 61 +++++++++++++++++++++++++++++++++++++++++++++
 rust/kernel/lib.rs  |  1 +
 3 files changed, 63 insertions(+)
 create mode 100644 rust/kernel/acpi.rs

diff --git a/MAINTAINERS b/MAINTAINERS
index a92290fffa16..6c809ea30e6a 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..f9a98dc4eb8a
--- /dev/null
+++ b/rust/kernel/acpi.rs
@@ -0,0 +1,61 @@
+// 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() };
+        let mut i = 0;
+        while i < src.len() {
+            acpi.id[i] = src[i];
+            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 6b4774b2b1c3..5bbf3627212f 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -51,6 +51,7 @@
 
 pub use ffi;
 
+pub mod acpi;
 pub mod alloc;
 #[cfg(CONFIG_AUXILIARY_BUS)]
 pub mod auxiliary;
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v4 2/5] rust: driver: Add ACPI id table support to Adapter trait
  2025-06-10 14:52 [PATCH v4 0/5] rust: Add ACPI match table support for Rust drivers Igor Korotin
  2025-06-10 15:00 ` [PATCH v4 1/5] rust: acpi: add `acpi::DeviceId` abstraction Igor Korotin
@ 2025-06-10 15:03 ` Igor Korotin
  2025-06-10 15:05 ` [PATCH v4 3/5] rust: driver: Consolidate `Adapter` methods using `#[cfg]` Igor Korotin
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 8+ messages in thread
From: Igor Korotin @ 2025-06-10 15:03 UTC (permalink / raw)
  To: ojeda, alex.gaynor, rafael, gregkh, linux-kernel, rust-for-linux,
	linux-acpi
  Cc: boqun.feng, gary, bjorn3_gh, benno.lossin, a.hindborg, aliceryhl,
	tmgross, dakr, lenb, wedsonaf, viresh.kumar, alex.hung,
	dingxiangfei2009

Extend the `Adapter` trait to support ACPI device identification.

This mirrors the existing Open Firmware (OF) support (`of_id_table`) and
enables Rust drivers to match and retrieve ACPI-specific device data
when `CONFIG_ACPI` is enabled.

To avoid breaking compilation, a stub implementation of `acpi_id_table()`
is added to the Platform adapter; the full implementation will be provided
in a subsequent patch.

Signed-off-by: Igor Korotin <igor.korotin.linux@gmail.com>
---
 rust/bindings/bindings_helper.h |  1 +
 rust/kernel/driver.rs           | 39 ++++++++++++++++++++++++++++++++-
 rust/kernel/platform.rs         |  6 ++++-
 3 files changed, 44 insertions(+), 2 deletions(-)

diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index bc494745f67b..dfb2dd500ef6 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -28,6 +28,7 @@
  */
 #include <linux/hrtimer_types.h>
 
+#include <linux/acpi.h>
 #include <drm/drm_device.h>
 #include <drm/drm_drv.h>
 #include <drm/drm_file.h>
diff --git a/rust/kernel/driver.rs b/rust/kernel/driver.rs
index ec9166cedfa7..638f1d270af8 100644
--- a/rust/kernel/driver.rs
+++ b/rust/kernel/driver.rs
@@ -6,7 +6,7 @@
 //! register using the [`Registration`] class.
 
 use crate::error::{Error, Result};
-use crate::{device, of, str::CStr, try_pin_init, types::Opaque, ThisModule};
+use crate::{acpi, device, of, str::CStr, try_pin_init, types::Opaque, ThisModule};
 use core::pin::Pin;
 use pin_init::{pin_data, pinned_drop, PinInit};
 
@@ -141,6 +141,38 @@ pub trait Adapter {
     /// The type holding driver private data about each device id supported by the driver.
     type IdInfo: 'static;
 
+    /// The [`acpi::IdTable`] of the corresponding driver
+    fn acpi_id_table() -> Option<acpi::IdTable<Self::IdInfo>>;
+
+    /// Returns the driver's private data from the matching entry in the [`acpi::IdTable`], if any.
+    ///
+    /// If this returns `None`, it means there is no match with an entry in the [`acpi::IdTable`].
+    #[cfg(CONFIG_ACPI)]
+    fn acpi_id_info(dev: &device::Device) -> Option<&'static Self::IdInfo> {
+        let table = Self::acpi_id_table()?;
+
+        // SAFETY:
+        // - `table` has static lifetime, hence it's valid for read,
+        // - `dev` is guaranteed to be valid while it's alive, and so is `pdev.as_ref().as_raw()`.
+        let raw_id = unsafe { bindings::acpi_match_device(table.as_ptr(), dev.as_raw()) };
+
+        if raw_id.is_null() {
+            None
+        } else {
+            // SAFETY: `DeviceId` is a `#[repr(transparent)` wrapper of `struct of_device_id` and
+            // does not add additional invariants, so it's safe to transmute.
+            let id = unsafe { &*raw_id.cast::<acpi::DeviceId>() };
+
+            Some(table.info(<acpi::DeviceId as crate::device_id::RawDeviceId>::index(id)))
+        }
+    }
+
+    #[cfg(not(CONFIG_ACPI))]
+    #[allow(missing_docs)]
+    fn acpi_id_info(_dev: &device::Device) -> Option<&'static Self::IdInfo> {
+        None
+    }
+
     /// The [`of::IdTable`] of the corresponding driver.
     fn of_id_table() -> Option<of::IdTable<Self::IdInfo>>;
 
@@ -178,6 +210,11 @@ fn of_id_info(_dev: &device::Device) -> Option<&'static Self::IdInfo> {
     /// If this returns `None`, it means that there is no match in any of the ID tables directly
     /// associated with a [`device::Device`].
     fn id_info(dev: &device::Device) -> Option<&'static Self::IdInfo> {
+        let id = Self::acpi_id_info(dev);
+        if id.is_some() {
+            return id;
+        }
+
         let id = Self::of_id_info(dev);
         if id.is_some() {
             return id;
diff --git a/rust/kernel/platform.rs b/rust/kernel/platform.rs
index 5b21fa517e55..5923d29a0511 100644
--- a/rust/kernel/platform.rs
+++ b/rust/kernel/platform.rs
@@ -5,7 +5,7 @@
 //! C header: [`include/linux/platform_device.h`](srctree/include/linux/platform_device.h)
 
 use crate::{
-    bindings, container_of, device, driver,
+    acpi, bindings, container_of, device, driver,
     error::{to_result, Result},
     of,
     prelude::*,
@@ -94,6 +94,10 @@ impl<T: Driver + 'static> driver::Adapter for Adapter<T> {
     fn of_id_table() -> Option<of::IdTable<Self::IdInfo>> {
         T::OF_ID_TABLE
     }
+
+    fn acpi_id_table() -> Option<acpi::IdTable<Self::IdInfo>> {
+        None
+    }
 }
 
 /// Declares a kernel module that exposes a single platform driver.
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v4 3/5] rust: driver: Consolidate `Adapter` methods using `#[cfg]`
  2025-06-10 14:52 [PATCH v4 0/5] rust: Add ACPI match table support for Rust drivers Igor Korotin
  2025-06-10 15:00 ` [PATCH v4 1/5] rust: acpi: add `acpi::DeviceId` abstraction Igor Korotin
  2025-06-10 15:03 ` [PATCH v4 2/5] rust: driver: Add ACPI id table support to Adapter trait Igor Korotin
@ 2025-06-10 15:05 ` Igor Korotin
  2025-06-10 15:15   ` Danilo Krummrich
  2025-06-10 15:07 ` [PATCH v4 4/5] rust: platform: Add ACPI match table support to `Driver` trait Igor Korotin
  2025-06-10 15:10 ` [PATCH v4 5/5] samples: rust: add ACPI match table example to platform driver Igor Korotin
  4 siblings, 1 reply; 8+ messages in thread
From: Igor Korotin @ 2025-06-10 15:05 UTC (permalink / raw)
  To: ojeda, alex.gaynor, rafael, gregkh, linux-kernel, rust-for-linux,
	linux-acpi
  Cc: boqun.feng, gary, bjorn3_gh, benno.lossin, a.hindborg, aliceryhl,
	tmgross, dakr, lenb, wedsonaf, viresh.kumar, alex.hung,
	dingxiangfei2009

Refactor the `acpi_id_info` and `of_id_info` methods in the `Adapter` trait
to reduce duplication. Previously, each method had two versions selected
via `#[cfg(...)]` and `#[cfg(not(...))]`. This change merges them into a
single method per case by using `#[cfg]` blocks within the method bodies.

Suggested-by: Benno Lossin <lossin@kernel.org>
Signed-off-by: Igor Korotin <igor.korotin.linux@gmail.com>
---
 rust/kernel/driver.rs | 76 +++++++++++++++++++++----------------------
 1 file changed, 38 insertions(+), 38 deletions(-)

diff --git a/rust/kernel/driver.rs b/rust/kernel/driver.rs
index 638f1d270af8..805f61bd4a50 100644
--- a/rust/kernel/driver.rs
+++ b/rust/kernel/driver.rs
@@ -147,30 +147,30 @@ pub trait Adapter {
     /// Returns the driver's private data from the matching entry in the [`acpi::IdTable`], if any.
     ///
     /// If this returns `None`, it means there is no match with an entry in the [`acpi::IdTable`].
-    #[cfg(CONFIG_ACPI)]
     fn acpi_id_info(dev: &device::Device) -> Option<&'static Self::IdInfo> {
-        let table = Self::acpi_id_table()?;
+        #[cfg(not(CONFIG_ACPI))] {
+            let _ = dev;
+            return None;
+        }
 
-        // SAFETY:
-        // - `table` has static lifetime, hence it's valid for read,
-        // - `dev` is guaranteed to be valid while it's alive, and so is `pdev.as_ref().as_raw()`.
-        let raw_id = unsafe { bindings::acpi_match_device(table.as_ptr(), dev.as_raw()) };
+        #[cfg(CONFIG_ACPI)] {
+            let table = Self::acpi_id_table()?;
 
-        if raw_id.is_null() {
-            None
-        } else {
-            // SAFETY: `DeviceId` is a `#[repr(transparent)` wrapper of `struct of_device_id` and
-            // does not add additional invariants, so it's safe to transmute.
-            let id = unsafe { &*raw_id.cast::<acpi::DeviceId>() };
+            // SAFETY:
+            // - `table` has static lifetime, hence it's valid for read,
+            // - `dev` is guaranteed to be valid while it's alive, and so is `pdev.as_ref().as_raw()`.
+            let raw_id = unsafe { bindings::acpi_match_device(table.as_ptr(), dev.as_raw()) };
 
-            Some(table.info(<acpi::DeviceId as crate::device_id::RawDeviceId>::index(id)))
-        }
-    }
+            if raw_id.is_null() {
+                None
+            } else {
+                // SAFETY: `DeviceId` is a `#[repr(transparent)` wrapper of `struct of_device_id` and
+                // does not add additional invariants, so it's safe to transmute.
+                let id = unsafe { &*raw_id.cast::<acpi::DeviceId>() };
 
-    #[cfg(not(CONFIG_ACPI))]
-    #[allow(missing_docs)]
-    fn acpi_id_info(_dev: &device::Device) -> Option<&'static Self::IdInfo> {
-        None
+                Some(table.info(<acpi::DeviceId as crate::device_id::RawDeviceId>::index(id)))
+            }
+        }
     }
 
     /// The [`of::IdTable`] of the corresponding driver.
@@ -179,30 +179,30 @@ fn acpi_id_info(_dev: &device::Device) -> Option<&'static Self::IdInfo> {
     /// Returns the driver's private data from the matching entry in the [`of::IdTable`], if any.
     ///
     /// If this returns `None`, it means there is no match with an entry in the [`of::IdTable`].
-    #[cfg(CONFIG_OF)]
     fn of_id_info(dev: &device::Device) -> Option<&'static Self::IdInfo> {
-        let table = Self::of_id_table()?;
+        #[cfg(not(CONFIG_OF))] {
+            let _ = dev;
+            return None;
+        }
 
-        // SAFETY:
-        // - `table` has static lifetime, hence it's valid for read,
-        // - `dev` is guaranteed to be valid while it's alive, and so is `pdev.as_ref().as_raw()`.
-        let raw_id = unsafe { bindings::of_match_device(table.as_ptr(), dev.as_raw()) };
+        #[cfg(CONFIG_OF)] {
+            let table = Self::of_id_table()?;
 
-        if raw_id.is_null() {
-            None
-        } else {
-            // SAFETY: `DeviceId` is a `#[repr(transparent)` wrapper of `struct of_device_id` and
-            // does not add additional invariants, so it's safe to transmute.
-            let id = unsafe { &*raw_id.cast::<of::DeviceId>() };
+            // SAFETY:
+            // - `table` has static lifetime, hence it's valid for read,
+            // - `dev` is guaranteed to be valid while it's alive, and so is `pdev.as_ref().as_raw()`.
+            let raw_id = unsafe { bindings::of_match_device(table.as_ptr(), dev.as_raw()) };
 
-            Some(table.info(<of::DeviceId as crate::device_id::RawDeviceId>::index(id)))
-        }
-    }
+            if raw_id.is_null() {
+                None
+            } else {
+                // SAFETY: `DeviceId` is a `#[repr(transparent)` wrapper of `struct of_device_id` and
+                // does not add additional invariants, so it's safe to transmute.
+                let id = unsafe { &*raw_id.cast::<of::DeviceId>() };
 
-    #[cfg(not(CONFIG_OF))]
-    #[allow(missing_docs)]
-    fn of_id_info(_dev: &device::Device) -> Option<&'static Self::IdInfo> {
-        None
+                Some(table.info(<of::DeviceId as crate::device_id::RawDeviceId>::index(id)))
+            }
+        }
     }
 
     /// Returns the driver's private data from the matching entry of any of the ID tables, if any.
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v4 4/5] rust: platform: Add ACPI match table support to `Driver` trait
  2025-06-10 14:52 [PATCH v4 0/5] rust: Add ACPI match table support for Rust drivers Igor Korotin
                   ` (2 preceding siblings ...)
  2025-06-10 15:05 ` [PATCH v4 3/5] rust: driver: Consolidate `Adapter` methods using `#[cfg]` Igor Korotin
@ 2025-06-10 15:07 ` Igor Korotin
  2025-06-10 15:10 ` [PATCH v4 5/5] samples: rust: add ACPI match table example to platform driver Igor Korotin
  4 siblings, 0 replies; 8+ messages in thread
From: Igor Korotin @ 2025-06-10 15:07 UTC (permalink / raw)
  To: ojeda, alex.gaynor, rafael, gregkh, linux-kernel, rust-for-linux,
	linux-acpi
  Cc: boqun.feng, gary, bjorn3_gh, benno.lossin, a.hindborg, aliceryhl,
	tmgross, dakr, lenb, wedsonaf, viresh.kumar, alex.hung,
	dingxiangfei2009

Extend the `platform::Driver` trait to support ACPI device matching by
adding the `ACPI_ID_TABLE` constant.

This allows Rust platform drivers to define ACPI match tables alongside
their existing OF match tables.

These changes mirror the existing OF support and allow Rust platform
drivers to match devices based on ACPI identifiers.

To avoid breaking compilation, a stub ACPI match table definition is
added to the Rust sample platform driver. Functional support for ACPI
matching in the sample driver will be provided in a follow-up patch.

Signed-off-by: Igor Korotin <igor.korotin.linux@gmail.com>
---
 rust/kernel/platform.rs              | 48 ++++++++++++++++++++++++++--
 samples/rust/rust_driver_platform.rs |  3 +-
 2 files changed, 48 insertions(+), 3 deletions(-)

diff --git a/rust/kernel/platform.rs b/rust/kernel/platform.rs
index 5923d29a0511..893b4189a10b 100644
--- a/rust/kernel/platform.rs
+++ b/rust/kernel/platform.rs
@@ -37,12 +37,18 @@ unsafe fn register(
             None => core::ptr::null(),
         };
 
+        let acpi_table = match T::ACPI_ID_TABLE {
+            Some(table) => table.as_ptr(),
+            None => core::ptr::null(),
+        };
+
         // SAFETY: It's safe to set the fields of `struct platform_driver` on initialization.
         unsafe {
             (*pdrv.get()).driver.name = name.as_char_ptr();
             (*pdrv.get()).probe = Some(Self::probe_callback);
             (*pdrv.get()).remove = Some(Self::remove_callback);
             (*pdrv.get()).driver.of_match_table = of_table;
+            (*pdrv.get()).driver.acpi_match_table = acpi_table;
         }
 
         // SAFETY: `pdrv` is guaranteed to be a valid `RegType`.
@@ -96,7 +102,7 @@ fn of_id_table() -> Option<of::IdTable<Self::IdInfo>> {
     }
 
     fn acpi_id_table() -> Option<acpi::IdTable<Self::IdInfo>> {
-        None
+        T::ACPI_ID_TABLE
     }
 }
 
@@ -127,7 +133,7 @@ macro_rules! module_platform_driver {
 /// # Example
 ///
 ///```
-/// # use kernel::{bindings, c_str, device::Core, of, platform};
+/// # use kernel::{acpi, bindings, c_str, device::Core, of, platform};
 ///
 /// struct MyDriver;
 ///
@@ -140,9 +146,44 @@ macro_rules! module_platform_driver {
 ///     ]
 /// );
 ///
+/// kernel::acpi_device_table!(
+///     ACPI_TABLE,
+///     MODULE_ACPI_TABLE,
+///     <MyDriver as platform::Driver>::IdInfo,
+///     [
+///         (acpi::DeviceId::new(c_str!("TEST4321")), ())
+///     ]
+/// );
+///
 /// impl platform::Driver for MyDriver {
 ///     type IdInfo = ();
 ///     const OF_ID_TABLE: Option<of::IdTable<Self::IdInfo>> = Some(&OF_TABLE);
+///     const ACPI_ID_TABLE: Option<acpi::IdTable<Self::IdInfo>> = Some(&ACPI_TABLE);
+///
+///     fn probe(
+///         _pdev: &platform::Device<Core>,
+///         _id_info: Option<&Self::IdInfo>,
+///     ) -> Result<Pin<KBox<Self>>> {
+///         Err(ENODEV)
+///     }
+/// }
+///```
+///
+/// The platform::Driver requires declaration of both OF_ID_TABLE and
+/// ACPI_ID_TABLE, but if driver is not going to use either of them
+/// it can implement one of them or both as None.
+///
+/// # Example:
+///
+///```
+/// # use kernel::{acpi, device::Core, of, platform};
+///
+/// struct MyDriver;
+///
+/// impl platform::Driver for MyDriver {
+///     type IdInfo = ();
+///     const OF_ID_TABLE: Option<of::IdTable<Self::IdInfo>> = None;
+///     const ACPI_ID_TABLE: Option<acpi::IdTable<Self::IdInfo>> = None;
 ///
 ///     fn probe(
 ///         _pdev: &platform::Device<Core>,
@@ -164,6 +205,9 @@ pub trait Driver: Send {
     /// The table of OF device ids supported by the driver.
     const OF_ID_TABLE: Option<of::IdTable<Self::IdInfo>>;
 
+    /// The table of ACPI device ids supported by the driver.
+    const ACPI_ID_TABLE: Option<acpi::IdTable<Self::IdInfo>>;
+
     /// Platform driver probe.
     ///
     /// Called when a new platform device is added or discovered.
diff --git a/samples/rust/rust_driver_platform.rs b/samples/rust/rust_driver_platform.rs
index 8b42b3cfb363..e3992e7a71e9 100644
--- a/samples/rust/rust_driver_platform.rs
+++ b/samples/rust/rust_driver_platform.rs
@@ -2,7 +2,7 @@
 
 //! Rust Platform driver sample.
 
-use kernel::{c_str, device::Core, of, platform, prelude::*, types::ARef};
+use kernel::{acpi, c_str, device::Core, of, platform, prelude::*, types::ARef};
 
 struct SampleDriver {
     pdev: ARef<platform::Device>,
@@ -20,6 +20,7 @@ struct SampleDriver {
 impl platform::Driver for SampleDriver {
     type IdInfo = Info;
     const OF_ID_TABLE: Option<of::IdTable<Self::IdInfo>> = Some(&OF_TABLE);
+    const ACPI_ID_TABLE: Option<acpi::IdTable<Self::IdInfo>> = None;
 
     fn probe(
         pdev: &platform::Device<Core>,
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v4 5/5] samples: rust: add ACPI match table example to platform driver
  2025-06-10 14:52 [PATCH v4 0/5] rust: Add ACPI match table support for Rust drivers Igor Korotin
                   ` (3 preceding siblings ...)
  2025-06-10 15:07 ` [PATCH v4 4/5] rust: platform: Add ACPI match table support to `Driver` trait Igor Korotin
@ 2025-06-10 15:10 ` Igor Korotin
  4 siblings, 0 replies; 8+ messages in thread
From: Igor Korotin @ 2025-06-10 15:10 UTC (permalink / raw)
  To: ojeda, alex.gaynor, rafael, gregkh, linux-kernel, rust-for-linux,
	linux-acpi
  Cc: boqun.feng, gary, bjorn3_gh, benno.lossin, a.hindborg, aliceryhl,
	tmgross, dakr, lenb, wedsonaf, viresh.kumar, alex.hung,
	dingxiangfei2009

Extend the Rust sample platform driver to probe using device/driver name
matching, OF ID table matching, or ACPI ID table matching.

Signed-off-by: Igor Korotin <igor.korotin.linux@gmail.com>
---
 samples/rust/rust_driver_platform.rs | 70 +++++++++++++++++++++++++++-
 1 file changed, 69 insertions(+), 1 deletion(-)

diff --git a/samples/rust/rust_driver_platform.rs b/samples/rust/rust_driver_platform.rs
index e3992e7a71e9..35d5067aa023 100644
--- a/samples/rust/rust_driver_platform.rs
+++ b/samples/rust/rust_driver_platform.rs
@@ -17,10 +17,78 @@ struct SampleDriver {
     [(of::DeviceId::new(c_str!("test,rust-device")), Info(42))]
 );
 
+// ACPI match table test
+//
+// This demonstrates how to test an ACPI-based Rust platform driver using QEMU
+// with a custom SSDT.
+//
+// Steps:
+//
+// 1. **Create an SSDT source file** (`ssdt.dsl`) with the following content:
+//
+//     ```asl
+//     DefinitionBlock ("", "SSDT", 2, "TEST", "VIRTACPI", 0x00000001)
+//     {
+//         Scope (\_SB)
+//         {
+//             Device (T432)
+//             {
+//                 Name (_HID, "TEST4321")  // ACPI hardware ID to match
+//                 Name (_UID, 1)
+//                 Name (_STA, 0x0F)        // Device present, enabled
+//                 Name (_CRS, ResourceTemplate ()
+//                 {
+//                     Memory32Fixed (ReadWrite, 0xFED00000, 0x1000)
+//                 })
+//             }
+//         }
+//     }
+//     ```
+//
+// 2. **Compile the table**:
+//
+//     ```sh
+//     iasl -tc ssdt.dsl
+//     ```
+//
+//     This generates `ssdt.aml`
+//
+// 3. **Run QEMU** with the compiled AML file:
+//
+//     ```sh
+//     qemu-system-x86_64 -m 512M \
+//         -enable-kvm \
+//         -kernel path/to/bzImage \
+//         -append "root=/dev/sda console=ttyS0" \
+//         -hda rootfs.img \
+//         -serial stdio \
+//         -acpitable file=ssdt.aml
+//     ```
+//
+//     Requirements:
+//     - The `rust_driver_platform` must be present either:
+//         - built directly into the kernel (`bzImage`), or
+//         - available as a `.ko` file and loadable from `rootfs.img`
+//
+// 4. **Verify it worked** by checking `dmesg`:
+//
+//     ```
+//     rust_driver_platform TEST4321:00: Probed with info: '0'.
+//     ```
+//
+// This demonstrates ACPI table matching using a custom ID in QEMU with a minimal SSDT
+
+kernel::acpi_device_table!(
+    ACPI_TABLE,
+    MODULE_ACPI_TABLE,
+    <SampleDriver as platform::Driver>::IdInfo,
+    [(acpi::DeviceId::new(c_str!("TEST4321")), Info(0))]
+);
+
 impl platform::Driver for SampleDriver {
     type IdInfo = Info;
     const OF_ID_TABLE: Option<of::IdTable<Self::IdInfo>> = Some(&OF_TABLE);
-    const ACPI_ID_TABLE: Option<acpi::IdTable<Self::IdInfo>> = None;
+    const ACPI_ID_TABLE: Option<acpi::IdTable<Self::IdInfo>> = Some(&ACPI_TABLE);
 
     fn probe(
         pdev: &platform::Device<Core>,
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v4 3/5] rust: driver: Consolidate `Adapter` methods using `#[cfg]`
  2025-06-10 15:05 ` [PATCH v4 3/5] rust: driver: Consolidate `Adapter` methods using `#[cfg]` Igor Korotin
@ 2025-06-10 15:15   ` Danilo Krummrich
  2025-06-10 16:14     ` Igor Korotin
  0 siblings, 1 reply; 8+ messages in thread
From: Danilo Krummrich @ 2025-06-10 15:15 UTC (permalink / raw)
  To: Igor Korotin
  Cc: ojeda, alex.gaynor, rafael, gregkh, linux-kernel, rust-for-linux,
	linux-acpi, boqun.feng, gary, bjorn3_gh, benno.lossin, a.hindborg,
	aliceryhl, tmgross, lenb, wedsonaf, viresh.kumar, alex.hung,
	dingxiangfei2009

On Tue, Jun 10, 2025 at 04:05:38PM +0100, Igor Korotin wrote:
> Refactor the `acpi_id_info` and `of_id_info` methods in the `Adapter` trait
> to reduce duplication. Previously, each method had two versions selected
> via `#[cfg(...)]` and `#[cfg(not(...))]`. This change merges them into a
> single method per case by using `#[cfg]` blocks within the method bodies.
> 
> Suggested-by: Benno Lossin <lossin@kernel.org>
> Signed-off-by: Igor Korotin <igor.korotin.linux@gmail.com>
> ---
>  rust/kernel/driver.rs | 76 +++++++++++++++++++++----------------------
>  1 file changed, 38 insertions(+), 38 deletions(-)
> 
> diff --git a/rust/kernel/driver.rs b/rust/kernel/driver.rs
> index 638f1d270af8..805f61bd4a50 100644
> --- a/rust/kernel/driver.rs
> +++ b/rust/kernel/driver.rs
> @@ -147,30 +147,30 @@ pub trait Adapter {
>      /// Returns the driver's private data from the matching entry in the [`acpi::IdTable`], if any.
>      ///
>      /// If this returns `None`, it means there is no match with an entry in the [`acpi::IdTable`].
> -    #[cfg(CONFIG_ACPI)]
>      fn acpi_id_info(dev: &device::Device) -> Option<&'static Self::IdInfo> {
> -        let table = Self::acpi_id_table()?;
> +        #[cfg(not(CONFIG_ACPI))] {
> +            let _ = dev;
> +            return None;
> +        }
>  
> -        // SAFETY:
> -        // - `table` has static lifetime, hence it's valid for read,
> -        // - `dev` is guaranteed to be valid while it's alive, and so is `pdev.as_ref().as_raw()`.
> -        let raw_id = unsafe { bindings::acpi_match_device(table.as_ptr(), dev.as_raw()) };
> +        #[cfg(CONFIG_ACPI)] {
> +            let table = Self::acpi_id_table()?;
>  
> -        if raw_id.is_null() {
> -            None
> -        } else {
> -            // SAFETY: `DeviceId` is a `#[repr(transparent)` wrapper of `struct of_device_id` and
> -            // does not add additional invariants, so it's safe to transmute.
> -            let id = unsafe { &*raw_id.cast::<acpi::DeviceId>() };
> +            // SAFETY:
> +            // - `table` has static lifetime, hence it's valid for read,
> +            // - `dev` is guaranteed to be valid while it's alive, and so is `pdev.as_ref().as_raw()`.
> +            let raw_id = unsafe { bindings::acpi_match_device(table.as_ptr(), dev.as_raw()) };
>  
> -            Some(table.info(<acpi::DeviceId as crate::device_id::RawDeviceId>::index(id)))
> -        }
> -    }
> +            if raw_id.is_null() {
> +                None
> +            } else {
> +                // SAFETY: `DeviceId` is a `#[repr(transparent)` wrapper of `struct of_device_id` and
> +                // does not add additional invariants, so it's safe to transmute.
> +                let id = unsafe { &*raw_id.cast::<acpi::DeviceId>() };
>  
> -    #[cfg(not(CONFIG_ACPI))]
> -    #[allow(missing_docs)]
> -    fn acpi_id_info(_dev: &device::Device) -> Option<&'static Self::IdInfo> {
> -        None
> +                Some(table.info(<acpi::DeviceId as crate::device_id::RawDeviceId>::index(id)))
> +            }
> +        }
>      }

It seems there has been a misunderstanding, the idea was to refactor
Adapter::of_id_info() in one patch and introduce Adapter::acpi_id_info() in a
subsequent patch already being the suggested implementation by Benno.

We try to avoid introducing code, just to change it again in a subsequent patch.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v4 3/5] rust: driver: Consolidate `Adapter` methods using `#[cfg]`
  2025-06-10 15:15   ` Danilo Krummrich
@ 2025-06-10 16:14     ` Igor Korotin
  0 siblings, 0 replies; 8+ messages in thread
From: Igor Korotin @ 2025-06-10 16:14 UTC (permalink / raw)
  To: Danilo Krummrich
  Cc: ojeda, alex.gaynor, rafael, gregkh, linux-kernel, rust-for-linux,
	linux-acpi, boqun.feng, gary, bjorn3_gh, benno.lossin, a.hindborg,
	aliceryhl, tmgross, lenb, wedsonaf, viresh.kumar, alex.hung,
	dingxiangfei2009

On Tue, Jun 10, 2025 at 4:15 PM Danilo Krummrich <dakr@kernel.org> wrote:
>
> On Tue, Jun 10, 2025 at 04:05:38PM +0100, Igor Korotin wrote:
> > Refactor the `acpi_id_info` and `of_id_info` methods in the `Adapter` trait
> > to reduce duplication. Previously, each method had two versions selected
> > via `#[cfg(...)]` and `#[cfg(not(...))]`. This change merges them into a
> > single method per case by using `#[cfg]` blocks within the method bodies.
> >
> > Suggested-by: Benno Lossin <lossin@kernel.org>
> > Signed-off-by: Igor Korotin <igor.korotin.linux@gmail.com>
> > ---
> >  rust/kernel/driver.rs | 76 +++++++++++++++++++++----------------------
> >  1 file changed, 38 insertions(+), 38 deletions(-)
> >
> > diff --git a/rust/kernel/driver.rs b/rust/kernel/driver.rs
> > index 638f1d270af8..805f61bd4a50 100644
> > --- a/rust/kernel/driver.rs
> > +++ b/rust/kernel/driver.rs
> > @@ -147,30 +147,30 @@ pub trait Adapter {
> >      /// Returns the driver's private data from the matching entry in the [`acpi::IdTable`], if any.
> >      ///
> >      /// If this returns `None`, it means there is no match with an entry in the [`acpi::IdTable`].
> > -    #[cfg(CONFIG_ACPI)]
> >      fn acpi_id_info(dev: &device::Device) -> Option<&'static Self::IdInfo> {
> > -        let table = Self::acpi_id_table()?;
> > +        #[cfg(not(CONFIG_ACPI))] {
> > +            let _ = dev;
> > +            return None;
> > +        }
> >
> > -        // SAFETY:
> > -        // - `table` has static lifetime, hence it's valid for read,
> > -        // - `dev` is guaranteed to be valid while it's alive, and so is `pdev.as_ref().as_raw()`.
> > -        let raw_id = unsafe { bindings::acpi_match_device(table.as_ptr(), dev.as_raw()) };
> > +        #[cfg(CONFIG_ACPI)] {
> > +            let table = Self::acpi_id_table()?;
> >
> > -        if raw_id.is_null() {
> > -            None
> > -        } else {
> > -            // SAFETY: `DeviceId` is a `#[repr(transparent)` wrapper of `struct of_device_id` and
> > -            // does not add additional invariants, so it's safe to transmute.
> > -            let id = unsafe { &*raw_id.cast::<acpi::DeviceId>() };
> > +            // SAFETY:
> > +            // - `table` has static lifetime, hence it's valid for read,
> > +            // - `dev` is guaranteed to be valid while it's alive, and so is `pdev.as_ref().as_raw()`.
> > +            let raw_id = unsafe { bindings::acpi_match_device(table.as_ptr(), dev.as_raw()) };
> >
> > -            Some(table.info(<acpi::DeviceId as crate::device_id::RawDeviceId>::index(id)))
> > -        }
> > -    }
> > +            if raw_id.is_null() {
> > +                None
> > +            } else {
> > +                // SAFETY: `DeviceId` is a `#[repr(transparent)` wrapper of `struct of_device_id` and
> > +                // does not add additional invariants, so it's safe to transmute.
> > +                let id = unsafe { &*raw_id.cast::<acpi::DeviceId>() };
> >
> > -    #[cfg(not(CONFIG_ACPI))]
> > -    #[allow(missing_docs)]
> > -    fn acpi_id_info(_dev: &device::Device) -> Option<&'static Self::IdInfo> {
> > -        None
> > +                Some(table.info(<acpi::DeviceId as crate::device_id::RawDeviceId>::index(id)))
> > +            }
> > +        }
> >      }
>
> It seems there has been a misunderstanding, the idea was to refactor
> Adapter::of_id_info() in one patch and introduce Adapter::acpi_id_info() in a
> subsequent patch already being the suggested implementation by Benno.
>
> We try to avoid introducing code, just to change it again in a subsequent patch.

That does make perfect sense. My bad. I'll reorder changes in the v5.

Sorry for the mess. Thanks for the review.

Best Regards
Igor

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2025-06-10 16:15 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-10 14:52 [PATCH v4 0/5] rust: Add ACPI match table support for Rust drivers Igor Korotin
2025-06-10 15:00 ` [PATCH v4 1/5] rust: acpi: add `acpi::DeviceId` abstraction Igor Korotin
2025-06-10 15:03 ` [PATCH v4 2/5] rust: driver: Add ACPI id table support to Adapter trait Igor Korotin
2025-06-10 15:05 ` [PATCH v4 3/5] rust: driver: Consolidate `Adapter` methods using `#[cfg]` Igor Korotin
2025-06-10 15:15   ` Danilo Krummrich
2025-06-10 16:14     ` Igor Korotin
2025-06-10 15:07 ` [PATCH v4 4/5] rust: platform: Add ACPI match table support to `Driver` trait Igor Korotin
2025-06-10 15:10 ` [PATCH v4 5/5] samples: rust: add ACPI match table example to platform driver Igor Korotin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).