Extend the Rust sample platform driver to probe using device/driver name
matching, OF ID table matching, or ACPI ID table matching.
Signed-off-by: Igor Korotin <igor.korotin.linux@gmail.com>
---
samples/rust/rust_driver_platform.rs | 71 +++++++++++++++++++++++++++-
1 file changed, 70 insertions(+), 1 deletion(-)
diff --git a/samples/rust/rust_driver_platform.rs b/samples/rust/rust_driver_platform.rs
index 8b42b3cfb363..35d5067aa023 100644
--- a/samples/rust/rust_driver_platform.rs
+++ b/samples/rust/rust_driver_platform.rs
@@ -2,7 +2,7 @@
//! Rust Platform driver sample.
-use kernel::{c_str, device::Core, of, platform, prelude::*, types::ARef};
+use kernel::{acpi, c_str, device::Core, of, platform, prelude::*, types::ARef};
struct SampleDriver {
pdev: ARef<platform::Device>,
@@ -17,9 +17,78 @@ struct SampleDriver {
[(of::DeviceId::new(c_str!("test,rust-device")), Info(42))]
);
+// ACPI match table test
+//
+// This demonstrates how to test an ACPI-based Rust platform driver using QEMU
+// with a custom SSDT.
+//
+// Steps:
+//
+// 1. **Create an SSDT source file** (`ssdt.dsl`) with the following content:
+//
+// ```asl
+// DefinitionBlock ("", "SSDT", 2, "TEST", "VIRTACPI", 0x00000001)
+// {
+// Scope (\_SB)
+// {
+// Device (T432)
+// {
+// Name (_HID, "TEST4321") // ACPI hardware ID to match
+// Name (_UID, 1)
+// Name (_STA, 0x0F) // Device present, enabled
+// Name (_CRS, ResourceTemplate ()
+// {
+// Memory32Fixed (ReadWrite, 0xFED00000, 0x1000)
+// })
+// }
+// }
+// }
+// ```
+//
+// 2. **Compile the table**:
+//
+// ```sh
+// iasl -tc ssdt.dsl
+// ```
+//
+// This generates `ssdt.aml`
+//
+// 3. **Run QEMU** with the compiled AML file:
+//
+// ```sh
+// qemu-system-x86_64 -m 512M \
+// -enable-kvm \
+// -kernel path/to/bzImage \
+// -append "root=/dev/sda console=ttyS0" \
+// -hda rootfs.img \
+// -serial stdio \
+// -acpitable file=ssdt.aml
+// ```
+//
+// Requirements:
+// - The `rust_driver_platform` must be present either:
+// - built directly into the kernel (`bzImage`), or
+// - available as a `.ko` file and loadable from `rootfs.img`
+//
+// 4. **Verify it worked** by checking `dmesg`:
+//
+// ```
+// rust_driver_platform TEST4321:00: Probed with info: '0'.
+// ```
+//
+// This demonstrates ACPI table matching using a custom ID in QEMU with a minimal SSDT
+
+kernel::acpi_device_table!(
+ ACPI_TABLE,
+ MODULE_ACPI_TABLE,
+ <SampleDriver as platform::Driver>::IdInfo,
+ [(acpi::DeviceId::new(c_str!("TEST4321")), Info(0))]
+);
+
impl platform::Driver for SampleDriver {
type IdInfo = Info;
const OF_ID_TABLE: Option<of::IdTable<Self::IdInfo>> = Some(&OF_TABLE);
+ const ACPI_ID_TABLE: Option<acpi::IdTable<Self::IdInfo>> = Some(&ACPI_TABLE);
fn probe(
pdev: &platform::Device<Core>,
--
2.43.0On Fri, Jun 13, 2025 at 02:54:07PM +0100, Igor Korotin wrote:
> Extend the Rust sample platform driver to probe using device/driver name
> matching, OF ID table matching, or ACPI ID table matching.
Can you please rebase onto driver-core-next? You may also want to add the three
patches I sent as a reply to this one to your series.
> Signed-off-by: Igor Korotin <igor.korotin.linux@gmail.com>
> ---
> samples/rust/rust_driver_platform.rs | 71 +++++++++++++++++++++++++++-
> 1 file changed, 70 insertions(+), 1 deletion(-)
>
> diff --git a/samples/rust/rust_driver_platform.rs b/samples/rust/rust_driver_platform.rs
> index 8b42b3cfb363..35d5067aa023 100644
> --- a/samples/rust/rust_driver_platform.rs
> +++ b/samples/rust/rust_driver_platform.rs
> @@ -2,7 +2,7 @@
>
> //! Rust Platform driver sample.
>
> -use kernel::{c_str, device::Core, of, platform, prelude::*, types::ARef};
> +use kernel::{acpi, c_str, device::Core, of, platform, prelude::*, types::ARef};
>
> struct SampleDriver {
> pdev: ARef<platform::Device>,
> @@ -17,9 +17,78 @@ struct SampleDriver {
> [(of::DeviceId::new(c_str!("test,rust-device")), Info(42))]
> );
>
> +// ACPI match table test
> +//
> +// This demonstrates how to test an ACPI-based Rust platform driver using QEMU
> +// with a custom SSDT.
> +//
> +// Steps:
> +//
> +// 1. **Create an SSDT source file** (`ssdt.dsl`) with the following content:
> +//
> +// ```asl
> +// DefinitionBlock ("", "SSDT", 2, "TEST", "VIRTACPI", 0x00000001)
> +// {
> +// Scope (\_SB)
> +// {
> +// Device (T432)
> +// {
> +// Name (_HID, "TEST4321") // ACPI hardware ID to match
> +// Name (_UID, 1)
> +// Name (_STA, 0x0F) // Device present, enabled
> +// Name (_CRS, ResourceTemplate ()
> +// {
> +// Memory32Fixed (ReadWrite, 0xFED00000, 0x1000)
> +// })
> +// }
> +// }
> +// }
> +// ```
> +//
> +// 2. **Compile the table**:
> +//
> +// ```sh
> +// iasl -tc ssdt.dsl
> +// ```
> +//
> +// This generates `ssdt.aml`
> +//
> +// 3. **Run QEMU** with the compiled AML file:
> +//
> +// ```sh
> +// qemu-system-x86_64 -m 512M \
> +// -enable-kvm \
> +// -kernel path/to/bzImage \
> +// -append "root=/dev/sda console=ttyS0" \
> +// -hda rootfs.img \
> +// -serial stdio \
> +// -acpitable file=ssdt.aml
> +// ```
> +//
> +// Requirements:
> +// - The `rust_driver_platform` must be present either:
> +// - built directly into the kernel (`bzImage`), or
> +// - available as a `.ko` file and loadable from `rootfs.img`
> +//
> +// 4. **Verify it worked** by checking `dmesg`:
> +//
> +// ```
> +// rust_driver_platform TEST4321:00: Probed with info: '0'.
> +// ```
> +//
> +// This demonstrates ACPI table matching using a custom ID in QEMU with a minimal SSDT
Can you please move this up into the module-level documentation?
> +kernel::acpi_device_table!(
> + ACPI_TABLE,
> + MODULE_ACPI_TABLE,
> + <SampleDriver as platform::Driver>::IdInfo,
> + [(acpi::DeviceId::new(c_str!("TEST4321")), Info(0))]
Let's use something like "SAMPLE_DRV0" instead.
> +);
> +
> impl platform::Driver for SampleDriver {
> type IdInfo = Info;
> const OF_ID_TABLE: Option<of::IdTable<Self::IdInfo>> = Some(&OF_TABLE);
> + const ACPI_ID_TABLE: Option<acpi::IdTable<Self::IdInfo>> = Some(&ACPI_TABLE);
>
> fn probe(
> pdev: &platform::Device<Core>,
> --
> 2.43.0
>
On 6/16/25 20:52, Danilo Krummrich wrote:
>> +kernel::acpi_device_table!(
>> + ACPI_TABLE,
>> + MODULE_ACPI_TABLE,
>> + <SampleDriver as platform::Driver>::IdInfo,
>> + [(acpi::DeviceId::new(c_str!("TEST4321")), Info(0))]
>
> Let's use something like "SAMPLE_DRV0" instead.
Nope, that's prohibited by ACPI HID naming rules. A brief description
can be found here:
https://uefi.org/specs/ACPI/6.5/06_Device_Configuration.html.
According to section 6.1.5, ACPI IDs must be 7 or 8 characters long and
follow the format "AAA####" or "AAAA####", where A is an uppercase
letter [A–Z] and # is an uppercase letter or digit [A–Z, 0–9]. No
underscores or special characters are allowed.
The `iasl` utility throws an error if an invalid HID string is used in a
dsl file.
I could suggest on of the following:
DRV0001
DRVR0001
PDRV0001
TEST0001
TST0001
Let me know which one you'd prefer for the code and documentation.
Best Regards
Igor
On Tue, Jun 17, 2025 at 05:39:07PM +0100, Igor Korotin wrote: > I could suggest on of the following: > DRV0001 > DRVR0001 > PDRV0001 This one looks reasonable to me. > TEST0001 > TST0001
On Tue, Jun 17, 2025 at 7:15 PM Danilo Krummrich <dakr@kernel.org> wrote: > > On Tue, Jun 17, 2025 at 05:39:07PM +0100, Igor Korotin wrote: > > I could suggest on of the following: > > DRV0001 > > DRVR0001 > > PDRV0001 > > This one looks reasonable to me. > > > TEST0001 > > TST0001 Can we please avoid using made up device IDs in the code, even if it is just sample code? It is way better to use a real one that's been reserved already as "never use in real ACPI tables" for some reason. I think I can find a few of these for you if need be. Thanks!
On Wed, Jun 25, 2025 at 11:17:33AM +0200, Rafael J. Wysocki wrote: > On Tue, Jun 17, 2025 at 7:15 PM Danilo Krummrich <dakr@kernel.org> wrote: > > > > On Tue, Jun 17, 2025 at 05:39:07PM +0100, Igor Korotin wrote: > > > I could suggest on of the following: > > > DRV0001 > > > DRVR0001 > > > PDRV0001 > > > > This one looks reasonable to me. > > > > > TEST0001 > > > TST0001 > > Can we please avoid using made up device IDs in the code, even if it > is just sample code? > > It is way better to use a real one that's been reserved already as > "never use in real ACPI tables" for some reason. I think I can find a > few of these for you if need be. > > Thanks! Sure! Please let me know which one you think fits best and I'll use it instead of "TST0001" when I apply the series. - Danilo
Implement FwNode::is_compatible() to check whether a given match string
is compatible with a FwNode.
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
rust/helpers/property.c | 6 ++++++
rust/kernel/device/property.rs | 9 +++++++++
2 files changed, 15 insertions(+)
diff --git a/rust/helpers/property.c b/rust/helpers/property.c
index 08f68e2dac4a..177b9ffd7ba4 100644
--- a/rust/helpers/property.c
+++ b/rust/helpers/property.c
@@ -6,3 +6,9 @@ void rust_helper_fwnode_handle_put(struct fwnode_handle *fwnode)
{
fwnode_handle_put(fwnode);
}
+
+bool rust_helper_fwnode_device_is_compatible(const struct fwnode_handle *fwnode,
+ const char *match)
+{
+ return fwnode_device_is_compatible(fwnode, match);
+}
diff --git a/rust/kernel/device/property.rs b/rust/kernel/device/property.rs
index 838509111e57..a946bf8d5571 100644
--- a/rust/kernel/device/property.rs
+++ b/rust/kernel/device/property.rs
@@ -93,6 +93,15 @@ pub fn property_present(&self, name: &CStr) -> bool {
unsafe { bindings::fwnode_property_present(self.as_raw().cast_const(), name.as_char_ptr()) }
}
+ /// Return `true` if this [`FwNode`] is compatible with `match_str`, `false` otherwise.
+ pub fn is_compatible(&self, match_str: &CStr) -> bool {
+ // SAFETY:
+ // - By the invariant of `CStr`, `name.as_char_ptr()` is valid and null-terminated.
+ // - The type invariant of `Self` guarantees that `self.as_raw() is a pointer to a valid
+ // `struct fwnode_handle`.
+ unsafe { bindings::fwnode_device_is_compatible(self.as_raw(), match_str.as_char_ptr()) }
+ }
+
/// Returns firmware property `name` boolean value.
pub fn property_read_bool(&self, name: &CStr) -> bool {
// SAFETY:
base-commit: 2a1ea59de83bf367215e2a4dd9bf8bbd061349b3
--
2.49.0In SampleDriver::probe() don't call pdev.as_ref() repeatedly, instead
introduce a dedicated &Device.
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
samples/rust/rust_driver_platform.rs | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/samples/rust/rust_driver_platform.rs b/samples/rust/rust_driver_platform.rs
index c0abf78d0683..000bb915af60 100644
--- a/samples/rust/rust_driver_platform.rs
+++ b/samples/rust/rust_driver_platform.rs
@@ -32,13 +32,15 @@ fn probe(
pdev: &platform::Device<Core>,
info: Option<&Self::IdInfo>,
) -> Result<Pin<KBox<Self>>> {
- dev_dbg!(pdev.as_ref(), "Probe Rust Platform driver sample.\n");
+ let dev = pdev.as_ref();
+
+ dev_dbg!(dev, "Probe Rust Platform driver sample.\n");
if let Some(info) = info {
- dev_info!(pdev.as_ref(), "Probed with info: '{}'.\n", info.0);
+ dev_info!(dev, "Probed with info: '{}'.\n", info.0);
}
- Self::properties_parse(pdev.as_ref())?;
+ Self::properties_parse(dev)?;
let drvdata = KBox::new(Self { pdev: pdev.into() }, GFP_KERNEL)?;
--
2.49.0On 16/06/2025 21:40, Danilo Krummrich wrote:
> In SampleDriver::probe() don't call pdev.as_ref() repeatedly, instead
> introduce a dedicated &Device.
>
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> ---
> samples/rust/rust_driver_platform.rs | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/samples/rust/rust_driver_platform.rs b/samples/rust/rust_driver_platform.rs
> index c0abf78d0683..000bb915af60 100644
> --- a/samples/rust/rust_driver_platform.rs
> +++ b/samples/rust/rust_driver_platform.rs
> @@ -32,13 +32,15 @@ fn probe(
> pdev: &platform::Device<Core>,
> info: Option<&Self::IdInfo>,
> ) -> Result<Pin<KBox<Self>>> {
> - dev_dbg!(pdev.as_ref(), "Probe Rust Platform driver sample.\n");
> + let dev = pdev.as_ref();
> +
> + dev_dbg!(dev, "Probe Rust Platform driver sample.\n");
>
> if let Some(info) = info {
> - dev_info!(pdev.as_ref(), "Probed with info: '{}'.\n", info.0);
> + dev_info!(dev, "Probed with info: '{}'.\n", info.0);
> }
>
> - Self::properties_parse(pdev.as_ref())?;
> + Self::properties_parse(dev)?;
>
> let drvdata = KBox::new(Self { pdev: pdev.into() }, GFP_KERNEL)?;
Yes!
Reviewed-by: Dirk Behme <dirk.behme@de.bosch.com>
Thanks,
Dirk
Only call Self::properties_parse() when the device is compatible with
"test,rust-device".
Once we add ACPI support, we don't want the ACPI device to fail probing
in Self::properties_parse().
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
samples/rust/rust_driver_platform.rs | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/samples/rust/rust_driver_platform.rs b/samples/rust/rust_driver_platform.rs
index 000bb915af60..036dd0b899b0 100644
--- a/samples/rust/rust_driver_platform.rs
+++ b/samples/rust/rust_driver_platform.rs
@@ -40,7 +40,12 @@ fn probe(
dev_info!(dev, "Probed with info: '{}'.\n", info.0);
}
- Self::properties_parse(dev)?;
+ if dev
+ .fwnode()
+ .is_some_and(|node| node.is_compatible(c_str!("test,rust-device")))
+ {
+ Self::properties_parse(dev)?;
+ }
let drvdata = KBox::new(Self { pdev: pdev.into() }, GFP_KERNEL)?;
--
2.49.0© 2016 - 2025 Red Hat, Inc.