linux-fbdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] backlight: pwm_bl: Use SIMPLE_DEV_PM_OPS()
@ 2014-03-29 10:11 Alexander Shiyan
  2014-03-31  7:38 ` Lee Jones
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: Alexander Shiyan @ 2014-03-29 10:11 UTC (permalink / raw)
  To: linux-fbdev

Use the SIMPLE_DEV_PM_OPS() macro to declare the driver's pm_ops.

Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
---
 drivers/video/backlight/pwm_bl.c | 17 ++++-------------
 1 file changed, 4 insertions(+), 13 deletions(-)

diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
index b75201f..d5e1f5b 100644
--- a/drivers/video/backlight/pwm_bl.c
+++ b/drivers/video/backlight/pwm_bl.c
@@ -359,8 +359,7 @@ static int pwm_backlight_remove(struct platform_device *pdev)
 	return 0;
 }
 
-#ifdef CONFIG_PM_SLEEP
-static int pwm_backlight_suspend(struct device *dev)
+static int __maybe_unused pwm_bl_suspend(struct device *dev)
 {
 	struct backlight_device *bl = dev_get_drvdata(dev);
 	struct pwm_bl_data *pb = bl_get_data(bl);
@@ -376,7 +375,7 @@ static int pwm_backlight_suspend(struct device *dev)
 	return 0;
 }
 
-static int pwm_backlight_resume(struct device *dev)
+static int __maybe_unused pwm_bl_resume(struct device *dev)
 {
 	struct backlight_device *bl = dev_get_drvdata(dev);
 
@@ -384,22 +383,14 @@ static int pwm_backlight_resume(struct device *dev)
 
 	return 0;
 }
-#endif
 
-static const struct dev_pm_ops pwm_backlight_pm_ops = {
-#ifdef CONFIG_PM_SLEEP
-	.suspend = pwm_backlight_suspend,
-	.resume = pwm_backlight_resume,
-	.poweroff = pwm_backlight_suspend,
-	.restore = pwm_backlight_resume,
-#endif
-};
+static SIMPLE_DEV_PM_OPS(pwm_bl_pm_ops, pwm_bl_suspend, pwm_bl_resume);
 
 static struct platform_driver pwm_backlight_driver = {
 	.driver		= {
 		.name		= "pwm-backlight",
 		.owner		= THIS_MODULE,
-		.pm		= &pwm_backlight_pm_ops,
+		.pm		= &pwm_bl_pm_ops,
 		.of_match_table	= of_match_ptr(pwm_backlight_of_match),
 	},
 	.probe		= pwm_backlight_probe,
-- 
1.8.3.2


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] backlight: pwm_bl: Use SIMPLE_DEV_PM_OPS()
  2014-03-29 10:11 [PATCH] backlight: pwm_bl: Use SIMPLE_DEV_PM_OPS() Alexander Shiyan
@ 2014-03-31  7:38 ` Lee Jones
  2014-03-31  8:16 ` Lee Jones
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Lee Jones @ 2014-03-31  7:38 UTC (permalink / raw)
  To: linux-fbdev

> Use the SIMPLE_DEV_PM_OPS() macro to declare the driver's pm_ops.
> 
> Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
> ---
>  drivers/video/backlight/pwm_bl.c | 17 ++++-------------
>  1 file changed, 4 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
> index b75201f..d5e1f5b 100644
> --- a/drivers/video/backlight/pwm_bl.c
> +++ b/drivers/video/backlight/pwm_bl.c

[...]

> -static int pwm_backlight_resume(struct device *dev)
> +static int __maybe_unused pwm_bl_resume(struct device *dev)

What's the __maybe_unused attribute for?

In include/linux/compiler-gcc.h it redefines the attribute as 'unused': 

  #define __maybe_unused                  __attribute__((unused))

... are you sure this is what you want?

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] backlight: pwm_bl: Use SIMPLE_DEV_PM_OPS()
@ 2014-03-31  7:41 Alexander Shiyan
  2014-03-31  8:18 ` Alexander Shiyan
  2014-03-31 10:36 ` Alexander Shiyan
  0 siblings, 2 replies; 10+ messages in thread
From: Alexander Shiyan @ 2014-03-31  7:41 UTC (permalink / raw)
  To: linux-fbdev

TW9uLCAzMSBNYXIgMjAxNCAwODozODo0OCArMDEwMCDQvtGCIExlZSBKb25lcyA8bGVlLmpvbmVz
QGxpbmFyby5vcmc+Ogo+ID4gVXNlIHRoZSBTSU1QTEVfREVWX1BNX09QUygpIG1hY3JvIHRvIGRl
Y2xhcmUgdGhlIGRyaXZlcidzIHBtX29wcy4KPiA+IAo+ID4gU2lnbmVkLW9mZi1ieTogQWxleGFu
ZGVyIFNoaXlhbiA8c2hjX3dvcmtAbWFpbC5ydT4KPiA+IC0tLQo+ID4gIGRyaXZlcnMvdmlkZW8v
YmFja2xpZ2h0L3B3bV9ibC5jIHwgMTcgKysrKy0tLS0tLS0tLS0tLS0KPiA+ICAxIGZpbGUgY2hh
bmdlZCwgNCBpbnNlcnRpb25zKCspLCAxMyBkZWxldGlvbnMoLSkKPiA+IAo+ID4gZGlmZiAtLWdp
dCBhL2RyaXZlcnMvdmlkZW8vYmFja2xpZ2h0L3B3bV9ibC5jIGIvZHJpdmVycy92aWRlby9iYWNr
bGlnaHQvcHdtX2JsLmMKPiA+IGluZGV4IGI3NTIwMWYuLmQ1ZTFmNWIgMTAwNjQ0Cj4gPiAtLS0g
YS9kcml2ZXJzL3ZpZGVvL2JhY2tsaWdodC9wd21fYmwuYwo+ID4gKysrIGIvZHJpdmVycy92aWRl
by9iYWNrbGlnaHQvcHdtX2JsLmMKPiAKPiBbLi4uXQo+IAo+ID4gLXN0YXRpYyBpbnQgcHdtX2Jh
Y2tsaWdodF9yZXN1bWUoc3RydWN0IGRldmljZSAqZGV2KQo+ID4gK3N0YXRpYyBpbnQgX19tYXli
ZV91bnVzZWQgcHdtX2JsX3Jlc3VtZShzdHJ1Y3QgZGV2aWNlICpkZXYpCj4gCj4gV2hhdCdzIHRo
ZSBfX21heWJlX3VudXNlZCBhdHRyaWJ1dGUgZm9yPwo+IAo+IEluIGluY2x1ZGUvbGludXgvY29t
cGlsZXItZ2NjLmggaXQgcmVkZWZpbmVzIHRoZSBhdHRyaWJ1dGUgYXMgJ3VudXNlZCc6IAo+IAo+
ICAgI2RlZmluZSBfX21heWJlX3VudXNlZCAgICAgICAgICAgICAgICAgIF9fYXR0cmlidXRlX18o
KHVudXNlZCkpCj4gCj4gLi4uIGFyZSB5b3Ugc3VyZSB0aGlzIGlzIHdoYXQgeW91IHdhbnQ/CgpZ
ZXMuIFRoaXMgYXZvaWRzIGNvbXBpbGVyIHdhcm5pbmdzIGlmIENPTkZJR19QTSBpcyB1bnNldC4K
Ci0tLQoK

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] backlight: pwm_bl: Use SIMPLE_DEV_PM_OPS()
  2014-03-29 10:11 [PATCH] backlight: pwm_bl: Use SIMPLE_DEV_PM_OPS() Alexander Shiyan
  2014-03-31  7:38 ` Lee Jones
@ 2014-03-31  8:16 ` Lee Jones
  2014-03-31  8:50 ` Lee Jones
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Lee Jones @ 2014-03-31  8:16 UTC (permalink / raw)
  To: linux-fbdev

> > > Use the SIMPLE_DEV_PM_OPS() macro to declare the driver's pm_ops.
> > > 
> > > Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
> > > ---
> > >  drivers/video/backlight/pwm_bl.c | 17 ++++-------------
> > >  1 file changed, 4 insertions(+), 13 deletions(-)
> > > 
> > > diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
> > > index b75201f..d5e1f5b 100644
> > > --- a/drivers/video/backlight/pwm_bl.c
> > > +++ b/drivers/video/backlight/pwm_bl.c
> > 
> > [...]
> > 
> > > -static int pwm_backlight_resume(struct device *dev)
> > > +static int __maybe_unused pwm_bl_resume(struct device *dev)
> > 
> > What's the __maybe_unused attribute for?
> > 
> > In include/linux/compiler-gcc.h it redefines the attribute as 'unused': 
> > 
> >   #define __maybe_unused                  __attribute__((unused))
> > 
> > ... are you sure this is what you want?
> 
> Yes. This avoids compiler warnings if CONFIG_PM is unset.

What are the advantages of this over the config option? Besides 2
lines of code?

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] backlight: pwm_bl: Use SIMPLE_DEV_PM_OPS()
  2014-03-31  7:41 Alexander Shiyan
