[PATCH v2 09/11] rust: driver: remove open-coded matching logic

Gary Guo <[email protected]>
Newsgroups org.kernel.vger.linux-pwm,dev.linux.lists.driver-core,dev.linux.lists.nova-gpu,org.freedesktop.lists.dri-devel,org.kernel.vger.linux-acpi,org.kernel.vger.linux-devicetree,org.kernel.vger.linux-kernel,org.kernel.vger.linux-pci,org.kernel.vger.linux-pm,org.kernel.vger.linux-usb,org.kernel.vger.netdev,org.kernel.vger.rust-for-linux
Message-ID <[email protected]>
With device ID info now including pointers instead of indices, the
open-coded ACPI/OF matching is no longer needed and can be replaced with
`device_get_match_data`.

With the removal of open-coded matching, the exposed functions and helpers
are also removed; this effectively reverts most of commit 2690d071584e
("rust: ACPI: fix missing match data for PRP0001").

Signed-off-by: Gary Guo <[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   | 137 ++++--------------------------------------------
 rust/kernel/i2c.rs      |   6 ++-
 rust/kernel/platform.rs |   3 +-
 8 files changed, 20 insertions(+), 161 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 15011f5752a9..07240e0b7451 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -297,7 +297,6 @@ 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 a30a904f6535..1fb0b10bf924 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -898,9 +898,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.
  */
-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)
+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)
 {
 	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 714d111d8053..7e57f9698f7c 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -189,10 +189,6 @@ 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 {
@@ -1000,13 +996,6 @@ 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
deleted file mode 100644
index e75c9807bbad..000000000000
--- a/rust/helpers/acpi.c
+++ /dev/null
@@ -1,16 +0,0 @@
-// 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 998e31052e66..fadd7acdd81c 100644
--- a/rust/helpers/helpers.c
+++ b/rust/helpers/helpers.c
@@ -38,7 +38,6 @@
 #define __rust_helper __always_inline
 #endif
 
-#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 824899d76fed..c9c74c4dde8f 100644
--- a/rust/kernel/driver.rs
+++ b/rust/kernel/driver.rs
@@ -107,7 +107,6 @@
 use crate::{
     acpi,
     device,
-    device_id::RawDeviceIdIndex,
     of,
     prelude::*,
     types::Opaque,
@@ -292,26 +291,6 @@ 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
@@ -325,117 +304,23 @@ pub trait Adapter {
     /// 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`].
-    fn acpi_id_info(dev: &device::Device) -> Option<&'static Self::IdInfo> {
-        #[cfg(not(CONFIG_ACPI))]
-        {
-            let _ = dev;
-            None
-        }
-
-        #[cfg(CONFIG_ACPI)]
-        {
-            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 `dev.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 acpi_device_id`
-                // and does not add additional invariants, so it's safe to transmute.
-                let id = unsafe { &*raw_id.cast::<acpi::DeviceId>() };
-
-                // SAFETY: `id` comes from `table` which is of type `IdArray<_, Self::IdInfo>`.
-                Some(unsafe { id.info_unchecked::<Self::IdInfo>() })
-            }
-        }
-    }
-
     /// The [`of::IdTable`] of the corresponding driver.
     fn of_id_table() -> Option<of::IdTable<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`].
-    fn of_id_info(dev: &device::Device) -> Option<&'static Self::IdInfo> {
-        let table = Self::of_id_table()?;
-
-        #[cfg(not(any(CONFIG_OF, CONFIG_ACPI)))]
-        {
-            let _ = (dev, table);
-        }
-
-        #[cfg(CONFIG_OF)]
-        {
-            // 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() {
-                // 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: `id` comes from `table` which is of type `IdArray<_, Self::IdInfo>`.
-                return Some(unsafe { id.info_unchecked::<Self::IdInfo>() });
-            }
-        }
-
-        #[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>() };
-
-                // SAFETY: `id` comes from `table` which is of type `IdArray<_, Self::IdInfo>`.
-                return Some(unsafe { id.info_unchecked::<Self::IdInfo>() });
-            }
-        }
-
-        None
-    }
-
     /// Returns the driver's private data from the matching entry of any of the ID tables, if any.
     ///
     /// 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;
-        }
-
-        None
+    ///
+    /// # Safety
+    ///
+    /// The caller must ensure that the `dev` matched data is of type `Self::IdInfo`.
+    #[inline]
+    unsafe fn id_info(dev: &device::Device) -> Option<&'static Self::IdInfo> {
+        // SAFETY: `dev` is guaranteed to be valid while it's alive, and so is `dev.as_raw()`.
+        let data = unsafe { bindings::device_get_match_data(dev.as_raw()) };
+
+        // SAFETY: Per safety requirement, `data` is of type `Self::IdInfo`.
+        unsafe { data.cast::<Self::IdInfo>().as_ref() }
     }
 }
diff --git a/rust/kernel/i2c.rs b/rust/kernel/i2c.rs
index 31f2cfe6be31..2d2191164e25 100644
--- a/rust/kernel/i2c.rs
+++ b/rust/kernel/i2c.rs
@@ -149,8 +149,10 @@ extern "C" fn probe_callback(idev: *mut bindings::i2c_client) -> kernel::ffi::c_
         // INVARIANT: `idev` is valid for the duration of `probe_callback()`.
         let idev = unsafe { &*idev.cast::<I2cClient<device::CoreInternal<'_>>>() };
 
-        let info =
-            Self::i2c_id_info(idev).or_else(|| <Self as driver::Adapter>::id_info(idev.as_ref()));
+        let info = Self::i2c_id_info(idev).or_else(|| {
+            // SAFETY: `idev` matched data is of type `Self::IdInfo`.
+            unsafe { <Self as driver::Adapter>::id_info(idev.as_ref()) }
+        });
 
         from_result(|| {
             let data = T::probe(idev, info);
diff --git a/rust/kernel/platform.rs b/rust/kernel/platform.rs
index 210a815925ce..e12e88113ca5 100644
--- a/rust/kernel/platform.rs
+++ b/rust/kernel/platform.rs
@@ -100,7 +100,8 @@ extern "C" fn probe_callback(pdev: *mut bindings::platform_device) -> kernel::ff
         //
         // INVARIANT: `pdev` is valid for the duration of `probe_callback()`.
         let pdev = unsafe { &*pdev.cast::<Device<device::CoreInternal<'_>>>() };
-        let info = <Self as driver::Adapter>::id_info(pdev.as_ref());
+        // SAFETY: `pdev` matched data is of type `Self::IdInfo`.
+        let info = unsafe { <Self as driver::Adapter>::id_info(pdev.as_ref()) };
 
         from_result(|| {
             let data = T::probe(pdev, info);

-- 
2.54.0
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.