mbox series

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

Message ID 20250605161956.3658374-1-igor.korotin.linux@gmail.com
Headers show
Series rust: Add ACPI match table support for Rust drivers | expand

Message

Igor Korotin June 5, 2025, 4:19 p.m. UTC
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

Comments

Danilo Krummrich June 6, 2025, 1:26 p.m. UTC | #1
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'.
Danilo Krummrich June 6, 2025, 2:32 p.m. UTC | #2
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.
Danilo Krummrich June 6, 2025, 3:14 p.m. UTC | #3
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/
Danilo Krummrich June 6, 2025, 3:38 p.m. UTC | #4
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.