@ 2014-03-31  8:18 ` Alexander Shiyan
  2014-03-31 10:36 ` Alexander Shiyan
  1 sibling, 0 replies; 10+ messages in thread
From: Alexander Shiyan @ 2014-03-31  8:18 UTC (permalink / raw)
  To: linux-fbdev

TW9uLCAzMSBNYXIgMjAxNCAwOToxNjozOCArMDEwMCDQvtGCIExlZSBKb25lcyA8bGVlLmpvbmVz
QGxpbmFyby5vcmc+Ogo+ID4gPiA+IFVzZSB0aGUgU0lNUExFX0RFVl9QTV9PUFMoKSBtYWNybyB0
byBkZWNsYXJlIHRoZSBkcml2ZXIncyBwbV9vcHMuCj4gPiA+ID4gCj4gPiA+ID4gU2lnbmVkLW9m
Zi1ieTogQWxleGFuZGVyIFNoaXlhbiA8c2hjX3dvcmtAbWFpbC5ydT4KPiA+ID4gPiAtLS0KPiA+
ID4gPiAgZHJpdmVycy92aWRlby9iYWNrbGlnaHQvcHdtX2JsLmMgfCAxNyArKysrLS0tLS0tLS0t
LS0tLQo+ID4gPiA+ICAxIGZpbGUgY2hhbmdlZCwgNCBpbnNlcnRpb25zKCspLCAxMyBkZWxldGlv
bnMoLSkKPiA+ID4gPiAKPiA+ID4gPiBkaWZmIC0tZ2l0IGEvZHJpdmVycy92aWRlby9iYWNrbGln
aHQvcHdtX2JsLmMgYi9kcml2ZXJzL3ZpZGVvL2JhY2tsaWdodC9wd21fYmwuYwo+ID4gPiA+IGlu
ZGV4IGI3NTIwMWYuLmQ1ZTFmNWIgMTAwNjQ0Cj4gPiA+ID4gLS0tIGEvZHJpdmVycy92aWRlby9i
YWNrbGlnaHQvcHdtX2JsLmMKPiA+ID4gPiArKysgYi9kcml2ZXJzL3ZpZGVvL2JhY2tsaWdodC9w
d21fYmwuYwo+ID4gPiAKPiA+ID4gWy4uLl0KPiA+ID4gCj4gPiA+ID4gLXN0YXRpYyBpbnQgcHdt
X2JhY2tsaWdodF9yZXN1bWUoc3RydWN0IGRldmljZSAqZGV2KQo+ID4gPiA+ICtzdGF0aWMgaW50
IF9fbWF5YmVfdW51c2VkIHB3bV9ibF9yZXN1bWUoc3RydWN0IGRldmljZSAqZGV2KQo+ID4gPiAK
PiA+ID4gV2hhdCdzIHRoZSBfX21heWJlX3VudXNlZCBhdHRyaWJ1dGUgZm9yPwo+ID4gPiAKPiA+
ID4gSW4gaW5jbHVkZS9saW51eC9jb21waWxlci1nY2MuaCBpdCByZWRlZmluZXMgdGhlIGF0dHJp
YnV0ZSBhcyAndW51c2VkJzogCj4gPiA+IAo+ID4gPiAgICNkZWZpbmUgX19tYXliZV91bnVzZWQg
ICAgICAgICAgICAgICAgICBfX2F0dHJpYnV0ZV9fKCh1bnVzZWQpKQo+ID4gPiAKPiA+ID4gLi4u
IGFyZSB5b3Ugc3VyZSB0aGlzIGlzIHdoYXQgeW91IHdhbnQ/Cj4gPiAKPiA+IFllcy4gVGhpcyBh
dm9pZHMgY29tcGlsZXIgd2FybmluZ3MgaWYgQ09ORklHX1BNIGlzIHVuc2V0Lgo+IAo+IFdoYXQg
YXJlIHRoZSBhZHZhbnRhZ2VzIG9mIHRoaXMgb3ZlciB0aGUgY29uZmlnIG9wdGlvbj8gQmVzaWRl
cyAyCj4gbGluZXMgb2YgY29kZT8KCkp1c3QgYW4gaW5jcmVhc2UgY29tcGlsZSBjb3ZlcmFnZS4K
Ci0tLQoK

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] backlight: pwm_bl: Use SIMPLE_DEV_PM_OPS()
  2014-03-29 10:11 [PATCH] backlight: pwm_bl: Use SIMPLE_DEV_PM_OPS() Alexander Shiyan
  2014-03-31  7:38 ` Lee Jones
  2014-03-31  8:16 ` Lee Jones
