[PATCH v2 0/5] rust: Add ACPI match table support for Rust drivers

[PATCH v2 0/5] rust: Add ACPI match table support for Rust drivers

[Igor Korotin]

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`.
  - A helper function `is_of_node()` for determining fwnode types.
  - 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: helpers: Add `is_of_node` helper function
  rust: driver: Add ACPI id table support to Adapter trait
  rust: platform: Add ACPI match table support to `Driver` trait
  samples: rust: add ACPI match table example to platform driver

Changelog
---------
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                          |  2 +
 rust/bindings/bindings_helper.h      |  1 +
 rust/helpers/helpers.c               |  1 +
 rust/helpers/of.c                    |  6 +++
 rust/kernel/acpi.rs                  | 62 ++++++++++++++++++++++++++++
 rust/kernel/driver.rs                | 58 ++++++++++++++++++++++++--
 rust/kernel/lib.rs                   |  1 +
 rust/kernel/platform.rs              | 17 +++++++-
 samples/rust/rust_driver_platform.rs | 41 +++++++++++++++++-
 9 files changed, 183 insertions(+), 6 deletions(-)
 create mode 100644 rust/helpers/of.c
 create mode 100644 rust/kernel/acpi.rs

-- 
2.43.0

[PATCH v2 1/5] rust: acpi: add `acpi::DeviceId` abstraction

[Igor Korotin]

`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 | 62 +++++++++++++++++++++++++++++++++++++++++++++
 rust/kernel/lib.rs  |  1 +
 3 files changed, 64 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..2f526a0b2ed9
--- /dev/null
+++ b/rust/kernel/acpi.rs
@@ -0,0 +1,62 @@
+// 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 6e9287136cac..c2761bc7cfe6 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_BLOCK)]
 pub mod block;
-- 
2.43.0

[PATCH v2 2/5] rust: helpers: Add `is_of_node` helper function

[Igor Korotin]

Add a helper function to determine whether a given device appears to be
an Open Firmware (OF) node.

This function will be used in Rust driver abstractions to support both
ACPI and OF matching simultaneously in subsequent patches.

Signed-off-by: Igor Korotin <igor.korotin.linux@gmail.com>
---
 MAINTAINERS            | 1 +
 rust/helpers/helpers.c | 1 +
 rust/helpers/of.c      | 6 ++++++
 3 files changed, 8 insertions(+)
 create mode 100644 rust/helpers/of.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 5f8dfae08454..d6cadaa592aa 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -18215,6 +18215,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/helpers/of.c
 F:	rust/kernel/of.rs
 F:	scripts/dtc/
 F:	tools/testing/selftests/dt/
diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
index 80785b1e7a63..2fc5242aa47a 100644
--- a/rust/helpers/helpers.c
+++ b/rust/helpers/helpers.c
@@ -21,6 +21,7 @@
 #include "jump_label.c"
 #include "kunit.c"
 #include "mutex.c"
+#include "of.c"
 #include "page.c"
 #include "platform.c"
 #include "pci.c"
diff --git a/rust/helpers/of.c b/rust/helpers/of.c
new file mode 100644
index 000000000000..8913eaced716
--- /dev/null
+++ b/rust/helpers/of.c
@@ -0,0 +1,6 @@
+#include <linux/of.h>
+
+bool rust_helper_is_of_node(const struct fwnode_handle *fwnode)
+{
+    return is_of_node(fwnode);
+}
-- 
2.43.0

[PATCH v2 3/5] rust: driver: Add ACPI id table support to Adapter trait

[Igor Korotin]

From: Igor Korotin <igor.korotin.linux@gmail.com>

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.
---
 rust/bindings/bindings_helper.h |  1 +
 rust/kernel/driver.rs           | 58 ++++++++++++++++++++++++++++++---
 rust/kernel/platform.rs         |  5 +++
 3 files changed, 60 insertions(+), 4 deletions(-)

diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index e0bcd130b494..d974fc6c141f 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -6,6 +6,7 @@
  * Sorted alphabetically.
  */
 
+#include <linux/acpi.h>
 #include <kunit/test.h>
 #include <linux/blk-mq.h>
 #include <linux/blk_types.h>
diff --git a/rust/kernel/driver.rs b/rust/kernel/driver.rs
index ec9166cedfa7..d4098596188a 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::{device, of, acpi, 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,9 +210,27 @@ 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::of_id_info(dev);
-        if id.is_some() {
-            return id;
+        // SAFETY: `id_info` is called from `Adapter::probe_callback` with a valid `dev` argument.
+        let fwnode = unsafe{ (*dev.as_raw()).fwnode};
+
+        // SAFETY: `bindings::is_acpi_device_node` checks `fwnode` before accessing `fwnode->ops`,
+        // and only compares it with the address of `acpi_device_fwnode_ops`.
+        if unsafe { bindings::is_acpi_device_node(fwnode) } {
+            let id = Self::acpi_id_info(dev);
+
+            if id.is_some() {
+                return id;
+            }
+        }
+
+        // SAFETY: `bindings::is_of_node` checks `fwnode` before accessing `fwnode->ops`,
+        // and only compares it with the address of `of_fwnode_ops`.
+        if unsafe { bindings::is_of_node(fwnode) } {
+            let id = Self::of_id_info(dev);
+
+            if id.is_some() {
+                return id;
+            }
         }
 
         None
diff --git a/rust/kernel/platform.rs b/rust/kernel/platform.rs
index fd4a494f30e8..3cc9fe6ccfcf 100644
--- a/rust/kernel/platform.rs
+++ b/rust/kernel/platform.rs
@@ -5,6 +5,7 @@
 //! C header: [`include/linux/platform_device.h`](srctree/include/linux/platform_device.h)
 
 use crate::{
+    acpi,
     bindings, device, driver,
     error::{to_result, Result},
     of,
@@ -95,6 +96,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

[PATCH v2 3/5] rust: driver: Add ACPI id table support to Adapter trait

[Igor Korotin]

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.
---
 rust/bindings/bindings_helper.h |  1 +
 rust/kernel/driver.rs           | 58 ++++++++++++++++++++++++++++++---
 rust/kernel/platform.rs         |  5 +++
 3 files changed, 60 insertions(+), 4 deletions(-)

diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index e0bcd130b494..d974fc6c141f 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -6,6 +6,7 @@
  * Sorted alphabetically.
  */
 
+#include <linux/acpi.h>
 #include <kunit/test.h>
 #include <linux/blk-mq.h>
 #include <linux/blk_types.h>
diff --git a/rust/kernel/driver.rs b/rust/kernel/driver.rs
index ec9166cedfa7..d4098596188a 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::{device, of, acpi, 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,9 +210,27 @@ 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::of_id_info(dev);
-        if id.is_some() {
-            return id;
+        // SAFETY: `id_info` is called from `Adapter::probe_callback` with a valid `dev` argument.
+        let fwnode = unsafe{ (*dev.as_raw()).fwnode};
+
+        // SAFETY: `bindings::is_acpi_device_node` checks `fwnode` before accessing `fwnode->ops`,
+        // and only compares it with the address of `acpi_device_fwnode_ops`.
+        if unsafe { bindings::is_acpi_device_node(fwnode) } {
+            let id = Self::acpi_id_info(dev);
+
+            if id.is_some() {
+                return id;
+            }
+        }
+
+        // SAFETY: `bindings::is_of_node` checks `fwnode` before accessing `fwnode->ops`,
+        // and only compares it with the address of `of_fwnode_ops`.
+        if unsafe { bindings::is_of_node(fwnode) } {
+            let id = Self::of_id_info(dev);
+
+            if id.is_some() {
+                return id;
+            }
         }
 
         None
diff --git a/rust/kernel/platform.rs b/rust/kernel/platform.rs
index fd4a494f30e8..3cc9fe6ccfcf 100644
--- a/rust/kernel/platform.rs
+++ b/rust/kernel/platform.rs
@@ -5,6 +5,7 @@
 //! C header: [`include/linux/platform_device.h`](srctree/include/linux/platform_device.h)
 
 use crate::{
+    acpi,
     bindings, device, driver,
     error::{to_result, Result},
     of,
@@ -95,6 +96,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

[PATCH v2 4/5] rust: platform: Add ACPI match table support to `Driver` trait

[Igor Korotin]

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              | 14 ++++++++++++--
 samples/rust/rust_driver_platform.rs |  3 ++-
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/rust/kernel/platform.rs b/rust/kernel/platform.rs
index 3cc9fe6ccfcf..e6cc878a5a37 100644
--- a/rust/kernel/platform.rs
+++ b/rust/kernel/platform.rs
@@ -39,12 +39,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`.
@@ -98,7 +104,7 @@ fn of_id_table() -> Option<of::IdTable<Self::IdInfo>> {
     }
 
     fn acpi_id_table() -> Option<acpi::IdTable<Self::IdInfo>> {
-        None
+        T::ACPI_ID_TABLE
     }
 }
 
@@ -129,7 +135,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;
 ///
@@ -145,6 +151,7 @@ macro_rules! module_platform_driver {
 /// 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>,
@@ -165,6 +172,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

[PATCH v2 5/5] samples: rust: add ACPI match table example to platform driver

[Igor Korotin]

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 | 40 +++++++++++++++++++++++++++-
 1 file changed, 39 insertions(+), 1 deletion(-)

diff --git a/samples/rust/rust_driver_platform.rs b/samples/rust/rust_driver_platform.rs
index e3992e7a71e9..ee0780c1d6ae 100644
--- a/samples/rust/rust_driver_platform.rs
+++ b/samples/rust/rust_driver_platform.rs
@@ -17,10 +17,48 @@ struct SampleDriver {
     [(of::DeviceId::new(c_str!("test,rust-device")), Info(42))]
 );
 
+kernel::acpi_device_table!(
+    ACPI_TABLE,
+    MODULE_ACPI_TABLE,
+    <SampleDriver as platform::Driver>::IdInfo,
+    [(acpi::DeviceId::new(c_str!("TEST4321")), Info(0))]
+);
+
+/// OF/ACPI match tables for Platform Driver implementation
+///
+/// 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:
+///
+///```
+/// impl platform::Driver for SampleDriver {
+///     type IdInfo = Info;
+///     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>,
+///         info: Option<&Self::IdInfo>,
+///     ) -> Result<Pin<KBox<Self>>> {
+///         dev_dbg!(pdev.as_ref(), "Probe Rust Platform driver sample.\n");
+///
+///         if let Some(info) = info {
+///             dev_info!(pdev.as_ref(), "Probed with info: '{}'.\n", info.0);
+///         }
+///
+///         let drvdata = KBox::new(Self { pdev: pdev.into() }, GFP_KERNEL)?;
+///
+///         Ok(drvdata.into())
+///     }
+/// }
+///```
+
 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

Re: [PATCH v2 1/5] rust: acpi: add `acpi::DeviceId` abstraction

[kernel test robot]

Hi Igor,

kernel test robot noticed the following build errors:

[auto build test ERROR on char-misc/char-misc-testing]
[also build test ERROR on char-misc/char-misc-next char-misc/char-misc-linus linus/master v6.15 next-20250605]
[cannot apply to rust/rust-next driver-core/driver-core-testing driver-core/driver-core-next driver-core/driver-core-linus]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Igor-Korotin/rust-acpi-add-acpi-DeviceId-abstraction/20250606-042551
base:   char-misc/char-misc-testing
patch link:    https://lore.kernel.org/r/20250605162326.3659046-1-igor.korotin.linux%40gmail.com
patch subject: [PATCH v2 1/5] rust: acpi: add `acpi::DeviceId` abstraction
config: x86_64-rhel-9.4-rust (https://download.01.org/0day-ci/archive/20250606/202506060925.Thw3s9xE-lkp@intel.com/config)
compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff)
rustc: rustc 1.78.0 (9b00956e5 2024-04-29)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250606/202506060925.Thw3s9xE-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202506060925.Thw3s9xE-lkp@intel.com/

All errors (new ones prefixed by >>):

   PATH=/opt/cross/clang-18/bin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
   INFO PATH=/opt/cross/rustc-1.78.0-bindgen-0.65.1/cargo/bin:/opt/cross/clang-18/bin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
   /usr/bin/timeout -k 100 12h /usr/bin/make KCFLAGS= -Wno-error=return-type -Wreturn-type -funsigned-char -Wundef W=1 --keep-going LLVM=1 -j32 -C source O=/kbuild/obj/consumer/x86_64-rhel-9.4-rust ARCH=x86_64 SHELL=/bin/bash rustfmtcheck
   make: Entering directory '/kbuild/src/consumer'
   make[1]: Entering directory '/kbuild/obj/consumer/x86_64-rhel-9.4-rust'
>> Diff in rust/kernel/acpi.rs at line 59:
            $crate::module_device_table!("acpi", $module_table_name, $table_name);
        };
    }
   -
    
>> Diff in rust/kernel/acpi.rs at line 59:
            $crate::module_device_table!("acpi", $module_table_name, $table_name);
        };
    }
   -
    
   make[2]: *** [Makefile:1826: rustfmt] Error 123
   make[2]: Target 'rustfmtcheck' not remade because of errors.
   make[1]: Leaving directory '/kbuild/obj/consumer/x86_64-rhel-9.4-rust'
   make[1]: *** [Makefile:248: __sub-make] Error 2
   make[1]: Target 'rustfmtcheck' not remade because of errors.
   make: *** [Makefile:248: __sub-make] Error 2
   make: Target 'rustfmtcheck' not remade because of errors.
   make: Leaving directory '/kbuild/src/consumer'

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v2 3/5] rust: driver: Add ACPI id table support to Adapter trait

[kernel test robot]

Hi Igor,

kernel test robot noticed the following build errors:

[auto build test ERROR on char-misc/char-misc-testing]
[also build test ERROR on char-misc/char-misc-next char-misc/char-misc-linus v6.15]
[cannot apply to rust/rust-next driver-core/driver-core-testing driver-core/driver-core-next driver-core/driver-core-linus linus/master next-20250605]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Igor-Korotin/rust-acpi-add-acpi-DeviceId-abstraction/20250606-042551
base:   char-misc/char-misc-testing
patch link:    https://lore.kernel.org/r/20250605165109.3663553-1-igor.korotin.linux%40gmail.com
patch subject: [PATCH v2 3/5] rust: driver: Add ACPI id table support to Adapter trait
config: x86_64-rhel-9.4-rust (https://download.01.org/0day-ci/archive/20250606/202506060941.Q8GECvnj-lkp@intel.com/config)
compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff)
rustc: rustc 1.78.0 (9b00956e5 2024-04-29)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250606/202506060941.Q8GECvnj-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202506060941.Q8GECvnj-lkp@intel.com/

All errors (new ones prefixed by >>):

   PATH=/opt/cross/clang-18/bin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
   INFO PATH=/opt/cross/rustc-1.78.0-bindgen-0.65.1/cargo/bin:/opt/cross/clang-18/bin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
   /usr/bin/timeout -k 100 12h /usr/bin/make KCFLAGS= -Wno-error=return-type -Wreturn-type -funsigned-char -Wundef W=1 --keep-going LLVM=1 -j32 -C source O=/kbuild/obj/consumer/x86_64-rhel-9.4-rust ARCH=x86_64 SHELL=/bin/bash rustfmtcheck
   make: Entering directory '/kbuild/src/consumer'
   make[1]: Entering directory '/kbuild/obj/consumer/x86_64-rhel-9.4-rust'
   Diff in rust/kernel/acpi.rs at line 59:
            $crate::module_device_table!("acpi", $module_table_name, $table_name);
        };
    }
   -
    
>> Diff in rust/kernel/driver.rs at line 6:
    //! register using the [`Registration`] class.
    
>>  use crate::error::{Error, Result};
   -use crate::{device, of, acpi, 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};
    
   Diff in rust/kernel/driver.rs at line 211:
        /// associated with a [`device::Device`].
        fn id_info(dev: &device::Device) -> Option<&'static Self::IdInfo> {
            // SAFETY: `id_info` is called from `Adapter::probe_callback` with a valid `dev` argument.
   -        let fwnode = unsafe{ (*dev.as_raw()).fwnode};
   +        let fwnode = unsafe { (*dev.as_raw()).fwnode };
    
            // SAFETY: `bindings::is_acpi_device_node` checks `fwnode` before accessing `fwnode->ops`,
            // and only compares it with the address of `acpi_device_fwnode_ops`.
>> Diff in rust/kernel/platform.rs at line 5:
    //! C header: [`include/linux/platform_device.h`](srctree/include/linux/platform_device.h)
    
    use crate::{
   -    acpi,
   -    bindings, device, driver,
   +    acpi, bindings, device, driver,
        error::{to_result, Result},
        of,
        prelude::*,
   Diff in rust/kernel/acpi.rs at line 59:
            $crate::module_device_table!("acpi", $module_table_name, $table_name);
        };
    }
   -
    
