public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: David Zeuthen <davidz@redhat.com>
To: Greg KH <greg@kroah.com>
Cc: David Woodhouse <dwmw2@infradead.org>,
	linux-kernel@vger.kernel.org, olpc-dev@laptop.org,
	mjg59@srcf.ucam.org, len.brown@intel.com, sfr@canb.auug.org.au,
	benh@kernel.crashing.org
Subject: Re: Battery class driver.
Date: Mon, 23 Oct 2006 21:31:09 -0400	[thread overview]
Message-ID: <1161653469.2801.91.camel@zelda.fubar.dk> (raw)
In-Reply-To: <20061023225905.GA10977@kroah.com>

On Mon, 2006-10-23 at 15:59 -0700, Greg KH wrote:
> > So, perhaps the battery class should provide a file called 'timestamp'
> > or something that is only writable by the super user. If you read from
> > that file it gives the time when the information was last updated. If
> > you write to the file it will force the driver query the hardware and
> > update the other files. Reading any other file than 'timestamp' will
> > just read cached information. 
> 
> You can poll the sysfs file, which means you just sleep until something
> changes in it and then you wake up and read it.  Sound accepatble?

Yea, however I still think there needs to be a 'timestamp' file so 

 a) we don't need to poll every file in /sys/class/battery/BAT0
   (if we had to do that, we would (potentially) wake up N times!); and 

 b) if the kernel ensures that the 'timestamp' file is updated last,
    we get atomic updates (which might matter for drawing pretty graphs,
    guestimating remaining time etc.); and

 c) it provides some mechanism to instruct the driver to go read the
    values from the hardware (if that is what user space wants)
    nonwithstanding that the hardware / driver delivers asynchronous
    updates once in a while via an IRQ.

    Notably user space can see _when_ the values from the hardware was
    retrieved the last time which makes it easier to work around with
    hardware / drivers that don't provide asynchronous updates even
    when they are supposed to (if there is some bug with e.g. the ACPI
    stack).

This implies that the battery class should probably cache the values as
to make the platform driver as simple as possible. I've got a bad
feeling things like caching is badly frowned upon in kernel space but I
thought I'd ask for it anyway :-). I hope I've stated my case :-)

(Anyway, the point is that I want to avoid having an open file
descriptor for each and every file in /sys/class/battery/BAT0 to put it
into the huge poll() stmt in my main loop (granted with things like glib
it's dead simple), that's all.. Caching might also avoid excessive round
trips to the hardware but one can argue both way in most cases.)

So.. how all this relates to hwmon I'm not sure.. looking briefly at
Documentation/hwmon/sysfs-interface no such thing seems to be available,
I'm not sure how libsensors get by here but the problem is sorta
similar. I do think batteries in itself deserves it's own abstraction
instead of using hwmon but I'm no expert on this.

Btw, an OLPC specific feature is also to instruct the Embedded
Controller (EC) to stop delivering IRQ's on battery/ac_adapter changes.
This is so the host CPU won't be woken up when e.g. in e-book reader
mode (or whatever) where the host CPU is supposed to be turned off to
save juice. 
This is simple right now as the EC currently doesn't deliver any IRQ's
at all [1] and I guess a simple sysfs file in the OLPC platform device
will let us do that once we actually get IRQ delivery. Thus, I'm not
sure "stop IRQ delivery" belongs in the battery class proper. This is
also why we need device link to the OLPC platform device.

     David

[1] : but see http://dev.laptop.org/ticket/224




  reply	other threads:[~2006-10-24  1:31 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-10-23 18:20 Battery class driver David Woodhouse
2006-10-23 18:26 ` Matthew Garrett
2006-10-23 18:30   ` David Woodhouse
2006-10-24  3:36   ` Benjamin Herrenschmidt
2006-10-23 18:30 ` Greg KH
2006-10-23 18:32   ` Greg KH
2006-10-23 18:50   ` David Woodhouse
2006-10-24  3:39     ` Benjamin Herrenschmidt
2006-10-23 21:04   ` Jean Delvare
2006-10-23 22:15 ` David Zeuthen
2006-10-23 22:59   ` Greg KH
2006-10-24  1:31     ` David Zeuthen [this message]
2006-10-24  3:04       ` Shem Multinymous
2006-10-24  2:56   ` Shem Multinymous
2006-10-24  3:27     ` Matthew Garrett
2006-10-24  3:48       ` Benjamin Herrenschmidt
2006-10-24  3:53         ` Matthew Garrett
2006-10-24  5:43           ` Benjamin Herrenschmidt
2006-10-24 11:09           ` Shem Multinymous
2006-10-24  2:04 ` Shem Multinymous
2006-10-25 10:45 ` Pavel Machek
     [not found] <1161628327.19446.391.camel@pmac.infradead.org>
2006-10-23 19:18 ` Dan Williams
2006-10-23 19:58   ` Richard Hughes
2006-10-23 20:10     ` Roland Dreier
2006-10-23 20:48     ` David Woodhouse
2006-10-24  3:44       ` Benjamin Herrenschmidt
2006-10-24 17:18       ` Richard Hughes
2006-10-24  3:41     ` Benjamin Herrenschmidt

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=1161653469.2801.91.camel@zelda.fubar.dk \
    --to=davidz@redhat.com \
    --cc=benh@kernel.crashing.org \
    --cc=dwmw2@infradead.org \
    --cc=greg@kroah.com \
    --cc=len.brown@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mjg59@srcf.ucam.org \
    --cc=olpc-dev@laptop.org \
    --cc=sfr@canb.auug.org.au \
    /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