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 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: signature.asc
Description: This is a digitally signed message part


[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux