Re: Battery class driver.

From: Richard Hughes
Date: Tue Oct 24 2006 - 13:19:44 EST


On Mon, 2006-10-23 at 21:48 +0100, David Woodhouse wrote:
> On Mon, 2006-10-23 at 20:58 +0100, Richard Hughes wrote:
> > No, I think the distinction between batteries and ac_adapter is large
> > enough to have different classes of devices. You may have many
> > batteries, but you'll only ever have one ac_adapter. I'm not sure it's
> > an obvious abstraction to make.
>
> â machines with more than one individual AC adapter. Which want
> individual notification when one of them goes down.
>
> You're right that they don't necessarily fit into the 'battery' class,
> but I'm not entirely sure if it's worth putting them elsewhere, because
> the information about them is usually available in the same place as the
> information about the batteries, at least in the laptop case.

Sure, makes sense I guess.

> The simple case of AC adapters, where we have only 'present' or
> 'absent', is a subset of what batteries can do. If you have more
> complicated monitoring, then it's _also_ going to bear a remarkable
> similarity to what you get from batteries -- you'll be able to monitor
> temperatures, voltage, current, etc. So they're not _that_ much out of
> place in a 'power supply' class.

Sure, psu could be a nice class. We would need buy-in from ACPI, APM and
PMU maintainers to avoid just creating *another* standard that HAL has
to read.

> It makes _less_ sense, imho, to have 'ac present' as a property of a
> battery -- which is what I've done for now.

Agree.

> > How are battery change notifications delivered to userspace? I know acpi
> > is using the input layer for buttons in the future (very sane IMO), so
> > using sysfs events for each property changing would probably be nice.
>
> For selected properties, yes. I wouldn't want it happening every time
> the current draw changes by a millivolt but for 'battery removed' or 'ac
> power applied' events it makes some sense.

Maybe still send events for large changes, like > whole % changes in
value. Then HAL hasn't got to poll at all.

> For sane hardware where we get an interrupt on such changes, that's fine
> -- but I'm wary of having to implement that by making the kernel poll
> for them; especially if/when there's nothing in userspace which cares.

HAL and gnome-power-manager? There should only be a few changing values
on charging and discharging, and one every percentage point change isn't
a lot.

> > Comments on your patch:
> >
> > > +#define BAT_INFO_TEMP2 (2) /* ÂC/1000 */
> > Temperature expressed in degrees C/1000? - what if the temperature goes
> > below 0?
>
> It's signed.

Sure, n/p.

> > What about just using mK (kelvin / 1000) - I don't know what is
> > used in the kernel elsewhere tho. Also, are you allowed the  sign in
> > kernel source now?
>
> Welcome to the 21st century.

Fair play. :-)

> > > +#define BAT_INFO_CURRENT (6) /* mA */
> > Can't this also be expressed in mW according to the ACPI spec?
>
> No, it can't. The Watt is not a unit of current.

Ahh, current as in electron flow, rather than current power use,
apologies.

> I intended the ACPI 'present rate' to map to the 'charge_rate' property,
> which is why we have the 'charge_unit' property. I don't like that much,
> but it seems necessary unless we're going to do something like separate
> 'charge_rate_mA' and 'charge_rate_mW' properties.

Not sure how to best do this for the kernel - maybe just expose the
value and the format separately. In HAL we normalise the rate to mWh
anyway using the rate in mAh and reported voltage.

> Actually, I suspect that on reflection I would prefer that latter
> option. DavidZ?
>
> > > +#define BAT_STAT_FIRE (1<<7)
> > I know there is precedent for "FIRE" but maybe CRITICAL or DANGER might
> > be better chosen words. We can reserve the word FIRE for when the faulty
> > battery really is going to explode...
>
> Yes, feasibly. I don't quite know what the 'destroy' bit in the OLPC
> embedded controller is supposed to mean, and 'FIRE' seemed as good as
> anything else.

Then maybe just set present to false as a destroyed battery isn't much
use anyway...

Richard.


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/