On Mon, Aug 8, 2022 at 11:49 AM Paul Cercueil <paul@xxxxxxxxxxxxxxx> wrote:
>
>
>
> Le lun., août 8 2022 at 11:39:56 +0200, Andy Shevchenko
> <andy.shevchenko@xxxxxxxxx> a écrit :
> > On Mon, Aug 8, 2022 at 11:35 AM Paul Cercueil <paul@xxxxxxxxxxxxxxx>
> > wrote:
> >> Le lun., août 8 2022 at 11:28:12 +0200, Andy Shevchenko
> >> <andy.shevchenko@xxxxxxxxx> a écrit :
> >> > On Sun, Aug 7, 2022 at 8:46 PM Jonathan Cameron <jic23@xxxxxxxxxx>
> >> > wrote:
> >
> > ...
> >
> >> >> In this case we can't simply use DEFINE_RUNTIME_DEV_PM_OPS()
> >> because
> >> >> that would provide suspend and resume functions without the
> >> >> checks the driver is doing before calling runtime_pm functions
> >> >> (whether the necessary GPIO is provided). It may be possible to
> >> >> clean that up in future by moving the checks into the callbacks.
> >> >
> >> > ...
> >> >
> >> >> static const struct dev_pm_ops srf04_pm_ops = {
> >> >> - SET_RUNTIME_PM_OPS(srf04_pm_runtime_suspend,
> >> >> - srf04_pm_runtime_resume, NULL)
> >> >> + RUNTIME_PM_OPS(srf04_pm_runtime_suspend,
> >> >> + srf04_pm_runtime_resume, NULL)
> >> >> };
> >> >
> >> > static DEFINE_RUNTIME_DEV_PM_OPS(...);
> >> >
> >> > ?
> >>
> >> Read the commit message?
> >
> > Yes, and I'm not sure how that part is relevant. The callbacks won't
> > be called if pm_ptr() equals no-op, no?
>
> Have a look at the definition of DEFINE_RUNTIME_DEV_PM_OPS(). I believe
> it does not do what you think it does.
>
> What the commit message says is that using DEFINE_RUNTIME_DEV_PM_OPS()
> would add .suspend/.resume callbacks, which aren't provided with the
> current code.
Effectively the use of DEFINE_RUNTIME_DEV_PM_OPS() enables system
sleep with the same callbacks as runtime PM. I don't see any
disadvantages of using it instead of keeping runtime PM only. That
said, I don't see the commit message that describes that nuance. It
rather says, as I said, something irrelevant.
--
With Best Regards,
Andy Shevchenko
