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

"Rafael J. Wysocki" <[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 <CAJZ5v0hBqxm93-eQ9UZZjbYmE_1+8KaBBe7-5LVz8cdDgwsyxA@mail.gmail.com>
On Tue, Apr 7, 2026 at 11:42 PM Markus Probst <[email protected]> wrote:
>
> 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]>

From the ACPI side

Acked-by: Rafael J. Wysocki (Intel) <[email protected]>

but I'm not a Rust expert.

> ---
> 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         |  4 +++
>  rust/bindings/bindings_helper.h |  5 +++
>  rust/helpers/acpi.c             |  8 +++++
>  rust/helpers/helpers.c          |  1 +
>  rust/kernel/driver.rs           | 70 ++++++++++++++++++++++++++++++++---------
>  7 files changed, 78 insertions(+), 17 deletions(-)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index c3fe46d7c4bc..3a7b3b5f2a28 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..d0098f24346f 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 {
> diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
> index 083cc44aa952..e47643ce8b50 100644
> --- a/rust/bindings/bindings_helper.h
> +++ b/rust/bindings/bindings_helper.h
> @@ -96,6 +96,11 @@
>   */
>  #include <../../drivers/base/base.h>
>
> +/*
> + * The driver-core Rust code needs to call `acpi_of_match_device`.
> + */
> +#include <acpi/acpi_bus.h>
> +
>  #if defined(CONFIG_DRM_PANIC_SCREEN_QR_CODE)
>  // Used by `#[export]` in `drivers/gpu/drm/drm_panic_qr.rs`.
>  #include <drm/drm_panic.h>
> diff --git a/rust/helpers/acpi.c b/rust/helpers/acpi.c
> new file mode 100644
> index 000000000000..f2aa00ec99c2
> --- /dev/null
> +++ b/rust/helpers/acpi.c
> @@ -0,0 +1,8 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <linux/acpi.h>
> +
> +__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..62d7d1ab776c 100644
> --- a/rust/kernel/driver.rs
> +++ b/rust/kernel/driver.rs
> @@ -96,13 +96,18 @@
>
>  use crate::{
>      acpi,
> -    device,
> +    device::{
> +        self,
> +        property::FwNode, //
> +    },
>      of,
>      prelude::*,
>      types::Opaque,
>      ThisModule, //
>  };
>
> +use core::ptr;
> +
>  /// Trait describing the layout of a specific device driver.
>  ///
>  /// This trait describes the layout of a specific driver structure, such as `struct pci_driver` or
> @@ -278,6 +283,18 @@ fn init(
>      }
>  }
>
> +#[inline(never)]
> +#[allow(clippy::missing_safety_doc)]
> +#[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_device_id`.
> +    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 +346,60 @@ 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)]
> +        {
> +            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: 0e5d0a0b5ca6ea4e391d6786266405c5871e0151
> 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.