Re: [PATCH v2 1/3] rust: add runtime PM support

On Tue, Aug 04, 2026 at 08:13:13AM +0000, Alice Ryhl wrote:
> On Tue, Jul 21, 2026 at 05:34:02PM +0200, Beata Michalska wrote:
> > +/// Runtime PM callbacks implemented by a driver.
> > +///
> > +/// Defines the [`PMOps`] trait and its corresponding [`bindings::dev_pm_ops`].
> > +///
> > +/// Each generated C callback recovers the Rust bus device from the raw
> > +/// `struct device *`, borrows the registered runtime PM payload, and delegates
> > +/// to the matching [`PMOps`] trait method.
> > +///
> > +/// # Safety
> > +///
> > +/// `PMContext::<T>::PM_OPS` must only be installed for devices whose concrete
> > +/// bus type is `T::DeviceType`, and whose runtime PM registration data was
> > +/// installed by [`Registration::new`] for the same `T`.
> > +///
> > +macro_rules! define_pm_ops {
> > +    ($($name:ident $( : $desc:tt )? ),+ $(,)?) => {
> > +        $( define_pm_callback!( @parse_desc $name $( $desc )?); )+
> > +        define_pm_ops!(@common $( $name ), +);
> > +    };
> > +
> > +    (@common $( $name:ident),+ ) => {
> > +        /// Runtime PM callbacks implemented by a driver
> > +        #[vtable]
> > +        pub trait PMOps: Sized
> > +        {
> > +            /// Type of a bus device
> > +            type DeviceType: AsBusDevice<device::Bound>;
> > +            /// Type of the data associated with a PM transitions:
> > +            type RuntimePayloadType: Send;
> > +
> > +            $(
> > +                #[allow(missing_docs)]
> > +                // Callback-specific docs are provided by the generated `dev_pm_ops`
> > +                // contract on `PMOps`.
> > +
> > +                fn $name<'a>(
> > +                    _dev:  &'a Self::DeviceType,
> > +                    _payload: Option<Self::RuntimePayloadType>,
> > +                ) -> Result<Option<Self::RuntimePayloadType>, (Option<Self::RuntimePayloadType>, Error)> {
> > +                    build_error!(VTABLE_DEFAULT_ERROR)
> > +                }
> > +            )+
> > +        }
> > +        paste!(
> > +            impl<'a, T:PMOps> PMContext<'a, T> {
> > +                /// Driver-provided runtime PM operations.
> > +                ///
> > +                /// A driver implements this trait to handle runtime PM
> > +                /// transitions for its device type.
> > +                ///
> > +                /// Each callback receives the device and the current payload.
> > +                /// On success, it returns the payload to keep for the next
> > +                /// transition. On failure, it returns the payload together
> > +                /// with the error so the previous, or otherwise sane state
> > +                /// can be preserved.
> > +                pub const PM_OPS: bindings::dev_pm_ops = bindings::dev_pm_ops {
> > +                    $( [<$name>]: if T::[<HAS_ $name:upper>] {
> > +                        Some([<$name _callback>]::<T>)
> > +                    } else {
> > +                        None
> > +                    }, )+
> > +                    ..PMOPS_NONE
> > +                };
> > +            }
> > +        );
> > +    }
> > +}
> 
> > +
> > +define_pm_ops!(
> > +    // PM state change
> > +    runtime_suspend,
> > +    runtime_resume,
> > +);
> 
> I think using a macro to define this trait is overkill. Just write it
> out:
> 
> pub trait PMOps: Sized {
>     /// Type of a bus device
>     type DeviceType: AsBusDevice<device::Bound>;
>     /// Type of the data associated with a PM transitions:
>     type RuntimePayloadType: Send;
> 
>     fn runtime_suspend<'a>(
>         _dev:  &'a Self::DeviceType,
>         _payload: Option<Self::RuntimePayloadType>,
>     ) -> Result<Option<Self::RuntimePayloadType>, (Option<Self::RuntimePayloadType>, Error)> {
>         build_error!(VTABLE_DEFAULT_ERROR)
>     }
> 
>     fn runtime_resume<'a>(
>         _dev:  &'a Self::DeviceType,
>         _payload: Option<Self::RuntimePayloadType>,
>     ) -> Result<Option<Self::RuntimePayloadType>, (Option<Self::RuntimePayloadType>, Error)> {
>         build_error!(VTABLE_DEFAULT_ERROR)
>     }
> }
> 
> This is a lot easier to read.
It is, though there are more collbacks, that I would expect to be added at some
point, so macro is just to avoid several code blocks that would only differ in
function name.
But I do see your point.
Happy to change that if you still believe there is no much point to it.

---
BR
Beata
> 
> Alice