[PATCH v5] rust: ACPI: fix missing match data for PRP0001

Markus Probst <[email protected]>
Newsgroups dev.linux.lists.acpica-devel,dev.linux.lists.driver-core,org.kernel.vger.linux-acpi,org.kernel.vger.linux-kernel,org.kernel.vger.rust-for-linux
Message-ID <[email protected]>
Export `acpi_of_match_device` function and use it to match the of device
table against ACPI PRP0001 in Rust.

This fixes id_info being None on ACPI PRP0001 devices.

Using `device_get_match_data` is not possible, because Rust stores an
index in the of device id instead of a data pointer. This was done this
way to provide a convenient and obvious API for drivers, which can be
evaluated in const context without the use of any unstable language
features.

Fixes: 7a718a1f26d1 ("rust: driver: implement `Adapter`")
Signed-off-by: Markus Probst <[email protected]>
---
Changes in v5:
- provide a stub instead of canditionalize the caller
- Link to v4: https://lore.kernel.org/r/[email protected]

Changes in v4:
- fix compilation on !CONFIG_ACPI
- fix typo
- add doc to `acpi_of_match_device` and remove from header file include
- Link to v3: https://lore.kernel.org/r/[email protected]

Changes in v3:
- use acpi/acpi_bus.h instead of drivers/acpi/internal.h to declare
  `acpi_of_match_device`
- Link to v2: https://lore.kernel.org/r/[email protected]

Changes in v2:
- keep unsafe block as short as possible
- don't export acpi_of_match_device
- add implementation decisions to commit msg
- Link to v1: https://lore.kernel.org/r/[email protected]
---
 MAINTAINERS             |  1 +
 drivers/acpi/bus.c      |  6 ++--
 include/acpi/acpi_bus.h | 11 ++++++++
 rust/helpers/acpi.c     | 16 +++++++++++
 rust/helpers/helpers.c  |  1 +
 rust/kernel/driver.rs   | 74 ++++++++++++++++++++++++++++++++++++++++---------
 6 files changed, 93 insertions(+), 16 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index d1cc0e12fe1f..7da4eea48330 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/helpers/acpi.c
 F:	rust/kernel/acpi.rs
 F:	tools/power/acpi/
 
diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index 2ec095e2009e..554dbddb1c8c 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -831,9 +831,9 @@ const struct acpi_device *acpi_companion_match(const struct device *dev)
  * identifiers and a _DSD object with the "compatible" property, use that
  * property to match against the given list of identifiers.
  */
-static bool acpi_of_match_device(const struct acpi_device *adev,
-				 const struct of_device_id *of_match_table,
-				 const struct of_device_id **of_id)
+bool acpi_of_match_device(const struct acpi_device *adev,
+			  const struct of_device_id *of_match_table,
+			  const struct of_device_id **of_id)
 {
 	const union acpi_object *of_compatible, *obj;
 	int i, nval;
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index aad1a95e6863..049301e71a11 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -187,6 +187,10 @@ struct acpi_driver {
  * -----------
  */
 
+bool acpi_of_match_device(const struct acpi_device *adev,
+			  const struct of_device_id *of_match_table,
+			  const struct of_device_id **of_id);
+
 /* Status (_STA) */
 
 struct acpi_device_status {
@@ -998,6 +1002,13 @@ int acpi_scan_add_dep(acpi_handle handle, struct acpi_handle_list *dep_devices);
 u32 arch_acpi_add_auto_dep(acpi_handle handle);
 #else	/* CONFIG_ACPI */
 
+static inline bool acpi_of_match_device(const struct acpi_device *adev,
+					const struct of_device_id *of_match_table,
+					const struct of_device_id **of_id)
+{
+	return false;
+}
+
 static inline int register_acpi_bus_type(void *bus) { return 0; }
 static inline int unregister_acpi_bus_type(void *bus) { return 0; }
 
diff --git a/rust/helpers/acpi.c b/rust/helpers/acpi.c
new file mode 100644
index 000000000000..e75c9807bbad
--- /dev/null
+++ b/rust/helpers/acpi.c
@@ -0,0 +1,16 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/acpi.h>
+#include <acpi/acpi_bus.h>
+
+__rust_helper bool rust_helper_acpi_of_match_device(const struct acpi_device *adev,
+						    const struct of_device_id *of_match_table,
+						    const struct of_device_id **of_id)
+{
+	return acpi_of_match_device(adev, of_match_table, of_id);
+}
+
+__rust_helper struct acpi_device *rust_helper_to_acpi_device_node(struct fwnode_handle *fwnode)
+{
+	return to_acpi_device_node(fwnode);
+}
diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
index a3c42e51f00a..8ecd1580aa69 100644
--- a/rust/helpers/helpers.c
+++ b/rust/helpers/helpers.c
@@ -9,6 +9,7 @@
 
 #define __rust_helper
 
+#include "acpi.c"
 #include "atomic.c"
 #include "atomic_ext.c"
 #include "auxiliary.c"
diff --git a/rust/kernel/driver.rs b/rust/kernel/driver.rs
index 36de8098754d..93e5dd6ae371 100644
--- a/rust/kernel/driver.rs
+++ b/rust/kernel/driver.rs
@@ -278,6 +278,26 @@ fn init(
     }
 }
 
+// Calling the FFI function directly from the `Adapter` impl may result in it being called
+// directly from driver modules. This happens since the Rust compiler will use monomorphisation, so
+// it might happen that functions are instantiated within the calling driver module. For now, work
+// around this with `#[inline(never)]` helpers.
+//
+// TODO: Remove once a more generic solution has been implemented. For instance, we may be able to
+// leverage `bindgen` to take care of this depending on whether a symbol is (already) exported.
+#[inline(never)]
+#[allow(clippy::missing_safety_doc)]
+#[allow(dead_code)]
+#[must_use]
+unsafe fn acpi_of_match_device(
+    adev: *const bindings::acpi_device,
+    of_match_table: *const bindings::of_device_id,
+    of_id: *mut *const bindings::of_device_id,
+) -> bool {
+    // SAFETY: Safety requirements are the same as `bindings::acpi_of_match_device`.
+    unsafe { bindings::acpi_of_match_device(adev, of_match_table, of_id) }
+}
+
 /// The bus independent adapter to match a drivers and a devices.
 ///
 /// This trait should be implemented by the bus specific adapter, which represents the connection
@@ -329,35 +349,63 @@ fn acpi_id_info(dev: &device::Device) -> Option<&'static Self::IdInfo> {
     ///
     /// If this returns `None`, it means there is no match with an entry in the [`of::IdTable`].
     fn of_id_info(dev: &device::Device) -> Option<&'static Self::IdInfo> {
-        #[cfg(not(CONFIG_OF))]
+        let table = Self::of_id_table()?;
+
+        #[cfg(not(any(CONFIG_OF, CONFIG_ACPI)))]
         {
-            let _ = dev;
-            None
+            let _ = (dev, table);
         }
 
         #[cfg(CONFIG_OF)]
         {
-            let table = Self::of_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 `dev.as_raw()`.
             let raw_id = unsafe { bindings::of_match_device(table.as_ptr(), dev.as_raw()) };
 
-            if raw_id.is_null() {
-                None
-            } else {
+            if !raw_id.is_null() {
                 // 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>() };
 
-                Some(
-                    table.info(<of::DeviceId as crate::device_id::RawDeviceIdIndex>::index(
-                        id,
-                    )),
-                )
+                return Some(table.info(
+                    <of::DeviceId as crate::device_id::RawDeviceIdIndex>::index(id),
+                ));
             }
         }
+
+        #[cfg(CONFIG_ACPI)]
+        {
+            use core::ptr;
+            use device::property::FwNode;
+
+            let mut raw_id = ptr::null();
+
+            let fwnode = dev.fwnode().map_or(ptr::null_mut(), FwNode::as_raw);
+
+            // SAFETY: `fwnode` is a pointer to a valid `fwnode_handle`. A null pointer will be
+            // passed through the function.
+            let adev = unsafe { bindings::to_acpi_device_node(fwnode) };
+
+            // SAFETY:
+            // - `adev` is a valid pointer to `acpi_device` or is null. It is guaranteed to be
+            //   valid as long as `dev` is alive.
+            // - `table` has static lifetime, hence it's valid for read.
+            if unsafe { acpi_of_match_device(adev, table.as_ptr(), &raw mut raw_id) } {
+                // SAFETY:
+                // - the function returns true, therefore `raw_id` has been set to a pointer to a
+                //   valid `of_device_id`.
+                // - `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>() };
+
+                return Some(table.info(
+                    <of::DeviceId as crate::device_id::RawDeviceIdIndex>::index(id),
+                ));
+            }
+        }
+
+        None
     }
 
     /// Returns the driver's private data from the matching entry of any of the ID tables, if any.

---
base-commit: 3131ff5a117498bb4b9db3a238bb311cbf8383ce
change-id: 20260401-rust_acpi_prp0001-a2971543b555
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.