* sysfs and power management @ 2010-10-27 10:59 Onkalo Samu 2010-10-27 11:48 ` Alan Cox 0 siblings, 1 reply; 15+ messages in thread From: Onkalo Samu @ 2010-10-27 10:59 UTC (permalink / raw) To: gregkh, Alan Cox, Andrew Morton; +Cc: linux-kernel Hi I have recently sent couple of sensor drivers to kernel. Interface is purely sysfs based. The hard part is to get power management to work reasonable way. With /dev/* interface power management is quite easy to do since driver gets clear open and release calls. With sysfs, there is only read / write methods available. My solution has been separate sysfs entry which controls operating state. However, there are some problems with that. In case of boolean type enable / disable control, there must be only one userspace component which decides sensor state. In case of counting type of control, there is a change that sensor is permanently left on if the userspace component crashes. This would lead to unbalanced enable / disable count. If the sensor functionality is based on interrupts (like threshold interrupt), it may take long time before there is any activity. In this kind of case timeout from the previous sysfs access doesn't make much sense. I started to wonder if it makes sense to enhance sysfs so that it optionally prodives open / close call backs. Internally sysfs has some bookkeeping about the refcount but this is not visible to the driver. Of course majority of the sysfs users doesn't need that at all and for them this is just overhead. Is it out of the question to provide call back to driver when some sysfs entry is opened (first time) and when refcnt goes back to 0? Regards, Samu ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: sysfs and power management 2010-10-27 10:59 sysfs and power management Onkalo Samu @ 2010-10-27 11:48 ` Alan Cox 2010-10-27 13:43 ` samu.p.onkalo 0 siblings, 1 reply; 15+ messages in thread From: Alan Cox @ 2010-10-27 11:48 UTC (permalink / raw) To: samu.p.onkalo; +Cc: gregkh, Andrew Morton, linux-kernel > I started to wonder if it makes sense to enhance sysfs so that it > optionally prodives open / close call backs. Internally sysfs has > some bookkeeping about the refcount but this is not visible to the > driver. Of course majority of the sysfs users doesn't need that at all > and for them this is just overhead. I think we need it. There doesn't need to be much overhead however as there is no need (or sense) in providing per sysfs node open/close hooks. The pm layer also lacks a clean race-free way to actually ascertain when the device was last kicked out of pm saving. ^ permalink raw reply [flat|nested] 15+ messages in thread
* RE: sysfs and power management 2010-10-27 11:48 ` Alan Cox @ 2010-10-27 13:43 ` samu.p.onkalo 2010-10-27 14:28 ` Alan Cox 0 siblings, 1 reply; 15+ messages in thread From: samu.p.onkalo @ 2010-10-27 13:43 UTC (permalink / raw) To: alan; +Cc: gregkh, akpm, linux-kernel >-----Original Message----- >From: ext Alan Cox [mailto:alan@linux.intel.com] > >> I started to wonder if it makes sense to enhance sysfs so that it >> optionally prodives open / close call backs. Internally sysfs has >> some bookkeeping about the refcount but this is not visible to the >> driver. Of course majority of the sysfs users doesn't need that at all >> and for them this is just overhead. > >I think we need it. There doesn't need to be much overhead however as >there is no need (or sense) in providing per sysfs node open/close >hooks. Do you mean per device hook and each sysfs open / close uses per device ref-counting. So the driver would just know that some sysfs entry is open or all the entries are closed. I think that this kind of behavior is ok for this purpose. > >The pm layer also lacks a clean race-free way to actually ascertain >when the device was last kicked out of pm saving. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: sysfs and power management 2010-10-27 13:43 ` samu.p.onkalo @ 2010-10-27 14:28 ` Alan Cox 2010-10-29 19:50 ` Greg KH 0 siblings, 1 reply; 15+ messages in thread From: Alan Cox @ 2010-10-27 14:28 UTC (permalink / raw) To: samu.p.onkalo; +Cc: alan, gregkh, akpm, linux-kernel > Do you mean per device hook and each sysfs open / close uses > per device ref-counting. Or you can do it by having an optional per device hook which is passed the sysfs node for each open/close and leave the rest up to the driver. If it needs to do clever stuff it can. Alan ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: sysfs and power management 2010-10-27 14:28 ` Alan Cox @ 2010-10-29 19:50 ` Greg KH 2010-10-30 14:00 ` Henrique de Moraes Holschuh 0 siblings, 1 reply; 15+ messages in thread From: Greg KH @ 2010-10-29 19:50 UTC (permalink / raw) To: Alan Cox; +Cc: samu.p.onkalo, alan, akpm, linux-kernel On Wed, Oct 27, 2010 at 03:28:09PM +0100, Alan Cox wrote: > > Do you mean per device hook and each sysfs open / close uses > > per device ref-counting. > > Or you can do it by having an optional per device hook which is passed > the sysfs node for each open/close and leave the rest up to the driver. > If it needs to do clever stuff it can. I really don't want to add open/close to the sysfs file model for the driver core. What is the specific problem with not doing any sensor work until userspace asks for the data? Then do the read from the hardware and go back to sleep. That's probably the best way to do this, as userspace isn't going to open the sysfs file and not close it instantly anyway after it has read the data (seeking on a sysfs file isn't really recommended, even if it sometimes seems to work.) thanks, greg k-h ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: sysfs and power management 2010-10-29 19:50 ` Greg KH @ 2010-10-30 14:00 ` Henrique de Moraes Holschuh 2010-10-31 11:57 ` Onkalo Samu 0 siblings, 1 reply; 15+ messages in thread From: Henrique de Moraes Holschuh @ 2010-10-30 14:00 UTC (permalink / raw) To: Greg KH; +Cc: Alan Cox, samu.p.onkalo, alan, akpm, linux-kernel On Fri, 29 Oct 2010, Greg KH wrote: > back to sleep. That's probably the best way to do this, as userspace > isn't going to open the sysfs file and not close it instantly anyway > after it has read the data (seeking on a sysfs file isn't really > recommended, even if it sometimes seems to work.) Well, it is documented that seek(start of file) on sysfs works, and it is ABI already (some userspace uses it on poll/select-capable attributes). So, maybe seek(somewhere that is not the start of the file) doesn't work -- and it should return an error in that case if it doesn't already... but it is a lot more deterministic than "sometimes" ;-) So yes, userspace will open() and not close() a sysfs attribute immediately afterwards. It is not only shell crap that interfaces to the kernel over sysfs :-) It would make a lot of sense to support the poll/select model on any sensor for which you have event-driven notifications of change from the hardware or firmware. I don't know about open/close notifications, however. Usually you need those when you're going to stream something to userspace, and there are FAR better interfaces to use in that case, such as the ring buffers used by the data acquisition devices, netlink (which userspace programmers seems not to like much :p), input devices, and generic character devices... -- "One disk to rule them all, One disk to find them. One disk to bring them all and in the darkness grind them. In the Land of Redmond where the shadows lie." -- The Silicon Valley Tarot Henrique Holschuh ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: sysfs and power management 2010-10-30 14:00 ` Henrique de Moraes Holschuh @ 2010-10-31 11:57 ` Onkalo Samu 2010-10-31 14:25 ` Greg KH 0 siblings, 1 reply; 15+ messages in thread From: Onkalo Samu @ 2010-10-31 11:57 UTC (permalink / raw) To: ext Henrique de Moraes Holschuh Cc: Greg KH, Alan Cox, alan, akpm, linux-kernel On Sat, 2010-10-30 at 16:00 +0200, ext Henrique de Moraes Holschuh wrote: > On Fri, 29 Oct 2010, Greg KH wrote: > > back to sleep. That's probably the best way to do this, as userspace > > isn't going to open the sysfs file and not close it instantly anyway > > after it has read the data (seeking on a sysfs file isn't really > > recommended, even if it sometimes seems to work.) > > Well, it is documented that seek(start of file) on sysfs works, and it > is ABI already (some userspace uses it on poll/select-capable > attributes). So, maybe seek(somewhere that is not the start of the > file) doesn't work -- and it should return an error in that case if it > doesn't already... but it is a lot more deterministic than "sometimes" > ;-) > > So yes, userspace will open() and not close() a sysfs attribute immediately > afterwards. It is not only shell crap that interfaces to the kernel over > sysfs :-) > What I would like to do is: Control sensors operating mode and regulators based on the userspace activity. If no-one is interested about the sensor, it can be turned totally off including its operating power via regulator framework. So far the only accepted interface for the small sensor seems to be sysfs. I tried use misc device but it was not accepted. For example ambient light sensor or proximity sensor may operate fully under interrupts. There is no need to poll status which is fine with sysfs. However, problem is that kernel driver have no idea if someone keeps some sysfs entry open and waits for the data. What I would like to have is just an indication that at least one of the sysfs entries is opened by some application. When that condition is true, the sensor would be powered on and kept running. And similarly, then all the users has gone, turn the device off. I can prepare a patch to show what I mean. -Samu > It would make a lot of sense to support the poll/select model on any > sensor for which you have event-driven notifications of change from the > hardware or firmware. > > I don't know about open/close notifications, however. Usually you need > those when you're going to stream something to userspace, and there are FAR > better interfaces to use in that case, such as the ring buffers used by the > data acquisition devices, netlink (which userspace programmers seems not to > like much :p), input devices, and generic character devices... > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: sysfs and power management 2010-10-31 11:57 ` Onkalo Samu @ 2010-10-31 14:25 ` Greg KH 2010-11-01 10:41 ` Onkalo Samu 0 siblings, 1 reply; 15+ messages in thread From: Greg KH @ 2010-10-31 14:25 UTC (permalink / raw) To: Onkalo Samu Cc: ext Henrique de Moraes Holschuh, Alan Cox, alan, akpm, linux-kernel On Sun, Oct 31, 2010 at 01:57:55PM +0200, Onkalo Samu wrote: > On Sat, 2010-10-30 at 16:00 +0200, ext Henrique de Moraes Holschuh > wrote: > > On Fri, 29 Oct 2010, Greg KH wrote: > > > back to sleep. That's probably the best way to do this, as userspace > > > isn't going to open the sysfs file and not close it instantly anyway > > > after it has read the data (seeking on a sysfs file isn't really > > > recommended, even if it sometimes seems to work.) > > > > Well, it is documented that seek(start of file) on sysfs works, and it > > is ABI already (some userspace uses it on poll/select-capable > > attributes). So, maybe seek(somewhere that is not the start of the > > file) doesn't work -- and it should return an error in that case if it > > doesn't already... but it is a lot more deterministic than "sometimes" > > ;-) > > > > So yes, userspace will open() and not close() a sysfs attribute immediately > > afterwards. It is not only shell crap that interfaces to the kernel over > > sysfs :-) > > > > What I would like to do is: > > Control sensors operating mode and regulators based on the userspace > activity. If no-one is interested about the sensor, it can be turned > totally off including its operating power via regulator framework. > > So far the only accepted interface for the small sensor seems to be > sysfs. I tried use misc device but it was not accepted. Look at the drivers/staging/iio/ subsystem. It is working on a framework that you can use through a character device (I think) to properly manage your drivers in this manner. Try working with those developers as I think it is what you are looking for here. thanks, greg k-h ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: sysfs and power management 2010-10-31 14:25 ` Greg KH @ 2010-11-01 10:41 ` Onkalo Samu 2010-11-01 16:57 ` Alan Cox 0 siblings, 1 reply; 15+ messages in thread From: Onkalo Samu @ 2010-11-01 10:41 UTC (permalink / raw) To: ext Greg KH Cc: ext Henrique de Moraes Holschuh, Alan Cox, alan, akpm, linux-kernel On Sun, 2010-10-31 at 15:25 +0100, ext Greg KH wrote: > > > > What I would like to do is: > > > > Control sensors operating mode and regulators based on the userspace > > activity. If no-one is interested about the sensor, it can be turned > > totally off including its operating power via regulator framework. > > > > So far the only accepted interface for the small sensor seems to be > > sysfs. I tried use misc device but it was not accepted. > > Look at the drivers/staging/iio/ subsystem. It is working on a > framework that you can use through a character device (I think) to > properly manage your drivers in this manner. > > Try working with those developers as I think it is what you are looking > for here. > I took a look to that. It seems that iio is more or less sysfs based. There are ring buffers and event device which are chardev based but still the data outside ring buffer and the control is sysfs based. By getting open and close from sysfs would be nice from the driver point of view. However, I understand that this is just overhead for majority of the cases. Regards, Samu ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: sysfs and power management 2010-11-01 10:41 ` Onkalo Samu @ 2010-11-01 16:57 ` Alan Cox 2010-11-01 18:07 ` Greg KH 0 siblings, 1 reply; 15+ messages in thread From: Alan Cox @ 2010-11-01 16:57 UTC (permalink / raw) To: samu.p.onkalo Cc: ext Greg KH, ext Henrique de Moraes Holschuh, Alan Cox, akpm, linux-kernel > I took a look to that. It seems that iio is more or less sysfs based. > There are ring buffers and event device which are chardev based > but still the data outside ring buffer and the control is sysfs based. IIO is sysfs dependant, heavyweight and makes no sense for some of the sysfs based drivers. IIO is also staging based and Linus already threw out the last attempt to unify these drivers sanely with an ALS layer - which was smaller, cleaner and better ! > By getting open and close from sysfs would be nice from the driver > point of view. However, I understand that this is just overhead for > majority of the cases. The alternative really is to end up with a parallel 'not quite sysfs' which is sysfs + open/close. My feeling is its cleaner to have a hook at the per device level (so we don't bloat the sysfs nodes at all) than have two copies of sysfs. Alan ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: sysfs and power management 2010-11-01 16:57 ` Alan Cox @ 2010-11-01 18:07 ` Greg KH 2010-11-03 9:44 ` Alan Cox 0 siblings, 1 reply; 15+ messages in thread From: Greg KH @ 2010-11-01 18:07 UTC (permalink / raw) To: Alan Cox Cc: samu.p.onkalo, ext Henrique de Moraes Holschuh, Alan Cox, akpm, linux-kernel On Mon, Nov 01, 2010 at 04:57:01PM +0000, Alan Cox wrote: > > I took a look to that. It seems that iio is more or less sysfs based. > > There are ring buffers and event device which are chardev based > > but still the data outside ring buffer and the control is sysfs based. > > IIO is sysfs dependant, heavyweight and makes no sense for some of the > sysfs based drivers. IIO is also staging based and Linus already threw > out the last attempt to unify these drivers sanely with an ALS layer - > which was smaller, cleaner and better ! I think we need to revisit this issue again, before iio is merged to the main kernel tree. I've been totally ignoring the iio user/kernel api at the moment, waiting for things to settle down there. thanks, greg k-h ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: sysfs and power management 2010-11-01 18:07 ` Greg KH @ 2010-11-03 9:44 ` Alan Cox 2010-11-03 10:48 ` samu.p.onkalo 2010-11-03 13:09 ` Greg KH 0 siblings, 2 replies; 15+ messages in thread From: Alan Cox @ 2010-11-03 9:44 UTC (permalink / raw) To: Greg KH Cc: samu.p.onkalo, ext Henrique de Moraes Holschuh, Alan Cox, akpm, linux-kernel On Mon, 1 Nov 2010 11:07:40 -0700 Greg KH <gregkh@suse.de> wrote: > On Mon, Nov 01, 2010 at 04:57:01PM +0000, Alan Cox wrote: > > > I took a look to that. It seems that iio is more or less sysfs > > > based. There are ring buffers and event device which are chardev > > > based but still the data outside ring buffer and the control is > > > sysfs based. > > > > IIO is sysfs dependant, heavyweight and makes no sense for some of > > the sysfs based drivers. IIO is also staging based and Linus > > already threw out the last attempt to unify these drivers sanely > > with an ALS layer - which was smaller, cleaner and better ! > > I think we need to revisit this issue again, before iio is merged to > the main kernel tree. I've been totally ignoring the iio user/kernel > api at the moment, waiting for things to settle down there Actually I think there is another way to do it cleanly Keep a flag per device (or per runtime pm struct of device) And on the open/close do if (runtime_pm on device && device has SYSFS_PM set) pm_runtime_foo so that devices that need to be powered up to handle sysfs requests can set a single flag and just work. ^ permalink raw reply [flat|nested] 15+ messages in thread
* RE: sysfs and power management 2010-11-03 9:44 ` Alan Cox @ 2010-11-03 10:48 ` samu.p.onkalo 2010-11-03 13:09 ` Greg KH 1 sibling, 0 replies; 15+ messages in thread From: samu.p.onkalo @ 2010-11-03 10:48 UTC (permalink / raw) To: alan, gregkh; +Cc: hmh, alan, akpm, linux-kernel >-----Original Message----- >From: ext Alan Cox [mailto:alan@linux.intel.com] >Sent: 03 November, 2010 11:45 >To: Greg KH >Cc: Onkalo Samu.P (Nokia-MS/Tampere); ext Henrique de Moraes Holschuh; >Alan Cox; akpm@linux-foundation.org; linux-kernel@vger.kernel.org >Subject: Re: sysfs and power management > >On Mon, 1 Nov 2010 11:07:40 -0700 >Greg KH <gregkh@suse.de> wrote: > >> On Mon, Nov 01, 2010 at 04:57:01PM +0000, Alan Cox wrote: >> > > I took a look to that. It seems that iio is more or less sysfs >> > > based. There are ring buffers and event device which are chardev >> > > based but still the data outside ring buffer and the control is >> > > sysfs based. >> > >> > IIO is sysfs dependant, heavyweight and makes no sense for some of >> > the sysfs based drivers. IIO is also staging based and Linus >> > already threw out the last attempt to unify these drivers sanely >> > with an ALS layer - which was smaller, cleaner and better ! >> >> I think we need to revisit this issue again, before iio is merged to >> the main kernel tree. I've been totally ignoring the iio user/kernel >> api at the moment, waiting for things to settle down there > >Actually I think there is another way to do it cleanly > >Keep a flag per device (or per runtime pm struct of device) > >And on the open/close do > > if (runtime_pm on device && device has SYSFS_PM set) > pm_runtime_foo > >so that devices that need to be powered up to handle sysfs requests can >set a single flag and just work. That is one quite clean way. sysfs_ops still needs function pointer to device core function which does pm_runtime calls. And there is one drawback. Driver doesn't know about new users after the first one. It may want to refresh results whenever a new user appears. But that is probably not a big issue. -Samu ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: sysfs and power management 2010-11-03 9:44 ` Alan Cox 2010-11-03 10:48 ` samu.p.onkalo @ 2010-11-03 13:09 ` Greg KH 2010-11-03 15:00 ` samu.p.onkalo 1 sibling, 1 reply; 15+ messages in thread From: Greg KH @ 2010-11-03 13:09 UTC (permalink / raw) To: Alan Cox Cc: samu.p.onkalo, ext Henrique de Moraes Holschuh, Alan Cox, akpm, linux-kernel On Wed, Nov 03, 2010 at 09:44:52AM +0000, Alan Cox wrote: > On Mon, 1 Nov 2010 11:07:40 -0700 > Greg KH <gregkh@suse.de> wrote: > > > On Mon, Nov 01, 2010 at 04:57:01PM +0000, Alan Cox wrote: > > > > I took a look to that. It seems that iio is more or less sysfs > > > > based. There are ring buffers and event device which are chardev > > > > based but still the data outside ring buffer and the control is > > > > sysfs based. > > > > > > IIO is sysfs dependant, heavyweight and makes no sense for some of > > > the sysfs based drivers. IIO is also staging based and Linus > > > already threw out the last attempt to unify these drivers sanely > > > with an ALS layer - which was smaller, cleaner and better ! > > > > I think we need to revisit this issue again, before iio is merged to > > the main kernel tree. I've been totally ignoring the iio user/kernel > > api at the moment, waiting for things to settle down there > > Actually I think there is another way to do it cleanly > > Keep a flag per device (or per runtime pm struct of device) > > And on the open/close do > > if (runtime_pm on device && device has SYSFS_PM set) > pm_runtime_foo > > so that devices that need to be powered up to handle sysfs requests can > set a single flag and just work. That sounds like a reasonable idea. thanks, greg k-h ^ permalink raw reply [flat|nested] 15+ messages in thread
* RE: sysfs and power management 2010-11-03 13:09 ` Greg KH @ 2010-11-03 15:00 ` samu.p.onkalo 0 siblings, 0 replies; 15+ messages in thread From: samu.p.onkalo @ 2010-11-03 15:00 UTC (permalink / raw) To: gregkh, alan; +Cc: hmh, alan, akpm, linux-kernel >-----Original Message----- >From: ext Greg KH [mailto:gregkh@suse.de] >Sent: 03 November, 2010 15:09 >To: Alan Cox >Cc: Onkalo Samu.P (Nokia-MS/Tampere); ext Henrique de Moraes Holschuh; >Alan Cox; akpm@linux-foundation.org; linux-kernel@vger.kernel.org >Subject: Re: sysfs and power management > >On Wed, Nov 03, 2010 at 09:44:52AM +0000, Alan Cox wrote: >> On Mon, 1 Nov 2010 11:07:40 -0700 >> Greg KH <gregkh@suse.de> wrote: >> >> > On Mon, Nov 01, 2010 at 04:57:01PM +0000, Alan Cox wrote: >> > > > I took a look to that. It seems that iio is more or less sysfs >> > > > based. There are ring buffers and event device which are chardev >> > > > based but still the data outside ring buffer and the control is >> > > > sysfs based. >> > > >> > > IIO is sysfs dependant, heavyweight and makes no sense for some of >> > > the sysfs based drivers. IIO is also staging based and Linus >> > > already threw out the last attempt to unify these drivers sanely >> > > with an ALS layer - which was smaller, cleaner and better ! >> > >> > I think we need to revisit this issue again, before iio is merged to >> > the main kernel tree. I've been totally ignoring the iio >user/kernel >> > api at the moment, waiting for things to settle down there >> >> Actually I think there is another way to do it cleanly >> >> Keep a flag per device (or per runtime pm struct of device) >> >> And on the open/close do >> >> if (runtime_pm on device && device has SYSFS_PM set) >> pm_runtime_foo >> >> so that devices that need to be powered up to handle sysfs requests >can >> set a single flag and just work. > >That sounds like a reasonable idea. > I'm working with the implementation which adds possibility to add per attribute open_close_notification method with minimal overhead to current system. -Samu ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2010-11-03 15:01 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2010-10-27 10:59 sysfs and power management Onkalo Samu 2010-10-27 11:48 ` Alan Cox 2010-10-27 13:43 ` samu.p.onkalo 2010-10-27 14:28 ` Alan Cox 2010-10-29 19:50 ` Greg KH 2010-10-30 14:00 ` Henrique de Moraes Holschuh 2010-10-31 11:57 ` Onkalo Samu 2010-10-31 14:25 ` Greg KH 2010-11-01 10:41 ` Onkalo Samu 2010-11-01 16:57 ` Alan Cox 2010-11-01 18:07 ` Greg KH 2010-11-03 9:44 ` Alan Cox 2010-11-03 10:48 ` samu.p.onkalo 2010-11-03 13:09 ` Greg KH 2010-11-03 15:00 ` samu.p.onkalo
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.