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

[Tomi Valkeinen <tomi.valkeinen@ti.com>]

[-- Attachment #1: Type: text/plain, Size: 1554 bytes --]

On Tue, 2012-06-26 at 22:31 +0530, Jassi Brar wrote:

> While I think your patch is simpler and achieve the same, I also think
> your fears about this patch are unfounded.

Perhaps. But I do get a bad feeling from your patch, and I don't like
when that happens =). What I fear with changes like you made is that
they'll hide problems that should be fixed in other ways.

And I think that was the case here also. I think we should not call
dispc_runtime_get() during suspend. If pm_runtime_get returns -EACCES,
we do have a possible problem, and we should not silently ignore it.

> A quick snack for thought...
> >
> > But if pm_runtime_get_sync() returns an error, it means the HW has not
> > been resumed successfully, and is not operational,
> >
> Not always. The HW could be in RPM_ACTIVE state while PM on it could
> be disabled, if the returned error is -EACCESS.   And
> pm_runtime_enabled() only catches a potential -EACCESS.

True. But the HW could also be in disabled state. And that would lead to
a crash when accessing the registers.

It is not a fatal error if pm_runtime_get returns -EACCES, but we sure
shouldn't ignore it (or avoid it with pm_runtime_enabled()), but handle
it. In some rare cases it could be ok to get -EACCES, but that's a
special case, not standard.

> BTW, I just tested your patch and it worked for me as well. But as
> suspected, it doesn't help the stack spew of CONFIG_PM_RUNTIME:=n
> 
> So I understand, I only need to resend the other three patches ?

Yes, please.

 Tomi


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]