[PATCH 0/3] pm: Introduce __pm to mark power management code

[PATCH 0/3] pm: Introduce __pm to mark power management code

[Guenter Roeck]

The following patch series introduces a marker for power management functions
and data. This this marker, #ifdef CONFIG_PM and #ifdef CONFIG_PM_SLEEP
can be removed from most of the code. This ensures that the conditional code
still compiles but is not included in the object file.

As a side effect, drivers declaring struct dev_pm_ops unconditionally
get a bit smaller if CONFIG_PM_SLEEP is not configured.

The first patch in the series introduces the marker, the following
two patches introduce the marker in two drivers to demonstrate its use.

The patch series depends on the "PM: Add pm_ops_ptr() macro" patch
submitted by Jingoo Han.

[PATCH 1/3] pm: Introduce __pm to mark power management functions and data

[Guenter Roeck]

By marking power management functions and data with __pm, #ifdef CONFIG_PM
and #ifdef CONFIG_PM_SLEEP is no longer necessary in most cases.
This ensures that the power management code still compiles even if power
management is disabled, but does not consume space in the object file.
As a side effect, drivers declaring struct dev_pm_ops unconditionally
get a bit smaller if CONFIG_PM_SLEEP is disabled.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 include/linux/pm.h |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/linux/pm.h b/include/linux/pm.h
index fe70d9b..46df155 100644
--- a/include/linux/pm.h
+++ b/include/linux/pm.h
@@ -43,9 +43,11 @@ struct device;
 #ifdef CONFIG_PM
 extern const char power_group_name[];		/* = "power" */
 #define pm_ops_ptr(_ptr)	(_ptr)
+#define __pm
 #else
 #define power_group_name	NULL
 #define pm_ops_ptr(_ptr)	NULL
+#define __pm			__section(.discard)
 #endif
 
 typedef struct pm_message {
-- 
1.7.9.7

[PATCH 2/3] hwmon: (tmp102) Convert to use __pm instead of #ifdef CONFIG_PM

[Guenter Roeck]

With this change, code only needed is power management is active still
compiles if it is disabled, but does not consume space in the object file.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/hwmon/tmp102.c |   14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/drivers/hwmon/tmp102.c b/drivers/hwmon/tmp102.c
index 523dd89..8e5d267 100644
--- a/drivers/hwmon/tmp102.c
+++ b/drivers/hwmon/tmp102.c
@@ -236,8 +236,7 @@ static int tmp102_remove(struct i2c_client *client)
 	return 0;
 }
 
-#ifdef CONFIG_PM
-static int tmp102_suspend(struct device *dev)
+static int __pm tmp102_suspend(struct device *dev)
 {
 	struct i2c_client *client = to_i2c_client(dev);
 	int config;
@@ -250,7 +249,7 @@ static int tmp102_suspend(struct device *dev)
 	return i2c_smbus_write_word_swapped(client, TMP102_CONF_REG, config);
 }
 
-static int tmp102_resume(struct device *dev)
+static int __pm tmp102_resume(struct device *dev)
 {
 	struct i2c_client *client = to_i2c_client(dev);
 	int config;
@@ -263,16 +262,11 @@ static int tmp102_resume(struct device *dev)
 	return i2c_smbus_write_word_swapped(client, TMP102_CONF_REG, config);
 }
 
