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 --- 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 -#include - -__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>; - /// 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::() }; - - // SAFETY: `id` comes from `table` which is of type `IdArray<_, Self::IdInfo>`. - Some(unsafe { id.info_unchecked::() }) - } - } - } - /// The [`of::IdTable`] of the corresponding driver. fn of_id_table() -> Option>; - /// 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::() }; - - // SAFETY: `id` comes from `table` which is of type `IdArray<_, Self::IdInfo>`. - return Some(unsafe { id.info_unchecked::() }); - } - } - - #[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::() }; - - // SAFETY: `id` comes from `table` which is of type `IdArray<_, Self::IdInfo>`. - return Some(unsafe { id.info_unchecked::() }); - } - } - - 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::().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::>>() }; - let info = - Self::i2c_id_info(idev).or_else(|| ::id_info(idev.as_ref())); + let info = Self::i2c_id_info(idev).or_else(|| { + // SAFETY: `idev` matched data is of type `Self::IdInfo`. + unsafe { ::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::>>() }; - let info = ::id_info(pdev.as_ref()); + // SAFETY: `pdev` matched data is of type `Self::IdInfo`. + let info = unsafe { ::id_info(pdev.as_ref()) }; from_result(|| { let data = T::probe(pdev, info); -- 2.54.0