Re: [PATCH] OMAPDSS: Check if RPM enabled before trying to change state

On Tue, 26 Jun 2012, Tomi Valkeinen wrote:

quoted

Failure to suspend a device should not be regarded as particularly bad, 
because it doesn't affect the device's functionality.  That's true even 
when CONFIG_RUNTIME_PM is enabled.

This makes sense. So if CONFIG_RUNTIME_PM=n, using pm_runtime_get_sync()
will return 1, meaning the HW is already enabled, and using
pm_runtime_put_sync() will return -ENOSYS, meaning the hardware cannot
be suspended.

With CONFIG_RUNTIME_PM=y, it's a bit more complex. If I read the code
correctly, when I call pm_runtime_get_sync(), the usage counter is
always increased, even if the pm_runtime_resume() fails. So a get()
needs to be always matched with a put(), even if get() has returned an
error.

Right.  Of course, it doesn't hurt to match a get() with a put() even 
when CONFIG_RUNTIME_PM=n.

But if pm_runtime_get_sync() returns an error, it means the HW has not
been resumed successfully, and is not operational, so the code should
bail out somehow. So basically I'd use this kind of pattern everywhere I
use pm_runtime_get_sync():

---

r = pm_runtime_get_sync(dev);
if (r < 0) {
	pm_runtime_put_sync(dev);

Here you could just as well call pm_runtime_put_noidle().  Since the 
device wasn't resumed, the put operation doesn't need to try to suspend 
it.

	/* handle error */
	return -ESOMETHING;
}

/* do the work */

pm_runtime_put_sync(dev);

---

Is this correct?

Yep, you've got it.

Alan Stern