-static const struct dev_pm_ops tmp102_dev_pm_ops = {
+static const struct dev_pm_ops __pm tmp102_dev_pm_ops = {
 	.suspend	= tmp102_suspend,
 	.resume		= tmp102_resume,
 };
 
-#define TMP102_DEV_PM_OPS (&tmp102_dev_pm_ops)
-#else
-#define	TMP102_DEV_PM_OPS NULL
-#endif /* CONFIG_PM */
-
 static const struct i2c_device_id tmp102_id[] = {
 	{ "tmp102", 0 },
 	{ }
@@ -281,7 +275,7 @@ MODULE_DEVICE_TABLE(i2c, tmp102_id);
 
 static struct i2c_driver tmp102_driver = {
 	.driver.name	= DRIVER_NAME,
-	.driver.pm	= TMP102_DEV_PM_OPS,
+	.driver.pm	= pm_ops_ptr(&tmp102_dev_pm_ops),
 	.probe		= tmp102_probe,
 	.remove		= tmp102_remove,
 	.id_table	= tmp102_id,
-- 
1.7.9.7

[PATCH 3/3] hwmon: (max6639) Convert to use __pm instead of #ifdef CONFIG_PM_SLEEP

[Guenter Roeck]

Drops #ifdef from code, and ensures that conditional code still compiles
if power management is disabled. Resulting code is dropped from object file.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/hwmon/max6639.c |   13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/hwmon/max6639.c b/drivers/hwmon/max6639.c
index 3e7b426..6080913 100644
--- a/drivers/hwmon/max6639.c
+++ b/drivers/hwmon/max6639.c
@@ -591,8 +591,7 @@ static int max6639_remove(struct i2c_client *client)
 	return 0;
 }
 
-#ifdef CONFIG_PM_SLEEP
-static int max6639_suspend(struct device *dev)
+static int __pm max6639_suspend(struct device *dev)
 {
 	struct i2c_client *client = to_i2c_client(dev);
 	int data = i2c_smbus_read_byte_data(client, MAX6639_REG_GCONFIG);
@@ -603,7 +602,7 @@ static int max6639_suspend(struct device *dev)
 			MAX6639_REG_GCONFIG, data | MAX6639_GCONFIG_STANDBY);
 }
 
-static int max6639_resume(struct device *dev)
+static int __pm max6639_resume(struct device *dev)
 {
 	struct i2c_client *client = to_i2c_client(dev);
 	int data = i2c_smbus_read_byte_data(client, MAX6639_REG_GCONFIG);
@@ -613,7 +612,6 @@ static int max6639_resume(struct device *dev)
 	return i2c_smbus_write_byte_data(client,
 			MAX6639_REG_GCONFIG, data & ~MAX6639_GCONFIG_STANDBY);
 }
-#endif /* CONFIG_PM_SLEEP */
 
 static const struct i2c_device_id max6639_id[] = {
 	{"max6639", 0},
@@ -622,15 +620,16 @@ static const struct i2c_device_id max6639_id[] = {
 
 MODULE_DEVICE_TABLE(i2c, max6639_id);
 
-static const struct dev_pm_ops max6639_pm_ops = {
-	SET_SYSTEM_SLEEP_PM_OPS(max6639_suspend, max6639_resume)
+static const struct dev_pm_ops __pm max6639_pm_ops = {
+	.suspend = max6639_suspend,
+	.resume = max6639_resume,
 };
 
 static struct i2c_driver max6639_driver = {
 	.class = I2C_CLASS_HWMON,
 	.driver = {
 		   .name = "max6639",
-		   .pm = &max6639_pm_ops,
+		   .pm = pm_ops_ptr(&max6639_pm_ops),
 		   },
 	.probe = max6639_probe,
 	.remove = max6639_remove,
-- 
1.7.9.7

Re: [PATCH 0/3] pm: Introduce __pm to mark power management code

[Pavel Machek]

On Thu 2013-05-09 10:09:48, Guenter Roeck wrote:
> The following patch series introduces a marker for power management functions
> and data. This this marker, #ifdef CONFIG_PM and #ifdef CONFIG_PM_SLEEP
> can be removed from most of the code. This ensures that the conditional code
> still compiles but is not included in the object file.

Was it compile-tested for both PM_SLEEP and !PM_SLEEP cases?

If driver wants to have pm-specific fields in its structures, this
will make it harder.

Ifdefs are ugly, and this has better compiler coverage in !PM_SLEEP
case. Good. OTOH most people are running with PM_SLEEP, so advantage
is not great...
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH 0/3] pm: Introduce __pm to mark power management code

[Guenter Roeck]

On Thu, May 09, 2013 at 09:15:00PM +0200, Rafael J. Wysocki wrote:
> On Thursday, May 09, 2013 10:09:23 AM Guenter Roeck wrote:
> > The following patch series introduces a marker for power management functions
> > and data. This this marker, #ifdef CONFIG_PM and #ifdef CONFIG_PM_SLEEP
> > can be removed from most of the code. This ensures that the conditional code
> > still compiles but is not included in the object file.
> > 
> > As a side effect, drivers declaring struct dev_pm_ops unconditionally
> > get a bit smaller if CONFIG_PM_SLEEP is not configured.
> > 
> > The first patch in the series introduces the marker, the following
> > two patches introduce the marker in two drivers to demonstrate its use.
> > 
> > The patch series depends on the "PM: Add pm_ops_ptr() macro" patch
> > submitted by Jingoo Han.
> 
> What about CCing a PM core maintainer?
> 
Actually, that was the idea. Somehow my send script got screwed up.
No idea what happened. Sorry for that.

Guenter

Re: [PATCH 0/3] pm: Introduce __pm to mark power management code

[Rafael J. Wysocki]

On Thursday, May 09, 2013 10:09:23 AM Guenter Roeck wrote:
> The following patch series introduces a marker for power management functions
> and data. This this marker, #ifdef CONFIG_PM and #ifdef CONFIG_PM_SLEEP
> can be removed from most of the code. This ensures that the conditional code
> still compiles but is not included in the object file.
> 
> As a side effect, drivers declaring struct dev_pm_ops unconditionally
> get a bit smaller if CONFIG_PM_SLEEP is not configured.
> 
> The first patch in the series introduces the marker, the following
> two patches introduce the marker in two drivers to demonstrate its use.
> 
> The patch series depends on the "PM: Add pm_ops_ptr() macro" patch
> submitted by Jingoo Han.

What about CCing a PM core maintainer?

Please see my reply to Alan Stern for my opinion.

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH 0/3] pm: Introduce __pm to mark power management code

[Rafael J. Wysocki]

On Thursday, May 09, 2013 02:31:46 PM Alan Stern wrote:
> On Thu, 9 May 2013, Guenter Roeck wrote:
> 
> > On Thu, May 09, 2013 at 01:38:36PM -0400, Alan Stern wrote:
> > > On Thu, 9 May 2013, Guenter Roeck wrote:
> > > 
> > > > The following patch series introduces a marker for power management functions
> > > > and data. This this marker, #ifdef CONFIG_PM and #ifdef CONFIG_PM_SLEEP
> > > > can be removed from most of the code. This ensures that the conditional code
> > > > still compiles but is not included in the object file.
> > > > 
> > > > As a side effect, drivers declaring struct dev_pm_ops unconditionally
> > > > get a bit smaller if CONFIG_PM_SLEEP is not configured.
> > > 
> > > What about code that depends on CONFIG_PM_RUNTIME?  Or code that 
> > > depends on CONFIG_PM_SLEEP but not on CONFIG_PM_RUNTIME?
> > > 
> > Should we also introduce __pm_sleep and __pm_runtime ?
> 
> If you want to implement this correctly, I think you have to.
> 
> As for whether the additional complication is desirable ... I'll leave 
> that up to Rafael to decide.

Well, if that had been so easy to do, we would have done it already before.

I think that we first should try to combine PM_SLEEP with PM_RUNTIME (plus
some other power management options related to CPU PM) and then introduce
something like __pm.  Otherwise, it's going to be a mess.

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH 0/3] pm: Introduce __pm to mark power management code

[Alan Stern]

On Thu, 9 May 2013, Guenter Roeck wrote:

> On Thu, May 09, 2013 at 01:38:36PM -0400, Alan Stern wrote:
> > On Thu, 9 May 2013, Guenter Roeck wrote:
> > 
> > > The following patch series introduces a marker for power management functions
> > > and data. This this marker, #ifdef CONFIG_PM and #ifdef CONFIG_PM_SLEEP
> > > can be removed from most of the code. This ensures that the conditional code
> > > still compiles but is not included in the object file.
> > > 
> > > As a side effect, drivers declaring struct dev_pm_ops unconditionally
> > > get a bit smaller if CONFIG_PM_SLEEP is not configured.
> > 
> > What about code that depends on CONFIG_PM_RUNTIME?  Or code that 
> > depends on CONFIG_PM_SLEEP but not on CONFIG_PM_RUNTIME?
> > 
> Should we also introduce __pm_sleep and __pm_runtime ?

If you want to implement this correctly, I think you have to.

As for whether the additional complication is desirable ... I'll leave 
that up to Rafael to decide.

Alan Stern

Re: [PATCH 0/3] pm: Introduce __pm to mark power management code

[Guenter Roeck]

On Thu, May 09, 2013 at 01:38:36PM -0400, Alan Stern wrote:
> On Thu, 9 May 2013, Guenter Roeck wrote:
> 
> > The following patch series introduces a marker for power management functions
> > and data. This this marker, #ifdef CONFIG_PM and #ifdef CONFIG_PM_SLEEP
> > can be removed from most of the code. This ensures that the conditional code
> > still compiles but is not included in the object file.
> > 
> > As a side effect, drivers declaring struct dev_pm_ops unconditionally
> > get a bit smaller if CONFIG_PM_SLEEP is not configured.
> 
> What about code that depends on CONFIG_PM_RUNTIME?  Or code that 
> depends on CONFIG_PM_SLEEP but not on CONFIG_PM_RUNTIME?
> 
Should we also introduce __pm_sleep and __pm_runtime ?

Thanks,
Guenter

Re: [PATCH 0/3] pm: Introduce __pm to mark power management code

[Alan Stern]

On Thu, 9 May 2013, Guenter Roeck wrote:

> The following patch series introduces a marker for power management functions
> and data. This this marker, #ifdef CONFIG_PM and #ifdef CONFIG_PM_SLEEP
> can be removed from most of the code. This ensures that the conditional code
> still compiles but is not included in the object file.
> 
> As a side effect, drivers declaring struct dev_pm_ops unconditionally
> get a bit smaller if CONFIG_PM_SLEEP is not configured.

What about code that depends on CONFIG_PM_RUNTIME?  Or code that 
depends on CONFIG_PM_SLEEP but not on CONFIG_PM_RUNTIME?

Alan Stern

[PATCH 0/3] pm: Introduce __pm to mark power management code

[Guenter Roeck]

The following patch series introduces a marker for power management functions
and data. This this marker, #ifdef CONFIG_PM and #ifdef CONFIG_PM_SLEEP
can be removed from most of the code. This ensures that the conditional code
still compiles but is not included in the object file.

As a side effect, drivers declaring struct dev_pm_ops unconditionally
get a bit smaller if CONFIG_PM_SLEEP is not configured.

The first patch in the series introduces the marker, the following
two patches introduce the marker in two drivers to demonstrate its use.

The patch series depends on the "PM: Add pm_ops_ptr() macro" patch
submitted by Jingoo Han.