On Wed, 2012-06-27 at 10:12 +0530, Jassi Brar wrote: > > 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. > > > You are mixing up generic concepts with what we have in omapdss. > Believe me, I do understand it's bad to proceed without caring for > returned _errors_. > The way omapdss is organized -EACCESS is _not_ an error, it just > denotes PM is disabled on the device and that DISPC is in RPM_ACTIVE > is backed by the fact that HDMI always hold a reference between > resume-suspend and DISPC goes to suspend last and resume first. I'm not arguing that your solution would not work with the omapdss code we have now, and presuming the underlying frameworks work fine. But I want omapdss to have code that works also in the future, when other parts of omapdss change. It doesn't matter how omapdss is organized, -EACCES _is_ an error. It tells us that something unexpected happened, and we should react to it somehow. When we call, for example, dispc_runtime_get(), we normally expect that runtime PM is enabled, and it 1) "gets" it, increasing the use count, and 2) makes sure the HW is enabled so it can be used. Your patch breaks both of these if runtime PM is disabled. Sure, in the current omapdss neither is a breaking problem, because 1) the matching dispc_runtime_put() is called also with runtime PM disabled, and thus we don't decrease the use count, and 2) the HW happens to be already enabled. But that's just by "luck", and tomorrow omapdss could be different. If, for some reason, we need to call dispc_runtime_get/put during suspend, the caller should either 1) realize that we're suspending, runtime PM is disabled, but the HW is anyway enabled, and thus skip calling both get and put, or 2) call both get and put, but handle the -EACCES, understanding what it means and knowing the HW is anyway enabled. Tomi
Attachment:
signature.asc
Description: This is a digitally signed message part