On Wed, Jun 05, 2013 at 10:39:03PM +0300, Ville Syrj?l? wrote: > On Wed, Jun 05, 2013 at 08:04:48PM +0200, Daniel Vetter wrote: > > On Wed, Jun 5, 2013 at 7:20 PM, Paulo Zanoni <przanoni at gmail.com> wrote: > > >> So all together I vote for the generic interface to be called something > > >> like > > >> > > >> intel_aux_display_runtime_forbid/allow > > > > > > I changed to intel_aux_display_shutdown_forbid/allow, because we > > > forbid the shutdown, not the "runtime". > > > > Just one quick comment: The idea was to be 100% consisten with the > > runtime pm api (I've dropped the pm_ part unfortunately), which is: > > > > extern void pm_runtime_allow(struct device *dev); > > extern void pm_runtime_forbid(struct device *dev); > > I've not seen those guys before, but they don't seem like something > we ought to be using. In fact it seems they're there solely for the > purpose of sysfs. They frob the runtime_auto boolean which has no ref > count protecting it, so someone writing to the sysfs "control" file > could undo what we just did from the driver, and vice versa. There's > still the usage_count though so the device should remain alive, but > then that's what _get()/_put() also modify. Yeah, I've been confused here. _get/_put seem to be the ones we actually want to emulate. > I do see quite a few pm_runtime_allow/forbid calls from drivers though, > which is confusing to say the least. Either I'm confused about the > purpose of these functions, or there has been a serious problem with > driver review. I think it's to prevent runtime pm in driver init code, e.g. usb devices which crap out when autosuspended. Since presumable the driver will do the allow/forbid only once it should all work out. And root is allowed to break stuff by touching randomg things in sysfs ;-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
