au1xmmc: dev_pm_ops conversion

PATCH New Albin Tonnerre 7 participants 25 messages Aug 3, 2009 – Aug 6, 2009

Subsystems (3)

Suspend to RAM Hibernation Power management core

lore

Share & tools

lore

Albin Tonnerre

author

2009-08-03 16:33 UTC #0 lore raw

Quoted 3 lines · Frans Pop

Yes, I see that in drivers/base/platform.c (legacy) .suspend resp. .resume also got called for those cases?
Ouch :-(

This really looks like it's error-prone (I made this mistake for atmel_serial and wouldn't have noticed if Frans hadn't told me), and AFAICS, for a number of drivers we'll have suspend = freeze = poweroff and resume = thaw = restore. Maybe putting something like this in pm.h would help ?

#define PM_OPS(name, suspend, resume) \

struct dev_pm_ops name = { \
	.suspend = suspend, \
	.resume = resume, \
	.freeze = suspend, \
	.thaw = resume, \
	.poweroff = suspend, \
	.restore = resume, \
};

Cheers,

Signature · 4 lines

Albin Tonnerre, Free Electrons
Kernel, drivers and embedded Linux development,
consulting, training and support.
http://free-electrons.com

Rafael J. Wysocki

2009-08-03 19:22 UTC #1 lore raw

Quoted 11 lines · Albin Tonnerre

Quoted 3 lines · Frans Pop
Yes, I see that in drivers/base/platform.c (legacy) .suspend resp. .resume also got called for those cases?
Ouch :-(
This really looks like it's error-prone (I made this mistake for atmel_serial and wouldn't have noticed if Frans hadn't told me), and AFAICS, for a number of drivers we'll have suspend = freeze = poweroff and resume = thaw = restore. Maybe putting something like this in pm.h would help ?
#define PM_OPS(name, suspend, resume) \

Quoted 8 lines

struct dev_pm_ops name = { \
	.suspend = suspend, \
	.resume = resume, \
	.freeze = suspend, \
	.thaw = resume, \
	.poweroff = suspend, \
	.restore = resume, \
};

That looks like a good idea, but please call that macro SIMPLE_PM_OPS (or something similar) and submit a patch.

Best,
Rafael

Rafael J. Wysocki

2009-08-03 19:23 UTC #24 lore raw

In reply to Albin Tonnerre

Quoted 11 lines · Albin Tonnerre

Quoted 3 lines · Frans Pop
Yes, I see that in drivers/base/platform.c (legacy) .suspend resp. .resume also got called for those cases?
Ouch :-(
This really looks like it's error-prone (I made this mistake for atmel_serial and wouldn't have noticed if Frans hadn't told me), and AFAICS, for a number of drivers we'll have suspend = freeze = poweroff and resume = thaw = restore. Maybe putting something like this in pm.h would help ?
#define PM_OPS(name, suspend, resume) \

Quoted 8 lines

struct dev_pm_ops name = { \
	.suspend = suspend, \
	.resume = resume, \
	.freeze = suspend, \
	.thaw = resume, \
	.poweroff = suspend, \
	.restore = resume, \
};

That looks like a good idea, but please call that macro SIMPLE_PM_OPS (or something similar) and submit a patch.

Best,
Rafael

Albin Tonnerre

author

2009-08-04 09:36 UTC #2 lore raw

PATCH 1/1 Add SIMPLE_PM_OPS: make switching to dev_pm_ops less error-prone

In reply to Rafael J. Wysocki

In a number of cases, the .suspend, .freeze, .poweroff and .resume, .thaw, .restore functions are identical. However, they all need to be assigned to avoid regressionsm as the previous code called .suspend resp. .resume in all those cases. SIMPLE_PM_OPS allows to deal with this case.

Signed-off-by Albin Tonnerre <[email protected]>

---
 include/linux/pm.h |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

include/linux/pm.h +10 -0

index b3f7476..994a62f 100644
@@ -184,6 +184,16 @@ struct dev_pm_ops {
 	int (*restore_noirq)(struct device *dev);
 };
 
+#define SIMPLE_PM_OPS(name, suspend_fn, resume_fn) \
+struct dev_pm_ops name = { \
+	.suspend = suspend_fn, \
+	.resume = resume_fn, \
+	.freeze = suspend_fn, \
+	.thaw = resume_fn, \
+	.poweroff = suspend_fn, \
+	.resume = resume_fn, \
+}
+
 /**
  * PM_EVENT_ messages
  *

Signature · 4 lines

Albin Tonnerre, Free Electrons
Kernel, drivers and embedded Linux development,
consulting, training and support.
http://free-electrons.com

Albin Tonnerre

author

2009-08-04 09:36 UTC #3 lore raw

PATCH 1/1 Add SIMPLE_PM_OPS: make switching to dev_pm_ops less error-prone

In reply to Rafael J. Wysocki

Signed-off-by Albin Tonnerre <[email protected]>

---
 include/linux/pm.h |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

include/linux/pm.h +10 -0

index b3f7476..994a62f 100644
@@ -184,6 +184,16 @@ struct dev_pm_ops {
 	int (*restore_noirq)(struct device *dev);
 };
 
+#define SIMPLE_PM_OPS(name, suspend_fn, resume_fn) \
+struct dev_pm_ops name = { \
+	.suspend = suspend_fn, \
+	.resume = resume_fn, \
+	.freeze = suspend_fn, \
+	.thaw = resume_fn, \
+	.poweroff = suspend_fn, \
+	.resume = resume_fn, \
+}
+
 /**
  * PM_EVENT_ messages
  *

Signature · 4 lines

Albin Tonnerre, Free Electrons
Kernel, drivers and embedded Linux development,
consulting, training and support.
http://free-electrons.com

Dmitry Torokhov

2009-08-05 04:55 UTC #4 lore raw

Re: [PATCH] Add SIMPLE_PM_OPS: make switching to dev_pm_ops less error-prone

Quoted 6 lines · Albin Tonnerre

In a number of cases, the .suspend, .freeze, .poweroff and .resume, .thaw, .restore functions are identical. However, they all need to be assigned to avoid regressionsm as the previous code called .suspend resp. .resume in all those cases. SIMPLE_PM_OPS allows to deal with this case.

I'd much rather have conversions done with a bit more analysis now that our framework is more flexible and we can have specialized routines for hibernation and suspend.

Maybe we should try changing from run-time to build time warning so that users are not overly concerned with it?

Thanks.

Signature · 1 line

Dmitry

Dmitry Torokhov

2009-08-05 04:55 UTC #5 lore raw

Re: [PATCH] Add SIMPLE_PM_OPS: make switching to dev_pm_ops less error-prone

In reply to Albin Tonnerre

Quoted 6 lines · Albin Tonnerre

In a number of cases, the .suspend, .freeze, .poweroff and .resume, .thaw, .restore functions are identical. However, they all need to be assigned to avoid regressionsm as the previous code called .suspend resp. .resume in all those cases. SIMPLE_PM_OPS allows to deal with this case.

I'd much rather have conversions done with a bit more analysis now that our framework is more flexible and we can have specialized routines for hibernation and suspend.

Maybe we should try changing from run-time to build time warning so that users are not overly concerned with it?

Thanks.

Signature · 1 line

Dmitry

Albin Tonnerre

author

2009-08-05 09:37 UTC #6 lore raw

Re: [PATCH] Add SIMPLE_PM_OPS: make switching to dev_pm_ops less error-prone

Quoted 11 lines · Dmitry Torokhov

Quoted 5 lines · Albin Tonnerre
In a number of cases, the .suspend, .freeze, .poweroff and .resume, .thaw, .restore functions are identical. However, they all need to be assigned to avoid regressionsm as the previous code called .suspend resp. .resume in all those cases. SIMPLE_PM_OPS allows to deal with this case.
I'd much rather have conversions done with a bit more analysis now that our framework is more flexible and we can have specialized routines for hibernation and suspend.

I still think that even though they can, quite a number of drivers won't /need/ to have different functions for this, but maybe I'm mistaken.

Quoted 2 lines

Maybe we should try changing from run-time to build time warning so that users are not overly concerned with it?

I'm not sure that solves the problem. The fact is that even for developers, it's easy to overlook that assiging only the .suspend and .resume fields is probably a mistake.

Regards,

Signature · 4 lines

Albin Tonnerre, Free Electrons
Kernel, drivers and embedded Linux development,
consulting, training and support.
http://free-electrons.com

Rafael J. Wysocki

2009-08-05 18:46 UTC #7 lore raw

Re: [PATCH] Add SIMPLE_PM_OPS: make switching to dev_pm_ops less error-prone

Quoted 16 lines · Albin Tonnerre

Quoted 11 lines · Dmitry Torokhov
Quoted 5 lines · Albin Tonnerre
In a number of cases, the .suspend, .freeze, .poweroff and .resume, .thaw, .restore functions are identical. However, they all need to be assigned to avoid regressionsm as the previous code called .suspend resp. .resume in all those cases. SIMPLE_PM_OPS allows to deal with this case.
I'd much rather have conversions done with a bit more analysis now that our framework is more flexible and we can have specialized routines for hibernation and suspend.
I still think that even though they can, quite a number of drivers won't /need/ to have different functions for this, but maybe I'm mistaken.

Quoted 6 lines

Quoted 2 lines
Maybe we should try changing from run-time to build time warning so that users are not overly concerned with it?
I'm not sure that solves the problem. The fact is that even for developers, it's easy to overlook that assiging only the .suspend and .resume fields is probably a mistake.

I agree, so I'm going to take the patch.

I'll add a comment describing what the macro is for, though.

Thanks,
Rafael

Rafael J. Wysocki

2009-08-05 18:47 UTC #23 lore raw

Re: [PATCH] Add SIMPLE_PM_OPS: make switching to dev_pm_ops less error-prone

In reply to Albin Tonnerre

Quoted 16 lines · Albin Tonnerre

Quoted 11 lines · Dmitry Torokhov
Quoted 5 lines · Albin Tonnerre
In a number of cases, the .suspend, .freeze, .poweroff and .resume, .thaw, .restore functions are identical. However, they all need to be assigned to avoid regressionsm as the previous code called .suspend resp. .resume in all those cases. SIMPLE_PM_OPS allows to deal with this case.
I'd much rather have conversions done with a bit more analysis now that our framework is more flexible and we can have specialized routines for hibernation and suspend.
I still think that even though they can, quite a number of drivers won't /need/ to have different functions for this, but maybe I'm mistaken.

Quoted 6 lines

Quoted 2 lines
Maybe we should try changing from run-time to build time warning so that users are not overly concerned with it?
I'm not sure that solves the problem. The fact is that even for developers, it's easy to overlook that assiging only the .suspend and .resume fields is probably a mistake.

I agree, so I'm going to take the patch.

I'll add a comment describing what the macro is for, though.

Thanks,
Rafael

Rafael J. Wysocki

2009-08-05 20:04 UTC #8 lore raw

PATCH 1/1 PM: Add convenience macro to make switching to dev_pm_ops less error-prone

In reply to Rafael J. Wysocki

Quoted 20 lines · Rafael J. Wysocki

Quoted 19 lines · Albin Tonnerre
Quoted 11 lines · Dmitry Torokhov
Quoted 5 lines · Albin Tonnerre
In a number of cases, the .suspend, .freeze, .poweroff and .resume, .thaw, .restore functions are identical. However, they all need to be assigned to avoid regressionsm as the previous code called .suspend resp. .resume in all those cases. SIMPLE_PM_OPS allows to deal with this case.
I'd much rather have conversions done with a bit more analysis now that our framework is more flexible and we can have specialized routines for hibernation and suspend.
I still think that even though they can, quite a number of drivers won't /need/ to have different functions for this, but maybe I'm mistaken.
Quoted 2 lines
Maybe we should try changing from run-time to build time warning so that users are not overly concerned with it?

Quoted 7 lines

Quoted 3 lines
I'm not sure that solves the problem. The fact is that even for developers, it's easy to overlook that assiging only the .suspend and .resume fields is probably a mistake.
I agree, so I'm going to take the patch.
I'll add a comment describing what the macro is for, though.

Strictly speaking, I'm going to add the appended patch to the linux-next branch of the suspend-2.6 tree.

Thanks,
Rafael

---
From: Albin Tonnerre <[email protected]>
Subject: PM: Add convenience macro to make switching to dev_pm_ops less error-prone

[rjw: Changed the name of the macro and added the comment explaining its
purpose.]

Signed-off-by Albin Tonnerre <[email protected]>

Signed-off-by Rafael J. Wysocki <[email protected]>

---
 include/linux/pm.h |   14 ++++++++++++++
 1 file changed, 14 insertions(+)

include/linux/pm.h +14 -0

Index: linux-2.6/include/linux/pm.h
===================================================================
@@ -184,6 +184,20 @@ struct dev_pm_ops {
 	int (*restore_noirq)(struct device *dev);
 };
 
+/*
+ * Use this if you want to use the same suspend and resume callbacks for suspend
+ * to RAM and hibernation.
+ */
+#define SIMPLE_DEV_PM_OPS(name, suspend_fn, resume_fn) \
+struct dev_pm_ops name = { \
+	.suspend = suspend_fn, \
+	.resume = resume_fn, \
+	.freeze = suspend_fn, \
+	.thaw = resume_fn, \
+	.poweroff = suspend_fn, \
+	.resume = resume_fn, \
+}
+
 /**
  * PM_EVENT_ messages
  *

Rafael J. Wysocki

2009-08-05 20:05 UTC #22 lore raw

PATCH 1/1 PM: Add convenience macro to make switching to dev_pm_ops less error-prone

In reply to Rafael J. Wysocki

Quoted 20 lines · Rafael J. Wysocki

Quoted 19 lines · Albin Tonnerre
Quoted 11 lines · Dmitry Torokhov
Quoted 5 lines · Albin Tonnerre
In a number of cases, the .suspend, .freeze, .poweroff and .resume, .thaw, .restore functions are identical. However, they all need to be assigned to avoid regressionsm as the previous code called .suspend resp. .resume in all those cases. SIMPLE_PM_OPS allows to deal with this case.
I'd much rather have conversions done with a bit more analysis now that our framework is more flexible and we can have specialized routines for hibernation and suspend.
I still think that even though they can, quite a number of drivers won't /need/ to have different functions for this, but maybe I'm mistaken.
Quoted 2 lines
Maybe we should try changing from run-time to build time warning so that users are not overly concerned with it?

Quoted 7 lines

Quoted 3 lines
I'm not sure that solves the problem. The fact is that even for developers, it's easy to overlook that assiging only the .suspend and .resume fields is probably a mistake.
I agree, so I'm going to take the patch.
I'll add a comment describing what the macro is for, though.

Strictly speaking, I'm going to add the appended patch to the linux-next branch of the suspend-2.6 tree.

Thanks,
Rafael

---
From: Albin Tonnerre <[email protected]>
Subject: PM: Add convenience macro to make switching to dev_pm_ops less error-prone

[rjw: Changed the name of the macro and added the comment explaining its
purpose.]

Signed-off-by Albin Tonnerre <[email protected]>

Signed-off-by Rafael J. Wysocki <[email protected]>

---
 include/linux/pm.h |   14 ++++++++++++++
 1 file changed, 14 insertions(+)

include/linux/pm.h +14 -0

Index: linux-2.6/include/linux/pm.h
===================================================================
@@ -184,6 +184,20 @@ struct dev_pm_ops {
 	int (*restore_noirq)(struct device *dev);
 };
 
+/*
+ * Use this if you want to use the same suspend and resume callbacks for suspend
+ * to RAM and hibernation.
+ */
+#define SIMPLE_DEV_PM_OPS(name, suspend_fn, resume_fn) \
+struct dev_pm_ops name = { \
+	.suspend = suspend_fn, \
+	.resume = resume_fn, \
+	.freeze = suspend_fn, \
+	.thaw = resume_fn, \
+	.poweroff = suspend_fn, \
+	.resume = resume_fn, \
+}
+
 /**
  * PM_EVENT_ messages
  *

pHilipp Zabel

2009-08-05 20:17 UTC #9 lore raw

Re: [PATCH] PM: Add convenience macro to make switching to dev_pm_ops less error-prone

In reply to Rafael J. Wysocki

Quoted 64 lines · Rafael J. Wysocki

Quoted 20 lines · Rafael J. Wysocki
Quoted 19 lines · Albin Tonnerre
Quoted 11 lines · Dmitry Torokhov
Quoted 5 lines · Albin Tonnerre
In a number of cases, the .suspend, .freeze, .poweroff and .resume, .thaw, .restore functions are identical. However, they all need to be assigned to avoid regressionsm as the previous code called .suspend resp. .resume in all those cases. SIMPLE_PM_OPS allows to deal with this case.
I'd much rather have conversions done with a bit more analysis now that our framework is more flexible and we can have specialized routines for hibernation and suspend.
I still think that even though they can, quite a number of drivers won't /need/ to have different functions for this, but maybe I'm mistaken.
Quoted 2 lines
Maybe we should try changing from run-time to build time warning so that users are not overly concerned with it?
Quoted 7 lines
Quoted 3 lines
I'm not sure that solves the problem. The fact is that even for developers, it's easy to overlook that assiging only the .suspend and .resume fields is probably a mistake.
I agree, so I'm going to take the patch.
I'll add a comment describing what the macro is for, though.
Strictly speaking, I'm going to add the appended patch to the linux-next branch of the suspend-2.6 tree.
Thanks,
Rafael
---
From: Albin Tonnerre <[email protected]>
Subject: PM: Add convenience macro to make switching to dev_pm_ops less error-prone
In a number of cases, the .suspend, .freeze, .poweroff and .resume, .thaw, .restore functions are identical. However, they all need to be assigned to avoid regressionsm as the previous code called .suspend resp. .resume in all those cases. SIMPLE_DEV_PM_OPS helps to deal with this case.
[rjw: Changed the name of the macro and added the comment explaining its purpose.]
Signed-off-by Albin Tonnerre <[email protected]>
Signed-off-by Rafael J. Wysocki <[email protected]>
---
include/linux/pm.h | 14 ++++++++++++++
 1 file changed, 14 insertions(+)
Index: linux-2.6/include/linux/pm.h
===================================================================
include/linux/pm.h +2 -0
@@ -184,6 +184,20 @@ struct dev_pm_ops {
        int (*restore_noirq)(struct device *dev);
  };
 
+/*
+ * Use this if you want to use the same suspend and resume callbacks for suspend

include/linux/pm.h struct dev_pm_ops { · line 198

Quoted 10 lines

include/linux/pm.h +10 -0

+ * to RAM and hibernation.
+ */
+#define SIMPLE_DEV_PM_OPS(name, suspend_fn, resume_fn) \
+struct dev_pm_ops name = { \
+       .suspend = suspend_fn, \
+       .resume = resume_fn, \
+       .freeze = suspend_fn, \
+       .thaw = resume_fn, \
+       .poweroff = suspend_fn, \
+       .resume = resume_fn, \

I think that second .resume was intended to be .restore instead.

regards
Philipp

pHilipp Zabel

2009-08-05 20:17 UTC #10 lore raw

Re: [PATCH] PM: Add convenience macro to make switching to dev_pm_ops less error-prone

In reply to Rafael J. Wysocki

Quoted 64 lines · Rafael J. Wysocki

Quoted 20 lines · Rafael J. Wysocki
Quoted 19 lines · Albin Tonnerre
Quoted 11 lines · Dmitry Torokhov
Quoted 5 lines · Albin Tonnerre
In a number of cases, the .suspend, .freeze, .poweroff and .resume, .thaw, .restore functions are identical. However, they all need to be assigned to avoid regressionsm as the previous code called .suspend resp. .resume in all those cases. SIMPLE_PM_OPS allows to deal with this case.
I'd much rather have conversions done with a bit more analysis now that our framework is more flexible and we can have specialized routines for hibernation and suspend.
I still think that even though they can, quite a number of drivers won't /need/ to have different functions for this, but maybe I'm mistaken.
Quoted 2 lines
Maybe we should try changing from run-time to build time warning so that users are not overly concerned with it?
Quoted 7 lines
Quoted 3 lines
I'm not sure that solves the problem. The fact is that even for developers, it's easy to overlook that assiging only the .suspend and .resume fields is probably a mistake.
I agree, so I'm going to take the patch.
I'll add a comment describing what the macro is for, though.
Strictly speaking, I'm going to add the appended patch to the linux-next branch of the suspend-2.6 tree.
Thanks,
Rafael
---
From: Albin Tonnerre <[email protected]>
Subject: PM: Add convenience macro to make switching to dev_pm_ops less error-prone
In a number of cases, the .suspend, .freeze, .poweroff and .resume, .thaw, .restore functions are identical. However, they all need to be assigned to avoid regressionsm as the previous code called .suspend resp. .resume in all those cases. SIMPLE_DEV_PM_OPS helps to deal with this case.
[rjw: Changed the name of the macro and added the comment explaining its purpose.]
Signed-off-by Albin Tonnerre <[email protected]>
Signed-off-by Rafael J. Wysocki <[email protected]>
---
include/linux/pm.h | 14 ++++++++++++++
 1 file changed, 14 insertions(+)
Index: linux-2.6/include/linux/pm.h
===================================================================
include/linux/pm.h +2 -0
@@ -184,6 +184,20 @@ struct dev_pm_ops {
        int (*restore_noirq)(struct device *dev);
  };
 
+/*
+ * Use this if you want to use the same suspend and resume callbacks for suspend

include/linux/pm.h struct dev_pm_ops { · line 198

Quoted 10 lines

include/linux/pm.h +10 -0

+ * to RAM and hibernation.
+ */
+#define SIMPLE_DEV_PM_OPS(name, suspend_fn, resume_fn) \
+struct dev_pm_ops name = { \
+       .suspend = suspend_fn, \
+       .resume = resume_fn, \
+       .freeze = suspend_fn, \
+       .thaw = resume_fn, \
+       .poweroff = suspend_fn, \
+       .resume = resume_fn, \

I think that second .resume was intended to be .restore instead.

regards
Philipp

Frans Pop

2009-08-05 20:22 UTC #11 lore raw

Re: [PATCH] PM: Add convenience macro to make switching to dev_pm_ops less error-prone

In reply to Rafael J. Wysocki

include/linux/pm.h struct dev_pm_ops { · line 198

Quoted 8 lines · Rafael J. Wysocki

include/linux/pm.h +8 -0

+#define SIMPLE_DEV_PM_OPS(name, suspend_fn, resume_fn) \
+struct dev_pm_ops name = { \
+       .suspend = suspend_fn, \
+       .resume = resume_fn, \
+       .freeze = suspend_fn, \
+       .thaw = resume_fn, \
+       .poweroff = suspend_fn, \
+       .resume = resume_fn, \

That defines .resume twice. Guess the last should be restore.

Cheers,
FJP

Frans Pop

2009-08-05 20:22 UTC #12 lore raw

Re: [PATCH] PM: Add convenience macro to make switching to dev_pm_ops less error-prone

In reply to Rafael J. Wysocki

include/linux/pm.h struct dev_pm_ops { · line 198

Quoted 8 lines · Rafael J. Wysocki

include/linux/pm.h +8 -0

+#define SIMPLE_DEV_PM_OPS(name, suspend_fn, resume_fn) \
+struct dev_pm_ops name = { \
+       .suspend = suspend_fn, \
+       .resume = resume_fn, \
+       .freeze = suspend_fn, \
+       .thaw = resume_fn, \
+       .poweroff = suspend_fn, \
+       .resume = resume_fn, \

That defines .resume twice. Guess the last should be restore.

Cheers,
FJP

Rafael J. Wysocki

2009-08-05 21:29 UTC #13 lore raw

Re: [PATCH] PM: Add convenience macro to make switching to dev_pm_ops less error-prone

Quoted 12 lines · Frans Pop

include/linux/pm.h struct dev_pm_ops { · line 198
Quoted 8 lines · Rafael J. Wysocki
include/linux/pm.h +8 -0
+#define SIMPLE_DEV_PM_OPS(name, suspend_fn, resume_fn) \
+struct dev_pm_ops name = { \
+       .suspend = suspend_fn, \
+       .resume = resume_fn, \
+       .freeze = suspend_fn, \
+       .thaw = resume_fn, \
+       .poweroff = suspend_fn, \
+       .resume = resume_fn, \
That defines .resume twice. Guess the last should be restore.

Yes, thanks to you and Philipp for noticing that and sorry for the mistake.

Rafael

Rafael J. Wysocki

2009-08-05 21:30 UTC #21 lore raw

Re: [PATCH] PM: Add convenience macro to make switching to dev_pm_ops less error-prone

In reply to Frans Pop

Quoted 12 lines · Frans Pop

include/linux/pm.h struct dev_pm_ops { · line 198
Quoted 8 lines · Rafael J. Wysocki
include/linux/pm.h +8 -0
+#define SIMPLE_DEV_PM_OPS(name, suspend_fn, resume_fn) \
+struct dev_pm_ops name = { \
+       .suspend = suspend_fn, \
+       .resume = resume_fn, \
+       .freeze = suspend_fn, \
+       .thaw = resume_fn, \
+       .poweroff = suspend_fn, \
+       .resume = resume_fn, \
That defines .resume twice. Guess the last should be restore.

Yes, thanks to you and Philipp for noticing that and sorry for the mistake.

Rafael

Daniel Mack

2009-08-06 08:51 UTC #14 lore raw

Re: [PATCH] PM: Add convenience macro to make switching to dev_pm_ops less error-prone

In reply to Rafael J. Wysocki

Quoted 14 lines · Rafael J. Wysocki

Quoted 12 lines · Frans Pop
include/linux/pm.h struct dev_pm_ops { · line 198
Quoted 8 lines · Rafael J. Wysocki
include/linux/pm.h +8 -0
+#define SIMPLE_DEV_PM_OPS(name, suspend_fn, resume_fn) \
+struct dev_pm_ops name = { \
+       .suspend = suspend_fn, \
+       .resume = resume_fn, \
+       .freeze = suspend_fn, \
+       .thaw = resume_fn, \
+       .poweroff = suspend_fn, \
+       .resume = resume_fn, \
That defines .resume twice. Guess the last should be restore.

Quoted 1 line

Yes, thanks to you and Philipp for noticing that and sorry for the mistake.

When is a good point to resend patches that use this macro? I guess they won't make it to .31 anyway, right? So I just wait for the next merge window?

Daniel

Daniel Mack

2009-08-06 08:51 UTC #15 lore raw

Re: [PATCH] PM: Add convenience macro to make switching to dev_pm_ops less error-prone

In reply to Rafael J. Wysocki

Quoted 14 lines · Rafael J. Wysocki

Quoted 12 lines · Frans Pop
include/linux/pm.h struct dev_pm_ops { · line 198
Quoted 8 lines · Rafael J. Wysocki
include/linux/pm.h +8 -0
+#define SIMPLE_DEV_PM_OPS(name, suspend_fn, resume_fn) \
+struct dev_pm_ops name = { \
+       .suspend = suspend_fn, \
+       .resume = resume_fn, \
+       .freeze = suspend_fn, \
+       .thaw = resume_fn, \
+       .poweroff = suspend_fn, \
+       .resume = resume_fn, \
That defines .resume twice. Guess the last should be restore.

Quoted 1 line

Yes, thanks to you and Philipp for noticing that and sorry for the mistake.

When is a good point to resend patches that use this macro? I guess they won't make it to .31 anyway, right? So I just wait for the next merge window?

Daniel

Rafael J. Wysocki

2009-08-06 12:16 UTC #16 lore raw

Re: [PATCH] PM: Add convenience macro to make switching to dev_pm_ops less error-prone

Quoted 15 lines · Daniel Mack

Quoted 14 lines · Rafael J. Wysocki
Quoted 12 lines · Frans Pop
include/linux/pm.h struct dev_pm_ops { · line 198
Quoted 8 lines · Rafael J. Wysocki
include/linux/pm.h +8 -0
+#define SIMPLE_DEV_PM_OPS(name, suspend_fn, resume_fn) \
+struct dev_pm_ops name = { \
+       .suspend = suspend_fn, \
+       .resume = resume_fn, \
+       .freeze = suspend_fn, \
+       .thaw = resume_fn, \
+       .poweroff = suspend_fn, \
+       .resume = resume_fn, \
That defines .resume twice. Guess the last should be restore.

Quoted 5 lines

Quoted 1 line
Yes, thanks to you and Philipp for noticing that and sorry for the mistake.
When is a good point to resend patches that use this macro? I guess they won't make it to .31 anyway, right? So I just wait for the next merge window?

Yes, I think that would be the right thing to do.

Best,
Rafael

Rafael J. Wysocki

2009-08-06 12:16 UTC #20 lore raw

Re: [PATCH] PM: Add convenience macro to make switching to dev_pm_ops less error-prone

In reply to Daniel Mack

Quoted 15 lines · Daniel Mack

Quoted 14 lines · Rafael J. Wysocki
Quoted 12 lines · Frans Pop
include/linux/pm.h struct dev_pm_ops { · line 198
Quoted 8 lines · Rafael J. Wysocki
include/linux/pm.h +8 -0
+#define SIMPLE_DEV_PM_OPS(name, suspend_fn, resume_fn) \
+struct dev_pm_ops name = { \
+       .suspend = suspend_fn, \
+       .resume = resume_fn, \
+       .freeze = suspend_fn, \
+       .thaw = resume_fn, \
+       .poweroff = suspend_fn, \
+       .resume = resume_fn, \
That defines .resume twice. Guess the last should be restore.

Quoted 5 lines

Quoted 1 line
Yes, thanks to you and Philipp for noticing that and sorry for the mistake.
When is a good point to resend patches that use this macro? I guess they won't make it to .31 anyway, right? So I just wait for the next merge window?

Yes, I think that would be the right thing to do.

Best,
Rafael

Magnus Damm

2009-08-06 13:10 UTC #17 lore raw

Re: [PATCH] PM: Add convenience macro to make switching to dev_pm_ops less error-prone

In reply to Rafael J. Wysocki

Quoted 6 lines · Rafael J. Wysocki

Quoted 3 lines · Daniel Mack
When is a good point to resend patches that use this macro? I guess they won't make it to .31 anyway, right? So I just wait for the next merge window?
Yes, I think that would be the right thing to do.

Is it a good idea to disable the warning before the final 2.6.31 release?

Cheers,

/ magnus

Rafael J. Wysocki

2009-08-06 15:14 UTC #18 lore raw

Re: [PATCH] PM: Add convenience macro to make switching to dev_pm_ops less error-prone

Quoted 9 lines · Magnus Damm

Quoted 6 lines · Rafael J. Wysocki
Quoted 3 lines · Daniel Mack
When is a good point to resend patches that use this macro? I guess they won't make it to .31 anyway, right? So I just wait for the next merge window?
Yes, I think that would be the right thing to do.
Is it a good idea to disable the warning before the final 2.6.31 release?

I guess so. Let's disable it.

Thanks,
Rafael

Rafael J. Wysocki

2009-08-06 15:14 UTC #19 lore raw

Re: [PATCH] PM: Add convenience macro to make switching to dev_pm_ops less error-prone

In reply to Magnus Damm

Quoted 9 lines · Magnus Damm

Quoted 6 lines · Rafael J. Wysocki
Quoted 3 lines · Daniel Mack
When is a good point to resend patches that use this macro? I guess they won't make it to .31 anyway, right? So I just wait for the next merge window?
Yes, I think that would be the right thing to do.
Is it a good idea to disable the warning before the final 2.6.31 release?

I guess so. Let's disable it.

Thanks,
Rafael