@ 2014-03-31  8:50 ` Lee Jones
  2014-04-01  0:08 ` Jingoo Han
  2018-12-16  4:21 ` Alexander Shiyan
  4 siblings, 0 replies; 10+ messages in thread
From: Lee Jones @ 2014-03-31  8:50 UTC (permalink / raw)
  To: linux-fbdev

> > > > > Use the SIMPLE_DEV_PM_OPS() macro to declare the driver's pm_ops.
> > > > > 
> > > > > Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
> > > > > ---
> > > > >  drivers/video/backlight/pwm_bl.c | 17 ++++-------------
> > > > >  1 file changed, 4 insertions(+), 13 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
> > > > > index b75201f..d5e1f5b 100644
> > > > > --- a/drivers/video/backlight/pwm_bl.c
> > > > > +++ b/drivers/video/backlight/pwm_bl.c
> > > > 
> > > > [...]
> > > > 
> > > > > -static int pwm_backlight_resume(struct device *dev)
> > > > > +static int __maybe_unused pwm_bl_resume(struct device *dev)
> > > > 
> > > > What's the __maybe_unused attribute for?
> > > > 
> > > > In include/linux/compiler-gcc.h it redefines the attribute as 'unused': 
> > > > 
> > > >   #define __maybe_unused                  __attribute__((unused))
> > > > 
> > > > ... are you sure this is what you want?
> > > 
> > > Yes. This avoids compiler warnings if CONFIG_PM is unset.
> > 
> > What are the advantages of this over the config option? Besides 2
> > lines of code?
> 
> Just an increase compile coverage.

What does this mean? I replied to another one of your patches with the
same question.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] backlight: pwm_bl: Use SIMPLE_DEV_PM_OPS()
  2014-03-31  7:41 Alexander Shiyan
  2014-03-31  8:18 ` Alexander Shiyan
