public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: "Éric Piel" <Eric.Piel@tremplin-utc.net>
To: samu.p.onkalo@nokia.com
Cc: pavel@ucw.cz, daniel@caiaq.de, lm-sensors@lm-sensors.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 0/6] lis3lv02d: Power management, click and threshold interrupts
Date: Fri, 12 Feb 2010 13:21:15 +0100	[thread overview]
Message-ID: <4B7547BB.1020900@tremplin-utc.net> (raw)
In-Reply-To: <62697B07E9803846BC582181BD6FB6B82661C5029E@NOK-EUMSG-02.mgdnok.nokia.com>

Op 12-02-10 07:31, samu.p.onkalo@nokia.com schreef:
>> The only hiccup is in patch 6: it declares the joystick with 3 buttons,
>> although on this hardware it's impossible to have this feature. So, it
>> would be nice if input_set_capability() could be conditionned.
> 
> I'll add configuration option to platform data or do you prefer some other
> way to enable this?

> You obviously meant that enable those only for 8 bit device, since
> 12 bit doesn't have click detection. I can do that.
> 
Perfect!

>> I also fully agree with the comments from others that it would be better
>> if it could use the special runtime power-management framework from
>> Rafael.
> 
> It looks like the corrent way to do that. However, I don't have suitable environment
> to do that right now. I need to study this more if I can somehow verify it.
That would be great :-)

>  BTW, what is exactly the usecase for the "active" sysfs
>> attribute? To me, it looks like it just prevents userspace from using
>> the position attribute, while the joystick interface is still
>> accessible, so it is not very effective!
> 
> That is exactly what it does. Input and freefall interfaces control the chip state
> separately. So, this is only for sysfs interface since driver doesn't know
> if somebody is using sysfs interface.
> 
> So this is the logic:
> - If application wants to use sysfs, it must set 'active' to 1 (which is the default to
> maintain backward compatibility).
> Otherwise chip state is controlled by the number of users in
> input and freefall device handles.
> When 'active' is 0, only input / freefall device handles controls
> the chip state.
> 
> When active is 0 but chip is enabled via input / freefall entries,
> position entry returns error code. This is done to prevent the case where
> chip is suddenly powered down when input  / freefall devices are closed. 
> So position entry is working only when it is separately  enabled.
> 
> When there are no users at all, chip is powered down. 
> 
I understand. The main problem is that powering up and down the chip
every time someone opens the sysfs position attribute is very expensive.

Having an explicit attribute to turn on/off the powersaving means that
the userspace must be aware of it. This is a burden, and especially if
you run your userspace application as a normal user, while the active
attribute obviously requires root level.

Previously, when I implemented the auto power down feature, I had put a
timer after 30s of usage to work around this. It's obviously not
optimal, but it has the advantage of hiding completely the powersaving
feature from userspace. Do you think it would be fine it implement it
this way?

See you,
Eric

      parent reply	other threads:[~2010-02-12 12:21 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-02-04  8:24 [PATCH 0/6] lis3lv02d: Power management, click and threshold interrupts Samu Onkalo
2010-02-04  8:24 ` [PATCH 1/6] lis3: Add missing constants for 8bit device Samu Onkalo
2010-02-04  8:24 ` [PATCH 2/6] lis3: Separate configuration function for 8 bit device Samu Onkalo
2010-02-04  8:24 ` [PATCH 3/6] lis3: Introduce platform data for second ff / wu unit Samu Onkalo
2010-02-04  8:24 ` [PATCH 4/6] lis3: Power control for the chip Samu Onkalo
2010-02-04  8:24 ` [PATCH 5/6] lis3: Add skeletons for interrupt handlers Samu Onkalo
2010-02-04  8:24 ` [PATCH 6/6] lis3: Interrupt handlers for 8bit wakeup and click events Samu Onkalo
2010-02-05  2:12 ` [PATCH 0/6] lis3lv02d: Power management, click and threshold interrupts Daniel Mack
2010-02-11 19:01 ` Daniel Mack
2010-02-11 19:17 ` Éric Piel
2010-02-12  6:31   ` samu.p.onkalo
2010-02-12  9:56     ` [lm-sensors] " samu.p.onkalo
2010-02-12 12:21     ` Éric Piel [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4B7547BB.1020900@tremplin-utc.net \
    --to=eric.piel@tremplin-utc.net \
    --cc=daniel@caiaq.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lm-sensors@lm-sensors.org \
    --cc=pavel@ucw.cz \
    --cc=samu.p.onkalo@nokia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox