Re: [PATCH 1/3] PM / Runtime: Add notifiers for device runtime PM events
Hi Rafael, On 2016-06-08 19:18, Rafael J. Wysocki wrote: On Wed, Jun 8, 2016 at 12:25 PM, Marek Szyprowski wrote: From: Krzysztof Kozlowski Allow drivers registering for certain runtime PM events of other devices. Some drivers in power domain are more or less coupled. When one driver is suspending (thus leading to power domain being also turned off) the other might have to perform some necessary steps. For example Exynos IOMMU has to save its context. Based on previous work of Sylwester Nawrocki. Signed-off-by: Krzysztof Kozlowski Signed-off-by: Marek Szyprowski No, this is not the right way to address this and using notifiers for that is just wrong (because of the potential ordering issues). Also, the problem is not limited to runtime PM, but also to system suspend/resume and initialization/shutdown. I posted a series of device dependencies patches a few months ago that might help to address this problem, but there was almost no interest in it at that time. I spent some time digging for your patches. Sadly no list archives had them all and I finally found them only in the linux-pm patchwork. This may explain why there was almost no interest in them. After some debugging I've managed to get it working for my case. I will include your patches in my v2 patchset together with the fixes needed to get it working. Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/3] PM / Runtime: Add notifiers for device runtime PM events
On Wed, Jun 8, 2016 at 12:25 PM, Marek Szyprowski
wrote:
> From: Krzysztof Kozlowski
>
> Allow drivers registering for certain runtime PM events of other
> devices. Some drivers in power domain are more or less coupled. When one
> driver is suspending (thus leading to power domain being also turned
> off) the other might have to perform some necessary steps. For example
> Exynos IOMMU has to save its context.
>
> Based on previous work of Sylwester Nawrocki .
>
> Signed-off-by: Krzysztof Kozlowski
> Signed-off-by: Marek Szyprowski
No, this is not the right way to address this and using notifiers for
that is just wrong (because of the potential ordering issues).
Also, the problem is not limited to runtime PM, but also to system
suspend/resume and initialization/shutdown.
I posted a series of device dependencies patches a few months ago that
might help to address this problem, but there was almost no interest
in it at that time.
Thanks,
Rafael
> ---
> drivers/base/power/generic_ops.c | 9 +++
> drivers/base/power/power.h | 6 +
> drivers/base/power/runtime.c | 34 +--
> include/linux/pm.h | 2 ++
> include/linux/pm_runtime.h | 51
>
> 5 files changed, 100 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/base/power/generic_ops.c
> b/drivers/base/power/generic_ops.c
> index 07c3c4a9522d..f0838229b781 100644
> --- a/drivers/base/power/generic_ops.c
> +++ b/drivers/base/power/generic_ops.c
> @@ -10,6 +10,7 @@
> #include
> #include
> #include
> +#include "power.h"
>
> #ifdef CONFIG_PM
> /**
> @@ -25,8 +26,12 @@ int pm_generic_runtime_suspend(struct device *dev)
> const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
> int ret;
>
> + pm_runtime_notifier_call(dev, RPM_EVENT_SUSPEND_PRE);
> +
> ret = pm && pm->runtime_suspend ? pm->runtime_suspend(dev) : 0;
>
> + pm_runtime_notifier_call(dev, RPM_EVENT_SUSPEND_POST);
> +
> return ret;
> }
> EXPORT_SYMBOL_GPL(pm_generic_runtime_suspend);
> @@ -44,8 +49,12 @@ int pm_generic_runtime_resume(struct device *dev)
> const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
> int ret;
>
> + pm_runtime_notifier_call(dev, RPM_EVENT_RESUME_PRE);
> +
> ret = pm && pm->runtime_resume ? pm->runtime_resume(dev) : 0;
>
> + pm_runtime_notifier_call(dev, RPM_EVENT_RESUME_POST);
> +
> return ret;
> }
> EXPORT_SYMBOL_GPL(pm_generic_runtime_resume);
> diff --git a/drivers/base/power/power.h b/drivers/base/power/power.h
> index 50e30e7b059d..30b6319ce96c 100644
> --- a/drivers/base/power/power.h
> +++ b/drivers/base/power/power.h
> @@ -1,4 +1,5 @@
> #include
> +#include
>
> static inline void device_pm_init_common(struct device *dev)
> {
> @@ -20,6 +21,7 @@ static inline void pm_runtime_early_init(struct device *dev)
> extern void pm_runtime_init(struct device *dev);
> extern void pm_runtime_reinit(struct device *dev);
> extern void pm_runtime_remove(struct device *dev);
> +extern int pm_runtime_notifier_call(struct device *dev, enum rpm_event
> event);
>
> struct wake_irq {
> struct device *dev;
> @@ -87,6 +89,10 @@ static inline void pm_runtime_early_init(struct device
> *dev)
> static inline void pm_runtime_init(struct device *dev) {}
> static inline void pm_runtime_reinit(struct device *dev) {}
> static inline void pm_runtime_remove(struct device *dev) {}
> +static inline pm_runtime_notifier_call(struct device *dev, enum rpm_event
> event)
> +{
> + return 0;
> +}
>
> static inline int dpm_sysfs_add(struct device *dev) { return 0; }
> static inline void dpm_sysfs_remove(struct device *dev) {}
> diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
> index b74690418504..3a5637ca8400 100644
> --- a/drivers/base/power/runtime.c
> +++ b/drivers/base/power/runtime.c
> @@ -227,6 +227,27 @@ void pm_runtime_set_memalloc_noio(struct device *dev,
> bool enable)
> }
> EXPORT_SYMBOL_GPL(pm_runtime_set_memalloc_noio);
>
> +int pm_runtime_notifier_call(struct device *dev, enum rpm_event event)
> +{
> + return atomic_notifier_call_chain(&dev->power.runtime_notifier,
> + event, dev);
> +}
> +
> +int pm_runtime_register_notifier(struct device *dev, struct notifier_block
> *nb)
> +{
> + return atomic_notifier_chain_register(&dev->power.runtime_notifier,
> + nb);
> +}
> +EXPORT_SYMBOL_GPL(pm_runtime_register_notifier);
> +
> +int pm_runtime_unregister_notifier(struct device *dev,
> + struct notifier_block *nb)
> +{
> + return atomic_notifier_chain_unregister(&dev->power.runtime_notifier,
> + nb);
> +}
> +EXPORT_SYMBOL_GPL(pm_runtime_unregister_notifier);
> +
> /**
> * rpm_check_suspend_allowed - Test whether a device may be s
[PATCH 1/3] PM / Runtime: Add notifiers for device runtime PM events
From: Krzysztof Kozlowski
Allow drivers registering for certain runtime PM events of other
devices. Some drivers in power domain are more or less coupled. When one
driver is suspending (thus leading to power domain being also turned
off) the other might have to perform some necessary steps. For example
Exynos IOMMU has to save its context.
Based on previous work of Sylwester Nawrocki .
Signed-off-by: Krzysztof Kozlowski
Signed-off-by: Marek Szyprowski
---
drivers/base/power/generic_ops.c | 9 +++
drivers/base/power/power.h | 6 +
drivers/base/power/runtime.c | 34 +--
include/linux/pm.h | 2 ++
include/linux/pm_runtime.h | 51
5 files changed, 100 insertions(+), 2 deletions(-)
diff --git a/drivers/base/power/generic_ops.c b/drivers/base/power/generic_ops.c
index 07c3c4a9522d..f0838229b781 100644
--- a/drivers/base/power/generic_ops.c
+++ b/drivers/base/power/generic_ops.c
@@ -10,6 +10,7 @@
#include
#include
#include
+#include "power.h"
#ifdef CONFIG_PM
/**
@@ -25,8 +26,12 @@ int pm_generic_runtime_suspend(struct device *dev)
const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
int ret;
+ pm_runtime_notifier_call(dev, RPM_EVENT_SUSPEND_PRE);
+
ret = pm && pm->runtime_suspend ? pm->runtime_suspend(dev) : 0;
+ pm_runtime_notifier_call(dev, RPM_EVENT_SUSPEND_POST);
+
return ret;
}
EXPORT_SYMBOL_GPL(pm_generic_runtime_suspend);
@@ -44,8 +49,12 @@ int pm_generic_runtime_resume(struct device *dev)
const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
int ret;
+ pm_runtime_notifier_call(dev, RPM_EVENT_RESUME_PRE);
+
ret = pm && pm->runtime_resume ? pm->runtime_resume(dev) : 0;
+ pm_runtime_notifier_call(dev, RPM_EVENT_RESUME_POST);
+
return ret;
}
EXPORT_SYMBOL_GPL(pm_generic_runtime_resume);
diff --git a/drivers/base/power/power.h b/drivers/base/power/power.h
index 50e30e7b059d..30b6319ce96c 100644
--- a/drivers/base/power/power.h
+++ b/drivers/base/power/power.h
@@ -1,4 +1,5 @@
#include
+#include
static inline void device_pm_init_common(struct device *dev)
{
@@ -20,6 +21,7 @@ static inline void pm_runtime_early_init(struct device *dev)
extern void pm_runtime_init(struct device *dev);
extern void pm_runtime_reinit(struct device *dev);
extern void pm_runtime_remove(struct device *dev);
+extern int pm_runtime_notifier_call(struct device *dev, enum rpm_event event);
struct wake_irq {
struct device *dev;
@@ -87,6 +89,10 @@ static inline void pm_runtime_early_init(struct device *dev)
static inline void pm_runtime_init(struct device *dev) {}
static inline void pm_runtime_reinit(struct device *dev) {}
static inline void pm_runtime_remove(struct device *dev) {}
+static inline pm_runtime_notifier_call(struct device *dev, enum rpm_event
event)
+{
+ return 0;
+}
static inline int dpm_sysfs_add(struct device *dev) { return 0; }
static inline void dpm_sysfs_remove(struct device *dev) {}
diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
index b74690418504..3a5637ca8400 100644
--- a/drivers/base/power/runtime.c
+++ b/drivers/base/power/runtime.c
@@ -227,6 +227,27 @@ void pm_runtime_set_memalloc_noio(struct device *dev, bool
enable)
}
EXPORT_SYMBOL_GPL(pm_runtime_set_memalloc_noio);
+int pm_runtime_notifier_call(struct device *dev, enum rpm_event event)
+{
+ return atomic_notifier_call_chain(&dev->power.runtime_notifier,
+ event, dev);
+}
+
+int pm_runtime_register_notifier(struct device *dev, struct notifier_block *nb)
+{
+ return atomic_notifier_chain_register(&dev->power.runtime_notifier,
+ nb);
+}
+EXPORT_SYMBOL_GPL(pm_runtime_register_notifier);
+
+int pm_runtime_unregister_notifier(struct device *dev,
+ struct notifier_block *nb)
+{
+ return atomic_notifier_chain_unregister(&dev->power.runtime_notifier,
+ nb);
+}
+EXPORT_SYMBOL_GPL(pm_runtime_unregister_notifier);
+
/**
* rpm_check_suspend_allowed - Test whether a device may be suspended.
* @dev: Device to test.
@@ -1174,6 +1195,7 @@ void __pm_runtime_disable(struct device *dev, bool
check_resume)
goto out;
}
+ pm_runtime_notifier_call(dev, RPM_EVENT_DISABLE_PRE);
/*
* Wake up the device if there's a resume request pending, because that
* means there probably is some I/O to process and disabling runtime PM
@@ -1195,6 +1217,7 @@ void __pm_runtime_disable(struct device *dev, bool
check_resume)
if (!dev->power.disable_depth++)
__pm_runtime_barrier(dev);
+ pm_runtime_notifier_call(dev, RPM_EVENT_DISABLE_POST);
out:
spin_unlock_irq(&dev->power.lock);
}
@@ -1210,10 +1233,16 @@ void pm