>> Diff in rust/kernel/platform.rs at line 5:
    //! C header: [`include/linux/platform_device.h`](srctree/include/linux/platform_device.h)
    
    use crate::{
   -    acpi,
   -    bindings, device, driver,
   +    acpi, bindings, device, driver,
        error::{to_result, Result},
        of,
        prelude::*,
>> Diff in rust/kernel/driver.rs at line 6:
    //! register using the [`Registration`] class.
    
>>  use crate::error::{Error, Result};
   -use crate::{device, of, acpi, 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};
    
   Diff in rust/kernel/driver.rs at line 211:
        /// associated with a [`device::Device`].
        fn id_info(dev: &device::Device) -> Option<&'static Self::IdInfo> {
            // SAFETY: `id_info` is called from `Adapter::probe_callback` with a valid `dev` argument.
   -        let fwnode = unsafe{ (*dev.as_raw()).fwnode};
   +        let fwnode = unsafe { (*dev.as_raw()).fwnode };
    
            // SAFETY: `bindings::is_acpi_device_node` checks `fwnode` before accessing `fwnode->ops`,
            // and only compares it with the address of `acpi_device_fwnode_ops`.
   make[1]: Leaving directory '/kbuild/obj/consumer/x86_64-rhel-9.4-rust'
   make[2]: *** [Makefile:1826: rustfmt] Error 123
   make[1]: *** [Makefile:248: __sub-make] Error 2
   make[2]: Target 'rustfmtcheck' not remade because of errors.
   make[1]: Target 'rustfmtcheck' not remade because of errors.
   make: *** [Makefile:248: __sub-make] Error 2
   make: Target 'rustfmtcheck' not remade because of errors.
   make: Leaving directory '/kbuild/src/consumer'

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v2 0/5] rust: Add ACPI match table support for Rust drivers

