2013-04-27 02:54:54

by Jingoo Han

[permalink] [raw]
Subject: [PATCH] PM: Add pm_ops_ptr() macro

Add pm_ops_ptr() macro that allows the .pm entry in the driver structures
to be assigned without having an #define xxx NULL for the case that PM is
not enabled.

Signed-off-by: Jingoo Han <[email protected]>
---
include/linux/pm.h | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/include/linux/pm.h b/include/linux/pm.h
index a224c7f..bd50d15 100644
--- a/include/linux/pm.h
+++ b/include/linux/pm.h
@@ -55,8 +55,10 @@ struct device;

#ifdef CONFIG_PM
extern const char power_group_name[]; /* = "power" */
+#define pm_ops_ptr(_ptr) (_ptr)
#else
#define power_group_name NULL
+#define pm_ops_ptr(_ptr) NULL
#endif

typedef struct pm_message {
--
1.7.2.5


2013-04-27 14:32:29

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH] PM: Add pm_ops_ptr() macro

On Saturday, April 27, 2013 11:54:50 AM Jingoo Han wrote:
> Add pm_ops_ptr() macro that allows the .pm entry in the driver structures
> to be assigned without having an #define xxx NULL for the case that PM is
> not enabled.
>
> Signed-off-by: Jingoo Han <[email protected]>

First, I'm not taking any more PM patches for v3.10 (that don't fix recent
regressions).

Second, please add that macro along with a user.

Thanks,
Rafael


> ---
> include/linux/pm.h | 2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/include/linux/pm.h b/include/linux/pm.h
> index a224c7f..bd50d15 100644
> --- a/include/linux/pm.h
> +++ b/include/linux/pm.h
> @@ -55,8 +55,10 @@ struct device;
>
> #ifdef CONFIG_PM
> extern const char power_group_name[]; /* = "power" */
> +#define pm_ops_ptr(_ptr) (_ptr)
> #else
> #define power_group_name NULL
> +#define pm_ops_ptr(_ptr) NULL
> #endif
>
> typedef struct pm_message {
>
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

2013-04-29 05:41:52

by Jingoo Han

[permalink] [raw]
Subject: Re: [PATCH] PM: Add pm_ops_ptr() macro

On Saturday, April 27, 2013 11:41 PM, Rafael J. Wysocki:
>
> On Saturday, April 27, 2013 11:54:50 AM Jingoo Han wrote:
> > Add pm_ops_ptr() macro that allows the .pm entry in the driver structures
> > to be assigned without having an #define xxx NULL for the case that PM is
> > not enabled.
> >
> > Signed-off-by: Jingoo Han <[email protected]>
>
> First, I'm not taking any more PM patches for v3.10 (that don't fix recent
> regressions).
>
> Second, please add that macro along with a user.

Hi Rafael,

This macro can be used as below:
This macro cannot affect the procedure of suspend/resume
calls; thus, there is no side effect.
It just reduces the code size of each drivers.
In this way, of_match_ptr() macro has been already used.


--- a/drivers/i2c/busses/i2c-s3c2410.c
+++ b/drivers/i2c/busses/i2c-s3c2410.c
@@ -1218,7 +1218,6 @@ static int s3c24xx_i2c_resume(struct device *dev)
}
#endif

