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

On Tue, Aug 04, 2026 at 02:27:25PM +0200, Beata Michalska wrote:
> 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:
> > > +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.

I generally believe that for traits such as this one where end-users are
likely to look up the definition, it's much more important that it's
easy to read when you look up the source, than it being easy to write.
Therefore, I think it should not use a macro, even if there are many
methods.

Alice