| Message ID | 1366120731-31427-1-git-send-email-fabio.estevam@freescale.com |
|---|---|
| State | Superseded, archived |
| Delegated to: | David Miller |
| Headers | show |
Hello. On 16-04-2013 17:58, Fabio Estevam wrote: > Using SIMPLE_DEV_PM_OPS can make the code smaller and simpler. > Also change CONFIG_PM to CONFIG_PM_SLEEP. > Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com> > --- > drivers/net/ethernet/freescale/fec_main.c | 20 +++++++------------- > 1 file changed, 7 insertions(+), 13 deletions(-) > diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c > index 806a56d..2e8bb58 100644 > --- a/drivers/net/ethernet/freescale/fec_main.c > +++ b/drivers/net/ethernet/freescale/fec_main.c [...] > @@ -1946,23 +1946,17 @@ fec_resume(struct device *dev) > return 0; > } > > -static const struct dev_pm_ops fec_pm_ops = { > - .suspend = fec_suspend, > - .resume = fec_resume, > - .freeze = fec_suspend, > - .thaw = fec_resume, > - .poweroff = fec_suspend, > - .restore = fec_resume, > -}; > -#endif > +static SIMPLE_DEV_PM_OPS(fec_pm_ops, fec_suspend, fec_resume); > +#define FEC_PM_OPS (&fec_pm_ops) Nit: parens are useless here. WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Apr 16, 2013 at 11:32 AM, Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> wrote: >> +static SIMPLE_DEV_PM_OPS(fec_pm_ops, fec_suspend, fec_resume); >> +#define FEC_PM_OPS (&fec_pm_ops) > > > Nit: parens are useless here. Have you tried removing parenthesis and run checkpatch? ERROR: Macros with complex values should be enclosed in parenthesis #45: FILE: drivers/net/ethernet/freescale/fec_main.c:1950: +#define FEC_PM_OPS &fec_pm_ops -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello. On 16-04-2013 19:22, Fabio Estevam wrote: >>> +static SIMPLE_DEV_PM_OPS(fec_pm_ops, fec_suspend, fec_resume); >>> +#define FEC_PM_OPS (&fec_pm_ops) >> Nit: parens are useless here. > Have you tried removing parenthesis and run checkpatch? Oops, I haven't. > ERROR: Macros with complex values should be enclosed in parenthesis > #45: FILE: drivers/net/ethernet/freescale/fec_main.c:1950: > +#define FEC_PM_OPS &fec_pm_ops Didn't know about this, sorry for the noise. WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 2013-04-16 at 21:11 +0400, Sergei Shtylyov wrote: > On 16-04-2013 19:22, Fabio Estevam wrote: > >>> +static SIMPLE_DEV_PM_OPS(fec_pm_ops, fec_suspend, fec_resume); > >>> +#define FEC_PM_OPS (&fec_pm_ops) > >> Nit: parens are useless here. Is this macro indirection even useful? Using &fec_pm_ops seems quite readable. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Apr 16, 2013 at 2:49 PM, Joe Perches <joe@perches.com> wrote:
> Is this macro indirection even useful?
Yes, it is useful. Please check the patch context. We use a define to
distinguish between the CONFIG_PM_SLEEP
and !CONFIG_PM_SLEEP
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Joe Perches <joe@perches.com> : > On Tue, 2013-04-16 at 21:11 +0400, Sergei Shtylyov wrote: > > On 16-04-2013 19:22, Fabio Estevam wrote: > > >>> +static SIMPLE_DEV_PM_OPS(fec_pm_ops, fec_suspend, fec_resume); > > >>> +#define FEC_PM_OPS (&fec_pm_ops) > > >> Nit: parens are useless here. > > Is this macro indirection even useful? When CONFIG_PM_SLEEP is not set, it saves a few bytes setting the pm reference to NULL instead of referencing a zero filled struct.
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 806a56d..2e8bb58 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -1911,7 +1911,7 @@ fec_drv_remove(struct platform_device *pdev) return 0; } -#ifdef CONFIG_PM +#ifdef CONFIG_PM_SLEEP static int fec_suspend(struct device *dev) { @@ -1946,23 +1946,17 @@ fec_resume(struct device *dev) return 0; } -static const struct dev_pm_ops fec_pm_ops = { - .suspend = fec_suspend, - .resume = fec_resume, - .freeze = fec_suspend, - .thaw = fec_resume, - .poweroff = fec_suspend, - .restore = fec_resume, -}; -#endif +static SIMPLE_DEV_PM_OPS(fec_pm_ops, fec_suspend, fec_resume); +#define FEC_PM_OPS (&fec_pm_ops) +#else +#define FEC_PM_OPS NULL +#endif /* CONFIG_PM_SLEEP */ static struct platform_driver fec_driver = { .driver = { .name = DRIVER_NAME, .owner = THIS_MODULE, -#ifdef CONFIG_PM - .pm = &fec_pm_ops, -#endif + .pm = FEC_PM_OPS, .of_match_table = fec_dt_ids, }, .id_table = fec_devtype,
Using SIMPLE_DEV_PM_OPS can make the code smaller and simpler. Also change CONFIG_PM to CONFIG_PM_SLEEP. Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com> --- drivers/net/ethernet/freescale/fec_main.c | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-)