-#ifdef CONFIG_PM
static const struct dev_pm_ops s3c24xx_i2c_dev_pm_ops = {
#ifdef CONFIG_PM_SLEEP
.suspend_noirq = s3c24xx_i2c_suspend_noirq,
@@ -1226,11 +1225,6 @@ static const struct dev_pm_ops s3c24xx_i2c_dev_pm_ops = {
#endif
};

-#define S3C24XX_DEV_PM_OPS (&s3c24xx_i2c_dev_pm_ops)
-#else
-#define S3C24XX_DEV_PM_OPS NULL
-#endif
-
/* device driver for platform bus bits */

static struct platform_driver s3c24xx_i2c_driver = {
@@ -1240,7 +1234,7 @@ static struct platform_driver s3c24xx_i2c_driver = {
.driver = {
.owner = THIS_MODULE,
.name = "s3c-i2c",
- .pm = S3C24XX_DEV_PM_OPS,
+ .pm = pm_ops_ptr(&s3c24xx_i2c_dev_pm_ops),
.of_match_table = of_match_ptr(s3c24xx_i2c_match),
},
};

Best regards,
Jingoo Han


>
> Thanks,
> Rafael
>
>
> > ---
> > include/linux/pm.h | 2 ++
> > 1 files changed, 2 insertions(+), 0 deletions(-)
> >
> > diff --git a/include/linux/pm.h b/include/linux/pm.h
> > index a224c7f..bd50d15 100644
> > --- a/include/linux/pm.h
> > +++ b/include/linux/pm.h
> > @@ -55,8 +55,10 @@ struct device;
> >
> > #ifdef CONFIG_PM
> > extern const char power_group_name[]; /* = "power" */
> > +#define pm_ops_ptr(_ptr) (_ptr)
> > #else
> > #define power_group_name NULL
> > +#define pm_ops_ptr(_ptr) NULL
> > #endif
> >
> > typedef struct pm_message {
> >
> --
> I speak only for myself.
> Rafael J. Wysocki, Intel Open Source Technology Center.

2013-04-29 11:26:55

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH] PM: Add pm_ops_ptr() macro

On Monday, April 29, 2013 02:41:48 PM Jingoo Han wrote:
> On Saturday, April 27, 2013 11:41 PM, Rafael J. Wysocki:
> >
> > On Saturday, April 27, 2013 11:54:50 AM Jingoo Han wrote:
> > > Add pm_ops_ptr() macro that allows the .pm entry in the driver structures
> > > to be assigned without having an #define xxx NULL for the case that PM is
> > > not enabled.
> > >
> > > Signed-off-by: Jingoo Han <[email protected]>
> >
> > First, I'm not taking any more PM patches for v3.10 (that don't fix recent
> > regressions).
> >
> > Second, please add that macro along with a user.
>
> Hi Rafael,
>
> This macro can be used as below:
> This macro cannot affect the procedure of suspend/resume
> calls; thus, there is no side effect.
> It just reduces the code size of each drivers.
> In this way, of_match_ptr() macro has been already used.

OK, so care to fold the below into your original patch?

Rafael


> --- a/drivers/i2c/busses/i2c-s3c2410.c
> +++ b/drivers/i2c/busses/i2c-s3c2410.c
> @@ -1218,7 +1218,6 @@ static int s3c24xx_i2c_resume(struct device *dev)
> }
> #endif
>
> -#ifdef CONFIG_PM
> static const struct dev_pm_ops s3c24xx_i2c_dev_pm_ops = {
> #ifdef CONFIG_PM_SLEEP
> .suspend_noirq = s3c24xx_i2c_suspend_noirq,
> @@ -1226,11 +1225,6 @@ static const struct dev_pm_ops s3c24xx_i2c_dev_pm_ops = {
> #endif
> };
>
> -#define S3C24XX_DEV_PM_OPS (&s3c24xx_i2c_dev_pm_ops)
> -#else
> -#define S3C24XX_DEV_PM_OPS NULL
> -#endif
> -
> /* device driver for platform bus bits */
>
> static struct platform_driver s3c24xx_i2c_driver = {
> @@ -1240,7 +1234,7 @@ static struct platform_driver s3c24xx_i2c_driver = {
> .driver = {
> .owner = THIS_MODULE,
> .name = "s3c-i2c",
> - .pm = S3C24XX_DEV_PM_OPS,
> + .pm = pm_ops_ptr(&s3c24xx_i2c_dev_pm_ops),
> .of_match_table = of_match_ptr(s3c24xx_i2c_match),
> },
> };
>
> Best regards,
> Jingoo Han
>
>
> >
> > Thanks,
> > Rafael
> >
> >
> > > ---
> > > include/linux/pm.h | 2 ++
> > > 1 files changed, 2 insertions(+), 0 deletions(-)
> > >
> > > diff --git a/include/linux/pm.h b/include/linux/pm.h
> > > index a224c7f..bd50d15 100644
> > > --- a/include/linux/pm.h
> > > +++ b/include/linux/pm.h
> > > @@ -55,8 +55,10 @@ struct device;
> > >
> > > #ifdef CONFIG_PM
> > > extern const char power_group_name[]; /* = "power" */
> > > +#define pm_ops_ptr(_ptr) (_ptr)
> > > #else
> > > #define power_group_name NULL
> > > +#define pm_ops_ptr(_ptr) NULL
> > > #endif
> > >
> > > typedef struct pm_message {
> > >
> > --
> > I speak only for myself.
> > Rafael J. Wysocki, Intel Open Source Technology Center.
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

2013-04-29 12:13:37

by Lars-Peter Clausen

[permalink] [raw]
Subject: Re: [PATCH] PM: Add pm_ops_ptr() macro

On 04/29/2013 07:41 AM, Jingoo Han wrote:
> On Saturday, April 27, 2013 11:41 PM, Rafael J. Wysocki:
>>
>> On Saturday, April 27, 2013 11:54:50 AM Jingoo Han wrote:
>>> Add pm_ops_ptr() macro that allows the .pm entry in the driver structures
>>> to be assigned without having an #define xxx NULL for the case that PM is
>>> not enabled.
>>>
>>> Signed-off-by: Jingoo Han <[email protected]>
>>
>> First, I'm not taking any more PM patches for v3.10 (that don't fix recent
>> regressions).
>>
>> Second, please add that macro along with a user.
>
> Hi Rafael,
>
> This macro can be used as below:
> This macro cannot affect the procedure of suspend/resume
> calls; thus, there is no side effect.
> It just reduces the code size of each drivers.
> In this way, of_match_ptr() macro has been already used.
>
>
> --- a/drivers/i2c/busses/i2c-s3c2410.c
> +++ b/drivers/i2c/busses/i2c-s3c2410.c
> @@ -1218,7 +1218,6 @@ static int s3c24xx_i2c_resume(struct device *dev)
> }
> #endif
>
> -#ifdef CONFIG_PM
> static const struct dev_pm_ops s3c24xx_i2c_dev_pm_ops = {
> #ifdef CONFIG_PM_SLEEP
> .suspend_noirq = s3c24xx_i2c_suspend_noirq,
> @@ -1226,11 +1225,6 @@ static const struct dev_pm_ops s3c24xx_i2c_dev_pm_ops = {
> #endif
> };
>
> -#define S3C24XX_DEV_PM_OPS (&s3c24xx_i2c_dev_pm_ops)
> -#else
> -#define S3C24XX_DEV_PM_OPS NULL
> -#endif
> -
> /* device driver for platform bus bits */
>
> static struct platform_driver s3c24xx_i2c_driver = {
> @@ -1240,7 +1234,7 @@ static struct platform_driver s3c24xx_i2c_driver = {
> .driver = {
> .owner = THIS_MODULE,
> .name = "s3c-i2c",
> - .pm = S3C24XX_DEV_PM_OPS,
> + .pm = pm_ops_ptr(&s3c24xx_i2c_dev_pm_ops),
> .of_match_table = of_match_ptr(s3c24xx_i2c_match),
> },
> };

Won't that generate a warning about an unused s3c24xx_i2c_dev_pm_ops when
CONFIG_PM is not enabled?

2013-04-30 00:21:44

by Jingoo Han

[permalink] [raw]
Subject: Re: [PATCH] PM: Add pm_ops_ptr() macro

On Monday, April 29, 2013 8:35 PM, Rafael J. Wysocki:
> On Monday, April 29, 2013 02:41:48 PM Jingoo Han wrote:
> > On Saturday, April 27, 2013 11:41 PM, Rafael J. Wysocki:
> > >
> > > On Saturday, April 27, 2013 11:54:50 AM Jingoo Han wrote:
> > > > Add pm_ops_ptr() macro that allows the .pm entry in the driver structures
> > > > to be assigned without having an #define xxx NULL for the case that PM is
> > > > not enabled.
> > > >
> > > > Signed-off-by: Jingoo Han <[email protected]>
> > >
> > > First, I'm not taking any more PM patches for v3.10 (that don't fix recent
> > > regressions).
> > >
> > > Second, please add that macro along with a user.
> >
> > Hi Rafael,
> >
> > This macro can be used as below:
> > This macro cannot affect the procedure of suspend/resume
> > calls; thus, there is no side effect.
> > It just reduces the code size of each drivers.
> > In this way, of_match_ptr() macro has been already used.
>
> OK, so care to fold the below into your original patch?

OK, I will fold it into my original patch.
Then, I will send v2 patch, soon.
Thank you.

Best regards,
Jingoo Han

>
> Rafael
>
>
> > --- a/drivers/i2c/busses/i2c-s3c2410.c
> > +++ b/drivers/i2c/busses/i2c-s3c2410.c
> > @@ -1218,7 +1218,6 @@ static int s3c24xx_i2c_resume(struct device *dev)
> > }
> > #endif
> >
> > -#ifdef CONFIG_PM
> > static const struct dev_pm_ops s3c24xx_i2c_dev_pm_ops = {
> > #ifdef CONFIG_PM_SLEEP
> > .suspend_noirq = s3c24xx_i2c_suspend_noirq,
> > @@ -1226,11 +1225,6 @@ static const struct dev_pm_ops s3c24xx_i2c_dev_pm_ops = {
> > #endif
> > };
> >
> > -#define S3C24XX_DEV_PM_OPS (&s3c24xx_i2c_dev_pm_ops)
> > -#else
> > -#define S3C24XX_DEV_PM_OPS NULL
> > -#endif
> > -
> > /* device driver for platform bus bits */
> >
> > static struct platform_driver s3c24xx_i2c_driver = {
> > @@ -1240,7 +1234,7 @@ static struct platform_driver s3c24xx_i2c_driver = {
> > .driver = {
> > .owner = THIS_MODULE,
> > .name = "s3c-i2c",
> > - .pm = S3C24XX_DEV_PM_OPS,
> > + .pm = pm_ops_ptr(&s3c24xx_i2c_dev_pm_ops),
> > .of_match_table = of_match_ptr(s3c24xx_i2c_match),
> > },
> > };
> >
> > Best regards,
> > Jingoo Han
> >
> >
> > >
> > > Thanks,
> > > Rafael
> > >
> > >
> > > > ---
> > > > include/linux/pm.h | 2 ++
> > > > 1 files changed, 2 insertions(+), 0 deletions(-)
> > > >
> > > > diff --git a/include/linux/pm.h b/include/linux/pm.h
> > > > index a224c7f..bd50d15 100644
> > > > --- a/include/linux/pm.h
> > > > +++ b/include/linux/pm.h
> > > > @@ -55,8 +55,10 @@ struct device;
> > > >
> > > > #ifdef CONFIG_PM
> > > > extern const char power_group_name[]; /* = "power" */
> > > > +#define pm_ops_ptr(_ptr) (_ptr)
> > > > #else
> > > > #define power_group_name NULL
> > > > +#define pm_ops_ptr(_ptr) NULL
> > > > #endif
> > > >
> > > > typedef struct pm_message {
> > > >
> > > --
> > > I speak only for myself.
> > > Rafael J. Wysocki, Intel Open Source Technology Center.
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> > the body of a message to [email protected]
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> --
> I speak only for myself.
> Rafael J. Wysocki, Intel Open Source Technology Center.

2013-04-30 00:54:13

by Jingoo Han

[permalink] [raw]
Subject: Re: [PATCH] PM: Add pm_ops_ptr() macro

On Monday, April 29, 2013 9:13 PM, Lars-Peter Clausen wrote:
> On 04/29/2013 07:41 AM, Jingoo Han wrote:
> > On Saturday, April 27, 2013 11:41 PM, Rafael J. Wysocki:
> >>
> >> On Saturday, April 27, 2013 11:54:50 AM Jingoo Han wrote:
> >>> Add pm_ops_ptr() macro that allows the .pm entry in the driver structures
> >>> to be assigned without having an #define xxx NULL for the case that PM is
> >>> not enabled.
> >>>
> >>> Signed-off-by: Jingoo Han <[email protected]>
> >>
> >> First, I'm not taking any more PM patches for v3.10 (that don't fix recent
> >> regressions).
> >>
> >> Second, please add that macro along with a user.
> >
> > Hi Rafael,
> >
> > This macro can be used as below:
> > This macro cannot affect the procedure of suspend/resume
> > calls; thus, there is no side effect.
> > It just reduces the code size of each drivers.
> > In this way, of_match_ptr() macro has been already used.
> >
> >
> > --- a/drivers/i2c/busses/i2c-s3c2410.c
> > +++ b/drivers/i2c/busses/i2c-s3c2410.c
> > @@ -1218,7 +1218,6 @@ static int s3c24xx_i2c_resume(struct device *dev)
> > }
> > #endif
> >
> > -#ifdef CONFIG_PM
> > static const struct dev_pm_ops s3c24xx_i2c_dev_pm_ops = {
> > #ifdef CONFIG_PM_SLEEP
> > .suspend_noirq = s3c24xx_i2c_suspend_noirq,
> > @@ -1226,11 +1225,6 @@ static const struct dev_pm_ops s3c24xx_i2c_dev_pm_ops = {
> > #endif
> > };
> >
> > -#define S3C24XX_DEV_PM_OPS (&s3c24xx_i2c_dev_pm_ops)
> > -#else
> > -#define S3C24XX_DEV_PM_OPS NULL
> > -#endif
> > -
> > /* device driver for platform bus bits */
> >
> > static struct platform_driver s3c24xx_i2c_driver = {
> > @@ -1240,7 +1234,7 @@ static struct platform_driver s3c24xx_i2c_driver = {
> > .driver = {
> > .owner = THIS_MODULE,
> > .name = "s3c-i2c",
> > - .pm = S3C24XX_DEV_PM_OPS,
> > + .pm = pm_ops_ptr(&s3c24xx_i2c_dev_pm_ops),
> > .of_match_table = of_match_ptr(s3c24xx_i2c_match),
> > },
> > };
>
> Won't that generate a warning about an unused s3c24xx_i2c_dev_pm_ops when
> CONFIG_PM is not enabled?

No, there is no build warning, when CONFIG_PM is not enabled.
In this way, of_match_ptr() macro has been already used
without build warnings.


Best regards,
Jingoo Han