Re: [PATCH 2/2] gpio: mpc8xxx: switch to using DEFINE_RUNTIME_DEV_PM_OPS()

[Martyn Welch <martyn.welch@collabora.com>]

On Tue, 2024-09-03 at 20:36 +0200, Bartosz Golaszewski wrote:
> On Tue, 3 Sept 2024 at 18:25, Andy Shevchenko
> <andriy.shevchenko@intel.com> wrote:
> > 
> > On Tue, Sep 03, 2024 at 07:23:31PM +0300, Andy Shevchenko wrote:
> > > On Tue, Sep 03, 2024 at 05:45:33PM +0200, Bartosz Golaszewski
> > > wrote:
> > 
> > ...
> > 
> > > >  #include <linux/mod_devicetable.h>
> > > >  #include <linux/of.h>
> > > >  #include <linux/platform_device.h>
> > > > +#include <linux/pm_runtime.h>
> > > 
> > > You need pm.h as macros defined there.
> > 
> > ...or both...
> > 
> 
> pm_runtime.h implies pm.h.
> 
> > > >  #include <linux/property.h>
> > > >  #include <linux/slab.h>
> > > >  #include <linux/spinlock.h>
> > 
> > ...
> > 
> > > > +static DEFINE_RUNTIME_DEV_PM_OPS(mpc8xx_pm_ops,
> > > > mpc8xxx_suspend,
> > > > +                            mpc8xxx_resume, NULL);
> > 
> > This one comes from pm_runtime.h, but pm*_ptr() ones from pm.h.
> > 
> > And it seems you wanted pm_ptr().
> > 
> 
> Yeah, I'm not sure really. The suspend and resume callbacks for
> platform devices are not documented but it looks like they're only
> used for system sleep. Martyn: which one do we actually need?
> 

Looking at commit 9d8619190031 (PM: runtime: Add
DEFINE_RUNTIME_DEV_PM_OPS() macro), the use of
DEFINE_RUNTIME_DEV_PM_OPS() sets up PM operations for all situations,
not just sleeping, so I think we need pm_ptr() here.

I'd explicitly added the functionality for suspension due to sleep,
though if a GPIO line is being used as some form of external interrupt,
there may well be cases where it would be beneficial to be able to
receive it in other low power states?

Martyn  

> Bart
> 
> > --
> > With Best Regards,
> > Andy Shevchenko
> > 
> >