[PATCH] fec: Use SIMPLE_DEV_PM_OPS

[PATCH] fec: Use SIMPLE_DEV_PM_OPS

[Fabio Estevam]

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
@@ -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,
-- 
1.7.9.5

Re: [PATCH] fec: Use SIMPLE_DEV_PM_OPS

[Sergei Shtylyov]

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

Re: [PATCH] fec: Use SIMPLE_DEV_PM_OPS

[Fabio Estevam]

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

Re: [PATCH] fec: Use SIMPLE_DEV_PM_OPS

[Sergei Shtylyov]

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

Re: [PATCH] fec: Use SIMPLE_DEV_PM_OPS

[Joe Perches]

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.

Re: [PATCH] fec: Use SIMPLE_DEV_PM_OPS

[Fabio Estevam]

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

Re: [PATCH] fec: Use SIMPLE_DEV_PM_OPS

[Francois Romieu]

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.

-- 
Ueimor