[PATCH] Staging: ste_rmi4: Use SIMPLE_DEV_PM_OPS() macro

[PATCH] Staging: ste_rmi4: Use SIMPLE_DEV_PM_OPS() macro

[Vaishali Thakkar]

Macro SIMPLE_DEV_PM_OPS() can be used when same suspend
and resume callbacks are used for suspend to RAM and
hibernation. So, here use SIMPLE_DEV_PM_OPS to make code
shorter and cleaner.

Signed-off-by: Vaishali Thakkar <vthakkar1994@gmail.com>
---
 drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c b/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c
index 6385b33..0f524bb 100644
--- a/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c
+++ b/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c
@@ -1112,12 +1112,11 @@ static int synaptics_rmi4_resume(struct device *dev)
 	return 0;
 }
 
-static const struct dev_pm_ops synaptics_rmi4_dev_pm_ops = {
-	.suspend = synaptics_rmi4_suspend,
-	.resume  = synaptics_rmi4_resume,
-};
 #endif
 
+static SIMPLE_DEV_PM_OPS(synaptics_rmi4_dev_pm_ops, synaptics_rmi4_suspend,
+			 synaptics_rmi4_resume);
+
 static const struct i2c_device_id synaptics_rmi4_id_table[] = {
 	{ DRIVER_NAME, 0 },
 	{ },
@@ -1128,9 +1127,7 @@ static struct i2c_driver synaptics_rmi4_driver = {
 	.driver = {
 		.name	=	DRIVER_NAME,
 		.owner	=	THIS_MODULE,
-#ifdef CONFIG_PM
 		.pm	=	&synaptics_rmi4_dev_pm_ops,
-#endif
 	},
 	.probe		=	synaptics_rmi4_probe,
 	.remove		=	synaptics_rmi4_remove,
-- 
1.9.1

Re: [Outreachy kernel] [PATCH] Staging: ste_rmi4: Use SIMPLE_DEV_PM_OPS() macro

[Julia Lawall]

On Fri, 20 Mar 2015, Vaishali Thakkar wrote:

> Macro SIMPLE_DEV_PM_OPS() can be used when same suspend
> and resume callbacks are used for suspend to RAM and
> hibernation. So, here use SIMPLE_DEV_PM_OPS to make code
> shorter and cleaner.
>
> Signed-off-by: Vaishali Thakkar <vthakkar1994@gmail.com>
> ---
>  drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c b/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c
> index 6385b33..0f524bb 100644
> --- a/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c
> +++ b/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c
> @@ -1112,12 +1112,11 @@ static int synaptics_rmi4_resume(struct device *dev)
>  	return 0;
>  }
>
> -static const struct dev_pm_ops synaptics_rmi4_dev_pm_ops = {
> -	.suspend = synaptics_rmi4_suspend,
> -	.resume  = synaptics_rmi4_resume,
> -};
>  #endif
>
> +static SIMPLE_DEV_PM_OPS(synaptics_rmi4_dev_pm_ops, synaptics_rmi4_suspend,
> +			 synaptics_rmi4_resume);
> +
>  static const struct i2c_device_id synaptics_rmi4_id_table[] = {
>  	{ DRIVER_NAME, 0 },
>  	{ },
> @@ -1128,9 +1127,7 @@ static struct i2c_driver synaptics_rmi4_driver = {
>  	.driver = {
>  		.name	=	DRIVER_NAME,
>  		.owner	=	THIS_MODULE,
> -#ifdef CONFIG_PM
>  		.pm	=	&synaptics_rmi4_dev_pm_ops,
> -#endif

How does this work?  I guess that before the pm field could sometimes be
null, if power management is not supported.  Not the field is not null.
Are the fields for the functions now null if there is no power management,
or are there just trivial definitions?  (It's not a suggestion about
changing the patch; I am just wondering).

julia

>  	},
>  	.probe		=	synaptics_rmi4_probe,
>  	.remove		=	synaptics_rmi4_remove,
> --
> 1.9.1
>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20150320071750.GA8322%40vaishali-Ideapad-Z570.
> For more options, visit https://groups.google.com/d/optout.
>

Re: [Outreachy kernel] [PATCH] Staging: ste_rmi4: Use SIMPLE_DEV_PM_OPS() macro

[Vaishali Thakkar]

On Fri, Mar 20, 2015 at 1:37 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
> On Fri, 20 Mar 2015, Vaishali Thakkar wrote:
>
>> Macro SIMPLE_DEV_PM_OPS() can be used when same suspend
>> and resume callbacks are used for suspend to RAM and
>> hibernation. So, here use SIMPLE_DEV_PM_OPS to make code
>> shorter and cleaner.
>>
>> Signed-off-by: Vaishali Thakkar <vthakkar1994@gmail.com>
>> ---
>>  drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c | 9 +++------
>>  1 file changed, 3 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c b/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c
>> index 6385b33..0f524bb 100644
>> --- a/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c
>> +++ b/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c
>> @@ -1112,12 +1112,11 @@ static int synaptics_rmi4_resume(struct device *dev)
>>       return 0;
>>  }
>>
>> -static const struct dev_pm_ops synaptics_rmi4_dev_pm_ops = {
>> -     .suspend = synaptics_rmi4_suspend,
>> -     .resume  = synaptics_rmi4_resume,
>> -};
>>  #endif
>>
>> +static SIMPLE_DEV_PM_OPS(synaptics_rmi4_dev_pm_ops, synaptics_rmi4_suspend,
>> +                      synaptics_rmi4_resume);
>> +
>>  static const struct i2c_device_id synaptics_rmi4_id_table[] = {
>>       { DRIVER_NAME, 0 },
>>       { },
>> @@ -1128,9 +1127,7 @@ static struct i2c_driver synaptics_rmi4_driver = {
>>       .driver = {
>>               .name   =       DRIVER_NAME,
>>               .owner  =       THIS_MODULE,
>> -#ifdef CONFIG_PM
>>               .pm     =       &synaptics_rmi4_dev_pm_ops,
>> -#endif
>
> How does this work?  I guess that before the pm field could sometimes be
> null, if power management is not supported.  Not the field is not null.
> Are the fields for the functions now null if there is no power management,
> or are there just trivial definitions?  (It's not a suggestion about
> changing the patch; I am just wondering).

Yes. I think members of  synaptics_rmi4_dev_pm_ops can be null if there
is no power management after this change.

As far as I understand, 'SIMPLE_DEV_PM_OPS' macro is defined as follows:

#define SIMPLE_DEV_PM_OPS(name, suspend_fn, resume_fn) \
const struct dev_pm_ops name = { \
        SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
}

And then 'SET_SYSTEM_SLEEP_PM_OPS' like this:

#ifdef CONFIG_PM_SLEEP
#define SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
        .suspend = suspend_fn, \
        .resume = resume_fn, \
        .freeze = suspend_fn, \
        .thaw = resume_fn, \
        .poweroff = suspend_fn, \
        .restore = resume_fn,
#else
#define SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn)
#endif

So, when CONFIG_PM is not enabled, we can simply remove these
guards. And we can write like this:
static const struct dev_pm_ops synaptics_rmi4_dev_pm_ops{
};

However, at first I was unsure about this change. But as this patch is
not giving any kind of build error I guess this change should work.

Although this is my understanding. It may possible that I am wrong.
Correct me if I am missing something.

Thank You.

> julia
>
>>       },
>>       .probe          =       synaptics_rmi4_probe,
>>       .remove         =       synaptics_rmi4_remove,
>> --
>> 1.9.1
>>
>> --
>> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
>> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
>> To post to this group, send email to outreachy-kernel@googlegroups.com.
>> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20150320071750.GA8322%40vaishali-Ideapad-Z570.
>> For more options, visit https://groups.google.com/d/optout.
>>



-- 
Vaishali

Re: [Outreachy kernel] [PATCH] Staging: ste_rmi4: Use SIMPLE_DEV_PM_OPS() macro

[Vaishali Thakkar]

On Fri, Mar 20, 2015 at 2:14 PM, Vaishali Thakkar
<vthakkar1994@gmail.com> wrote:
> On Fri, Mar 20, 2015 at 1:37 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
>> On Fri, 20 Mar 2015, Vaishali Thakkar wrote:
>>
>>> Macro SIMPLE_DEV_PM_OPS() can be used when same suspend
>>> and resume callbacks are used for suspend to RAM and
>>> hibernation. So, here use SIMPLE_DEV_PM_OPS to make code
>>> shorter and cleaner.
>>>
>>> Signed-off-by: Vaishali Thakkar <vthakkar1994@gmail.com>
>>> ---
>>>  drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c | 9 +++------
>>>  1 file changed, 3 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c b/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c
>>> index 6385b33..0f524bb 100644
>>> --- a/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c
>>> +++ b/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c
>>> @@ -1112,12 +1112,11 @@ static int synaptics_rmi4_resume(struct device *dev)
>>>       return 0;
>>>  }
>>>
>>> -static const struct dev_pm_ops synaptics_rmi4_dev_pm_ops = {
>>> -     .suspend = synaptics_rmi4_suspend,
>>> -     .resume  = synaptics_rmi4_resume,
>>> -};
>>>  #endif
>>>
>>> +static SIMPLE_DEV_PM_OPS(synaptics_rmi4_dev_pm_ops, synaptics_rmi4_suspend,
>>> +                      synaptics_rmi4_resume);
>>> +
>>>  static const struct i2c_device_id synaptics_rmi4_id_table[] = {
>>>       { DRIVER_NAME, 0 },
>>>       { },
>>> @@ -1128,9 +1127,7 @@ static struct i2c_driver synaptics_rmi4_driver = {
>>>       .driver = {
>>>               .name   =       DRIVER_NAME,
>>>               .owner  =       THIS_MODULE,
>>> -#ifdef CONFIG_PM
>>>               .pm     =       &synaptics_rmi4_dev_pm_ops,
>>> -#endif
>>
>> How does this work?  I guess that before the pm field could sometimes be
>> null, if power management is not supported.  Not the field is not null.
>> Are the fields for the functions now null if there is no power management,
>> or are there just trivial definitions?  (It's not a suggestion about
>> changing the patch; I am just wondering).
>
> Yes. I think members of  synaptics_rmi4_dev_pm_ops can be null if there
> is no power management after this change.
>
> As far as I understand, 'SIMPLE_DEV_PM_OPS' macro is defined as follows:
>
> #define SIMPLE_DEV_PM_OPS(name, suspend_fn, resume_fn) \
> const struct dev_pm_ops name = { \
>         SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
> }
>
> And then 'SET_SYSTEM_SLEEP_PM_OPS' like this:
>
> #ifdef CONFIG_PM_SLEEP
> #define SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
>         .suspend = suspend_fn, \
>         .resume = resume_fn, \
>         .freeze = suspend_fn, \
>         .thaw = resume_fn, \
>         .poweroff = suspend_fn, \
>         .restore = resume_fn,
> #else
> #define SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn)
> #endif
>
> So, when CONFIG_PM is not enabled, we can simply remove these
> guards. And we can write like this:
> static const struct dev_pm_ops synaptics_rmi4_dev_pm_ops{
> };
>
> However, at first I was unsure about this change. But as this patch is
> not giving any kind of build error I guess this change should work.
>
> Although this is my understanding. It may possible that I am wrong.
> Correct me if I am missing something.
>
> Thank You.

Ah, I think I am missing something. May be I can use this macro when
there is CONFIG_PM_SLEEP. But the way here CONFIG_PM is used
it seems like it should be CONFIG_PM_SLEEP. Though I don't have
knowledge about this particular driver. So, it would be good if you can
enlighten me on this particular topic.

Thank You.

>> julia
>>
>>>       },
>>>       .probe          =       synaptics_rmi4_probe,
>>>       .remove         =       synaptics_rmi4_remove,
>>> --
>>> 1.9.1
>>>
>>> --
>>> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
>>> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
>>> To post to this group, send email to outreachy-kernel@googlegroups.com.
>>> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20150320071750.GA8322%40vaishali-Ideapad-Z570.
>>> For more options, visit https://groups.google.com/d/optout.
>>>
>
>
>
> --
> Vaishali



-- 
Vaishali

Re: [Outreachy kernel] [PATCH] Staging: ste_rmi4: Use SIMPLE_DEV_PM_OPS() macro

[Julia Lawall]

> >> How does this work?  I guess that before the pm field could sometimes be
> >> null, if power management is not supported.  Not the field is not null.
> >> Are the fields for the functions now null if there is no power management,
> >> or are there just trivial definitions?  (It's not a suggestion about
> >> changing the patch; I am just wondering).
> >
> > Yes. I think members of  synaptics_rmi4_dev_pm_ops can be null if there
> > is no power management after this change.
> >
> > As far as I understand, 'SIMPLE_DEV_PM_OPS' macro is defined as follows:
> >
> > #define SIMPLE_DEV_PM_OPS(name, suspend_fn, resume_fn) \
> > const struct dev_pm_ops name = { \
> >         SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
> > }
> >
> > And then 'SET_SYSTEM_SLEEP_PM_OPS' like this:
> >
> > #ifdef CONFIG_PM_SLEEP
> > #define SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
> >         .suspend = suspend_fn, \
> >         .resume = resume_fn, \
> >         .freeze = suspend_fn, \
> >         .thaw = resume_fn, \
> >         .poweroff = suspend_fn, \
> >         .restore = resume_fn,
> > #else
> > #define SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn)
> > #endif
> >
> > So, when CONFIG_PM is not enabled, we can simply remove these
> > guards. And we can write like this:
> > static const struct dev_pm_ops synaptics_rmi4_dev_pm_ops{
> > };
> >
> > However, at first I was unsure about this change. But as this patch is
> > not giving any kind of build error I guess this change should work.
> >
> > Although this is my understanding. It may possible that I am wrong.
> > Correct me if I am missing something.
> >
> > Thank You.
>
> Ah, I think I am missing something. May be I can use this macro when
> there is CONFIG_PM_SLEEP. But the way here CONFIG_PM is used
> it seems like it should be CONFIG_PM_SLEEP. Though I don't have
> knowledge about this particular driver. So, it would be good if you can
> enlighten me on this particular topic.

Actually, I don't know.  There is just the observation that a NULL value
is turned into a & value, which cannot be NULL.  So one could check on
what the driver infrastructure does with a NULL value in the field, and
what it now does with a non-NULL value in the field, but a NULL value in
each of the substructures.

julia