[PATCH 1/2] i2c: designware: add CONFIG_PM_SLEEP to suspend/resume functions

[PATCH 1/2] i2c: designware: add CONFIG_PM_SLEEP to suspend/resume functions

[Jingoo Han]

Add CONFIG_PM_SLEEP to suspend/resume functions to fix the following
build warning when CONFIG_PM_SLEEP is not selected. This is because
sleep PM callbacks defined by SIMPLE_DEV_PM_OPS are only used when
the CONFIG_PM_SLEEP is enabled.

drivers/i2c/busses/i2c-designware-platdrv.c:253:12: warning: 'dw_i2c_suspend' defined but not used [-Wunused-function]
drivers/i2c/busses/i2c-designware-platdrv.c:263:12: warning: 'dw_i2c_resume' defined but not used [-Wunused-function]

Signed-off-by: Jingoo Han <jg1.han-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
---
 drivers/i2c/busses/i2c-designware-platdrv.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 0ceb6e1..f45b9cc 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -249,7 +249,7 @@ static const struct of_device_id dw_i2c_of_match[] = {
 MODULE_DEVICE_TABLE(of, dw_i2c_of_match);
 #endif
 
-#ifdef CONFIG_PM
+#ifdef CONFIG_PM_SLEEP
 static int dw_i2c_suspend(struct device *dev)
 {
 	struct platform_device *pdev = to_platform_device(dev);
-- 
1.7.2.5

[PATCH 2/2] i2c: ocores: add CONFIG_PM_SLEEP to suspend/resume functions

[Jingoo Han]

Add CONFIG_PM_SLEEP to suspend/resume functions to fix the following
build warning when CONFIG_PM_SLEEP is not selected. This is because
sleep PM callbacks defined by SIMPLE_DEV_PM_OPS are only used when
the CONFIG_PM_SLEEP is enabled.

drivers/i2c/busses/i2c-ocores.c:460:12: warning: 'ocores_i2c_suspend' defined but not used [-Wunused-function]
drivers/i2c/busses/i2c-ocores.c:471:12: warning: 'ocores_i2c_resume' defined but not used [-Wunused-function]

Signed-off-by: Jingoo Han <jg1.han-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
---
 drivers/i2c/busses/i2c-ocores.c |    9 +++------
 1 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/i2c/busses/i2c-ocores.c b/drivers/i2c/busses/i2c-ocores.c
index 0e1f824..1ecea5e 100644
--- a/drivers/i2c/busses/i2c-ocores.c
+++ b/drivers/i2c/busses/i2c-ocores.c
@@ -456,7 +456,7 @@ static int ocores_i2c_remove(struct platform_device *pdev)
 	return 0;
 }
 
-#ifdef CONFIG_PM
+#ifdef CONFIG_PM_SLEEP
 static int ocores_i2c_suspend(struct device *dev)
 {
 	struct ocores_i2c *i2c = dev_get_drvdata(dev);
@@ -476,12 +476,9 @@ static int ocores_i2c_resume(struct device *dev)
 
 	return 0;
 }
+#endif
 
 static SIMPLE_DEV_PM_OPS(ocores_i2c_pm, ocores_i2c_suspend, ocores_i2c_resume);
-#define OCORES_I2C_PM	(&ocores_i2c_pm)
-#else
-#define OCORES_I2C_PM	NULL
-#endif
 
 static struct platform_driver ocores_i2c_driver = {
 	.probe   = ocores_i2c_probe,
@@ -490,7 +487,7 @@ static struct platform_driver ocores_i2c_driver = {
 		.owner = THIS_MODULE,
 		.name = "ocores-i2c",
 		.of_match_table = ocores_i2c_match,
-		.pm = OCORES_I2C_PM,
+		.pm = &ocores_i2c_pm,
 	},
 };
 
-- 
1.7.2.5

Re: [PATCH 1/2] i2c: designware: add CONFIG_PM_SLEEP to suspend/resume functions

[Wolfram Sang]

On Fri, Mar 22, 2013 at 11:13:07AM +0900, Jingoo Han wrote:
> Add CONFIG_PM_SLEEP to suspend/resume functions to fix the following
> build warning when CONFIG_PM_SLEEP is not selected. This is because
> sleep PM callbacks defined by SIMPLE_DEV_PM_OPS are only used when
> the CONFIG_PM_SLEEP is enabled.
> 
> drivers/i2c/busses/i2c-designware-platdrv.c:253:12: warning: 'dw_i2c_suspend' defined but not used [-Wunused-function]
> drivers/i2c/busses/i2c-designware-platdrv.c:263:12: warning: 'dw_i2c_resume' defined but not used [-Wunused-function]
> 
> Signed-off-by: Jingoo Han <jg1.han-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>

What about doing it like the s3c2410 driver which also adds CONFIG_PM
around the whole structure? (comment applies for both patches)

Thanks,

   Wolfram

Re: [PATCH 1/2] i2c: designware: add CONFIG_PM_SLEEP to suspend/resume functions

[Jingoo Han]

On Friday, March 22, 2013 8:47 PM, Wolfram Sang wrote:
> 
> On Fri, Mar 22, 2013 at 11:13:07AM +0900, Jingoo Han wrote:
> > Add CONFIG_PM_SLEEP to suspend/resume functions to fix the following
> > build warning when CONFIG_PM_SLEEP is not selected. This is because
> > sleep PM callbacks defined by SIMPLE_DEV_PM_OPS are only used when
> > the CONFIG_PM_SLEEP is enabled.
> >
> > drivers/i2c/busses/i2c-designware-platdrv.c:253:12: warning: 'dw_i2c_suspend' defined but not used [-
> Wunused-function]
> > drivers/i2c/busses/i2c-designware-platdrv.c:263:12: warning: 'dw_i2c_resume' defined but not used [-
> Wunused-function]
> >
> > Signed-off-by: Jingoo Han <jg1.han-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> 
> What about doing it like the s3c2410 driver which also adds CONFIG_PM
> around the whole structure? (comment applies for both patches)

Hi Wolfram Sang,
Sorry for being late.

It is similar with s3c2410 driver.
However, CONFIG_PM is not necessary, when only SIMPLE_DEV_PM_OPS is
used.

Please refer to 'include/linux/pm.h'.
SIMPLE_DEV_PM_OPS macro uses SET_SYSTEM_SLEEP_PM_OPS macro.
Also, SET_SYSTEM_SLEEP_PM_OPS macro is defined as below.
Thus, adding CONFIG_PM is not necessary, when only SIMPLE_DEV_PM_OPS
is used.

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

302 #ifdef CONFIG_PM_SLEEP
303 #define SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
304         .suspend = suspend_fn, \
305         .resume = resume_fn, \
306         .freeze = suspend_fn, \
307         .thaw = resume_fn, \
308         .poweroff = suspend_fn, \
309         .restore = resume_fn,
310 #else
311 #define SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn)
312 #endif

Best regards,
Jingoo Han


> 
> Thanks,
> 
>    Wolfram

Re: [PATCH 1/2] i2c: designware: add CONFIG_PM_SLEEP to suspend/resume functions

[Wolfram Sang]

On Tue, Apr 09, 2013 at 09:07:10AM +0900, Jingoo Han wrote:
> On Friday, March 22, 2013 8:47 PM, Wolfram Sang wrote:
> > 
> > On Fri, Mar 22, 2013 at 11:13:07AM +0900, Jingoo Han wrote:
> > > Add CONFIG_PM_SLEEP to suspend/resume functions to fix the following
> > > build warning when CONFIG_PM_SLEEP is not selected. This is because
> > > sleep PM callbacks defined by SIMPLE_DEV_PM_OPS are only used when
> > > the CONFIG_PM_SLEEP is enabled.
> > >
> > > drivers/i2c/busses/i2c-designware-platdrv.c:253:12: warning: 'dw_i2c_suspend' defined but not used [-
> > Wunused-function]
> > > drivers/i2c/busses/i2c-designware-platdrv.c:263:12: warning: 'dw_i2c_resume' defined but not used [-
> > Wunused-function]
> > >
> > > Signed-off-by: Jingoo Han <jg1.han-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> > 
> > What about doing it like the s3c2410 driver which also adds CONFIG_PM
> > around the whole structure? (comment applies for both patches)
> 
> Hi Wolfram Sang,
> Sorry for being late.
> 
> It is similar with s3c2410 driver.
> However, CONFIG_PM is not necessary, when only SIMPLE_DEV_PM_OPS is
> used.

Still, you are always creating a dev_pm_ops structure, even if empty in
case of no CONFIG_PM. The s3c driver skips that and uses NULL for
setting the pm member of the driver struct then.

Re: [PATCH 1/2] i2c: designware: add CONFIG_PM_SLEEP to suspend/resume functions

[Jingoo Han]

On Tuesday, April 09, 2013 5:51 PM, Wolfram Sang wrote:
> On Tue, Apr 09, 2013 at 09:07:10AM +0900, Jingoo Han wrote:
> > On Friday, March 22, 2013 8:47 PM, Wolfram Sang wrote:
> > >
> > > On Fri, Mar 22, 2013 at 11:13:07AM +0900, Jingoo Han wrote:
> > > > Add CONFIG_PM_SLEEP to suspend/resume functions to fix the following
> > > > build warning when CONFIG_PM_SLEEP is not selected. This is because
> > > > sleep PM callbacks defined by SIMPLE_DEV_PM_OPS are only used when
> > > > the CONFIG_PM_SLEEP is enabled.
> > > >
> > > > drivers/i2c/busses/i2c-designware-platdrv.c:253:12: warning: 'dw_i2c_suspend' defined but not used
> [-
> > > Wunused-function]
> > > > drivers/i2c/busses/i2c-designware-platdrv.c:263:12: warning: 'dw_i2c_resume' defined but not used
> [-
> > > Wunused-function]
> > > >
> > > > Signed-off-by: Jingoo Han <jg1.han-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> > >
> > > What about doing it like the s3c2410 driver which also adds CONFIG_PM
> > > around the whole structure? (comment applies for both patches)
> >
> > Hi Wolfram Sang,
> > Sorry for being late.
> >
> > It is similar with s3c2410 driver.
> > However, CONFIG_PM is not necessary, when only SIMPLE_DEV_PM_OPS is
> > used.
> 
> Still, you are always creating a dev_pm_ops structure, even if empty in
> case of no CONFIG_PM. The s3c driver skips that and uses NULL for
> setting the pm member of the driver struct then.

I know it.

When SIMPLE_DEV_PM_OPS is used and CONFIG_PM is NOT selected,
call back functions (suspend, resume) are empty.
Then it cannot be called by PM framework.

Of course in this case, it requires additional NULL checking by
PM framework.

However, in my opinion, it would not be critical.
Now, many drivers are using SIMPLE_DEV_PM_OPS macro in this way,
not adding CONFIG_PM. There is no side effect.


Best regards,
Jingoo Han

Re: [PATCH 1/2] i2c: designware: add CONFIG_PM_SLEEP to suspend/resume functions

[Jingoo Han]

On Tuesday, April 09, 2013 5:51 PM, Wolfram Sang wrote:
> -----Original Message-----
> From: Wolfram Sang [mailto:wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org]
> Sent: Tuesday, April 09, 2013 5:51 PM
> To: Jingoo Han
> Cc: 'Wolfram Sang'; linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; 'Ben Dooks'
> Subject: Re: [PATCH 1/2] i2c: designware: add CONFIG_PM_SLEEP to suspend/resume functions
> 
> On Tue, Apr 09, 2013 at 09:07:10AM +0900, Jingoo Han wrote:
> > On Friday, March 22, 2013 8:47 PM, Wolfram Sang wrote:
> > >
> > > On Fri, Mar 22, 2013 at 11:13:07AM +0900, Jingoo Han wrote:
> > > > Add CONFIG_PM_SLEEP to suspend/resume functions to fix the following
> > > > build warning when CONFIG_PM_SLEEP is not selected. This is because
> > > > sleep PM callbacks defined by SIMPLE_DEV_PM_OPS are only used when
> > > > the CONFIG_PM_SLEEP is enabled.
> > > >
> > > > drivers/i2c/busses/i2c-designware-platdrv.c:253:12: warning: 'dw_i2c_suspend' defined but not used
> [-
> > > Wunused-function]
> > > > drivers/i2c/busses/i2c-designware-platdrv.c:263:12: warning: 'dw_i2c_resume' defined but not used
> [-
> > > Wunused-function]
> > > >
> > > > Signed-off-by: Jingoo Han <jg1.han-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> > >
> > > What about doing it like the s3c2410 driver which also adds CONFIG_PM
> > > around the whole structure? (comment applies for both patches)
> >
> > Hi Wolfram Sang,
> > Sorry for being late.
> >
> > It is similar with s3c2410 driver.
> > However, CONFIG_PM is not necessary, when only SIMPLE_DEV_PM_OPS is
> > used.
> 
> Still, you are always creating a dev_pm_ops structure, even if empty in
> case of no CONFIG_PM. The s3c driver skips that and uses NULL for
> setting the pm member of the driver struct then.

CC'ed Rafael J. Wysocki


I know it.

When SIMPLE_DEV_PM_OPS is used and CONFIG_PM is NOT selected,
call back functions (suspend, resume) are empty.
Then it cannot be called by PM framework.

Of course in this case, it requires additional NULL checking by
PM framework.

However, in my opinion, it would not be critical.
Now, many drivers are using SIMPLE_DEV_PM_OPS macro in this way,
not adding CONFIG_PM. There is no side effect.


Best regards,
Jingoo Han