@ 2014-03-31 10:36 ` Alexander Shiyan
  1 sibling, 0 replies; 10+ messages in thread
From: Alexander Shiyan @ 2014-03-31 10:36 UTC (permalink / raw)
  To: linux-fbdev

TW9uLCAzMSBNYXIgMjAxNCAwOTo1MDo0NyArMDEwMCDQvtGCIExlZSBKb25lcyA8bGVlLmpvbmVz
QGxpbmFyby5vcmc+Ogo+ID4gPiA+ID4gPiBVc2UgdGhlIFNJTVBMRV9ERVZfUE1fT1BTKCkgbWFj
cm8gdG8gZGVjbGFyZSB0aGUgZHJpdmVyJ3MgcG1fb3BzLgo+ID4gPiA+ID4gPiAKPiA+ID4gPiA+
ID4gU2lnbmVkLW9mZi1ieTogQWxleGFuZGVyIFNoaXlhbiA8c2hjX3dvcmtAbWFpbC5ydT4KPiA+
ID4gPiA+ID4gLS0tCj4gPiA+ID4gPiA+ICBkcml2ZXJzL3ZpZGVvL2JhY2tsaWdodC9wd21fYmwu
YyB8IDE3ICsrKystLS0tLS0tLS0tLS0tCj4gPiA+ID4gPiA+ICAxIGZpbGUgY2hhbmdlZCwgNCBp
bnNlcnRpb25zKCspLCAxMyBkZWxldGlvbnMoLSkKPiA+ID4gPiA+ID4gCj4gPiA+ID4gPiA+IGRp
ZmYgLS1naXQgYS9kcml2ZXJzL3ZpZGVvL2JhY2tsaWdodC9wd21fYmwuYyBiL2RyaXZlcnMvdmlk
ZW8vYmFja2xpZ2h0L3B3bV9ibC5jCj4gPiA+ID4gPiA+IGluZGV4IGI3NTIwMWYuLmQ1ZTFmNWIg
MTAwNjQ0Cj4gPiA+ID4gPiA+IC0tLSBhL2RyaXZlcnMvdmlkZW8vYmFja2xpZ2h0L3B3bV9ibC5j
Cj4gPiA+ID4gPiA+ICsrKyBiL2RyaXZlcnMvdmlkZW8vYmFja2xpZ2h0L3B3bV9ibC5jCj4gPiA+
ID4gPiAKPiA+ID4gPiA+IFsuLi5dCj4gPiA+ID4gPiAKPiA+ID4gPiA+ID4gLXN0YXRpYyBpbnQg
cHdtX2JhY2tsaWdodF9yZXN1bWUoc3RydWN0IGRldmljZSAqZGV2KQo+ID4gPiA+ID4gPiArc3Rh
dGljIGludCBfX21heWJlX3VudXNlZCBwd21fYmxfcmVzdW1lKHN0cnVjdCBkZXZpY2UgKmRldikK
PiA+ID4gPiA+IAo+ID4gPiA+ID4gV2hhdCdzIHRoZSBfX21heWJlX3VudXNlZCBhdHRyaWJ1dGUg
Zm9yPwo+ID4gPiA+ID4gCj4gPiA+ID4gPiBJbiBpbmNsdWRlL2xpbnV4L2NvbXBpbGVyLWdjYy5o
IGl0IHJlZGVmaW5lcyB0aGUgYXR0cmlidXRlIGFzICd1bnVzZWQnOiAKPiA+ID4gPiA+IAo+ID4g
PiA+ID4gICAjZGVmaW5lIF9fbWF5YmVfdW51c2VkICAgICAgICAgICAgICAgICAgX19hdHRyaWJ1
dGVfXygodW51c2VkKSkKPiA+ID4gPiA+IAo+ID4gPiA+ID4gLi4uIGFyZSB5b3Ugc3VyZSB0aGlz
IGlzIHdoYXQgeW91IHdhbnQ/Cj4gPiA+ID4gCj4gPiA+ID4gWWVzLiBUaGlzIGF2b2lkcyBjb21w
aWxlciB3YXJuaW5ncyBpZiBDT05GSUdfUE0gaXMgdW5zZXQuCj4gPiA+IAo+ID4gPiBXaGF0IGFy
ZSB0aGUgYWR2YW50YWdlcyBvZiB0aGlzIG92ZXIgdGhlIGNvbmZpZyBvcHRpb24/IEJlc2lkZXMg
Mgo+ID4gPiBsaW5lcyBvZiBjb2RlPwo+ID4gCj4gPiBKdXN0IGFuIGluY3JlYXNlIGNvbXBpbGUg
Y292ZXJhZ2UuCj4gCj4gV2hhdCBkb2VzIHRoaXMgbWVhbj8gSSByZXBsaWVkIHRvIGFub3RoZXIg
b25lIG9mIHlvdXIgcGF0Y2hlcyB3aXRoIHRoZQo+IHNhbWUgcXVlc3Rpb24uCgpDb2RlIHBhcnRz
IGZvciBzdXNwZW5kKCkgYW5kIHJlc3VtZSgpIHdpbGwgYmUgY29tcGlsZWQgcmVnYXJkbGVzcyBv
ZgpDT05GSUdfUE0gb3B0aW9uIGFuZCB3aWxsIGJlIGRpc2NhcmRlZCBieSBsaW5rZXIgaWYgQ09O
RklHX1BNIGlzIG5vdCBzZXQuCgotLS0KCg=

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] backlight: pwm_bl: Use SIMPLE_DEV_PM_OPS()
  2014-03-29 10:11 [PATCH] backlight: pwm_bl: Use SIMPLE_DEV_PM_OPS() Alexander Shiyan
                   ` (2 preceding siblings ...)
  2014-03-31  8:50 ` Lee Jones
@ 2014-04-01  0:08 ` Jingoo Han
  2018-12-16  4:21 ` Alexander Shiyan
  4 siblings, 0 replies; 10+ messages in thread
From: Jingoo Han @ 2014-04-01  0:08 UTC (permalink / raw)
  To: linux-fbdev

On Monday, March 31, 2014 7:37 PM, Lee Jones wrote:
> Mon, 31 Mar 2014 09:50:47 +0100 от Lee Jones <lee.jones@linaro.org>:
> > > > > > > Use the SIMPLE_DEV_PM_OPS() macro to declare the driver's pm_ops.
> > > > > > >
> > > > > > > Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
> > > > > > > ---
> > > > > > >  drivers/video/backlight/pwm_bl.c | 17 ++++-------------
> > > > > > >  1 file changed, 4 insertions(+), 13 deletions(-)
> > > > > > >
> > > > > > > diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
> > > > > > > index b75201f..d5e1f5b 100644
> > > > > > > --- a/drivers/video/backlight/pwm_bl.c
> > > > > > > +++ b/drivers/video/backlight/pwm_bl.c
> > > > > >
> > > > > > [...]
> > > > > >
> > > > > > > -static int pwm_backlight_resume(struct device *dev)
> > > > > > > +static int __maybe_unused pwm_bl_resume(struct device *dev)
> > > > > >
> > > > > > What's the __maybe_unused attribute for?
> > > > > >
> > > > > > In include/linux/compiler-gcc.h it redefines the attribute as 'unused':
> > > > > >
> > > > > >   #define __maybe_unused                  __attribute__((unused))
> > > > > >
> > > > > > ... are you sure this is what you want?
> > > > >
> > > > > Yes. This avoids compiler warnings if CONFIG_PM is unset.
> > > >
> > > > What are the advantages of this over the config option? Besides 2
> > > > lines of code?
> > >
> > > Just an increase compile coverage.
> >
> > What does this mean? I replied to another one of your patches with the
> > same question.
> 
> Code parts for suspend() and resume() will be compiled regardless of
> CONFIG_PM option and will be discarded by linker if CONFIG_PM is not set.

(+cc Joe Perches, Dan Carpenter)

Original patch is 
http://www.spinics.net/lists/linux-fbdev/msg14177.html

I don't think that __maybe_unused is good.
Currently, SIMPLE_DEV_PM_OPS is using "#ifdef CONFIG_PM_SLEEP" as below.
So, suspend_fn, resume_fn should be protected by "#ifdef CONFIG_PM_SLEEP".
Maybe, it is necessary to fix SET_SYSTEM_SLEEP_PM_OPS define,
in order to increase compile coverage.

./include/linux/pm.h
#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) \
}

#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


Please, don’t remove "#ifdef CONFIG_PM_SLEEP".
Your patch can be modified as below:

--- a/drivers/video/backlight/pwm_bl.c
+++ b/drivers/video/backlight/pwm_bl.c
@@ -386,14 +386,7 @@ static int pwm_backlight_resume(struct device *dev)
 }
 #endif

-static const struct dev_pm_ops pwm_backlight_pm_ops = {
-#ifdef CONFIG_PM_SLEEP
-       .suspend = pwm_backlight_suspend,
-       .resume = pwm_backlight_resume,
-       .poweroff = pwm_backlight_suspend,
-       .restore = pwm_backlight_resume,
-#endif
-};
+static SIMPLE_DEV_PM_OPS(pwm_bl_pm_ops, pwm_bl_suspend, pwm_bl_resume);

Best regards,
Jingoo Han


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH] backlight: pwm_bl: Use SIMPLE_DEV_PM_OPS()
  2014-03-29 10:11 [PATCH] backlight: pwm_bl: Use SIMPLE_DEV_PM_OPS() Alexander Shiyan
                   ` (3 preceding siblings ...)
  2014-04-01  0:08 ` Jingoo Han
@ 2018-12-16  4:21 ` Alexander Shiyan
  2018-12-17 12:05   ` Daniel Thompson
  4 siblings, 1 reply; 10+ messages in thread
From: Alexander Shiyan @ 2018-12-16  4:21 UTC (permalink / raw)
  To: linux-fbdev
  Cc: linux-pwm, Daniel Thompson, Alexander Shiyan,
	Bartlomiej Zolnierkiewicz, Jingoo Han, dri-devel, Thierry Reding,
	Lee Jones

Use the SIMPLE_DEV_PM_OPS() macro to declare the driver's pm_ops.
As a side effect, removing #ifdef CONFIG_PM_SLEEP will improve
compilation coverage.

Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
---
 drivers/video/backlight/pwm_bl.c | 17 ++++-------------
 1 file changed, 4 insertions(+), 13 deletions(-)

diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
index f9ef067..33503ce 100644
--- a/drivers/video/backlight/pwm_bl.c
+++ b/drivers/video/backlight/pwm_bl.c
@@ -667,8 +667,7 @@ static void pwm_backlight_shutdown(struct platform_device *pdev)
 	pwm_backlight_power_off(pb);
 }
 
-#ifdef CONFIG_PM_SLEEP
-static int pwm_backlight_suspend(struct device *dev)
+static int __maybe_unused pwm_bl_suspend(struct device *dev)
 {
 	struct backlight_device *bl = dev_get_drvdata(dev);
 	struct pwm_bl_data *pb = bl_get_data(bl);
@@ -684,7 +683,7 @@ static int pwm_backlight_suspend(struct device *dev)
 	return 0;
 }
 
-static int pwm_backlight_resume(struct device *dev)
+static int __maybe_unused pwm_bl_resume(struct device *dev)
 {
 	struct backlight_device *bl = dev_get_drvdata(dev);
 
@@ -692,21 +691,13 @@ static int pwm_backlight_resume(struct device *dev)
 
 	return 0;
 }
-#endif
 
-static const struct dev_pm_ops pwm_backlight_pm_ops = {
-#ifdef CONFIG_PM_SLEEP
-	.suspend = pwm_backlight_suspend,
-	.resume = pwm_backlight_resume,
-	.poweroff = pwm_backlight_suspend,
-	.restore = pwm_backlight_resume,
-#endif
-};
+static SIMPLE_DEV_PM_OPS(pwm_bl_pm_ops, pwm_bl_suspend, pwm_bl_resume);
 
 static struct platform_driver pwm_backlight_driver = {
 	.driver		= {
 		.name		= "pwm-backlight",
-		.pm		= &pwm_backlight_pm_ops,
+		.pm		= &pwm_bl_pm_ops,
 		.of_match_table	= of_match_ptr(pwm_backlight_of_match),
 	},
 	.probe		= pwm_backlight_probe,
-- 
2.10.2

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] backlight: pwm_bl: Use SIMPLE_DEV_PM_OPS()
  2018-12-16  4:21 ` Alexander Shiyan
