Re: [PATCH v2 2/3] rust: platform: wire runtime PM callbacks
Alice Ryhl <[email protected]> Tue, 4 Aug 2026 09:16:11 +0000
| Newsgroups | org.kernel.vger.rust-for-linux,dev.linux.lists.driver-core,org.kernel.vger.linux-kernel,org.kernel.vger.linux-pm |
|---|---|
| Message-ID | <[email protected]> |
On Tue, Jul 21, 2026 at 05:34:03PM +0200, Beata Michalska wrote: > Allow Rust platform drivers to expose runtime PM callbacks to the driver core. > > The runtime PM abstraction builds a dev_pm_ops table for the concrete driver > implementation, but the platform bus still needs to receive that table through > struct platform_driver. Add an optional PM_OPS associated constant to > platform::Driver and initialize the coresponding C device_driver struct > accordingly during registration. The platform glue only wires the callback > table into the C driver model; ownership of the callback payload and > runtime PM teardown remain with the pm module. > > Signed-off-by: Beata Michalska <[email protected]> > --- > rust/kernel/platform.rs | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/rust/kernel/platform.rs b/rust/kernel/platform.rs > index d8d48f60b0b9..b7e422388634 100644 > --- a/rust/kernel/platform.rs > +++ b/rust/kernel/platform.rs > @@ -72,6 +72,11 @@ unsafe fn register( > None => core::ptr::null(), > }; > > + let pm_ops = match T::PM_OPS { > + Some(ops) => ops, > + None => core::ptr::null(), > + }; > + > // SAFETY: It's safe to set the fields of `struct platform_driver` on initialization. > unsafe { > (*pdrv.get()).driver.name = name.as_char_ptr(); > @@ -79,6 +84,7 @@ unsafe fn register( > (*pdrv.get()).remove = Some(Self::remove_callback); > (*pdrv.get()).driver.of_match_table = of_table; > (*pdrv.get()).driver.acpi_match_table = acpi_table; > + (*pdrv.get()).driver.pm = pm_ops; > } > > // SAFETY: `pdrv` is guaranteed to be a valid `DriverType`. > @@ -222,6 +228,9 @@ pub trait Driver { > /// The table of ACPI device ids supported by the driver. > const ACPI_ID_TABLE: Option<acpi::IdTable<Self::IdInfo>> = None; > > + /// Runtime PM callbacks > + const PM_OPS: Option<&'static bindings::dev_pm_ops> = None; > + > /// Platform driver probe. > /// > /// Called when a new platform device is added or discovered. I've been thinking more about this, and I can't help but wonder whether we could significantly simplify it. Why not just do this: 1. Update rust/kernel/platform.rs Driver trait to include pm ops directly in the trait: #[vtable] pub trait Driver { type IdInfo: 'static; type Data<'bound>: Send + 'bound; const OF_ID_TABLE: Option<of::IdTable<Self::IdInfo>> = None; const ACPI_ID_TABLE: Option<acpi::IdTable<Self::IdInfo>> = None; fn probe<'bound>( dev: &'bound Device<device::Core<'_>>, id_info: Option<&'bound Self::IdInfo>, ) -> impl PinInit<Self::Data<'bound>, Error> + 'bound; fn unbind<'bound>(dev: &'bound Device<device::Core<'_>>, this: Pin<&Self::Data<'bound>>) { let _ = (dev, this); } // Add these methods. fn runtime_suspend<'bound>( dev: &'bound Device<device::Bound>, data: &Self::Data<'bound>, ) -> Result { build_error!(VTABLE_DEFAULT_ERROR) } fn runtime_resume<'bound>( dev: &'bound Device<device::Bound>, data: &Self::Data<'bound>, ) -> Result { build_error!(VTABLE_DEFAULT_ERROR) } } By marking the trait with #[vtable], we know whether the user has overridden runtime_suspend() and runtime_resume() and then the platform abstraction can enable PM in that scenario: - If `T::HAS_RUNTIME_SUSPEND && T::HAS_RUNTIME_RESUME` then set `(*pdrv.get()).driver.pm` to a table using those methods in register(). - If `T::HAS_RUNTIME_SUSPEND && T::HAS_RUNTIME_RESUME` then invoke `pm_runtime_enable()` from `probe_callback()` after the `set_callback()` line. Note that this call is infallible. - Do the same from unplug to disable PM. And then you automatically PM whenever you implement those two methods in the `platform::Driver` trait, and that's all you need to do. Since we invoke `pm_runtime_enable()` in `probe_callback()` after setting the private data, there's no issue with passing the device private data to the callbacks. Note that we can trigger a const eval panic on `T::HAS_RUNTIME_SUSPEND != T::HAS_RUNTIME_RESUME` to enforce that you must implement both methods if you implement either one. Thoughts? I know we have gone down this route before, but I just think it would be so so much simpler than the current approach. I know that this diverges from IRQ and such by not having a Registration, but I actually think that's ok. PM is already different from IRQ callbacks in the sense that the platform abstractions need PM-specific code *anyway* to properly set pm_ops in the `struct platform_device`. Alice