On Fri, Feb 28, 2014 at 09:37:02AM -0800, Greg KH wrote:
> On Fri, Feb 28, 2014 at 02:25:31PM +0900, Jingoo Han wrote:
> > On Thursday, February 27, 2014 11:41 PM, Roger Quadros wrote:
> > > On 02/27/2014 01:47 PM, Jingoo Han wrote:
> > > > Use SIMPLE_DEV_PM_OPS macro in order to make the code simpler.
> > > >
> > > > Signed-off-by: Jingoo Han <jg1.han@xxxxxxxxxxx>
> > > > ---
> > > > drivers/usb/phy/phy-am335x.c | 4 +---
> > > > 1 file changed, 1 insertion(+), 3 deletions(-)
> > > >
> > > > diff --git a/drivers/usb/phy/phy-am335x.c b/drivers/usb/phy/phy-am335x.c
> > > > index 12fc346..ebf8373 100644
> > > > --- a/drivers/usb/phy/phy-am335x.c
> > > > +++ b/drivers/usb/phy/phy-am335x.c
> > > > @@ -123,9 +123,7 @@ static int am335x_phy_resume(struct device *dev)
> > > > return 0;
> > > > }
> > > >
> > > > -static const struct dev_pm_ops am335x_pm_ops = {
> > > > - SET_SYSTEM_SLEEP_PM_OPS(am335x_phy_suspend, am335x_phy_resume)
> > > > -};
> > > > +static SIMPLE_DEV_PM_OPS(am335x_pm_ops, am335x_phy_suspend, am335x_phy_resume);
> > >
> > > You need to move this outside the #ifdef CONFIG_PM_SLEEP condition and get rid
> > > of the DEV_PM_OPS macro below as well.
> >
> > 1. Your suggestion - Removing '#define DEV_PM_OPS NULL'
> >
> > #ifdef CONFIG_PM_SLEEP
> > static int am335x_phy_suspend(struct device *dev)
> > static int am335x_phy_resume(struct device *dev)
> > #endif
> > static SIMPLE_DEV_PM_OPS(am335x_pm_ops, am335x_phy_suspend, am335x_phy_resume);
> >
> > In this case, code can be simpler. However, a dev_pm_ops structure is
> > created, and it requires additional NULL checking by PM framework,
> > when "CONFIG_PM_SLEEP=n && CONFIG_PM=y".
> >
> > 2. Current code - Keeping '#define DEV_PM_OPS NULL'
> >
> > #ifdef CONFIG_PM_SLEEP
> > static int am335x_phy_suspend(struct device *dev)
> > static int am335x_phy_resume(struct device *dev)
> > static SIMPLE_DEV_PM_OPS(am335x_pm_ops, am335x_phy_suspend, am335x_phy_resume);
> > #define DEV_PM_OPS (&am335x_pm_ops)
> > #else
> > #define DEV_PM_OPS NULL
> > #endif
> >
> > In this case, code size is larger. However, a dev_pm_ops structure is
> > NOT created, and it does NOT require additional NULL checking by PM framework,
> > when "CONFIG_PM_SLEEP=n && CONFIG_PM=y".
> >
> > Personally, I prefer the first one. Additional NULL checking is not
> > crucial. Is there any other opinions?
>
> Yes, none of this at all, just fix the structures to always have the
> fields no matter what the config option is, and then deal with it in the
the fields in dev_pm_ops *are* always available the problem is with
SET_SYSTEM_SLEEP_PM_OPS and SET_PM_RUNTIME_OPS which do nothing if
!PM_{SLEEP,RUNTIME}.
I would be more than glad if someone got PM fellows to accept removing
the ifdeffery around both macros and just *always* leave PM hooks
available and ready to go.
cheers
--
balbi
Attachment:
signature.asc
Description: Digital signature
