Re: [PATCH 3/4] OMAP: PM: use omap_device API for suspend/resume
One correction here:
On Wed, 23 Jun 2010, Paul Walmsley wrote:
On Mon, 21 Jun 2010, Kevin Hilman wrote:
Just to clarify. The functions I'm overriding here is the bus
functions (bus-pm-[suspend|resume]_noirq(), not any driver functions
OK, I see that now - this code was confusing in the patch's
platform_pm_suspend_noirq():
+ if (drv-pm) {
+ if (drv-pm-suspend_noirq)
+ ret = drv-pm-suspend_noirq(dev);
+ }
This is already done by the DPM core in
drivers/base/power/main.c:device_suspend_noirq() and will result in
re-executing the driver's suspend_noirq function, which is potentially
quite bad. Same thing for platform_pm_resume_noirq() in the patch.
Sorry, misread this also. Indeed, something like this is necessary in
your platform bus override code - so please ignore this part of the
comments.
- Paul
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] OMAP: PM: use omap_device API for suspend/resume
Paul Walmsley p...@pwsan.com writes:
On Mon, 21 Jun 2010, Kevin Hilman wrote:
Paul Walmsley p...@pwsan.com writes:
I guess the intent of your patch is to avoid having to patch in
omap_device_{idle,enable}() into a bunch of driver DPM suspend/resume
callbacks?
Partially. Actually, I think many (most?) drivers can get rid of
static suspend/resume paths all together and just use runtime PM.
There are some cases though (MMC comes to mind, more on that below)
where static suspend has some extra steps necessary and behaves
differently than runtime PM.
It's not just MMC, any driver that can be in the middle of doing something
during DPM -suspend() will need to have DPM suspend code to stop what
it's doing and quiesce the device before returning from the suspend
callback.
I don't think we do a very good job of that today in most drivers, but
your point is valid.
Probably best to trick the runtime PM core by doing a runtime PM
put/get in the bus code as you suggest below to avoid this kind of
potential conflict.
Either that, or -suspend() needs to return an error and block
the system suspend process...
[...]
Right now the OMAP2+ codebase doesn't use any
shared interrupts for on-chip devices, as far as I can see. It
looks like you use -suspend_noirq() to ensure your code runs after
the existing driver suspend functions.
No. The primary reason for using _noirq() is that if any driver ever
did use the _noirq() hooks (for any atomic activity, or late wakeup
detection, etc.) the device will still be accessible.
Using -suspend_noirq() for this purpose seems like a hack.
A better place for that code would be in a bus-pm-suspend()
callback, which will run after the device's dev_pm_ops-suspend()
callback.
I originally put it in bus-pm-suspend, but moved it to
bus-pm-suspend_noirq() since I didn't do a full audit so see
if anything was using the _noirq hooks.
If we want to have a requirement that no on-chip devices can use the
_noirq hooks (or at least they can't touch hardware in those hooks)
then I can move it back to bus-pm-suspend().
That sounds like the best argument for keeping these hooks in
_noirq() -- some driver writer may be likely to use suspend_noirq()
without understanding that they shouldn't... maybe we should add some code
that looks for this and warns.
But thinking about it further, it also seems possible that a handful of
OMAP drivers might use shared IRQs at some point in the future. DSS comes
to mind -- that really is a shared IRQ. So, _noirq() is fine, then.
OK.
[...]
- It is not safe to call omap_device_{idle,enable}() from DPM
callbacks as the patch does, because they will race with runtime PM
calls to omap_device_{idle,enable}().
No, they cannot race.
Runtime PM transitions are prevented by the system PM core during a
system PM transition. The system suspend path does a pm_runtime_get()
for each device being suspended, and then does a _put() upon resume.
(see drivers/base/power/main.c, grep for pm_runtime_)
Yes, you are correct in terms of my original statement. But the problem
-- and the race -- that I did a poor job of describing is still present.
What if this pm_bus -suspend_noirq() calls omap_device_idle() while other
code in the driver is still in the middle of interacting with the device?
Although that would certainly be a driver bug, I certainly wouldn't trust
all of our existing driver DPM suspend* functions to adequately wait for
in-progress operations to complete and block new operations from starting
before returning.
Yes, I see the point now, and I agree that this is a problem.
We shouldn't idle (and potentially power off) a device unless we know its
driver is done with it. In an ideal world, this would always be taken
care of by driver -suspend()/-suspend_noirq() functions. But we know
this isn't always the case -- perhaps it's not even the case for most of
the OMAP drivers.
Yeah, I don't think we handle this very well currently in most drivers.
We can use the device's runtime PM state to figure out whether the driver
thinks it's done with the device. Unfortunately, the existing Linux DPM
suspend code makes it difficult to deal with this by calling
pm_runtime_get_noresume() before entering suspend and never calling
pm_runtime_put() until after resume -- no idea why.
I gathered it was intended to minimize potential conflicts between
system PM and runtime PM, but not sure. I may ask on linux-pm.
That strikes me as a bug. From a semantic perspective it is certainly
confusing: the PM runtime implementation will think the device is
still active after it's been suspended. I wouldn't want the full
Linux system suspend code to enter some low power state while some
driver still thought its device should stay powered.
Completely agree here, and this is the root cause of all this funny
business in the
Re: [PATCH 3/4] OMAP: PM: use omap_device API for suspend/resume
On Thu, 24 Jun 2010, Paul Walmsley wrote: Sorry, misread this also. Indeed, something like this is necessary in your platform bus override code - so please ignore this part of the comments. By the way, I owe you a more general apology, Kevin, that some of these comments have been erroneous and that you have had to spend time addressing the half-baked ones. They haven't measured up to my personal technical standards for comments... - Paul -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] OMAP: PM: use omap_device API for suspend/resume
Hi Kevin,
On Mon, 21 Jun 2010, Kevin Hilman wrote:
Paul Walmsley p...@pwsan.com writes:
I guess the intent of your patch is to avoid having to patch in
omap_device_{idle,enable}() into a bunch of driver DPM suspend/resume
callbacks?
Partially. Actually, I think many (most?) drivers can get rid of
static suspend/resume paths all together and just use runtime PM.
There are some cases though (MMC comes to mind, more on that below)
where static suspend has some extra steps necessary and behaves
differently than runtime PM.
It's not just MMC, any driver that can be in the middle of doing something
during DPM -suspend() will need to have DPM suspend code to stop what
it's doing and quiesce the device before returning from the suspend
callback. Either that, or -suspend() needs to return an error and block
the system suspend process...
If so, some comments:
- dev_pm_ops-suspend_noirq() is intended for use on devices with
shared interrupts.
Just to clarify. The functions I'm overriding here is the bus
functions (bus-pm-[suspend|resume]_noirq(), not any driver functions
OK, I see that now - this code was confusing in the patch's
platform_pm_suspend_noirq():
+ if (drv-pm) {
+ if (drv-pm-suspend_noirq)
+ ret = drv-pm-suspend_noirq(dev);
+ }
This is already done by the DPM core in
drivers/base/power/main.c:device_suspend_noirq() and will result in
re-executing the driver's suspend_noirq function, which is potentially
quite bad. Same thing for platform_pm_resume_noirq() in the patch.
Indeed, shared IRQs were an intended usage, but I don't see that as a
requirement. Indeed, Documentation/power/devices.txt even seems to
suggest that the _noirq version is the place to turn the device as
off as possible:
For simple drivers, suspend might quiesce the device using class code
and then turn its hardware as off as possible during suspend_noirq
Further down in that file, it says:
2. The suspend methods should quiesce the device to stop it from
performing I/O. They also may save the device registers and put it into
the
appropriate low-power state, depending on the bus type the device
is on, and they may enable wakeup events.
... and then:
The majority of subsystems and device drivers need not implement
[the suspend_noirq] callback. However, bus types allowing devices to
share interrupt vectors, like PCI, generally need it; otherwise a driver
might encounter an error during the suspend phase by fielding a shared
interrupt generated by some other device after its own device had been
set to low power.
Right now the OMAP2+ codebase doesn't use any
shared interrupts for on-chip devices, as far as I can see. It
looks like you use -suspend_noirq() to ensure your code runs after
the existing driver suspend functions.
No. The primary reason for using _noirq() is that if any driver ever
did use the _noirq() hooks (for any atomic activity, or late wakeup
detection, etc.) the device will still be accessible.
Using -suspend_noirq() for this purpose seems like a hack.
A better place for that code would be in a bus-pm-suspend()
callback, which will run after the device's dev_pm_ops-suspend()
callback.
I originally put it in bus-pm-suspend, but moved it to
bus-pm-suspend_noirq() since I didn't do a full audit so see
if anything was using the _noirq hooks.
If we want to have a requirement that no on-chip devices can use the
_noirq hooks (or at least they can't touch hardware in those hooks)
then I can move it back to bus-pm-suspend().
That sounds like the best argument for keeping these hooks in
_noirq() -- some driver writer may be likely to use suspend_noirq()
without understanding that they shouldn't... maybe we should add some code
that looks for this and warns.
But thinking about it further, it also seems possible that a handful of
OMAP drivers might use shared IRQs at some point in the future. DSS comes
to mind -- that really is a shared IRQ. So, _noirq() is fine, then.
But personally, I would see that as an artificial requirement based on
a very restrictive definiton of the _noirq() methods.
It's just the definition from the kernel documentation.
- It is not safe to call omap_device_{idle,enable}() from DPM
callbacks as the patch does, because they will race with runtime PM
calls to omap_device_{idle,enable}().
No, they cannot race.
Runtime PM transitions are prevented by the system PM core during a
system PM transition. The system suspend path does a pm_runtime_get()
for each device being suspended, and then does a _put() upon resume.
(see drivers/base/power/main.c, grep for pm_runtime_)
Yes, you are correct in terms of my original statement. But the problem
-- and the race -- that I did a poor job of describing is still present.
What if this pm_bus -suspend_noirq() calls omap_device_idle()
Re: [PATCH 3/4] OMAP: PM: use omap_device API for suspend/resume
On Tue, 1 Jun 2010, Kevin Hilman wrote:
Nayak, Rajendra rna...@ti.com writes:
[...]
diff --git a/arch/arm/mach-omap2/pm_bus.c
b/arch/arm/mach-omap2/pm_bus.c
index 69acaa5..3787da8 100644
--- a/arch/arm/mach-omap2/pm_bus.c
+++ b/arch/arm/mach-omap2/pm_bus.c
@@ -70,3 +70,64 @@ int platform_pm_runtime_idle(struct device *dev)
};
#endif /* CONFIG_PM_RUNTIME */
+#ifdef CONFIG_SUSPEND
+int platform_pm_suspend_noirq(struct device *dev)
+{
+ struct device_driver *drv = dev-driver;
+ struct platform_device *pdev = to_platform_device(dev);
+ struct omap_device *odev = to_omap_device(pdev);
+ int ret = 0;
+
+ if (!drv)
+ return 0;
+
+ if (drv-pm) {
+ if (drv-pm-suspend_noirq)
+ ret = drv-pm-suspend_noirq(dev);
+ }
+
+ if (omap_device_is_valid(odev)) {
+ if (odev-flags OMAP_DEVICE_NO_BUS_SUSPEND) {
+ dev_dbg(dev, no automatic bus-level
system resume.\n);
+ return 0;
+ }
+
+ dev_dbg(dev, %s\n, __func__);
+ omap_device_idle(pdev);
Is it expected that a device is always in enabled state at this point?
If the device is already in idle a call to omap_device_idle unconditionally
throws up warnings from the omap_device api.
Hmm, good point. The device may already be idled (via runtime PM, or
maybe because it was never enabled.)
There are two options:
1. fixup the warnings in the omap_device_idle() to allow multiple
calls to _idle()
2. Add an omap_device_is_idle() check before calling _idle()
I much prefer (1).
Paul?
As far as I can tell, it's not safe for upper-layer code to idle a device
like this. The driver itself needs to be aware of the device's idle
state. For example, if the driver has exported some symbols, and some
other code is calling one of those functions, it's the driver code that
needs to be aware of its own idle-state and to return some kind of error
if the device is quiesced. I don't think there's an easy way for
upper-layer code to do that.
The runtime PM-related code, however, should be safe, since it's initiated
by the driver itself.
- Paul
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] OMAP: PM: use omap_device API for suspend/resume
Paul Walmsley p...@pwsan.com writes:
On Tue, 1 Jun 2010, Kevin Hilman wrote:
Nayak, Rajendra rna...@ti.com writes:
[...]
diff --git a/arch/arm/mach-omap2/pm_bus.c
b/arch/arm/mach-omap2/pm_bus.c
index 69acaa5..3787da8 100644
--- a/arch/arm/mach-omap2/pm_bus.c
+++ b/arch/arm/mach-omap2/pm_bus.c
@@ -70,3 +70,64 @@ int platform_pm_runtime_idle(struct device *dev)
};
#endif /* CONFIG_PM_RUNTIME */
+#ifdef CONFIG_SUSPEND
+int platform_pm_suspend_noirq(struct device *dev)
+{
+ struct device_driver *drv = dev-driver;
+ struct platform_device *pdev = to_platform_device(dev);
+ struct omap_device *odev = to_omap_device(pdev);
+ int ret = 0;
+
+ if (!drv)
+ return 0;
+
+ if (drv-pm) {
+ if (drv-pm-suspend_noirq)
+ ret = drv-pm-suspend_noirq(dev);
+ }
+
+ if (omap_device_is_valid(odev)) {
+ if (odev-flags OMAP_DEVICE_NO_BUS_SUSPEND) {
+ dev_dbg(dev, no automatic bus-level
system resume.\n);
+ return 0;
+ }
+
+ dev_dbg(dev, %s\n, __func__);
+ omap_device_idle(pdev);
Is it expected that a device is always in enabled state at this point?
If the device is already in idle a call to omap_device_idle unconditionally
throws up warnings from the omap_device api.
Hmm, good point. The device may already be idled (via runtime PM, or
maybe because it was never enabled.)
There are two options:
1. fixup the warnings in the omap_device_idle() to allow multiple
calls to _idle()
2. Add an omap_device_is_idle() check before calling _idle()
I much prefer (1).
Paul?
As far as I can tell, it's not safe for upper-layer code to idle a device
like this. The driver itself needs to be aware of the device's idle
state.
The driver is made aware using the standard dev_pm_ops callback for
suspend and resume.
Note that this is not just any upper-layer code that is blindly idling
a device. This is only happening at the system-suspend time, and in
particular this is happening using the _noirq() versions of the
dev_pm_ops hooks.
For example, if the driver has exported some symbols, and some
other code is calling one of those functions, it's the driver code that
needs to be aware of its own idle-state and to return some kind of error
if the device is quiesced. I don't think there's an easy way for
upper-layer code to do that.
Drivers already have to handle this today. If they have been
suspended and their functions have been called, they have to return
errors. This patch doesn't change that.
Kevin
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] OMAP: PM: use omap_device API for suspend/resume
On Mon, 21 Jun 2010, Kevin Hilman wrote:
Paul Walmsley p...@pwsan.com writes:
As far as I can tell, it's not safe for upper-layer code to idle a device
like this. The driver itself needs to be aware of the device's idle
state.
The driver is made aware using the standard dev_pm_ops callback for
suspend and resume.
I guess the intent of your patch is to avoid having to patch in
omap_device_{idle,enable}() into a bunch of driver DPM suspend/resume
callbacks? If so, some comments:
- dev_pm_ops-suspend_noirq() is intended for use on devices with
shared interrupts. Right now the OMAP2+ codebase doesn't use any
shared interrupts for on-chip devices, as far as I can see. It
looks like you use -suspend_noirq() to ensure your code runs after
the existing driver suspend functions. Using -suspend_noirq() for
this purpose seems like a hack. A better place for that code would
be in a bus-pm-suspend() callback, which will run after the
device's dev_pm_ops-suspend() callback.
- It is not safe to call omap_device_{idle,enable}() from DPM
callbacks as the patch does, because they will race with runtime PM
calls to omap_device_{idle,enable}(). (This is part of the reason
why it's important to be anal about enforcing the
omap_device_{enable,idle,shutdown}() state transition limitations -
so we get warned early when these types of conflicts appear.)
omap_device*() calls should either be in runtime PM functions, or
DPM -suspend/resume() callbacks, but not both. Since we've decided
to focus on runtime PM power management as our primary driver power
management approach, and we expect drivers to aggressively idle
their devices, it seems best to keep the omap_device*() functions in
runtime PM code.
At that point, the common DPM suspend/idle callbacks that you
propose can simply block until runtime PM indicates that the device
is idle. For example, you could use a per-omap_device mutex.
A sketch of a sample implementation follows this message.
- Paul
In omap_device.c, add
int omap_device_suspend_mpu_irqs(struct omap_device *od)
{
/*
* For each hwmod, for each MPU IRQ, save current state,
* disable_irq(irq)
*/
}
int omap_device_resume_mpu_irqs(struct omap_device *od)
{
/*
* For each hwmod, for each MPU IRQ, enable_irq(irq) if
* it was previously
*/
}
In your bus-pm-suspend()/resume() functions:
int omap_bus_suspend(struct device *dev, pm_message_t state)
{
dev_dbg(waiting for device %s to go idle\n, dev_name(dev));
mutex_lock(od-active_mutex);
/* Device guaranteed to be idle at this point */
/*
* do anything else necessary to ensure that driver code is not
* entered after the unlock()
*/
omap_device_suspend_mpu_irqs(od);
mutex_unlock(od-active_mutex);
return 0;
}
int omap_bus_resume(struct device *dev, pm_message_t state)
{
mutex_lock(od-active_mutex);
/* Device guaranteed to be idle at this point */
/*
* do anything else necessary to ensure that driver code can
* be entered after the unlock()
*/
omap_device_resume_mpu_irqs(od);
mutex_unlock(od-active_mutex);
return 0;
}
Then in the omap_device code, add a mutex active_mutex in struct
omap_device, init the mutex in omap_device_build_ss(), then:
int omap_device_enable(struct platform_device *pdev)
{
struct omap_device *od;
od = _find_by_pdev(pdev);
...
WARN(!mutex_trylock(od-active_mutex),
State transition violation - should never happen\n);
mutex_lock(od-active_mutex);
...
}
int omap_device_idle(struct platform_device *pdev)
{
struct omap_device *od;
od = _find_by_pdev(pdev);
...
mutex_unlock(od-active_mutex);
...
}
int omap_device_shutdown(struct platform_device *pdev)
{
struct omap_device *od;
od = _find_by_pdev(pdev);
...
mutex_unlock(od-active_mutex);
...
}
The driver needs the usual:
- add a DPM -suspend() function that tries to stop whatever the
device is doing if it's in the middle of something that can be
stopped;
- ensure all paths that start new I/O check dev-power.status, and
return an error if it is DPM_SUSPENDING
- Paul
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] OMAP: PM: use omap_device API for suspend/resume
Paul Walmsley p...@pwsan.com writes:
On Mon, 21 Jun 2010, Kevin Hilman wrote:
Paul Walmsley p...@pwsan.com writes:
As far as I can tell, it's not safe for upper-layer code to idle a device
like this. The driver itself needs to be aware of the device's idle
state.
The driver is made aware using the standard dev_pm_ops callback for
suspend and resume.
I guess the intent of your patch is to avoid having to patch in
omap_device_{idle,enable}() into a bunch of driver DPM suspend/resume
callbacks?
Partially. Actually, I think many (most?) drivers can get rid of
static suspend/resume paths all together and just use runtime PM.
There are some cases though (MMC comes to mind, more on that below)
where static suspend has some extra steps necessary and behaves
differently than runtime PM.
For those cases, the goal is as you stated. Basically, to avoid having
all the drivers having to call omap_device_idle().
If so, some comments:
- dev_pm_ops-suspend_noirq() is intended for use on devices with
shared interrupts.
Just to clarify. The functions I'm overriding here is the bus
functions (bus-pm-[suspend|resume]_noirq(), not any driver functions
Indeed, shared IRQs were an intended usage, but I don't see that as a
requirement. Indeed, Documentation/power/devices.txt even seems to
suggest that the _noirq version is the place to turn the device as
off as possible:
For simple drivers, suspend might quiesce the device using class code
and then turn its hardware as off as possible during suspend_noirq
Right now the OMAP2+ codebase doesn't use any
shared interrupts for on-chip devices, as far as I can see. It
looks like you use -suspend_noirq() to ensure your code runs after
the existing driver suspend functions.
No. The primary reason for using _noirq() is that if any driver ever
did use the _noirq() hooks (for any atomic activity, or late wakeup
detection, etc.) the device will still be accessible.
Using -suspend_noirq() for this purpose seems like a hack.
A better place for that code would be in a bus-pm-suspend()
callback, which will run after the device's dev_pm_ops-suspend()
callback.
I originally put it in bus-pm-suspend, but moved it to
bus-pm-suspend_noirq() since I didn't do a full audit so see
if anything was using the _noirq hooks.
If we want to have a requirement that no on-chip devices can use the
_noirq hooks (or at least they can't touch hardware in those hooks)
then I can move it back to bus-pm-suspend(). But personally, I
would see that as an artificial requirement based on a very
restrictive definiton of the _noirq() methods.
- It is not safe to call omap_device_{idle,enable}() from DPM
callbacks as the patch does, because they will race with runtime PM
calls to omap_device_{idle,enable}().
No, they cannot race.
Runtime PM transitions are prevented by the system PM core during a
system PM transition. The system suspend path does a pm_runtime_get()
for each device being suspended, and then does a _put() upon resume.
(see drivers/base/power/main.c, grep for pm_runtime_)
(This is part of the reason
why it's important to be anal about enforcing the
omap_device_{enable,idle,shutdown}() state transition limitations -
so we get warned early when these types of conflicts appear.)
omap_device*() calls should either be in runtime PM functions, or
DPM -suspend/resume() callbacks, but not both.
Why not both? I don't follow the reason for this restriction. We
certainly did the equivalent clock enable/disable calls in both cases
in the past.
Since we've decided
to focus on runtime PM power management as our primary driver power
management approach, and we expect drivers to aggressively idle
their devices, it seems best to keep the omap_device*() functions in
runtime PM code.
I agree, mostly.
As I mentioned above, I expect most drivers not to even have system PM
methods implemented. They should implement everything as runtime PM.
However, there will be some exceptions.
The use case that brought this patch into existence was the MMC driver
where the suspend hook had to do some extra stuff before suspending.
Even if already runtime suspended, the MMC suspend hook has to
re-enable the device do stuff and then re-idle the device.
Initially, I tried to just use the same runtime PM calls in the
suspend hook, but that doesn't work since runtime PM transitions are
prevented during system PM. So, I was forced to call
omap_device_idle() in the suspend path, which led to the decision to
move that into common code.
At that point, the common DPM suspend/idle callbacks that you
propose can simply block until runtime PM indicates that the device
is idle.
For example, you could use a per-omap_device mutex.
A sketch of a sample implementation follows this message.
That is an option, but since there is no potential racing between
runtime PM and system PM, I don't see the
RE: [PATCH 3/4] OMAP: PM: use omap_device API for suspend/resume
-Original Message-
From: linux-omap-ow...@vger.kernel.org
[mailto:linux-omap-ow...@vger.kernel.org] On Behalf Of Kevin Hilman
Sent: Friday, May 28, 2010 2:43 AM
To: linux-omap@vger.kernel.org
Subject: [PATCH 3/4] OMAP: PM: use omap_device API for suspend/resume
Hook into the platform bus methods for suspend and resume and
use the omap_device API to automatically idle and enable the
device on suspend and resume.
This allows device drivers to get rid of direct management of their
clocks in their suspend/resume paths, and let omap_device do it for
them.
We currently use the _noirq (late suspend, early resume) versions of
the suspend/resume methods to ensure that the device is not disabled
too early for any drivers also using the _noirq methods.
NOTE: only works for devices with omap_hwmod support.
Signed-off-by: Kevin Hilman khil...@deeprootsystems.com
---
arch/arm/mach-omap2/pm_bus.c | 61
++
1 files changed, 61 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-omap2/pm_bus.c
b/arch/arm/mach-omap2/pm_bus.c
index 69acaa5..3787da8 100644
--- a/arch/arm/mach-omap2/pm_bus.c
+++ b/arch/arm/mach-omap2/pm_bus.c
@@ -70,3 +70,64 @@ int platform_pm_runtime_idle(struct device *dev)
};
#endif /* CONFIG_PM_RUNTIME */
+#ifdef CONFIG_SUSPEND
+int platform_pm_suspend_noirq(struct device *dev)
+{
+ struct device_driver *drv = dev-driver;
+ struct platform_device *pdev = to_platform_device(dev);
+ struct omap_device *odev = to_omap_device(pdev);
+ int ret = 0;
+
+ if (!drv)
+ return 0;
+
+ if (drv-pm) {
+ if (drv-pm-suspend_noirq)
+ ret = drv-pm-suspend_noirq(dev);
+ }
+
+ if (omap_device_is_valid(odev)) {
+ if (odev-flags OMAP_DEVICE_NO_BUS_SUSPEND) {
+ dev_dbg(dev, no automatic bus-level
system resume.\n);
+ return 0;
+ }
+
+ dev_dbg(dev, %s\n, __func__);
+ omap_device_idle(pdev);
Is it expected that a device is always in enabled state at this point?
If the device is already in idle a call to omap_device_idle unconditionally
throws up warnings from the omap_device api.
+ } else {
+ dev_dbg(dev, not an omap_device\n);
+ }
+
+ return ret;
+}
+
+int platform_pm_resume_noirq(struct device *dev)
+{
+ struct device_driver *drv = dev-driver;
+ struct platform_device *pdev = to_platform_device(dev);
+ struct omap_device *odev = to_omap_device(pdev);
+ int ret = 0;
+
+ if (omap_device_is_valid(odev)) {
+ if (odev-flags OMAP_DEVICE_NO_BUS_SUSPEND) {
+ dev_dbg(dev, no automatic bus-level
system resume.\n);
+ return 0;
+ }
+
+ dev_dbg(dev, %s\n, __func__);
+ omap_device_enable(pdev);
+ } else {
+ dev_dbg(dev, not an omap_device\n);
+ }
+
+ if (!drv)
+ return 0;
+
+ if (drv-pm) {
+ if (drv-pm-resume_noirq)
+ ret = drv-pm-resume_noirq(dev);
+ }
+
+ return ret;
+}
+#endif /* CONFIG_SUSPEND */
--
1.7.0.2
--
To unsubscribe from this list: send the line unsubscribe
linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] OMAP: PM: use omap_device API for suspend/resume
Nayak, Rajendra rna...@ti.com writes:
[...]
diff --git a/arch/arm/mach-omap2/pm_bus.c
b/arch/arm/mach-omap2/pm_bus.c
index 69acaa5..3787da8 100644
--- a/arch/arm/mach-omap2/pm_bus.c
+++ b/arch/arm/mach-omap2/pm_bus.c
@@ -70,3 +70,64 @@ int platform_pm_runtime_idle(struct device *dev)
};
#endif /* CONFIG_PM_RUNTIME */
+#ifdef CONFIG_SUSPEND
+int platform_pm_suspend_noirq(struct device *dev)
+{
+struct device_driver *drv = dev-driver;
+struct platform_device *pdev = to_platform_device(dev);
+struct omap_device *odev = to_omap_device(pdev);
+int ret = 0;
+
+if (!drv)
+return 0;
+
+if (drv-pm) {
+if (drv-pm-suspend_noirq)
+ret = drv-pm-suspend_noirq(dev);
+}
+
+if (omap_device_is_valid(odev)) {
+if (odev-flags OMAP_DEVICE_NO_BUS_SUSPEND) {
+dev_dbg(dev, no automatic bus-level
system resume.\n);
+return 0;
+}
+
+dev_dbg(dev, %s\n, __func__);
+omap_device_idle(pdev);
Is it expected that a device is always in enabled state at this point?
If the device is already in idle a call to omap_device_idle unconditionally
throws up warnings from the omap_device api.
Hmm, good point. The device may already be idled (via runtime PM, or
maybe because it was never enabled.)
There are two options:
1. fixup the warnings in the omap_device_idle() to allow multiple
calls to _idle()
2. Add an omap_device_is_idle() check before calling _idle()
I much prefer (1).
Paul?
Kevin
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/4] OMAP: PM: use omap_device API for suspend/resume
Hook into the platform bus methods for suspend and resume and
use the omap_device API to automatically idle and enable the
device on suspend and resume.
This allows device drivers to get rid of direct management of their
clocks in their suspend/resume paths, and let omap_device do it for
them.
We currently use the _noirq (late suspend, early resume) versions of
the suspend/resume methods to ensure that the device is not disabled
too early for any drivers also using the _noirq methods.
NOTE: only works for devices with omap_hwmod support.
Signed-off-by: Kevin Hilman khil...@deeprootsystems.com
---
arch/arm/mach-omap2/pm_bus.c | 61 ++
1 files changed, 61 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-omap2/pm_bus.c b/arch/arm/mach-omap2/pm_bus.c
index 69acaa5..3787da8 100644
--- a/arch/arm/mach-omap2/pm_bus.c
+++ b/arch/arm/mach-omap2/pm_bus.c
@@ -70,3 +70,64 @@ int platform_pm_runtime_idle(struct device *dev)
};
#endif /* CONFIG_PM_RUNTIME */
+#ifdef CONFIG_SUSPEND
+int platform_pm_suspend_noirq(struct device *dev)
+{
+ struct device_driver *drv = dev-driver;
+ struct platform_device *pdev = to_platform_device(dev);
+ struct omap_device *odev = to_omap_device(pdev);
+ int ret = 0;
+
+ if (!drv)
+ return 0;
+
+ if (drv-pm) {
+ if (drv-pm-suspend_noirq)
+ ret = drv-pm-suspend_noirq(dev);
+ }
+
+ if (omap_device_is_valid(odev)) {
+ if (odev-flags OMAP_DEVICE_NO_BUS_SUSPEND) {
+ dev_dbg(dev, no automatic bus-level system resume.\n);
+ return 0;
+ }
+
+ dev_dbg(dev, %s\n, __func__);
+ omap_device_idle(pdev);
+ } else {
+ dev_dbg(dev, not an omap_device\n);
+ }
+
+ return ret;
+}
+
+int platform_pm_resume_noirq(struct device *dev)
+{
+ struct device_driver *drv = dev-driver;
+ struct platform_device *pdev = to_platform_device(dev);
+ struct omap_device *odev = to_omap_device(pdev);
+ int ret = 0;
+
+ if (omap_device_is_valid(odev)) {
+ if (odev-flags OMAP_DEVICE_NO_BUS_SUSPEND) {
+ dev_dbg(dev, no automatic bus-level system resume.\n);
+ return 0;
+ }
+
+ dev_dbg(dev, %s\n, __func__);
+ omap_device_enable(pdev);
+ } else {
+ dev_dbg(dev, not an omap_device\n);
+ }
+
+ if (!drv)
+ return 0;
+
+ if (drv-pm) {
+ if (drv-pm-resume_noirq)
+ ret = drv-pm-resume_noirq(dev);
+ }
+
+ return ret;
+}
+#endif /* CONFIG_SUSPEND */
--
1.7.0.2
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
