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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, 2012-06-26 at 10:34 -0400, Alan Stern wrote:
> On Tue, 26 Jun 2012, Grazvydas Ignotas wrote:
> 
> > CCing some PM people, maybe they can comment?
> > 
> > On Tue, Jun 26, 2012 at 7:51 AM, Rajendra Nayak <rnayak@xxxxxx> wrote:
> > > On Monday 25 June 2012 06:20 PM, Tomi Valkeinen wrote:
> > >>
> > >> Do you know how the drivers should handle CONFIG_PM_RUNTIME=n?
> > >> Are they supposed to handle the error values returned by runtime PM
> > >> functions somehow, or should they use #ifdef CONFIG_PM_RUNTIME?
> > >
> > > hmm, I always though with CONFIG_RUNTIME_PM=n, the functions would
> > > be stubbed to return success and not failure.
> 
> Not exactly.  They are stubbed to indicate that the device cannot be 
> suspended, that it is always active.
> 
> 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.

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);
	/* handle error */
	return -ESOMETHING;
}

/* do the work */

pm_runtime_put_sync(dev);

---

Is this correct?

 Tomi

Attachment: signature.asc
Description: This is a digitally signed message part


[Index of Archives]     [Video for Linux]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Tourism]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux