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
drivers, if they want to (hint, they probably shouldn't), by specifying
the function callbacks or not.
This is a mess, please clean it up, and not force every driver to also
be messy...
greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