@ 2018-12-17 12:05   ` Daniel Thompson
  0 siblings, 0 replies; 10+ messages in thread
From: Daniel Thompson @ 2018-12-17 12:05 UTC (permalink / raw)
  To: Alexander Shiyan
  Cc: linux-pwm, linux-fbdev, Bartlomiej Zolnierkiewicz, Jingoo Han,
	dri-devel, Thierry Reding, Lee Jones

On Sun, Dec 16, 2018 at 07:21:12AM +0300, Alexander Shiyan wrote:
> Use the SIMPLE_DEV_PM_OPS() macro to declare the driver's pm_ops.
> As a side effect, removing #ifdef CONFIG_PM_SLEEP will improve
> compilation coverage.
> 
> Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
> ---
>  drivers/video/backlight/pwm_bl.c | 17 ++++-------------
>  1 file changed, 4 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
> index f9ef067..33503ce 100644
> --- a/drivers/video/backlight/pwm_bl.c
> +++ b/drivers/video/backlight/pwm_bl.c
> @@ -667,8 +667,7 @@ static void pwm_backlight_shutdown(struct platform_device *pdev)
>  	pwm_backlight_power_off(pb);
>  }
>  
> -#ifdef CONFIG_PM_SLEEP
> -static int pwm_backlight_suspend(struct device *dev)
> +static int __maybe_unused pwm_bl_suspend(struct device *dev)

Why rename the symbol. It no longer matches the coding style of the
file.


>  {
>  	struct backlight_device *bl = dev_get_drvdata(dev);
>  	struct pwm_bl_data *pb = bl_get_data(bl);
> @@ -684,7 +683,7 @@ static int pwm_backlight_suspend(struct device *dev)
>  	return 0;
>  }
>  
> -static int pwm_backlight_resume(struct device *dev)
> +static int __maybe_unused pwm_bl_resume(struct device *dev)

Same here.


>  {
>  	struct backlight_device *bl = dev_get_drvdata(dev);
>  
> @@ -692,21 +691,13 @@ static int pwm_backlight_resume(struct device *dev)
>  
>  	return 0;
>  }
> -#endif
>  
> -static const struct dev_pm_ops pwm_backlight_pm_ops = {
> -#ifdef CONFIG_PM_SLEEP
> -	.suspend = pwm_backlight_suspend,
> -	.resume = pwm_backlight_resume,
> -	.poweroff = pwm_backlight_suspend,
> -	.restore = pwm_backlight_resume,
> -#endif
> -};
> +static SIMPLE_DEV_PM_OPS(pwm_bl_pm_ops, pwm_bl_suspend, pwm_bl_resume);

And same here...


Daniel.


>  
>  static struct platform_driver pwm_backlight_driver = {
>  	.driver		= {
>  		.name		= "pwm-backlight",
> -		.pm		= &pwm_backlight_pm_ops,
> +		.pm		= &pwm_bl_pm_ops,
>  		.of_match_table	= of_match_ptr(pwm_backlight_of_match),
>  	},
>  	.probe		= pwm_backlight_probe,
> -- 
> 2.10.2
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2018-12-17 12:05 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-03-29 10:11 [PATCH] backlight: pwm_bl: Use SIMPLE_DEV_PM_OPS() Alexander Shiyan
2014-03-31  7:38 ` Lee Jones
2014-03-31  8:16 ` Lee Jones
2014-03-31  8:50 ` Lee Jones
2014-04-01  0:08 ` Jingoo Han
2018-12-16  4:21 ` Alexander Shiyan
2018-12-17 12:05   ` Daniel Thompson
  -- strict thread matches above, loose matches on Subject: below --
2014-03-31  7:41 Alexander Shiyan
2014-03-31  8:18 ` Alexander Shiyan
2014-03-31 10:36 ` Alexander Shiyan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).