[Danilo Krummrich]

On Thu, Jun 05, 2025 at 05:19:56PM +0100, Igor Korotin wrote:
> This patch series introduces support for ACPI match tables in Rust 
> drivers.

Which commit does this patch series apply to?

In general, I recommend to use '--base' on 'git format-patch'.

Re: [PATCH v2 3/5] rust: driver: Add ACPI id table support to Adapter trait

[Danilo Krummrich]

On Thu, Jun 05, 2025 at 05:27:26PM +0100, Igor Korotin wrote:
> From: Igor Korotin <igor.korotin.linux@gmail.com>
> 
> 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.
> ---
>  rust/bindings/bindings_helper.h |  1 +
>  rust/kernel/driver.rs           | 58 ++++++++++++++++++++++++++++++---
>  rust/kernel/platform.rs         |  5 +++
>  3 files changed, 60 insertions(+), 4 deletions(-)
> 
> diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
> index e0bcd130b494..d974fc6c141f 100644
> --- a/rust/bindings/bindings_helper.h
> +++ b/rust/bindings/bindings_helper.h
> @@ -6,6 +6,7 @@
>   * Sorted alphabetically.
>   */
>  
> +#include <linux/acpi.h>
>  #include <kunit/test.h>
>  #include <linux/blk-mq.h>
>  #include <linux/blk_types.h>
> diff --git a/rust/kernel/driver.rs b/rust/kernel/driver.rs
> index ec9166cedfa7..d4098596188a 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::{device, of, acpi, 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,9 +210,27 @@ 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::of_id_info(dev);
> -        if id.is_some() {
> -            return id;
> +        // SAFETY: `id_info` is called from `Adapter::probe_callback` with a valid `dev` argument.
> +        let fwnode = unsafe{ (*dev.as_raw()).fwnode};

There is an abstraction for FwNode on the list [1] that I plan to merge soon.
Generally, it would make sense to build on top of that.

However, I don't understand why we need this and the subsequent
is_acpi_device_node() and is_of_node() checks.

Instead, I think we can keep the existing code and just add the following.

	let id = Self::acpi_id_info(dev);
	if id.is_some() {
	   return id;
	}

[1] https://lore.kernel.org/lkml/20250530192856.1177011-1-remo@buenzli.dev/

> +
> +        // SAFETY: `bindings::is_acpi_device_node` checks `fwnode` before accessing `fwnode->ops`,
> +        // and only compares it with the address of `acpi_device_fwnode_ops`.
> +        if unsafe { bindings::is_acpi_device_node(fwnode) } {

As mentioned above, I think we don't need this check.

> +            let id = Self::acpi_id_info(dev);
> +
> +            if id.is_some() {
> +                return id;
> +            }
> +        }
> +
> +        // SAFETY: `bindings::is_of_node` checks `fwnode` before accessing `fwnode->ops`,
> +        // and only compares it with the address of `of_fwnode_ops`.
> +        if unsafe { bindings::is_of_node(fwnode) } {

Same here.

> +            let id = Self::of_id_info(dev);
> +
> +            if id.is_some() {
> +                return id;
> +            }
>          }
>  
>          None
> diff --git a/rust/kernel/platform.rs b/rust/kernel/platform.rs
> index fd4a494f30e8..3cc9fe6ccfcf 100644
> --- a/rust/kernel/platform.rs
> +++ b/rust/kernel/platform.rs
> @@ -5,6 +5,7 @@
>  //! C header: [`include/linux/platform_device.h`](srctree/include/linux/platform_device.h)
>  
>  use crate::{
> +    acpi,
>      bindings, device, driver,
>      error::{to_result, Result},
>      of,
> @@ -95,6 +96,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
> 

Re: [PATCH v2 5/5] samples: rust: add ACPI match table example to platform driver

[Danilo Krummrich]

On Thu, Jun 05, 2025 at 05:52:31PM +0100, Igor Korotin wrote:
> 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 | 40 +++++++++++++++++++++++++++-
>  1 file changed, 39 insertions(+), 1 deletion(-)
> 
> diff --git a/samples/rust/rust_driver_platform.rs b/samples/rust/rust_driver_platform.rs
> index e3992e7a71e9..ee0780c1d6ae 100644
> --- a/samples/rust/rust_driver_platform.rs
> +++ b/samples/rust/rust_driver_platform.rs
> @@ -17,10 +17,48 @@ struct SampleDriver {
>      [(of::DeviceId::new(c_str!("test,rust-device")), Info(42))]
>  );
>  
> +kernel::acpi_device_table!(
> +    ACPI_TABLE,
> +    MODULE_ACPI_TABLE,
> +    <SampleDriver as platform::Driver>::IdInfo,
> +    [(acpi::DeviceId::new(c_str!("TEST4321")), Info(0))]

Can you please explain add a comment explaining how to make this probe? In the
cover letter you mention:

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

> +);
> +
> +/// OF/ACPI match tables for Platform Driver implementation
> +///
> +/// 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:
> +///
> +///```
> +/// impl platform::Driver for SampleDriver {
> +///     type IdInfo = Info;
> +///     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>,
> +///         info: Option<&Self::IdInfo>,
> +///     ) -> Result<Pin<KBox<Self>>> {
> +///         dev_dbg!(pdev.as_ref(), "Probe Rust Platform driver sample.\n");
> +///
> +///         if let Some(info) = info {
> +///             dev_info!(pdev.as_ref(), "Probed with info: '{}'.\n", info.0);
> +///         }
> +///
> +///         let drvdata = KBox::new(Self { pdev: pdev.into() }, GFP_KERNEL)?;
> +///
> +///         Ok(drvdata.into())
> +///     }
> +/// }
> +///```

I assume you want to make clear that both the ACPI and OF table are optional;
not sure of that's required given their type is Option<...>. But I'm fine having
this additional comment and example.

Please make sure that it compiles though and remove everything unnecessary from
probe() please.

> +
>  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
> 

Re: [PATCH v2 3/5] rust: driver: Add ACPI id table support to Adapter trait

[Igor Korotin]

On Fri, Jun 6, 2025 at 2:50 PM Danilo Krummrich <dakr@kernel.org> wrote:
> However, I don't understand why we need this and the subsequent
> is_acpi_device_node() and is_of_node() checks.

The idea is to avoid unnecessary table lookups when both OF and ACPI
match tables are present. If we already know the fwnode type, these
simple pointer comparisons (is_acpi_device_node() / is_of_node()) let
us skip the irrelevant match function.

Those checks are cheap (just pointer comparisons), while
acpi_match_device() and of_match_device() iterate over tables.

So yeah, it’s a bit ugly, but it can save some CPU cycles during enumeration.

Thanks,
Igor

Re: [PATCH v2 3/5] rust: driver: Add ACPI id table support to Adapter trait

[Danilo Krummrich]

On Fri, Jun 06, 2025 at 03:26:13PM +0100, Igor Korotin wrote:
> On Fri, Jun 6, 2025 at 2:50 PM Danilo Krummrich <dakr@kernel.org> wrote:
> > However, I don't understand why we need this and the subsequent
> > is_acpi_device_node() and is_of_node() checks.
> 
> The idea is to avoid unnecessary table lookups when both OF and ACPI
> match tables are present.

Ok, that's fair -- let's build it on top of the FwNode abstractions though.

Re: [PATCH v2 3/5] rust: driver: Add ACPI id table support to Adapter trait

[Igor Korotin]

On Fri, Jun 6, 2025 at 3:32 PM Danilo Krummrich <dakr@kernel.org> wrote:
>
> On Fri, Jun 06, 2025 at 03:26:13PM +0100, Igor Korotin wrote:
> > On Fri, Jun 6, 2025 at 2:50 PM Danilo Krummrich <dakr@kernel.org> wrote:
> > > However, I don't understand why we need this and the subsequent
> > > is_acpi_device_node() and is_of_node() checks.
> >
> > The idea is to avoid unnecessary table lookups when both OF and ACPI
> > match tables are present.
>
> Ok, that's fair -- let's build it on top of the FwNode abstractions though.

I'm ok with the FwNode abstractions. Just to make sure I understood
you correctly:
I'll need to wait until these FwNode abstractions are pushed to the
rust-next branch, reimplement what is necessary and send v3. Is this
the course of actions?

Thanks
Igor

Re: [PATCH v2 3/5] rust: driver: Add ACPI id table support to Adapter trait

[Danilo Krummrich]

On Fri, Jun 06, 2025 at 03:58:11PM +0100, Igor Korotin wrote:
> On Fri, Jun 6, 2025 at 3:32 PM Danilo Krummrich <dakr@kernel.org> wrote:
> >
> > On Fri, Jun 06, 2025 at 03:26:13PM +0100, Igor Korotin wrote:
> > > On Fri, Jun 6, 2025 at 2:50 PM Danilo Krummrich <dakr@kernel.org> wrote:
> > > > However, I don't understand why we need this and the subsequent
> > > > is_acpi_device_node() and is_of_node() checks.
> > >
> > > The idea is to avoid unnecessary table lookups when both OF and ACPI
> > > match tables are present.
> >
> > Ok, that's fair -- let's build it on top of the FwNode abstractions though.
> 
> I'm ok with the FwNode abstractions. Just to make sure I understood
> you correctly:
> I'll need to wait until these FwNode abstractions are pushed to the
> rust-next branch, reimplement what is necessary and send v3. Is this
> the course of actions?

Not all Rust code goes through the Rust-for-Linux tree, it depends on who
maintains the code. For the FwNode and device property series I pointed you to,
the code is maintained by the driver-core maintainers and hence will go through
the driver-core tree [1].

(You can always check the corresponding entries in the MAINTAINERS file, they
document, who maintains a file and which tree changes go through. For
instance, if you want to know this for a specific file, you can run

	./scripts/get_maintainer.pl path/to/file

and the script tells you everything you need to know.

In general, before submitting patches you should run this script on your patches
to find out who you should send them to.)

However, there is no need to wait until the FwNode and device property series
lands in driver-core-next, you can just fetch the patch series from the mailing
list and build your patches on top of that.

If you do this, you should make sure to mention the exact series you build on
top of in the cover letter, ideally with a lore link to the specific version of
the series.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/driver-core/driver-core.git/

Re: [PATCH v2 3/5] rust: driver: Add ACPI id table support to Adapter trait

[Greg Kroah-Hartman]

On Fri, Jun 06, 2025 at 03:26:13PM +0100, Igor Korotin wrote:
> On Fri, Jun 6, 2025 at 2:50 PM Danilo Krummrich <dakr@kernel.org> wrote:
> > However, I don't understand why we need this and the subsequent
> > is_acpi_device_node() and is_of_node() checks.
> 
> The idea is to avoid unnecessary table lookups when both OF and ACPI
> match tables are present. If we already know the fwnode type, these
> simple pointer comparisons (is_acpi_device_node() / is_of_node()) let
> us skip the irrelevant match function.
> 
> Those checks are cheap (just pointer comparisons), while
> acpi_match_device() and of_match_device() iterate over tables.
> 
> So yeah, it’s a bit ugly, but it can save some CPU cycles during enumeration.

You have loads of CPU cycles during enumeration, keep things simple
first, only attempt to optimize things later on if it is actually
measureable.

thanks,

greg k-h

Re: [PATCH v2 3/5] rust: driver: Add ACPI id table support to Adapter trait

[Danilo Krummrich]

On 6/6/25 5:29 PM, Greg Kroah-Hartman wrote:
> On Fri, Jun 06, 2025 at 03:26:13PM +0100, Igor Korotin wrote:
>> On Fri, Jun 6, 2025 at 2:50 PM Danilo Krummrich <dakr@kernel.org> wrote:
>>> However, I don't understand why we need this and the subsequent
>>> is_acpi_device_node() and is_of_node() checks.
>>
>> The idea is to avoid unnecessary table lookups when both OF and ACPI
>> match tables are present. If we already know the fwnode type, these
>> simple pointer comparisons (is_acpi_device_node() / is_of_node()) let
>> us skip the irrelevant match function.
>>
>> Those checks are cheap (just pointer comparisons), while
>> acpi_match_device() and of_match_device() iterate over tables.
>>
>> So yeah, it’s a bit ugly, but it can save some CPU cycles during enumeration.
> 
> You have loads of CPU cycles during enumeration, keep things simple
> first, only attempt to optimize things later on if it is actually
> measureable.

I'm fine either way, I don't expect much value in optimizing this and at the
same time I don't see doing it adds significant complexity either.

If Greg prefers not to have this optimization to begin with, let's go without
it please.

Re: [PATCH v2 3/5] rust: driver: Add ACPI id table support to Adapter trait

[Igor Korotin]

On Fri, Jun 6, 2025 at 4:39 PM Danilo Krummrich <dakr@kernel.org> wrote:
>
> On 6/6/25 5:29 PM, Greg Kroah-Hartman wrote:
> > On Fri, Jun 06, 2025 at 03:26:13PM +0100, Igor Korotin wrote:
> >> On Fri, Jun 6, 2025 at 2:50 PM Danilo Krummrich <dakr@kernel.org> wrote:
> >>> However, I don't understand why we need this and the subsequent
> >>> is_acpi_device_node() and is_of_node() checks.
> >>
> >> The idea is to avoid unnecessary table lookups when both OF and ACPI
> >> match tables are present. If we already know the fwnode type, these
> >> simple pointer comparisons (is_acpi_device_node() / is_of_node()) let
> >> us skip the irrelevant match function.
> >>
> >> Those checks are cheap (just pointer comparisons), while
> >> acpi_match_device() and of_match_device() iterate over tables.
> >>
> >> So yeah, it’s a bit ugly, but it can save some CPU cycles during enumeration.
> >
> > You have loads of CPU cycles during enumeration, keep things simple
> > first, only attempt to optimize things later on if it is actually
> > measureable.
>
> I'm fine either way, I don't expect much value in optimizing this and at the
> same time I don't see doing it adds significant complexity either.
>
> If Greg prefers not to have this optimization to begin with, let's go without
> it please.

I'm ok with this as well. Will remove those checks. I assume that I
don't need to align with `FwNode` abstractions in that case. Also I'll
drop `is_of_node` rust helper in v3.

Thanks for the review and your comments, guys

Cheers
Igor