Re: [PATCH] of/platform: Implement support for dev_pm_ops
On Tue, 2009-10-13 at 02:44 +0400, Anton Vorontsov wrote: I agree that there is some room for improvements in general (e.g. merging platform and of_platform devices/drivers), but it's not as easy as you would like to think. Let's make it in a separate step that don't stop real features from being implemented (e.g. hibernate). For the six functions that we can reuse I can prepare a cleanup patch that we can merge via -mm, or it can just sit and collect needed acks and can be merged via any tree. But please, no cross-tree dependencies for the cruicial features. I agree. I'll take the patch for now. In the long run, I'm all for killing of_platform if we can find a proper way to replace it with platform. IE. With dev_archdata, any device carries the of device node, so of_platform doesn't really buy us much anymore. We could even default by populating platform device resources with standard-parsing of reg properties etc... So for devices who don't actually need anything more, we may get away re-using platform devices as-is, all we would need is some kind of conversion table or such to map OF match to platform dev names, or maybe a secondary match table in the drivers themselves. Anyway, that's an old discussion, something we still need to sort out... Ben. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH] of/platform: Implement support for dev_pm_ops
Linux power management subsystem supports vast amount of new PM
callbacks that are crucial for proper suspend and hibernation support
in drivers.
This patch implements support for dev_pm_ops, preserving support
for legacy callbacks.
Signed-off-by: Anton Vorontsov avoront...@ru.mvista.com
---
drivers/of/platform.c | 305 ++---
1 files changed, 290 insertions(+), 15 deletions(-)
diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index 298de0f..d58ade1 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -65,47 +65,322 @@ static int of_platform_device_remove(struct device *dev)
return 0;
}
-static int of_platform_device_suspend(struct device *dev, pm_message_t state)
+static void of_platform_device_shutdown(struct device *dev)
{
struct of_device *of_dev = to_of_device(dev);
struct of_platform_driver *drv = to_of_platform_driver(dev-driver);
- int error = 0;
- if (dev-driver drv-suspend)
- error = drv-suspend(of_dev, state);
- return error;
+ if (dev-driver drv-shutdown)
+ drv-shutdown(of_dev);
}
-static int of_platform_device_resume(struct device * dev)
+#ifdef CONFIG_PM_SLEEP
+
+static int of_platform_legacy_suspend(struct device *dev, pm_message_t mesg)
{
struct of_device *of_dev = to_of_device(dev);
struct of_platform_driver *drv = to_of_platform_driver(dev-driver);
- int error = 0;
+ int ret = 0;
- if (dev-driver drv-resume)
- error = drv-resume(of_dev);
- return error;
+ if (dev-driver drv-suspend)
+ ret = drv-suspend(of_dev, mesg);
+ return ret;
}
-static void of_platform_device_shutdown(struct device *dev)
+static int of_platform_legacy_resume(struct device *dev)
{
struct of_device *of_dev = to_of_device(dev);
struct of_platform_driver *drv = to_of_platform_driver(dev-driver);
+ int ret = 0;
- if (dev-driver drv-shutdown)
- drv-shutdown(of_dev);
+ if (dev-driver drv-resume)
+ ret = drv-resume(of_dev);
+ return ret;
+}
+
+static int of_platform_pm_prepare(struct device *dev)
+{
+ struct device_driver *drv = dev-driver;
+ int ret = 0;
+
+ if (drv drv-pm drv-pm-prepare)
+ ret = drv-pm-prepare(dev);
+
+ return ret;
+}
+
+static void of_platform_pm_complete(struct device *dev)
+{
+ struct device_driver *drv = dev-driver;
+
+ if (drv drv-pm drv-pm-complete)
+ drv-pm-complete(dev);
+}
+
+#ifdef CONFIG_SUSPEND
+
+static int of_platform_pm_suspend(struct device *dev)
+{
+ struct device_driver *drv = dev-driver;
+ int ret = 0;
+
+ if (!drv)
+ return 0;
+
+ if (drv-pm) {
+ if (drv-pm-suspend)
+ ret = drv-pm-suspend(dev);
+ } else {
+ ret = of_platform_legacy_suspend(dev, PMSG_SUSPEND);
+ }
+
+ return ret;
}
+static int of_platform_pm_suspend_noirq(struct device *dev)
+{
+ struct device_driver *drv = dev-driver;
+ int ret = 0;
+
+ if (!drv)
+ return 0;
+
+ if (drv-pm) {
+ if (drv-pm-suspend_noirq)
+ ret = drv-pm-suspend_noirq(dev);
+ }
+
+ return ret;
+}
+
+static int of_platform_pm_resume(struct device *dev)
+{
+ struct device_driver *drv = dev-driver;
+ int ret = 0;
+
+ if (!drv)
+ return 0;
+
+ if (drv-pm) {
+ if (drv-pm-resume)
+ ret = drv-pm-resume(dev);
+ } else {
+ ret = of_platform_legacy_resume(dev);
+ }
+
+ return ret;
+}
+
+static int of_platform_pm_resume_noirq(struct device *dev)
+{
+ struct device_driver *drv = dev-driver;
+ int ret = 0;
+
+ if (!drv)
+ return 0;
+
+ if (drv-pm) {
+ if (drv-pm-resume_noirq)
+ ret = drv-pm-resume_noirq(dev);
+ }
+
+ return ret;
+}
+
+#else /* !CONFIG_SUSPEND */
+
+#define of_platform_pm_suspend NULL
+#define of_platform_pm_resume NULL
+#define of_platform_pm_suspend_noirq NULL
+#define of_platform_pm_resume_noirqNULL
+
+#endif /* !CONFIG_SUSPEND */
+
+#ifdef CONFIG_HIBERNATION
+
+static int of_platform_pm_freeze(struct device *dev)
+{
+ struct device_driver *drv = dev-driver;
+ int ret = 0;
+
+ if (!drv)
+ return 0;
+
+ if (drv-pm) {
+ if (drv-pm-freeze)
+ ret = drv-pm-freeze(dev);
+ } else {
+ ret = of_platform_legacy_suspend(dev, PMSG_FREEZE);
+ }
+
+ return ret;
+}
+
+static int of_platform_pm_freeze_noirq(struct device *dev)
+{
+ struct device_driver *drv = dev-driver;
+ int ret = 0;
+
+ if (!drv)
+ return 0;
+
+ if (drv-pm) {
+
Re: [PATCH] of/platform: Implement support for dev_pm_ops
On Mon, Oct 12, 2009 at 8:50 AM, Anton Vorontsov
avoront...@ru.mvista.com wrote:
Linux power management subsystem supports vast amount of new PM
callbacks that are crucial for proper suspend and hibernation support
in drivers.
This patch implements support for dev_pm_ops, preserving support
for legacy callbacks.
Signed-off-by: Anton Vorontsov avoront...@ru.mvista.com
Hmmm... I'm not very familiar with the PM callbacks, but change
doesn't look right to me. In particular, a lot of these new hooks
don't do anything remotely of_platform bus specific. For example,
of_platform_pm_prepare() checks if there is drv, drv-pm, and
drv-pm-prepare. If all are true, then it calls drv-pm-prepare().
I see that the platform bus platform_pm_prepare() function is
absolutely identical. I haven't looked, but I wouldn't be surprised
if other busses do the same.
I think these simple pm ops should be made library functions that
platform, of_platform and other simple busses can just populate their
pm ops structure with.
g.
---
drivers/of/platform.c | 305
++---
1 files changed, 290 insertions(+), 15 deletions(-)
diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index 298de0f..d58ade1 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -65,47 +65,322 @@ static int of_platform_device_remove(struct device *dev)
return 0;
}
-static int of_platform_device_suspend(struct device *dev, pm_message_t state)
+static void of_platform_device_shutdown(struct device *dev)
{
struct of_device *of_dev = to_of_device(dev);
struct of_platform_driver *drv = to_of_platform_driver(dev-driver);
- int error = 0;
- if (dev-driver drv-suspend)
- error = drv-suspend(of_dev, state);
- return error;
+ if (dev-driver drv-shutdown)
+ drv-shutdown(of_dev);
}
-static int of_platform_device_resume(struct device * dev)
+#ifdef CONFIG_PM_SLEEP
+
+static int of_platform_legacy_suspend(struct device *dev, pm_message_t mesg)
{
struct of_device *of_dev = to_of_device(dev);
struct of_platform_driver *drv = to_of_platform_driver(dev-driver);
- int error = 0;
+ int ret = 0;
- if (dev-driver drv-resume)
- error = drv-resume(of_dev);
- return error;
+ if (dev-driver drv-suspend)
+ ret = drv-suspend(of_dev, mesg);
+ return ret;
}
-static void of_platform_device_shutdown(struct device *dev)
+static int of_platform_legacy_resume(struct device *dev)
{
struct of_device *of_dev = to_of_device(dev);
struct of_platform_driver *drv = to_of_platform_driver(dev-driver);
+ int ret = 0;
- if (dev-driver drv-shutdown)
- drv-shutdown(of_dev);
+ if (dev-driver drv-resume)
+ ret = drv-resume(of_dev);
+ return ret;
+}
+
+static int of_platform_pm_prepare(struct device *dev)
+{
+ struct device_driver *drv = dev-driver;
+ int ret = 0;
+
+ if (drv drv-pm drv-pm-prepare)
+ ret = drv-pm-prepare(dev);
+
+ return ret;
+}
+
+static void of_platform_pm_complete(struct device *dev)
+{
+ struct device_driver *drv = dev-driver;
+
+ if (drv drv-pm drv-pm-complete)
+ drv-pm-complete(dev);
+}
+
+#ifdef CONFIG_SUSPEND
+
+static int of_platform_pm_suspend(struct device *dev)
+{
+ struct device_driver *drv = dev-driver;
+ int ret = 0;
+
+ if (!drv)
+ return 0;
+
+ if (drv-pm) {
+ if (drv-pm-suspend)
+ ret = drv-pm-suspend(dev);
+ } else {
+ ret = of_platform_legacy_suspend(dev, PMSG_SUSPEND);
+ }
+
+ return ret;
}
+static int of_platform_pm_suspend_noirq(struct device *dev)
+{
+ struct device_driver *drv = dev-driver;
+ int ret = 0;
+
+ if (!drv)
+ return 0;
+
+ if (drv-pm) {
+ if (drv-pm-suspend_noirq)
+ ret = drv-pm-suspend_noirq(dev);
+ }
+
+ return ret;
+}
+
+static int of_platform_pm_resume(struct device *dev)
+{
+ struct device_driver *drv = dev-driver;
+ int ret = 0;
+
+ if (!drv)
+ return 0;
+
+ if (drv-pm) {
+ if (drv-pm-resume)
+ ret = drv-pm-resume(dev);
+ } else {
+ ret = of_platform_legacy_resume(dev);
+ }
+
+ return ret;
+}
+
+static int of_platform_pm_resume_noirq(struct device *dev)
+{
+ struct device_driver *drv = dev-driver;
+ int ret = 0;
+
+ if (!drv)
+ return 0;
+
+ if (drv-pm) {
+ if (drv-pm-resume_noirq)
+ ret = drv-pm-resume_noirq(dev);
+ }
+
+ return ret;
+}
+
+#else /* !CONFIG_SUSPEND
Re: [PATCH] of/platform: Implement support for dev_pm_ops
On Mon, Oct 12, 2009 at 03:09:53PM -0700, Grant Likely wrote: On Mon, Oct 12, 2009 at 8:50 AM, Anton Vorontsov avoront...@ru.mvista.com wrote: Linux power management subsystem supports vast amount of new PM callbacks that are crucial for proper suspend and hibernation support in drivers. This patch implements support for dev_pm_ops, preserving support for legacy callbacks. Signed-off-by: Anton Vorontsov avoront...@ru.mvista.com Hmmm... I'm not very familiar with the PM callbacks, but change doesn't look right to me. In particular, a lot of these new hooks don't do anything remotely of_platform bus specific. For example, of_platform_pm_prepare() checks if there is drv, drv-pm, and drv-pm-prepare. If all are true, then it calls drv-pm-prepare(). I see that the platform bus platform_pm_prepare() function is absolutely identical. I haven't looked, but I wouldn't be surprised if other busses do the same. I think these simple pm ops should be made library functions that platform, of_platform and other simple busses can just populate their pm ops structure with. Some hooks can be made as library functions, but some can't (for example that do of_plaform_driver-suspend(), as opposite to of_platform_driver-driver.suspend(), i.e. the legacy hooks). Also, if you look into PCI bus hooks, you'll see that these hooks aren't pure proxies for drivers, they do real work, so they won't like to reuse or share anything. For OF platfrom bus, we can share these functions with platform: of_platform_pm_suspend_noirq of_platform_pm_resume_noirq of_platform_pm_freeze_noirq of_platform_pm_thaw_noirq of_platform_pm_poweroff_noirq of_platform_pm_restore_noirq These we cannot share: of_platform_pm_suspend of_platform_pm_resume of_platform_pm_freeze of_platform_pm_thaw of_platform_pm_poweroff of_platform_pm_restore I agree that there is some room for improvements in general (e.g. merging platform and of_platform devices/drivers), but it's not as easy as you would like to think. Let's make it in a separate step that don't stop real features from being implemented (e.g. hibernate). For the six functions that we can reuse I can prepare a cleanup patch that we can merge via -mm, or it can just sit and collect needed acks and can be merged via any tree. But please, no cross-tree dependencies for the cruicial features. Thanks, -- Anton Vorontsov email: cbouatmai...@gmail.com irc://irc.freenode.net/bd2 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
