Re: [PATCH 3/7] [RFC] Battery monitoring class
Hi! > > I'll convert mXh to uXh a bit later, if there will no further objections > > against uXh. Also I'd like to hear if there any objections on > > mA/mV -> uA/uV conversion. I think we'd better keep all units at the > > same order/precision. > > Okay, would it make sense to use "long" instead of "int" after "milli" to > "micro" conversion? On 32 bit machines int gives +-2147483648 limit. So > 2147 volts/amperes/... long == int on 32bit machines. > Though 2147 amperes is unrealistic for batteries, but if used in > calculations it could be dangerous. Let the one doing calculations handle that ;-). Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/7] [RFC] Battery monitoring class
Hi! I'll convert mXh to uXh a bit later, if there will no further objections against uXh. Also I'd like to hear if there any objections on mA/mV - uA/uV conversion. I think we'd better keep all units at the same order/precision. Okay, would it make sense to use long instead of int after milli to micro conversion? On 32 bit machines int gives +-2147483648 limit. So 2147 volts/amperes/... long == int on 32bit machines. Though 2147 amperes is unrealistic for batteries, but if used in calculations it could be dangerous. Let the one doing calculations handle that ;-). Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Kernel-discuss] Re: [PATCH 3/7] [RFC] Battery monitoring class
Hi! > > > > That said, you may need to use uWh and uAh instead of mAh and mWh, > > > > though. > > > > > > Not sure. Is there any existing chip that can report uAh/uWh? That is > > > great precision. > > > > The way things are going, it should be feasible for small embedded systems > > quite soon. Refer to the previous thread. > > I see... is it also applicable to currents and voltages? I.e. should we > use uA and uV from the start? AFAICT, mobile phone in standby can eat less than 1000 uW... so uA/uV would indeed be nice. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Kernel-discuss] Re: [PATCH 3/7] [RFC] Battery monitoring class
Hi! That said, you may need to use uWh and uAh instead of mAh and mWh, though. Not sure. Is there any existing chip that can report uAh/uWh? That is great precision. The way things are going, it should be feasible for small embedded systems quite soon. Refer to the previous thread. I see... is it also applicable to currents and voltages? I.e. should we use uA and uV from the start? AFAICT, mobile phone in standby can eat less than 1000 uW... so uA/uV would indeed be nice. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/7] [RFC] Battery monitoring class
> > > Also, if you your battery can collect and report its approximated values > > > in additional to momentary values, you're free to add _AVG attributes > > > to standard ones and use them. > > > > No, that won't help much. IMO, we want the sanest set of standard > > attributes we can get, and weird as it might be, average reporting are > > common properties of battery control firmware on laptops (maybe because of > > SBS, but still...). > > Why that won't help? Is there something else besides momentary and > hardware-averaged values? It wouldn't help much to not have standard generic attributes for the averaged measurements. That said, yes, there are relative measurements (percent), and time measurements (some battery firmware can tell you how much *time* it will take to stop recharing, empty the battery, etc). > I guess the only question for you is whether we would use, "attr" as momentary > and "attr_avg" as averaged by hardware value, or will use "attr" as averaged > values, and something alike "attr_now" for momentary? And you voting for > "attr" being averaged and "attr_now" momentary. I don't care either way, as long as it is *documented* what is averaged, and what is instantaneous. -- "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 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Kernel-discuss] Re: [PATCH 3/7] [RFC] Battery monitoring class
On Mon, 2007-04-16 at 07:12 +0400, Anton Vorontsov wrote: > Why? With current battery class we can do whatever everyone needs. No > need for wrappers. > Because of your original design, simple batteries are stay simple, and > no noticing that there is some "complicated" attributes exists at all. > That's indeed great characteristic of that *universal* battery class. Indeed. Im just trying to make sure we dont bloat an otherwise very simple class. A word of caution only. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Kernel-discuss] Re: [PATCH 3/7] [RFC] Battery monitoring class
On Mon, 2007-04-16 at 07:12 +0400, Anton Vorontsov wrote: Why? With current battery class we can do whatever everyone needs. No need for wrappers. cut Because of your original design, simple batteries are stay simple, and no noticing that there is some complicated attributes exists at all. That's indeed great characteristic of that *universal* battery class. Indeed. Im just trying to make sure we dont bloat an otherwise very simple class. A word of caution only. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/7] [RFC] Battery monitoring class
Also, if you your battery can collect and report its approximated values in additional to momentary values, you're free to add _AVG attributes to standard ones and use them. No, that won't help much. IMO, we want the sanest set of standard attributes we can get, and weird as it might be, average reporting are common properties of battery control firmware on laptops (maybe because of SBS, but still...). Why that won't help? Is there something else besides momentary and hardware-averaged values? It wouldn't help much to not have standard generic attributes for the averaged measurements. That said, yes, there are relative measurements (percent), and time measurements (some battery firmware can tell you how much *time* it will take to stop recharing, empty the battery, etc). I guess the only question for you is whether we would use, attr as momentary and attr_avg as averaged by hardware value, or will use attr as averaged values, and something alike attr_now for momentary? And you voting for attr being averaged and attr_now momentary. I don't care either way, as long as it is *documented* what is averaged, and what is instantaneous. -- 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 - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Kernel-discuss] Re: [PATCH 3/7] [RFC] Battery monitoring class
On Mon, Apr 16, 2007 at 03:32:54AM +0100, ian wrote: > On Sun, 2007-04-15 at 21:57 -0300, Henrique de Moraes Holschuh wrote: > > > > No, that won't help much. IMO, we want the sanest set of standard > > attributes we can get, and weird as it might be, average reporting are > > common properties of battery control firmware on laptops (maybe > > because of SBS, but still...). > > We need to think very carefully here. > > charge, current, capacity, etc. are properties all batteries have, and > the current values can all be sampled instantaneously. > > funky values processed by 'black box' firmware are not universal > properties of all batteries. > > IOW, battery class should be for 'simple' battery types only. > > perhaps rename it to simple battery class to make it distinct? > > Userspace is the place to put the complications, in any case, and I see > nothing wrong with having both a simple battery class AND other > proprietary battery class an SBS battery class. (or a > toshiba_proprietary_bios battery class or whatever). > > Perhaps we need a 'libbattery.so' so that userspace can have a nice > consistent interface? Why? With current battery class we can do whatever everyone needs. No need for wrappers. Adding new properties is cheap and easy. Simple batteries using only "simple" properties/attributes, and complicated batteries using complicated attributes. Because of your original design, simple batteries are stay simple, and no noticing that there is some "complicated" attributes exists at all. That's indeed great characteristic of that *universal* battery class. For example, ds2760 is not really "simple" monitoring chip. ADC battery is (it's in -hh tree so far). So, ds2760 is somewhere in between SBS design, and dumb ADC batteries. So, my another purposal, which I very like now: Let's do self-documented properties. current_now, current_avg, e.t.c. No more just "current", lets remove any ambiguousness! -- Anton Vorontsov email: [EMAIL PROTECTED] backup email: [EMAIL PROTECTED] irc://irc.freenode.org/bd2 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Kernel-discuss] Re: [PATCH 3/7] [RFC] Battery monitoring class
On Sun, 2007-04-15 at 21:57 -0300, Henrique de Moraes Holschuh wrote: > > No, that won't help much. IMO, we want the sanest set of standard > attributes we can get, and weird as it might be, average reporting are > common properties of battery control firmware on laptops (maybe > because of SBS, but still...). We need to think very carefully here. charge, current, capacity, etc. are properties all batteries have, and the current values can all be sampled instantaneously. funky values processed by 'black box' firmware are not universal properties of all batteries. IOW, battery class should be for 'simple' battery types only. perhaps rename it to simple battery class to make it distinct? Userspace is the place to put the complications, in any case, and I see nothing wrong with having both a simple battery class AND other proprietary battery class an SBS battery class. (or a toshiba_proprietary_bios battery class or whatever). Perhaps we need a 'libbattery.so' so that userspace can have a nice consistent interface? - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/7] [RFC] Battery monitoring class
On Sun, Apr 15, 2007 at 09:57:22PM -0300, Henrique de Moraes Holschuh wrote: > On Mon, 16 Apr 2007, Anton Vorontsov wrote: > > Current battery class assumes values are not averaged. I.e. momentary > > values. In general, it's userspace' job to collect statistics. Though, > > if hardware can report only average values, it's just okay to use > > usual attributes. > > What about SBS-style battery firmware, which can report *both* ? This > includes just about all ThinkPads in the last five years, so we are talking > about a damn big lot of machines... Sure, no problem. I'm taking away my words "if hardware can report only average values, it's just okay to use usual attributes", and replacing them by "if hardware can report only average values, it should use _AVG attributes". So, this scheme should work now: 1. If userspace (nice GUI app) seeing _avg values, it will use these. 2. If userspace seeing no _avg values, it is using its own statistics mechanism, i.e. collecting information from momentary attributes. 3. If userspace seeing both, it can decide, most probably it will decide to use hardware averaged values, i.e. _avg. > I'd really appreciate if there is a standard way to communicate both. And > it is probably a good idea to define what should be averaged, and what > should be instantaneous when that matters, that way userspace actually has a > chance at not doing something braindamaged. > > Actually, IMHO, every attribute and alarm from SBS should be somehow > losslessly translatable to standard class attributes from day one, unless it > is something that makes no sense at all (and there is precious little of > that in the latest version of SBS, thankfully...). > > > Also, if you your battery can collect and report its approximated values > > in additional to momentary values, you're free to add _AVG attributes > > to standard ones and use them. > > No, that won't help much. IMO, we want the sanest set of standard > attributes we can get, and weird as it might be, average reporting are > common properties of battery control firmware on laptops (maybe because of > SBS, but still...). Why that won't help? Is there something else besides momentary and hardware-averaged values? > We don't have to get it perfect at the first try, but I really think we are > getting a bit too far from "as good as we can make it" at the first attempt > if we don't take the SBS into account properly, given the ammount of > circuits and firmware out there that are shaped along the SBS guidelines. I guess the only question for you is whether we would use, "attr" as momentary and "attr_avg" as averaged by hardware value, or will use "attr" as averaged values, and something alike "attr_now" for momentary? And you voting for "attr" being averaged and "attr_now" momentary. Actually, I don't see much difference, except default assumption of "attr" being averaged/momentary. Though, if it's real issue for you or SBS conformity, I can surely rename attrs. > -- > "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 > And again, much thanks for your comments! They're really helpful. -- Anton Vorontsov email: [EMAIL PROTECTED] backup email: [EMAIL PROTECTED] irc://irc.freenode.org/bd2 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/7] [RFC] Battery monitoring class
On Mon, 16 Apr 2007, Anton Vorontsov wrote: > Current battery class assumes values are not averaged. I.e. momentary > values. In general, it's userspace' job to collect statistics. Though, > if hardware can report only average values, it's just okay to use > usual attributes. What about SBS-style battery firmware, which can report *both* ? This includes just about all ThinkPads in the last five years, so we are talking about a damn big lot of machines... I'd really appreciate if there is a standard way to communicate both. And it is probably a good idea to define what should be averaged, and what should be instantaneous when that matters, that way userspace actually has a chance at not doing something braindamaged. Actually, IMHO, every attribute and alarm from SBS should be somehow losslessly translatable to standard class attributes from day one, unless it is something that makes no sense at all (and there is precious little of that in the latest version of SBS, thankfully...). > Also, if you your battery can collect and report its approximated values > in additional to momentary values, you're free to add _AVG attributes > to standard ones and use them. No, that won't help much. IMO, we want the sanest set of standard attributes we can get, and weird as it might be, average reporting are common properties of battery control firmware on laptops (maybe because of SBS, but still...). We don't have to get it perfect at the first try, but I really think we are getting a bit too far from "as good as we can make it" at the first attempt if we don't take the SBS into account properly, given the ammount of circuits and firmware out there that are shaped along the SBS guidelines. -- "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 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/7] [RFC] Battery monitoring class
Hi, On Mon, Apr 16, 2007 at 12:08:54AM +0200, Ondrej Zajicek wrote: > On Thu, Apr 12, 2007 at 03:25:03AM +0400, Anton Vorontsov wrote: > > Here is battery monitor class. According to first copyright string, we're > > maintaining it since 2003. I've took few days and cleaned it up to be > > more suitable for mainline inclusion. > > Just some ideas: > > - what about using exponents in values? > For example file "voltage" could contain "123 -3" to represent 123 mV. > Exponents could be hardcoded in drivers according to device's range > (so there is no complication in it), but interface is usable in great > range of values. And it is pretty easy to use from userspace. No, sorry. Common units is main goal of that class. If you're saying "energy" you always know that it's uWh. It's better for both userspace (don't bother to parse anything from kernel) and kernel itself. No need to invent new kernel<->userspace protocols, no need to do useless string manipulations in kernel itself. > - interface should allow to present values which are some monotonic > functions of common physical properties. For example when we know > where are some raw data from sensor, but we don't know where are > calibration tables to be able to compute value in some standard unit > (as V for voltage) - in this case it is better to show raw data > (or raw data after some transformation which makes them monotonic) > and specify that this is raw data than show nothing. > > - it would be nice to know whether presented value is from some > measurement or it is (inaccurate) estimation. Current battery class assumes values are not averaged. I.e. momentary values. In general, it's userspace' job to collect statistics. Though, if hardware can report only average values, it's just okay to use usual attributes. Also, if you your battery can collect and report its approximated values in additional to momentary values, you're free to add _AVG attributes to standard ones and use them. > -- > Elen sila lumenn' omentielvo > > Ondrej 'SanTiago' Zajicek (email: [EMAIL PROTECTED], jabber: [EMAIL > PROTECTED]) > OpenPGP encrypted e-mails preferred (KeyID 0x11DEADC3, wwwkeys.pgp.net) > "To err is human -- to blame it on a computer is even more so." Thanks for comments! -- Anton Vorontsov email: [EMAIL PROTECTED] backup email: [EMAIL PROTECTED] irc://irc.freenode.org/bd2 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/7] [RFC] Battery monitoring class
On Thu, Apr 12, 2007 at 03:25:03AM +0400, Anton Vorontsov wrote: > Here is battery monitor class. According to first copyright string, we're > maintaining it since 2003. I've took few days and cleaned it up to be > more suitable for mainline inclusion. Just some ideas: - what about using exponents in values? For example file "voltage" could contain "123 -3" to represent 123 mV. Exponents could be hardcoded in drivers according to device's range (so there is no complication in it), but interface is usable in great range of values. And it is pretty easy to use from userspace. - interface should allow to present values which are some monotonic functions of common physical properties. For example when we know where are some raw data from sensor, but we don't know where are calibration tables to be able to compute value in some standard unit (as V for voltage) - in this case it is better to show raw data (or raw data after some transformation which makes them monotonic) and specify that this is raw data than show nothing. - it would be nice to know whether presented value is from some measurement or it is (inaccurate) estimation. -- Elen sila lumenn' omentielvo Ondrej 'SanTiago' Zajicek (email: [EMAIL PROTECTED], jabber: [EMAIL PROTECTED]) OpenPGP encrypted e-mails preferred (KeyID 0x11DEADC3, wwwkeys.pgp.net) "To err is human -- to blame it on a computer is even more so." - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Kernel-discuss] Re: [PATCH 3/7] [RFC] Battery monitoring class
Hello Pavel, On Sun, Apr 15, 2007 at 07:56:56PM +, Pavel Machek wrote: > Hi! > > > * It insists on reusing its predefined attributes *and* their units. > > So, userspace getting expected values for any battery. > > > > Also common units is required for APM/ACPI emulation. > > > > Though our battery class insisting on re-usage, but not forces it. If some > > battery driver can't convert its own raw values (can't imagine why), then > > driver is free to implement its own attributes *and* additional _units > > attribute. Though, this scheme is discouraged. > > > > * LEDs support. Each battery register its trigger, and gadgets with LEDs > > can quickly bind to battery-charging / battery-full triggers. > > > > Here how it looks like from user space: > > > > # ls /sys/class/battery/main-battery/ > > capacity max_capacity max_voltage min_current power subsystem uevent > > current max_current min_capacity min_voltage status temp > > voltage > > # cat /sys/class/battery/main-battery/status > > Full > > # cat /sys/class/leds/h5400\:green-right/trigger > > none h5400-radio timer hwtimer main-battery-charging [main-battery-full] > > # cat /sys/class/leds/h5400\:green-right/brightness > > 255 > > Can we get few lines in Documentation? Yes, sure. > I guess min_capacity is > shutdown capacity at current temperature, but its surely non-obvious. > > Will min_capacity increase as batery gets old? Or will max_capacity > decrease? min_capacity and max_capacity depend on temperature. But some drivers can/want to interpolate, others can't/don't want. It's driver's matter. ds2760_battery just remembers last capacity when current was < 10 mA (i.e. battery charged full at given temperature) as max_capacity, and takes this as reference for calculations. > (Should we introduce design_capacity for ACPI systems that > know the difference?) Yup, I've introduced it in [take2] (it's in this thread, thus it's hard to find). Will resend it as separate thread soon, along with few other changes. > What is min_current? Granularity of amper meter? > > And min_voltage is shutdown voltage? Yes, should be. But, for example, ds2760 battery can't remember this value in its eeprom (iirc), thus this value passed by platform code, i.e. in our case (iPaqs) it's machine dependent value. Probably we should not even export this attribute in ds2760_battery driver, as it does not take any part in calculations. Plus this attribute highly depend on battery chemistry and temperature. Thus it's hardly anyhow useful, except if hardware itself reports it (not ds2760 case). > Otherwise it looks good to me. Something like this is really needed. Thanks! > > + * All voltages, currents, capacities and temperatures in mV, mA, mAh and > > + * tenths of a degree unless otherwise stated. It's driver's job to convert > > tenths of degree Celsius? Yup, fixed in [take2]. > > +#define BATTERY_STATUS_UNKNOWN 0 > > +#define BATTERY_STATUS_CHARGING 1 > > +#define BATTERY_STATUS_DISCHARGING 2 > > +#define BATTERY_STATUS_NOT_CHARGING 3 > > +#define BATTERY_STATUS_FULL 4 > > Perhaps we need STATUS_ERROR? At least on some machines it is > different from STATUS_NOT_CHARGING. I'm unsure about this. BATTERY_STATUS_* is mostly about charging process status (should I rename it to more verbose BATTERY_CHARGING_STATUS_, or just mention it in Documentation?). For errors things it might be better to create "health" attribute. > > + /* private */ > > + struct power_supplicant pst; > > + > > + #ifdef CONFIG_LEDS_TRIGGERS > > + struct led_trigger *charging_trig; > > + char *charging_trig_name; > > + struct led_trigger *full_trig; > > + char *full_trig_name; > > + #endif > > +}; > > #ifdefs need to be at column 0? Yup, fixed in [take2]. > > +/* > > + * This is recommended structure to specify static battery parameters. > > + * Generic one, parametrizable for different batteries. Battery device > > + * itself does bot use it, but that's what implementing most drivers, > > 'does not'? Thanks, will fix! > Pavel > -- > (english) http://www.livejournal.com/~pavelmachek > (cesky, pictures) > http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html Thanks, -- Anton Vorontsov email: [EMAIL PROTECTED] backup email: [EMAIL PROTECTED] irc://irc.freenode.org/bd2 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/7] [RFC] Battery monitoring class
Hi! > * It insists on reusing its predefined attributes *and* their units. > So, userspace getting expected values for any battery. > > Also common units is required for APM/ACPI emulation. > > Though our battery class insisting on re-usage, but not forces it. If some > battery driver can't convert its own raw values (can't imagine why), then > driver is free to implement its own attributes *and* additional _units > attribute. Though, this scheme is discouraged. > > * LEDs support. Each battery register its trigger, and gadgets with LEDs > can quickly bind to battery-charging / battery-full triggers. > > Here how it looks like from user space: > > # ls /sys/class/battery/main-battery/ > capacity max_capacity max_voltage min_current power subsystem uevent > current max_current min_capacity min_voltage status temp voltage > # cat /sys/class/battery/main-battery/status > Full > # cat /sys/class/leds/h5400\:green-right/trigger > none h5400-radio timer hwtimer main-battery-charging [main-battery-full] > # cat /sys/class/leds/h5400\:green-right/brightness > 255 Can we get few lines in Documentation? I guess min_capacity is shutdown capacity at current temperature, but its surely non-obvious. Will min_capacity increase as batery gets old? Or will max_capacity decrease? (Should we introduce design_capacity for ACPI systems that know the difference?) What is min_current? Granularity of amper meter? And min_voltage is shutdown voltage? Otherwise it looks good to me. Something like this is really needed. > + * All voltages, currents, capacities and temperatures in mV, mA, mAh and > + * tenths of a degree unless otherwise stated. It's driver's job to convert tenths of degree Celsius? Pavel > +#define BATTERY_STATUS_UNKNOWN 0 > +#define BATTERY_STATUS_CHARGING 1 > +#define BATTERY_STATUS_DISCHARGING 2 > +#define BATTERY_STATUS_NOT_CHARGING 3 > +#define BATTERY_STATUS_FULL 4 Perhaps we need STATUS_ERROR? At least on some machines it is different from STATUS_NOT_CHARGING. > + /* private */ > + struct power_supplicant pst; > + > + #ifdef CONFIG_LEDS_TRIGGERS > + struct led_trigger *charging_trig; > + char *charging_trig_name; > + struct led_trigger *full_trig; > + char *full_trig_name; > + #endif > +}; #ifdefs need to be at column 0? > +/* > + * This is recommended structure to specify static battery parameters. > + * Generic one, parametrizable for different batteries. Battery device > + * itself does bot use it, but that's what implementing most drivers, 'does not'? Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/7] [RFC] Battery monitoring class
On Thu, Apr 12, 2007 at 03:25:03AM +0400, Anton Vorontsov wrote: Here is battery monitor class. According to first copyright string, we're maintaining it since 2003. I've took few days and cleaned it up to be more suitable for mainline inclusion. Just some ideas: - what about using exponents in values? For example file voltage could contain 123 -3 to represent 123 mV. Exponents could be hardcoded in drivers according to device's range (so there is no complication in it), but interface is usable in great range of values. And it is pretty easy to use from userspace. - interface should allow to present values which are some monotonic functions of common physical properties. For example when we know where are some raw data from sensor, but we don't know where are calibration tables to be able to compute value in some standard unit (as V for voltage) - in this case it is better to show raw data (or raw data after some transformation which makes them monotonic) and specify that this is raw data than show nothing. - it would be nice to know whether presented value is from some measurement or it is (inaccurate) estimation. -- Elen sila lumenn' omentielvo Ondrej 'SanTiago' Zajicek (email: [EMAIL PROTECTED], jabber: [EMAIL PROTECTED]) OpenPGP encrypted e-mails preferred (KeyID 0x11DEADC3, wwwkeys.pgp.net) To err is human -- to blame it on a computer is even more so. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/7] [RFC] Battery monitoring class
Hi, On Mon, Apr 16, 2007 at 12:08:54AM +0200, Ondrej Zajicek wrote: On Thu, Apr 12, 2007 at 03:25:03AM +0400, Anton Vorontsov wrote: Here is battery monitor class. According to first copyright string, we're maintaining it since 2003. I've took few days and cleaned it up to be more suitable for mainline inclusion. Just some ideas: - what about using exponents in values? For example file voltage could contain 123 -3 to represent 123 mV. Exponents could be hardcoded in drivers according to device's range (so there is no complication in it), but interface is usable in great range of values. And it is pretty easy to use from userspace. No, sorry. Common units is main goal of that class. If you're saying energy you always know that it's uWh. It's better for both userspace (don't bother to parse anything from kernel) and kernel itself. No need to invent new kernel-userspace protocols, no need to do useless string manipulations in kernel itself. - interface should allow to present values which are some monotonic functions of common physical properties. For example when we know where are some raw data from sensor, but we don't know where are calibration tables to be able to compute value in some standard unit (as V for voltage) - in this case it is better to show raw data (or raw data after some transformation which makes them monotonic) and specify that this is raw data than show nothing. - it would be nice to know whether presented value is from some measurement or it is (inaccurate) estimation. Current battery class assumes values are not averaged. I.e. momentary values. In general, it's userspace' job to collect statistics. Though, if hardware can report only average values, it's just okay to use usual attributes. Also, if you your battery can collect and report its approximated values in additional to momentary values, you're free to add _AVG attributes to standard ones and use them. -- Elen sila lumenn' omentielvo Ondrej 'SanTiago' Zajicek (email: [EMAIL PROTECTED], jabber: [EMAIL PROTECTED]) OpenPGP encrypted e-mails preferred (KeyID 0x11DEADC3, wwwkeys.pgp.net) To err is human -- to blame it on a computer is even more so. Thanks for comments! -- Anton Vorontsov email: [EMAIL PROTECTED] backup email: [EMAIL PROTECTED] irc://irc.freenode.org/bd2 - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/7] [RFC] Battery monitoring class
On Mon, 16 Apr 2007, Anton Vorontsov wrote: Current battery class assumes values are not averaged. I.e. momentary values. In general, it's userspace' job to collect statistics. Though, if hardware can report only average values, it's just okay to use usual attributes. What about SBS-style battery firmware, which can report *both* ? This includes just about all ThinkPads in the last five years, so we are talking about a damn big lot of machines... I'd really appreciate if there is a standard way to communicate both. And it is probably a good idea to define what should be averaged, and what should be instantaneous when that matters, that way userspace actually has a chance at not doing something braindamaged. Actually, IMHO, every attribute and alarm from SBS should be somehow losslessly translatable to standard class attributes from day one, unless it is something that makes no sense at all (and there is precious little of that in the latest version of SBS, thankfully...). Also, if you your battery can collect and report its approximated values in additional to momentary values, you're free to add _AVG attributes to standard ones and use them. No, that won't help much. IMO, we want the sanest set of standard attributes we can get, and weird as it might be, average reporting are common properties of battery control firmware on laptops (maybe because of SBS, but still...). We don't have to get it perfect at the first try, but I really think we are getting a bit too far from as good as we can make it at the first attempt if we don't take the SBS into account properly, given the ammount of circuits and firmware out there that are shaped along the SBS guidelines. -- 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 - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/7] [RFC] Battery monitoring class
On Sun, Apr 15, 2007 at 09:57:22PM -0300, Henrique de Moraes Holschuh wrote: On Mon, 16 Apr 2007, Anton Vorontsov wrote: Current battery class assumes values are not averaged. I.e. momentary values. In general, it's userspace' job to collect statistics. Though, if hardware can report only average values, it's just okay to use usual attributes. What about SBS-style battery firmware, which can report *both* ? This includes just about all ThinkPads in the last five years, so we are talking about a damn big lot of machines... Sure, no problem. I'm taking away my words if hardware can report only average values, it's just okay to use usual attributes, and replacing them by if hardware can report only average values, it should use _AVG attributes. So, this scheme should work now: 1. If userspace (nice GUI app) seeing _avg values, it will use these. 2. If userspace seeing no _avg values, it is using its own statistics mechanism, i.e. collecting information from momentary attributes. 3. If userspace seeing both, it can decide, most probably it will decide to use hardware averaged values, i.e. _avg. I'd really appreciate if there is a standard way to communicate both. And it is probably a good idea to define what should be averaged, and what should be instantaneous when that matters, that way userspace actually has a chance at not doing something braindamaged. Actually, IMHO, every attribute and alarm from SBS should be somehow losslessly translatable to standard class attributes from day one, unless it is something that makes no sense at all (and there is precious little of that in the latest version of SBS, thankfully...). Also, if you your battery can collect and report its approximated values in additional to momentary values, you're free to add _AVG attributes to standard ones and use them. No, that won't help much. IMO, we want the sanest set of standard attributes we can get, and weird as it might be, average reporting are common properties of battery control firmware on laptops (maybe because of SBS, but still...). Why that won't help? Is there something else besides momentary and hardware-averaged values? We don't have to get it perfect at the first try, but I really think we are getting a bit too far from as good as we can make it at the first attempt if we don't take the SBS into account properly, given the ammount of circuits and firmware out there that are shaped along the SBS guidelines. I guess the only question for you is whether we would use, attr as momentary and attr_avg as averaged by hardware value, or will use attr as averaged values, and something alike attr_now for momentary? And you voting for attr being averaged and attr_now momentary. Actually, I don't see much difference, except default assumption of attr being averaged/momentary. Though, if it's real issue for you or SBS conformity, I can surely rename attrs. -- 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 And again, much thanks for your comments! They're really helpful. -- Anton Vorontsov email: [EMAIL PROTECTED] backup email: [EMAIL PROTECTED] irc://irc.freenode.org/bd2 - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Kernel-discuss] Re: [PATCH 3/7] [RFC] Battery monitoring class
On Sun, 2007-04-15 at 21:57 -0300, Henrique de Moraes Holschuh wrote: No, that won't help much. IMO, we want the sanest set of standard attributes we can get, and weird as it might be, average reporting are common properties of battery control firmware on laptops (maybe because of SBS, but still...). We need to think very carefully here. charge, current, capacity, etc. are properties all batteries have, and the current values can all be sampled instantaneously. funky values processed by 'black box' firmware are not universal properties of all batteries. IOW, battery class should be for 'simple' battery types only. perhaps rename it to simple battery class to make it distinct? Userspace is the place to put the complications, in any case, and I see nothing wrong with having both a simple battery class AND other proprietary battery class an SBS battery class. (or a toshiba_proprietary_bios battery class or whatever). Perhaps we need a 'libbattery.so' so that userspace can have a nice consistent interface? - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Kernel-discuss] Re: [PATCH 3/7] [RFC] Battery monitoring class
On Mon, Apr 16, 2007 at 03:32:54AM +0100, ian wrote: On Sun, 2007-04-15 at 21:57 -0300, Henrique de Moraes Holschuh wrote: No, that won't help much. IMO, we want the sanest set of standard attributes we can get, and weird as it might be, average reporting are common properties of battery control firmware on laptops (maybe because of SBS, but still...). We need to think very carefully here. charge, current, capacity, etc. are properties all batteries have, and the current values can all be sampled instantaneously. funky values processed by 'black box' firmware are not universal properties of all batteries. IOW, battery class should be for 'simple' battery types only. perhaps rename it to simple battery class to make it distinct? Userspace is the place to put the complications, in any case, and I see nothing wrong with having both a simple battery class AND other proprietary battery class an SBS battery class. (or a toshiba_proprietary_bios battery class or whatever). Perhaps we need a 'libbattery.so' so that userspace can have a nice consistent interface? Why? With current battery class we can do whatever everyone needs. No need for wrappers. Adding new properties is cheap and easy. Simple batteries using only simple properties/attributes, and complicated batteries using complicated attributes. Because of your original design, simple batteries are stay simple, and no noticing that there is some complicated attributes exists at all. That's indeed great characteristic of that *universal* battery class. For example, ds2760 is not really simple monitoring chip. ADC battery is (it's in -hh tree so far). So, ds2760 is somewhere in between SBS design, and dumb ADC batteries. So, my another purposal, which I very like now: Let's do self-documented properties. current_now, current_avg, e.t.c. No more just current, lets remove any ambiguousness! -- Anton Vorontsov email: [EMAIL PROTECTED] backup email: [EMAIL PROTECTED] irc://irc.freenode.org/bd2 - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/7] [RFC] Battery monitoring class
Hi! * It insists on reusing its predefined attributes *and* their units. So, userspace getting expected values for any battery. Also common units is required for APM/ACPI emulation. Though our battery class insisting on re-usage, but not forces it. If some battery driver can't convert its own raw values (can't imagine why), then driver is free to implement its own attributes *and* additional _units attribute. Though, this scheme is discouraged. * LEDs support. Each battery register its trigger, and gadgets with LEDs can quickly bind to battery-charging / battery-full triggers. Here how it looks like from user space: # ls /sys/class/battery/main-battery/ capacity max_capacity max_voltage min_current power subsystem uevent current max_current min_capacity min_voltage status temp voltage # cat /sys/class/battery/main-battery/status Full # cat /sys/class/leds/h5400\:green-right/trigger none h5400-radio timer hwtimer main-battery-charging [main-battery-full] # cat /sys/class/leds/h5400\:green-right/brightness 255 Can we get few lines in Documentation? I guess min_capacity is shutdown capacity at current temperature, but its surely non-obvious. Will min_capacity increase as batery gets old? Or will max_capacity decrease? (Should we introduce design_capacity for ACPI systems that know the difference?) What is min_current? Granularity of amper meter? And min_voltage is shutdown voltage? Otherwise it looks good to me. Something like this is really needed. + * All voltages, currents, capacities and temperatures in mV, mA, mAh and + * tenths of a degree unless otherwise stated. It's driver's job to convert tenths of degree Celsius? Pavel +#define BATTERY_STATUS_UNKNOWN 0 +#define BATTERY_STATUS_CHARGING 1 +#define BATTERY_STATUS_DISCHARGING 2 +#define BATTERY_STATUS_NOT_CHARGING 3 +#define BATTERY_STATUS_FULL 4 Perhaps we need STATUS_ERROR? At least on some machines it is different from STATUS_NOT_CHARGING. + /* private */ + struct power_supplicant pst; + + #ifdef CONFIG_LEDS_TRIGGERS + struct led_trigger *charging_trig; + char *charging_trig_name; + struct led_trigger *full_trig; + char *full_trig_name; + #endif +}; #ifdefs need to be at column 0? +/* + * This is recommended structure to specify static battery parameters. + * Generic one, parametrizable for different batteries. Battery device + * itself does bot use it, but that's what implementing most drivers, 'does not'? Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Kernel-discuss] Re: [PATCH 3/7] [RFC] Battery monitoring class
Hello Pavel, On Sun, Apr 15, 2007 at 07:56:56PM +, Pavel Machek wrote: Hi! * It insists on reusing its predefined attributes *and* their units. So, userspace getting expected values for any battery. Also common units is required for APM/ACPI emulation. Though our battery class insisting on re-usage, but not forces it. If some battery driver can't convert its own raw values (can't imagine why), then driver is free to implement its own attributes *and* additional _units attribute. Though, this scheme is discouraged. * LEDs support. Each battery register its trigger, and gadgets with LEDs can quickly bind to battery-charging / battery-full triggers. Here how it looks like from user space: # ls /sys/class/battery/main-battery/ capacity max_capacity max_voltage min_current power subsystem uevent current max_current min_capacity min_voltage status temp voltage # cat /sys/class/battery/main-battery/status Full # cat /sys/class/leds/h5400\:green-right/trigger none h5400-radio timer hwtimer main-battery-charging [main-battery-full] # cat /sys/class/leds/h5400\:green-right/brightness 255 Can we get few lines in Documentation? Yes, sure. I guess min_capacity is shutdown capacity at current temperature, but its surely non-obvious. Will min_capacity increase as batery gets old? Or will max_capacity decrease? min_capacity and max_capacity depend on temperature. But some drivers can/want to interpolate, others can't/don't want. It's driver's matter. ds2760_battery just remembers last capacity when current was 10 mA (i.e. battery charged full at given temperature) as max_capacity, and takes this as reference for calculations. (Should we introduce design_capacity for ACPI systems that know the difference?) Yup, I've introduced it in [take2] (it's in this thread, thus it's hard to find). Will resend it as separate thread soon, along with few other changes. What is min_current? Granularity of amper meter? And min_voltage is shutdown voltage? Yes, should be. But, for example, ds2760 battery can't remember this value in its eeprom (iirc), thus this value passed by platform code, i.e. in our case (iPaqs) it's machine dependent value. Probably we should not even export this attribute in ds2760_battery driver, as it does not take any part in calculations. Plus this attribute highly depend on battery chemistry and temperature. Thus it's hardly anyhow useful, except if hardware itself reports it (not ds2760 case). Otherwise it looks good to me. Something like this is really needed. Thanks! + * All voltages, currents, capacities and temperatures in mV, mA, mAh and + * tenths of a degree unless otherwise stated. It's driver's job to convert tenths of degree Celsius? Yup, fixed in [take2]. +#define BATTERY_STATUS_UNKNOWN 0 +#define BATTERY_STATUS_CHARGING 1 +#define BATTERY_STATUS_DISCHARGING 2 +#define BATTERY_STATUS_NOT_CHARGING 3 +#define BATTERY_STATUS_FULL 4 Perhaps we need STATUS_ERROR? At least on some machines it is different from STATUS_NOT_CHARGING. I'm unsure about this. BATTERY_STATUS_* is mostly about charging process status (should I rename it to more verbose BATTERY_CHARGING_STATUS_, or just mention it in Documentation?). For errors things it might be better to create health attribute. + /* private */ + struct power_supplicant pst; + + #ifdef CONFIG_LEDS_TRIGGERS + struct led_trigger *charging_trig; + char *charging_trig_name; + struct led_trigger *full_trig; + char *full_trig_name; + #endif +}; #ifdefs need to be at column 0? Yup, fixed in [take2]. +/* + * This is recommended structure to specify static battery parameters. + * Generic one, parametrizable for different batteries. Battery device + * itself does bot use it, but that's what implementing most drivers, 'does not'? Thanks, will fix! Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html Thanks, -- Anton Vorontsov email: [EMAIL PROTECTED] backup email: [EMAIL PROTECTED] irc://irc.freenode.org/bd2 - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/7] [RFC] Battery monitoring class
On Fri, Apr 13, 2007 at 05:49:39PM +0400, Anton Vorontsov wrote: > I'll convert mXh to uXh a bit later, if there will no further objections > against uXh. Also I'd like to hear if there any objections on > mA/mV -> uA/uV conversion. I think we'd better keep all units at the > same order/precision. Okay, would it make sense to use "long" instead of "int" after "milli" to "micro" conversion? On 32 bit machines int gives +-2147483648 limit. So 2147 volts/amperes/... Though 2147 amperes is unrealistic for batteries, but if used in calculations it could be dangerous. For example: di->life_sec = -((di->accum_current_uAh - di->empty_uAh) * 3600) / di->current_uA; It can be also solved (and I voting for it) by typecasting to long in the driver itself. Would it also make sense to use int64_t instead of long? And how should it passed to printk in portable way? I guess printk (vsprintf) does not support PRIx notation as defined in /usr/include/inttypes.h ? -- Anton Vorontsov email: [EMAIL PROTECTED] backup email: [EMAIL PROTECTED] irc://irc.freenode.org/bd2 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/7] [RFC] Battery monitoring class
On Fri, Apr 13, 2007 at 05:49:39PM +0400, Anton Vorontsov wrote: I'll convert mXh to uXh a bit later, if there will no further objections against uXh. Also I'd like to hear if there any objections on mA/mV - uA/uV conversion. I think we'd better keep all units at the same order/precision. Okay, would it make sense to use long instead of int after milli to micro conversion? On 32 bit machines int gives +-2147483648 limit. So 2147 volts/amperes/... Though 2147 amperes is unrealistic for batteries, but if used in calculations it could be dangerous. For example: di-life_sec = -((di-accum_current_uAh - di-empty_uAh) * 3600) / di-current_uA; It can be also solved (and I voting for it) by typecasting to long in the driver itself. Would it also make sense to use int64_t instead of long? And how should it passed to printk in portable way? I guess printk (vsprintf) does not support PRIx notation as defined in /usr/include/inttypes.h ? -- Anton Vorontsov email: [EMAIL PROTECTED] backup email: [EMAIL PROTECTED] irc://irc.freenode.org/bd2 - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/7] [RFC] Battery monitoring class
On Thu, Apr 12, 2007 at 03:25:03AM +0400, Anton Vorontsov wrote: > Here is battery monitor class. According to first copyright string, we're > maintaining it since 2003. I've took few days and cleaned it up to be > more suitable for mainline inclusion. > > It differs from battery class at git://git.infradead.org/battery-2.6.git: > > * It's using external power kernel interface, i.e. does not fake external > powers as batteries. (Same thing David Woodhouse planed last year). > > * It have predefined set of attributes, this eliminates code duplication > by battery drivers. And also gives opportunity to write emulation drivers > for legacy stuff (APM emulation driver follow). > > If driver can't afford some attribute, it will not appear in sysfs. > > * It insists on reusing its predefined attributes *and* their units. > So, userspace getting expected values for any battery. > > Also common units is required for APM/ACPI emulation. > > Though our battery class insisting on re-usage, but not forces it. If some > battery driver can't convert its own raw values (can't imagine why), then > driver is free to implement its own attributes *and* additional _units > attribute. Though, this scheme is discouraged. > > * LEDs support. Each battery register its trigger, and gadgets with LEDs > can quickly bind to battery-charging / battery-full triggers. > > Here how it looks like from user space: > > # ls /sys/class/battery/main-battery/ > capacity max_capacity max_voltage min_current power subsystem uevent > current max_current min_capacity min_voltage status temp voltage > # cat /sys/class/battery/main-battery/status > Full > # cat /sys/class/leds/h5400\:green-right/trigger > none h5400-radio timer hwtimer main-battery-charging [main-battery-full] > # cat /sys/class/leds/h5400\:green-right/brightness > 255 > Changes: - Cleanups based on comments from Randy Dunlap. - Attribute creation scheme changed drastically. No more tons of macro-created functions. Compiled code should be much smaller. Also adding new "standard" attributes is trivial task now (matter of adding two lines, one in battery.c and another in battery.h). - charge (as quantity) in mAh, energy in mWh. I'll convert mXh to uXh a bit later, if there will no further objections against uXh. Also I'd like to hear if there any objections on mA/mV -> uA/uV conversion. I think we'd better keep all units at the same order/precision. Subject: [PATCH] [take2] Battery monitoring class Signed-off-by: Anton Vorontsov <[EMAIL PROTECTED]> --- drivers/Kconfig |2 + drivers/Makefile |1 + drivers/battery/Kconfig | 11 ++ drivers/battery/Makefile |1 + drivers/battery/battery.c | 290 + include/linux/battery.h | 113 ++ 6 files changed, 418 insertions(+), 0 deletions(-) create mode 100644 drivers/battery/Kconfig create mode 100644 drivers/battery/Makefile create mode 100644 drivers/battery/battery.c create mode 100644 include/linux/battery.h diff --git a/drivers/Kconfig b/drivers/Kconfig index c546de3..c3a0038 100644 --- a/drivers/Kconfig +++ b/drivers/Kconfig @@ -56,6 +56,8 @@ source "drivers/w1/Kconfig" source "drivers/power/Kconfig" +source "drivers/battery/Kconfig" + source "drivers/hwmon/Kconfig" source "drivers/mfd/Kconfig" diff --git a/drivers/Makefile b/drivers/Makefile index 2bdaae7..7cbfd37 100644 --- a/drivers/Makefile +++ b/drivers/Makefile @@ -61,6 +61,7 @@ obj-$(CONFIG_RTC_LIB) += rtc/ obj-$(CONFIG_I2C) += i2c/ obj-$(CONFIG_W1) += w1/ obj-$(CONFIG_EXTERNAL_POWER) += power/ +obj-$(CONFIG_BATTERY) += battery/ obj-$(CONFIG_HWMON)+= hwmon/ obj-$(CONFIG_PHONE)+= telephony/ obj-$(CONFIG_MD) += md/ diff --git a/drivers/battery/Kconfig b/drivers/battery/Kconfig new file mode 100644 index 000..c386593 --- /dev/null +++ b/drivers/battery/Kconfig @@ -0,0 +1,11 @@ + +menu "Battery support" + +config BATTERY + tristate "Battery monitoring support" + select EXTERNAL_POWER + help + Say Y here to enable generic battery status reporting in + the /sys filesystem. + +endmenu diff --git a/drivers/battery/Makefile b/drivers/battery/Makefile new file mode 100644 index 000..a2239cb --- /dev/null +++ b/drivers/battery/Makefile @@ -0,0 +1 @@ +obj-$(CONFIG_BATTERY) += battery.o diff --git a/drivers/battery/battery.c b/drivers/battery/battery.c new file mode 100644 index 000..6c87fe3 --- /dev/null +++ b/drivers/battery/battery.c @@ -0,0 +1,290 @@ +/* + * Universal battery monitor class + * + * Copyright (c) 2007 Anton Vorontsov <[EMAIL PROTECTED]> + * Copyright (c) 2004 Szabolcs Gyurko + * Copyright (c) 2003 Ian Molton <[EMAIL PROTECTED]> + * + * Modified: 2004, Oct Szabolcs Gyurko + * + * You may use this code as per GPL version 2 + * + * All
Re: [Kernel-discuss] Re: [PATCH 3/7] [RFC] Battery monitoring class
On Fri, 13 Apr 2007, Anton Vorontsov wrote: > > With fixed-units files, having *_energy and *_capacity isn't too clear > > either... Nor is it consistent with SBS, since SBS uses "capacity" to > > refer to either energy or charge, depending on a units attribute. > > > > As a compromise, how about using "energy" and "charge" for quantities, > > and "charging" (i.e., a verb) when referring to the operation? > > It would be great compromise! Please please please! I can live with it, although I'd rather just use the units (zero margin of error or confusion). -- "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 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Kernel-discuss] Re: [PATCH 3/7] [RFC] Battery monitoring class
On Fri, 13 Apr 2007, Anton Vorontsov wrote: With fixed-units files, having *_energy and *_capacity isn't too clear either... Nor is it consistent with SBS, since SBS uses capacity to refer to either energy or charge, depending on a units attribute. As a compromise, how about using energy and charge for quantities, and charging (i.e., a verb) when referring to the operation? It would be great compromise! Please please please! I can live with it, although I'd rather just use the units (zero margin of error or confusion). -- 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 - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/7] [RFC] Battery monitoring class
On Thu, Apr 12, 2007 at 03:25:03AM +0400, Anton Vorontsov wrote: Here is battery monitor class. According to first copyright string, we're maintaining it since 2003. I've took few days and cleaned it up to be more suitable for mainline inclusion. It differs from battery class at git://git.infradead.org/battery-2.6.git: * It's using external power kernel interface, i.e. does not fake external powers as batteries. (Same thing David Woodhouse planed last year). * It have predefined set of attributes, this eliminates code duplication by battery drivers. And also gives opportunity to write emulation drivers for legacy stuff (APM emulation driver follow). If driver can't afford some attribute, it will not appear in sysfs. * It insists on reusing its predefined attributes *and* their units. So, userspace getting expected values for any battery. Also common units is required for APM/ACPI emulation. Though our battery class insisting on re-usage, but not forces it. If some battery driver can't convert its own raw values (can't imagine why), then driver is free to implement its own attributes *and* additional _units attribute. Though, this scheme is discouraged. * LEDs support. Each battery register its trigger, and gadgets with LEDs can quickly bind to battery-charging / battery-full triggers. Here how it looks like from user space: # ls /sys/class/battery/main-battery/ capacity max_capacity max_voltage min_current power subsystem uevent current max_current min_capacity min_voltage status temp voltage # cat /sys/class/battery/main-battery/status Full # cat /sys/class/leds/h5400\:green-right/trigger none h5400-radio timer hwtimer main-battery-charging [main-battery-full] # cat /sys/class/leds/h5400\:green-right/brightness 255 Changes: - Cleanups based on comments from Randy Dunlap. - Attribute creation scheme changed drastically. No more tons of macro-created functions. Compiled code should be much smaller. Also adding new standard attributes is trivial task now (matter of adding two lines, one in battery.c and another in battery.h). - charge (as quantity) in mAh, energy in mWh. I'll convert mXh to uXh a bit later, if there will no further objections against uXh. Also I'd like to hear if there any objections on mA/mV - uA/uV conversion. I think we'd better keep all units at the same order/precision. Subject: [PATCH] [take2] Battery monitoring class Signed-off-by: Anton Vorontsov [EMAIL PROTECTED] --- drivers/Kconfig |2 + drivers/Makefile |1 + drivers/battery/Kconfig | 11 ++ drivers/battery/Makefile |1 + drivers/battery/battery.c | 290 + include/linux/battery.h | 113 ++ 6 files changed, 418 insertions(+), 0 deletions(-) create mode 100644 drivers/battery/Kconfig create mode 100644 drivers/battery/Makefile create mode 100644 drivers/battery/battery.c create mode 100644 include/linux/battery.h diff --git a/drivers/Kconfig b/drivers/Kconfig index c546de3..c3a0038 100644 --- a/drivers/Kconfig +++ b/drivers/Kconfig @@ -56,6 +56,8 @@ source drivers/w1/Kconfig source drivers/power/Kconfig +source drivers/battery/Kconfig + source drivers/hwmon/Kconfig source drivers/mfd/Kconfig diff --git a/drivers/Makefile b/drivers/Makefile index 2bdaae7..7cbfd37 100644 --- a/drivers/Makefile +++ b/drivers/Makefile @@ -61,6 +61,7 @@ obj-$(CONFIG_RTC_LIB) += rtc/ obj-$(CONFIG_I2C) += i2c/ obj-$(CONFIG_W1) += w1/ obj-$(CONFIG_EXTERNAL_POWER) += power/ +obj-$(CONFIG_BATTERY) += battery/ obj-$(CONFIG_HWMON)+= hwmon/ obj-$(CONFIG_PHONE)+= telephony/ obj-$(CONFIG_MD) += md/ diff --git a/drivers/battery/Kconfig b/drivers/battery/Kconfig new file mode 100644 index 000..c386593 --- /dev/null +++ b/drivers/battery/Kconfig @@ -0,0 +1,11 @@ + +menu Battery support + +config BATTERY + tristate Battery monitoring support + select EXTERNAL_POWER + help + Say Y here to enable generic battery status reporting in + the /sys filesystem. + +endmenu diff --git a/drivers/battery/Makefile b/drivers/battery/Makefile new file mode 100644 index 000..a2239cb --- /dev/null +++ b/drivers/battery/Makefile @@ -0,0 +1 @@ +obj-$(CONFIG_BATTERY) += battery.o diff --git a/drivers/battery/battery.c b/drivers/battery/battery.c new file mode 100644 index 000..6c87fe3 --- /dev/null +++ b/drivers/battery/battery.c @@ -0,0 +1,290 @@ +/* + * Universal battery monitor class + * + * Copyright (c) 2007 Anton Vorontsov [EMAIL PROTECTED] + * Copyright (c) 2004 Szabolcs Gyurko + * Copyright (c) 2003 Ian Molton [EMAIL PROTECTED] + * + * Modified: 2004, Oct Szabolcs Gyurko + * + * You may use this code as per GPL version 2 + * + * All voltages, currents, charges, energies and temperatures in mV,
Re: [Kernel-discuss] Re: [PATCH 3/7] [RFC] Battery monitoring class
On Thu, Apr 12, 2007 at 10:34:06PM -0400, Shem Multinymous wrote: > Hi, > > On 4/12/07, Henrique de Moraes Holschuh <[EMAIL PROTECTED]> wrote: > >On Fri, 13 Apr 2007, Anton Vorontsov wrote: > >> * Yup, I've read last discussion regarding batteries, and I've seen > >> objections against "charge" term, quoting Shem Multinymous: > >> > >> "And, for the reasons I explained earlier, I strongly suggest not using > >> the term "charge" except when referring to the action of charging. > >> Hence: > >> s/charge_rate/rate/; s/charge/capacity/" > >> > >> But lets think about it once again? We'll make things much cleaner > >> if we'll drop "capacity" at all. > > > >I stand with Shem on this one. The people behind the SBS specification > >seems to agree... that specification is aimed at *engineers* and still > >avoids the obvious trap of using "charge" due to its high potential for > >confusion. > > > >I don't even want to know how much of a mess the people writing applets > >woudl make of it... > > With fixed-units files, having *_energy and *_capacity isn't too clear > either... Nor is it consistent with SBS, since SBS uses "capacity" to > refer to either energy or charge, depending on a units attribute. > > As a compromise, how about using "energy" and "charge" for quantities, > and "charging" (i.e., a verb) when referring to the operation? It would be great compromise! Please please please! -- Anton Vorontsov email: [EMAIL PROTECTED] backup email: [EMAIL PROTECTED] irc://irc.freenode.org/bd2 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Kernel-discuss] Re: [PATCH 3/7] [RFC] Battery monitoring class
Hi, On 4/12/07, Henrique de Moraes Holschuh <[EMAIL PROTECTED]> wrote: On Fri, 13 Apr 2007, Anton Vorontsov wrote: > * Yup, I've read last discussion regarding batteries, and I've seen > objections against "charge" term, quoting Shem Multinymous: > > "And, for the reasons I explained earlier, I strongly suggest not using > the term "charge" except when referring to the action of charging. > Hence: > s/charge_rate/rate/; s/charge/capacity/" > > But lets think about it once again? We'll make things much cleaner > if we'll drop "capacity" at all. I stand with Shem on this one. The people behind the SBS specification seems to agree... that specification is aimed at *engineers* and still avoids the obvious trap of using "charge" due to its high potential for confusion. I don't even want to know how much of a mess the people writing applets woudl make of it... With fixed-units files, having *_energy and *_capacity isn't too clear either... Nor is it consistent with SBS, since SBS uses "capacity" to refer to either energy or charge, depending on a units attribute. As a compromise, how about using "energy" and "charge" for quantities, and "charging" (i.e., a verb) when referring to the operation? BTW, tp_smapi uses "charge" and "charging" interchangeably; that was a mistake. Shem - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Kernel-discuss] Re: [PATCH 3/7] [RFC] Battery monitoring class
On Thu, Apr 12, 2007 at 09:51:12PM -0300, Henrique de Moraes Holschuh wrote:
> On Fri, 13 Apr 2007, Anton Vorontsov wrote:
> > Let's name attributes with mWh units as {min_,max_,design_,}energy,
> > and attributes with mAh units as {min_,max_,design_,}charge.
>
> [...]
>
> > * Yup, I've read last discussion regarding batteries, and I've seen
> > objections against "charge" term, quoting Shem Multinymous:
> >
> > "And, for the reasons I explained earlier, I strongly suggest not using
> > the term "charge" except when referring to the action of charging.
> > Hence:
> > s/charge_rate/rate/; s/charge/capacity/"
> >
> > But lets think about it once again? We'll make things much cleaner
> > if we'll drop "capacity" at all.
>
> I stand with Shem on this one. The people behind the SBS specification
> seems to agree... that specification is aimed at *engineers* and still
> avoids the obvious trap of using "charge" due to its high potential for
> confusion.
>
> I don't even want to know how much of a mess the people writing applets
> woudl make of it...
:-(
Okay, term "charge" is out of scope, I guess. But can we use "capacity"
for xAh, and "energy" for xWh? I just trying to separate these terms
somehow, and avoid "_units" stuff.
>
> > > That said, you may need to use uWh and uAh instead of mAh and mWh, though.
> >
> > Not sure. Is there any existing chip that can report uAh/uWh? That is
> > great precision.
>
> The way things are going, it should be feasible for small embedded systems
> quite soon. Refer to the previous thread.
I see... is it also applicable to currents and voltages? I.e. should we
use uA and uV from the start?
--
Anton Vorontsov
email: [EMAIL PROTECTED]
backup email: [EMAIL PROTECTED]
irc://irc.freenode.org/bd2
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Kernel-discuss] Re: [PATCH 3/7] [RFC] Battery monitoring class
On Fri, 13 Apr 2007, Anton Vorontsov wrote:
> Let's name attributes with mWh units as {min_,max_,design_,}energy,
> and attributes with mAh units as {min_,max_,design_,}charge.
[...]
> * Yup, I've read last discussion regarding batteries, and I've seen
> objections against "charge" term, quoting Shem Multinymous:
>
> "And, for the reasons I explained earlier, I strongly suggest not using
> the term "charge" except when referring to the action of charging.
> Hence:
> s/charge_rate/rate/; s/charge/capacity/"
>
> But lets think about it once again? We'll make things much cleaner
> if we'll drop "capacity" at all.
I stand with Shem on this one. The people behind the SBS specification
seems to agree... that specification is aimed at *engineers* and still
avoids the obvious trap of using "charge" due to its high potential for
confusion.
I don't even want to know how much of a mess the people writing applets
woudl make of it...
> > That said, you may need to use uWh and uAh instead of mAh and mWh, though.
>
> Not sure. Is there any existing chip that can report uAh/uWh? That is
> great precision.
The way things are going, it should be feasible for small embedded systems
quite soon. Refer to the previous thread.
--
"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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Kernel-discuss] Re: [PATCH 3/7] [RFC] Battery monitoring class
On Thu, Apr 12, 2007 at 03:56:30PM -0300, Henrique de Moraes Holschuh wrote:
> On Thu, 12 Apr 2007, Paul Sokolovsky wrote:
> > Yes, that's apparently the way to go. We just should consider
> > if mAh and mWh are enough, or we go wider and allow whole collection of
> > units. Btw, original handhelds.org code used Joules ;-).
>
> FWIW, SBS only mentions mAh and mWh. AFAIK, all other (meaningful) units
> should be able to be converted to either Ah or Wh, assuming enough precision
> on the math. I never heard of any other way to fuel-gauge batteries than
> these two main modes (current-based or capacity-based), but I don't work on
> the battery field.
Okay, I have an idea:
Let's name attributes with mWh units as {min_,max_,design_,}energy,
and attributes with mAh units as {min_,max_,design_,}charge.
Because both energy and charge represents ""capacity"" in some meanings,
and that's why we bothering with _units attribute. So, lets drop
"capacity"* and use more specific terms? I really don't want string
attributes by default (except status).
If we export attributes with predefined units, userspace developers
could just look into include/linux/battery.h and conclude: "Ah, great,
if battery reporting energy, then it's in mWh, and if battery reporting
charge it's always in mAh".
* Yup, I've read last discussion regarding batteries, and I've seen
objections against "charge" term, quoting Shem Multinymous:
"And, for the reasons I explained earlier, I strongly suggest not using
the term "charge" except when referring to the action of charging.
Hence:
s/charge_rate/rate/; s/charge/capacity/"
But lets think about it once again? We'll make things much cleaner
if we'll drop "capacity" at all.
> That said, you may need to use uWh and uAh instead of mAh and mWh, though.
Not sure. Is there any existing chip that can report uAh/uWh? That is
great precision.
--
Anton Vorontsov
email: [EMAIL PROTECTED]
backup email: [EMAIL PROTECTED]
irc://irc.freenode.org/bd2
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Kernel-discuss] Re: [PATCH 3/7] [RFC] Battery monitoring class
On Thu, 12 Apr 2007, Paul Sokolovsky wrote: > Yes, that's apparently the way to go. We just should consider > if mAh and mWh are enough, or we go wider and allow whole collection of > units. Btw, original handhelds.org code used Joules ;-). FWIW, SBS only mentions mAh and mWh. AFAIK, all other (meaningful) units should be able to be converted to either Ah or Wh, assuming enough precision on the math. I never heard of any other way to fuel-gauge batteries than these two main modes (current-based or capacity-based), but I don't work on the battery field. That said, you may need to use uWh and uAh instead of mAh and mWh, though. -- "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 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/7] [RFC] Battery monitoring class
Hi Anton,
On 4/12/07, Anton Vorontsov <[EMAIL PROTECTED]> wrote:
On Thu, Apr 12, 2007 at 11:00:07AM -0400, Shem Multinymous wrote:
> I suggest adding "remaining operating time" and "remaining charging
> time". You can try deducing these from the above attributes, but in
> practice this gives very inaccurate predictions. On laptops (e.g.,
> ThinkPad) the BIOS or EC often provides much better estimates, using a
> more accurate physical model.
Yes, sure. Feel free to add these attributes to the "standard" ones,
along with your drivers. See (1).
That's a sound way to go around i. We just need to be careful about
naming conventions. For example, "*_charge" rather than "*_capacity,
so that we can later add "*_energy" analogously.
But specifically about {operating,charge} time remaining readouts, it
seems important to have them there from the beginning and have all
drivers implement them. Otherwise, userspace will just go ahead and
implement its own crude computation, so by the time when new
attributes and drivers are introduced, userspace will be full of bad
code. I've seen this happening with the tp_smapi ThinkPad driver -- I
introduced those attributes at a recent version, and then needed to
encourage userspace utility authors to dump their extrapolation
calcultions and use the attribute instead.
The rest of the missing attributes are not as imporant in this
respect, because userspace can't try to estimate them.
Shem
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/7] [RFC] Battery monitoring class
Hello Randy,
On Wed, Apr 11, 2007 at 07:53:59PM -0700, Randy Dunlap wrote:
> On Thu, 12 Apr 2007 03:25:03 +0400 Anton Vorontsov wrote:
> > +void battery_status_changed(struct battery *bat)
> > +{
> > + pr_debug("%s\n", __FUNCTION__);
> > + #ifdef CONFIG_LEDS_TRIGGERS
>
> Please don't indent preprocessor controls (ifdef/endif etc.).
...
> Place 'switch' and 'case' at the same indent level. This prevents
> the "double-indent" for the code statements.
...
> We usually try to place a blank line between local data and code.
...
> space after "if"
...
Much thanks, will fix.
> > + device_remove_file(bat->dev, _attr_##name);
> > +
> > + goto success;
> > +
> > +capacity_failed: remove_bat_attr_conditional(current);
> > +current_failed: remove_bat_attr_conditional(voltage);
> > +voltage_failed: remove_bat_attr_conditional(temp);
> > +temp_failed: remove_bat_attr_conditional(max_capacity);
> > +max_capacity_failed: remove_bat_attr_conditional(max_current);
> > +max_current_failed: remove_bat_attr_conditional(max_voltage);
> > +max_voltage_failed: remove_bat_attr_conditional(min_capacity);
> > +min_capacity_failed: remove_bat_attr_conditional(min_current);
> > +min_current_failed: remove_bat_attr_conditional(min_voltage);
> > +min_voltage_failed: remove_bat_attr_conditional(status);
>
> I thought there was a class_remove() or something like that?
> but I'm not sure of it.
[class_]device_destroy()? Yeah, but it's exactly same thing as
[class_]device_unregister.
But this is really good question. Should I manually clean up attributes,
or sysfs will take care? I.e. whole battery directory removes, and
sysfs removes files in it, or they just leaks?
I've took worst scenario, and done removal manually (I've grep'ed in
drivers/, and almost every driver also doing so).
> ---
> ~Randy
> *** Remember to use Documentation/SubmitChecklist when testing your code ***
Thanks for comments!
--
Anton Vorontsov
email: [EMAIL PROTECTED]
backup email: [EMAIL PROTECTED]
irc://irc.freenode.org/bd2
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/7] [RFC] Battery monitoring class
Hello Shem, On Thu, Apr 12, 2007 at 11:00:07AM -0400, Shem Multinymous wrote: > Hi Anton, > > A few comments on the ever-contentious choice of battery attributes: > > On 4/11/07, Anton Vorontsov <[EMAIL PROTECTED]> wrote: > >+ * All voltages, currents, capacities and temperatures in mV, mA, mAh and > >+ * tenths of a degree unless otherwise stated. It's driver's job to > >convert > >+ * its raw values to which this class operates. If for some reason driver > >+ * can't afford this requirement, then it have to create its own > >attributes, > >+ * plus additional "XYZ_units" for each of them. > >+ */ > > Many (most, I believe) laptop batteries report capacity in mWh > (energy), not mAh (charge). You can't convert between the two without > a detailed, up-to-date physical model of the battery. This is too > common to relegate to a non-standardized "XYZ_units" extension. I see. Okay, I'll try to cook something regarding capacity units. > >+BATTERY_INT_ATTR(min_voltage); > >+BATTERY_INT_ATTR(min_current); > >+BATTERY_INT_ATTR(min_capacity); > >+BATTERY_INT_ATTR(max_voltage); > >+BATTERY_INT_ATTR(max_current); > >+BATTERY_INT_ATTR(max_capacity); > >+BATTERY_INT_ATTR(temp); > >+BATTERY_INT_ATTR(voltage); > >+BATTERY_INT_ATTR(current); > >+BATTERY_INT_ATTR(capacity); > > I suggest adding "remaining operating time" and "remaining charging > time". You can try deducing these from the above attributes, but in > practice this gives very inaccurate predictions. On laptops (e.g., > ThinkPad) the BIOS or EC often provides much better estimates, using a > more accurate physical model. Yes, sure. Feel free to add these attributes to the "standard" ones, along with your drivers. See (1). > > I also see you omitted a host of other common attributes, like design > capacity, cycle count, model string, and temperatures. There was an > extensive LKML discussion of the choice and naming of attributes, in > the context of David Woodhouse's patch; there are futher observations > there. Also, here's the list of attributes in my tp_smapi ThinkPad > driver: http://thinkwiki.org/wiki/tp_smapi#Battery_charge_control_features > > Does "max capaxity" correspond to what's usually denoted "last full > capacity". Yup. > I understand you allow adding custom attributes, but they're of > limited use if generic userspace tools don't know their name and > semantics. It's important to standardize all reasonably common > attributes. (1) You're free to add your attribute to "standard" ones in include/linux/battery.h if it's proven to widely used. Batteries which do not have such attribute will not even notice that addition, so you will not have to grep and modify other drivers. So, that is plain matter of code duplication. If two or more battery drivers have same attributes, then we can put it in battery.h. If only one driver using it, then code duplication impossible, and driver should keep this attribute private. In handhelds.org tree we have two battery drivers with attributes you see currently. Second driver is not ready for mainline inclusion yet (it depends on ADC framework, which we'll present soon), thus I've not posted it. Also, APM emulation driver should learn how to use one attribute, or another depending on which available/better. But this is only technical question, like if (battery have attribute1) do precise maths of something, because we have mWh; else do approx maths of something, because we have only mAh. else n/a. > Shem Thanks for comments! -- Anton Vorontsov email: [EMAIL PROTECTED] backup email: [EMAIL PROTECTED] irc://irc.freenode.org/bd2 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/7] [RFC] Battery monitoring class
Hi Anton, A few comments on the ever-contentious choice of battery attributes: On 4/11/07, Anton Vorontsov <[EMAIL PROTECTED]> wrote: + * All voltages, currents, capacities and temperatures in mV, mA, mAh and + * tenths of a degree unless otherwise stated. It's driver's job to convert + * its raw values to which this class operates. If for some reason driver + * can't afford this requirement, then it have to create its own attributes, + * plus additional "XYZ_units" for each of them. + */ Many (most, I believe) laptop batteries report capacity in mWh (energy), not mAh (charge). You can't convert between the two without a detailed, up-to-date physical model of the battery. This is too common to relegate to a non-standardized "XYZ_units" extension. +BATTERY_INT_ATTR(min_voltage); +BATTERY_INT_ATTR(min_current); +BATTERY_INT_ATTR(min_capacity); +BATTERY_INT_ATTR(max_voltage); +BATTERY_INT_ATTR(max_current); +BATTERY_INT_ATTR(max_capacity); +BATTERY_INT_ATTR(temp); +BATTERY_INT_ATTR(voltage); +BATTERY_INT_ATTR(current); +BATTERY_INT_ATTR(capacity); I suggest adding "remaining operating time" and "remaining charging time". You can try deducing these from the above attributes, but in practice this gives very inaccurate predictions. On laptops (e.g., ThinkPad) the BIOS or EC often provides much better estimates, using a more accurate physical model. I also see you omitted a host of other common attributes, like design capacity, cycle count, model string, and temperatures. There was an extensive LKML discussion of the choice and naming of attributes, in the context of David Woodhouse's patch; there are futher observations there. Also, here's the list of attributes in my tp_smapi ThinkPad driver: http://thinkwiki.org/wiki/tp_smapi#Battery_charge_control_features Does "max capaxity" correspond to what's usually denoted "last full capacity". I understand you allow adding custom attributes, but they're of limited use if generic userspace tools don't know their name and semantics. It's important to standardize all reasonably common attributes. Shem - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Kernel-discuss] Re: [PATCH 3/7] [RFC] Battery monitoring class
Hello Matthew, Thursday, April 12, 2007, 5:24:30 PM, you wrote: > On Thu, Apr 12, 2007 at 06:15:05PM +0400, Anton Vorontsov wrote: >> On Thu, Apr 12, 2007 at 02:08:18PM +0100, Matthew Garrett wrote: >> > ACPI batteries can report capacity and rate in either mA or mW. Given >> >> You sure, capacity in mA? Then I don't know. But you can safely >> fallback and create your own attribute (just as in David's battery class, >> where every battery required to create its own attributes), plus create >> capacity_units attribute. So, user space will know your driver's specific >> units. > Well, mAh, but yes. Clearly it's possible to add extra attributes, but > speccing standard attributes that don't entirely cover the most common > non-embedded battery class seems less than ideal. Why not just require > capacity_units and rate_units attributes? Yes, that's apparently the way to go. We just should consider if mAh and mWh are enough, or we go wider and allow whole collection of units. Btw, original handhelds.org code used Joules ;-). -- Best regards, Paulmailto:[EMAIL PROTECTED] - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/7] [RFC] Battery monitoring class
On Thu, Apr 12, 2007 at 06:15:05PM +0400, Anton Vorontsov wrote: > On Thu, Apr 12, 2007 at 02:08:18PM +0100, Matthew Garrett wrote: > > ACPI batteries can report capacity and rate in either mA or mW. Given > > You sure, capacity in mA? Then I don't know. But you can safely > fallback and create your own attribute (just as in David's battery class, > where every battery required to create its own attributes), plus create > capacity_units attribute. So, user space will know your driver's specific > units. Well, mAh, but yes. Clearly it's possible to add extra attributes, but speccing standard attributes that don't entirely cover the most common non-embedded battery class seems less than ideal. Why not just require capacity_units and rate_units attributes? -- Matthew Garrett | [EMAIL PROTECTED] - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/7] [RFC] Battery monitoring class
On Thu, Apr 12, 2007 at 02:08:18PM +0100, Matthew Garrett wrote: > On Thu, Apr 12, 2007 at 03:25:03AM +0400, Anton Vorontsov wrote: > > + * All voltages, currents, capacities and temperatures in mV, mA, mAh and > > + * tenths of a degree unless otherwise stated. It's driver's job to convert > > + * its raw values to which this class operates. If for some reason driver > > + * can't afford this requirement, then it have to create its own > > attributes, > > + * plus additional "XYZ_units" for each of them. > > ACPI batteries can report capacity and rate in either mA or mW. Given You sure, capacity in mA? Then I don't know. But you can safely fallback and create your own attribute (just as in David's battery class, where every battery required to create its own attributes), plus create capacity_units attribute. So, user space will know your driver's specific units. > the lack of a constant voltage, how do you accurately convert between > the two? Right now, I think this is a loss of functionality over the > current situation. > > -- > Matthew Garrett | [EMAIL PROTECTED] -- Anton Vorontsov email: [EMAIL PROTECTED] backup email: [EMAIL PROTECTED] irc://irc.freenode.org/bd2 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/7] [RFC] Battery monitoring class
Hello Greg,
On Wed, Apr 11, 2007 at 08:43:23PM -0700, Greg KH wrote:
> On Thu, Apr 12, 2007 at 03:25:03AM +0400, Anton Vorontsov wrote:
> > Here is battery monitor class. According to first copyright string, we're
> > maintaining it since 2003. I've took few days and cleaned it up to be
> > more suitable for mainline inclusion.
> >
> > It differs from battery class at git://git.infradead.org/battery-2.6.git:
>
> Why fork from David's work? Does he not like these changes for some
> reason?
It's not a fork, actually. Ian Molton started battery stuff years before
David's work.
Though ours and David's exported API functions are exactly the same
(don't count functions which are unique for our code), but our
implementation a little intersects with David's.
For me it was no matter if I'll take handhelds.org or David's code as a
start point, except that hh.o code familiar to me, and I can test it on
real devices.
Though, you're right, we're in situation when we've two battery classes
now. :-/ And another pity fact is that we also have 8 Mb of patches in
our CVS. It takes a lot of time to to cleanup code for mainline,
especially with limited man-power resources. But we're working hard.
So, this is brief explanation why it took so long for us to show up.
> > +static int battery_create_attrs(struct battery *bat)
> > +{
> > + int rc;
> > +
> > + #define create_bat_attr_conditional(name)\
> > + if(bat->get_##name) {\
> > + rc = device_create_file(bat->dev, _attr_##name); \
> > + if (rc) goto name##_failed; \
> > + }
> > +
> > + create_bat_attr_conditional(status);
> > + create_bat_attr_conditional(min_voltage);
> > + create_bat_attr_conditional(min_current);
> > + create_bat_attr_conditional(min_capacity);
> > + create_bat_attr_conditional(max_voltage);
> > + create_bat_attr_conditional(max_current);
> > + create_bat_attr_conditional(max_capacity);
> > + create_bat_attr_conditional(temp);
> > + create_bat_attr_conditional(voltage);
> > + create_bat_attr_conditional(current);
> > + create_bat_attr_conditional(capacity);
>
> Use an attribute group please. It's much simpler and will be created at
> the proper time so your userspace tools don't have to sit and spin in
> order to properly wait for them to show up.
>
> Ok, yes, you want a conditional type of attribute group, like the
> new firewire code does. I have no problem adding that if you like.
I'm not sure if it's possible to create that type of conditional
attribute group. Because the condition is "bat->func != NULL", not
attribute' function. And that condition is battery-specific, not class
specific.
But anyway, I guess you're talking about not yet existent API, so I'd be
glad to take a look.
> thanks,
>
> greg k-h
Thanks for comments!
--
Anton Vorontsov
email: [EMAIL PROTECTED]
backup email: [EMAIL PROTECTED]
irc://irc.freenode.org/bd2
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/7] [RFC] Battery monitoring class
On Thu, Apr 12, 2007 at 03:25:03AM +0400, Anton Vorontsov wrote: > + * All voltages, currents, capacities and temperatures in mV, mA, mAh and > + * tenths of a degree unless otherwise stated. It's driver's job to convert > + * its raw values to which this class operates. If for some reason driver > + * can't afford this requirement, then it have to create its own attributes, > + * plus additional "XYZ_units" for each of them. ACPI batteries can report capacity and rate in either mA or mW. Given the lack of a constant voltage, how do you accurately convert between the two? Right now, I think this is a loss of functionality over the current situation. -- Matthew Garrett | [EMAIL PROTECTED] - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/7] [RFC] Battery monitoring class
On Wed, 11 Apr 2007, Greg KH wrote: > Ok, yes, you want a conditional type of attribute group, like the > new firewire code does. I have no problem adding that if you like. Please do. I want that for ibm-acpi/thinkpad-acpi as well. Right now I need to use multiple attribute groups because of that. Some hwmon drivers are in the same boat, too. -- "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 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/7] [RFC] Battery monitoring class
On Wed, 11 Apr 2007, Greg KH wrote: Ok, yes, you want a conditional type of attribute group, like the new firewire code does. I have no problem adding that if you like. Please do. I want that for ibm-acpi/thinkpad-acpi as well. Right now I need to use multiple attribute groups because of that. Some hwmon drivers are in the same boat, too. -- 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 - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/7] [RFC] Battery monitoring class
On Thu, Apr 12, 2007 at 03:25:03AM +0400, Anton Vorontsov wrote: + * All voltages, currents, capacities and temperatures in mV, mA, mAh and + * tenths of a degree unless otherwise stated. It's driver's job to convert + * its raw values to which this class operates. If for some reason driver + * can't afford this requirement, then it have to create its own attributes, + * plus additional XYZ_units for each of them. ACPI batteries can report capacity and rate in either mA or mW. Given the lack of a constant voltage, how do you accurately convert between the two? Right now, I think this is a loss of functionality over the current situation. -- Matthew Garrett | [EMAIL PROTECTED] - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/7] [RFC] Battery monitoring class
Hello Greg,
On Wed, Apr 11, 2007 at 08:43:23PM -0700, Greg KH wrote:
On Thu, Apr 12, 2007 at 03:25:03AM +0400, Anton Vorontsov wrote:
Here is battery monitor class. According to first copyright string, we're
maintaining it since 2003. I've took few days and cleaned it up to be
more suitable for mainline inclusion.
It differs from battery class at git://git.infradead.org/battery-2.6.git:
Why fork from David's work? Does he not like these changes for some
reason?
It's not a fork, actually. Ian Molton started battery stuff years before
David's work.
Though ours and David's exported API functions are exactly the same
(don't count functions which are unique for our code), but our
implementation a little intersects with David's.
For me it was no matter if I'll take handhelds.org or David's code as a
start point, except that hh.o code familiar to me, and I can test it on
real devices.
Though, you're right, we're in situation when we've two battery classes
now. :-/ And another pity fact is that we also have 8 Mb of patches in
our CVS. It takes a lot of time to to cleanup code for mainline,
especially with limited man-power resources. But we're working hard.
So, this is brief explanation why it took so long for us to show up.
+static int battery_create_attrs(struct battery *bat)
+{
+ int rc;
+
+ #define create_bat_attr_conditional(name)\
+ if(bat-get_##name) {\
+ rc = device_create_file(bat-dev, dev_attr_##name); \
+ if (rc) goto name##_failed; \
+ }
+
+ create_bat_attr_conditional(status);
+ create_bat_attr_conditional(min_voltage);
+ create_bat_attr_conditional(min_current);
+ create_bat_attr_conditional(min_capacity);
+ create_bat_attr_conditional(max_voltage);
+ create_bat_attr_conditional(max_current);
+ create_bat_attr_conditional(max_capacity);
+ create_bat_attr_conditional(temp);
+ create_bat_attr_conditional(voltage);
+ create_bat_attr_conditional(current);
+ create_bat_attr_conditional(capacity);
Use an attribute group please. It's much simpler and will be created at
the proper time so your userspace tools don't have to sit and spin in
order to properly wait for them to show up.
Ok, yes, you want a conditional type of attribute group, like the
new firewire code does. I have no problem adding that if you like.
I'm not sure if it's possible to create that type of conditional
attribute group. Because the condition is bat-func != NULL, not
attribute' function. And that condition is battery-specific, not class
specific.
But anyway, I guess you're talking about not yet existent API, so I'd be
glad to take a look.
thanks,
greg k-h
Thanks for comments!
--
Anton Vorontsov
email: [EMAIL PROTECTED]
backup email: [EMAIL PROTECTED]
irc://irc.freenode.org/bd2
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/7] [RFC] Battery monitoring class
On Thu, Apr 12, 2007 at 02:08:18PM +0100, Matthew Garrett wrote: On Thu, Apr 12, 2007 at 03:25:03AM +0400, Anton Vorontsov wrote: + * All voltages, currents, capacities and temperatures in mV, mA, mAh and + * tenths of a degree unless otherwise stated. It's driver's job to convert + * its raw values to which this class operates. If for some reason driver + * can't afford this requirement, then it have to create its own attributes, + * plus additional XYZ_units for each of them. ACPI batteries can report capacity and rate in either mA or mW. Given You sure, capacity in mA? Then I don't know. But you can safely fallback and create your own attribute (just as in David's battery class, where every battery required to create its own attributes), plus create capacity_units attribute. So, user space will know your driver's specific units. the lack of a constant voltage, how do you accurately convert between the two? Right now, I think this is a loss of functionality over the current situation. -- Matthew Garrett | [EMAIL PROTECTED] -- Anton Vorontsov email: [EMAIL PROTECTED] backup email: [EMAIL PROTECTED] irc://irc.freenode.org/bd2 - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/7] [RFC] Battery monitoring class
On Thu, Apr 12, 2007 at 06:15:05PM +0400, Anton Vorontsov wrote: On Thu, Apr 12, 2007 at 02:08:18PM +0100, Matthew Garrett wrote: ACPI batteries can report capacity and rate in either mA or mW. Given You sure, capacity in mA? Then I don't know. But you can safely fallback and create your own attribute (just as in David's battery class, where every battery required to create its own attributes), plus create capacity_units attribute. So, user space will know your driver's specific units. Well, mAh, but yes. Clearly it's possible to add extra attributes, but speccing standard attributes that don't entirely cover the most common non-embedded battery class seems less than ideal. Why not just require capacity_units and rate_units attributes? -- Matthew Garrett | [EMAIL PROTECTED] - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Kernel-discuss] Re: [PATCH 3/7] [RFC] Battery monitoring class
Hello Matthew, Thursday, April 12, 2007, 5:24:30 PM, you wrote: On Thu, Apr 12, 2007 at 06:15:05PM +0400, Anton Vorontsov wrote: On Thu, Apr 12, 2007 at 02:08:18PM +0100, Matthew Garrett wrote: ACPI batteries can report capacity and rate in either mA or mW. Given You sure, capacity in mA? Then I don't know. But you can safely fallback and create your own attribute (just as in David's battery class, where every battery required to create its own attributes), plus create capacity_units attribute. So, user space will know your driver's specific units. Well, mAh, but yes. Clearly it's possible to add extra attributes, but speccing standard attributes that don't entirely cover the most common non-embedded battery class seems less than ideal. Why not just require capacity_units and rate_units attributes? Yes, that's apparently the way to go. We just should consider if mAh and mWh are enough, or we go wider and allow whole collection of units. Btw, original handhelds.org code used Joules ;-). -- Best regards, Paulmailto:[EMAIL PROTECTED] - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/7] [RFC] Battery monitoring class
Hi Anton, A few comments on the ever-contentious choice of battery attributes: On 4/11/07, Anton Vorontsov [EMAIL PROTECTED] wrote: + * All voltages, currents, capacities and temperatures in mV, mA, mAh and + * tenths of a degree unless otherwise stated. It's driver's job to convert + * its raw values to which this class operates. If for some reason driver + * can't afford this requirement, then it have to create its own attributes, + * plus additional XYZ_units for each of them. + */ Many (most, I believe) laptop batteries report capacity in mWh (energy), not mAh (charge). You can't convert between the two without a detailed, up-to-date physical model of the battery. This is too common to relegate to a non-standardized XYZ_units extension. +BATTERY_INT_ATTR(min_voltage); +BATTERY_INT_ATTR(min_current); +BATTERY_INT_ATTR(min_capacity); +BATTERY_INT_ATTR(max_voltage); +BATTERY_INT_ATTR(max_current); +BATTERY_INT_ATTR(max_capacity); +BATTERY_INT_ATTR(temp); +BATTERY_INT_ATTR(voltage); +BATTERY_INT_ATTR(current); +BATTERY_INT_ATTR(capacity); I suggest adding remaining operating time and remaining charging time. You can try deducing these from the above attributes, but in practice this gives very inaccurate predictions. On laptops (e.g., ThinkPad) the BIOS or EC often provides much better estimates, using a more accurate physical model. I also see you omitted a host of other common attributes, like design capacity, cycle count, model string, and temperatures. There was an extensive LKML discussion of the choice and naming of attributes, in the context of David Woodhouse's patch; there are futher observations there. Also, here's the list of attributes in my tp_smapi ThinkPad driver: http://thinkwiki.org/wiki/tp_smapi#Battery_charge_control_features Does max capaxity correspond to what's usually denoted last full capacity. I understand you allow adding custom attributes, but they're of limited use if generic userspace tools don't know their name and semantics. It's important to standardize all reasonably common attributes. Shem - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/7] [RFC] Battery monitoring class
Hello Shem, On Thu, Apr 12, 2007 at 11:00:07AM -0400, Shem Multinymous wrote: Hi Anton, A few comments on the ever-contentious choice of battery attributes: On 4/11/07, Anton Vorontsov [EMAIL PROTECTED] wrote: + * All voltages, currents, capacities and temperatures in mV, mA, mAh and + * tenths of a degree unless otherwise stated. It's driver's job to convert + * its raw values to which this class operates. If for some reason driver + * can't afford this requirement, then it have to create its own attributes, + * plus additional XYZ_units for each of them. + */ Many (most, I believe) laptop batteries report capacity in mWh (energy), not mAh (charge). You can't convert between the two without a detailed, up-to-date physical model of the battery. This is too common to relegate to a non-standardized XYZ_units extension. I see. Okay, I'll try to cook something regarding capacity units. +BATTERY_INT_ATTR(min_voltage); +BATTERY_INT_ATTR(min_current); +BATTERY_INT_ATTR(min_capacity); +BATTERY_INT_ATTR(max_voltage); +BATTERY_INT_ATTR(max_current); +BATTERY_INT_ATTR(max_capacity); +BATTERY_INT_ATTR(temp); +BATTERY_INT_ATTR(voltage); +BATTERY_INT_ATTR(current); +BATTERY_INT_ATTR(capacity); I suggest adding remaining operating time and remaining charging time. You can try deducing these from the above attributes, but in practice this gives very inaccurate predictions. On laptops (e.g., ThinkPad) the BIOS or EC often provides much better estimates, using a more accurate physical model. Yes, sure. Feel free to add these attributes to the standard ones, along with your drivers. See (1). I also see you omitted a host of other common attributes, like design capacity, cycle count, model string, and temperatures. There was an extensive LKML discussion of the choice and naming of attributes, in the context of David Woodhouse's patch; there are futher observations there. Also, here's the list of attributes in my tp_smapi ThinkPad driver: http://thinkwiki.org/wiki/tp_smapi#Battery_charge_control_features Does max capaxity correspond to what's usually denoted last full capacity. Yup. I understand you allow adding custom attributes, but they're of limited use if generic userspace tools don't know their name and semantics. It's important to standardize all reasonably common attributes. (1) You're free to add your attribute to standard ones in include/linux/battery.h if it's proven to widely used. Batteries which do not have such attribute will not even notice that addition, so you will not have to grep and modify other drivers. So, that is plain matter of code duplication. If two or more battery drivers have same attributes, then we can put it in battery.h. If only one driver using it, then code duplication impossible, and driver should keep this attribute private. In handhelds.org tree we have two battery drivers with attributes you see currently. Second driver is not ready for mainline inclusion yet (it depends on ADC framework, which we'll present soon), thus I've not posted it. Also, APM emulation driver should learn how to use one attribute, or another depending on which available/better. But this is only technical question, like if (battery have attribute1) do precise maths of something, because we have mWh; else do approx maths of something, because we have only mAh. else n/a. Shem Thanks for comments! -- Anton Vorontsov email: [EMAIL PROTECTED] backup email: [EMAIL PROTECTED] irc://irc.freenode.org/bd2 - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/7] [RFC] Battery monitoring class
Hello Randy,
On Wed, Apr 11, 2007 at 07:53:59PM -0700, Randy Dunlap wrote:
On Thu, 12 Apr 2007 03:25:03 +0400 Anton Vorontsov wrote:
+void battery_status_changed(struct battery *bat)
+{
+ pr_debug(%s\n, __FUNCTION__);
+ #ifdef CONFIG_LEDS_TRIGGERS
Please don't indent preprocessor controls (ifdef/endif etc.).
...
Place 'switch' and 'case' at the same indent level. This prevents
the double-indent for the code statements.
...
We usually try to place a blank line between local data and code.
...
space after if
...
Much thanks, will fix.
+ device_remove_file(bat-dev, dev_attr_##name);
+
+ goto success;
+
+capacity_failed: remove_bat_attr_conditional(current);
+current_failed: remove_bat_attr_conditional(voltage);
+voltage_failed: remove_bat_attr_conditional(temp);
+temp_failed: remove_bat_attr_conditional(max_capacity);
+max_capacity_failed: remove_bat_attr_conditional(max_current);
+max_current_failed: remove_bat_attr_conditional(max_voltage);
+max_voltage_failed: remove_bat_attr_conditional(min_capacity);
+min_capacity_failed: remove_bat_attr_conditional(min_current);
+min_current_failed: remove_bat_attr_conditional(min_voltage);
+min_voltage_failed: remove_bat_attr_conditional(status);
I thought there was a class_remove() or something like that?
but I'm not sure of it.
[class_]device_destroy()? Yeah, but it's exactly same thing as
[class_]device_unregister.
But this is really good question. Should I manually clean up attributes,
or sysfs will take care? I.e. whole battery directory removes, and
sysfs removes files in it, or they just leaks?
I've took worst scenario, and done removal manually (I've grep'ed in
drivers/, and almost every driver also doing so).
---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
Thanks for comments!
--
Anton Vorontsov
email: [EMAIL PROTECTED]
backup email: [EMAIL PROTECTED]
irc://irc.freenode.org/bd2
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/7] [RFC] Battery monitoring class
Hi Anton,
On 4/12/07, Anton Vorontsov [EMAIL PROTECTED] wrote:
On Thu, Apr 12, 2007 at 11:00:07AM -0400, Shem Multinymous wrote:
I suggest adding remaining operating time and remaining charging
time. You can try deducing these from the above attributes, but in
practice this gives very inaccurate predictions. On laptops (e.g.,
ThinkPad) the BIOS or EC often provides much better estimates, using a
more accurate physical model.
Yes, sure. Feel free to add these attributes to the standard ones,
along with your drivers. See (1).
That's a sound way to go around i. We just need to be careful about
naming conventions. For example, *_charge rather than *_capacity,
so that we can later add *_energy analogously.
But specifically about {operating,charge} time remaining readouts, it
seems important to have them there from the beginning and have all
drivers implement them. Otherwise, userspace will just go ahead and
implement its own crude computation, so by the time when new
attributes and drivers are introduced, userspace will be full of bad
code. I've seen this happening with the tp_smapi ThinkPad driver -- I
introduced those attributes at a recent version, and then needed to
encourage userspace utility authors to dump their extrapolation
calcultions and use the attribute instead.
The rest of the missing attributes are not as imporant in this
respect, because userspace can't try to estimate them.
Shem
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Kernel-discuss] Re: [PATCH 3/7] [RFC] Battery monitoring class
On Thu, 12 Apr 2007, Paul Sokolovsky wrote: Yes, that's apparently the way to go. We just should consider if mAh and mWh are enough, or we go wider and allow whole collection of units. Btw, original handhelds.org code used Joules ;-). FWIW, SBS only mentions mAh and mWh. AFAIK, all other (meaningful) units should be able to be converted to either Ah or Wh, assuming enough precision on the math. I never heard of any other way to fuel-gauge batteries than these two main modes (current-based or capacity-based), but I don't work on the battery field. That said, you may need to use uWh and uAh instead of mAh and mWh, though. -- 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 - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Kernel-discuss] Re: [PATCH 3/7] [RFC] Battery monitoring class
On Thu, Apr 12, 2007 at 03:56:30PM -0300, Henrique de Moraes Holschuh wrote:
On Thu, 12 Apr 2007, Paul Sokolovsky wrote:
Yes, that's apparently the way to go. We just should consider
if mAh and mWh are enough, or we go wider and allow whole collection of
units. Btw, original handhelds.org code used Joules ;-).
FWIW, SBS only mentions mAh and mWh. AFAIK, all other (meaningful) units
should be able to be converted to either Ah or Wh, assuming enough precision
on the math. I never heard of any other way to fuel-gauge batteries than
these two main modes (current-based or capacity-based), but I don't work on
the battery field.
Okay, I have an idea:
Let's name attributes with mWh units as {min_,max_,design_,}energy,
and attributes with mAh units as {min_,max_,design_,}charge.
Because both energy and charge represents capacity in some meanings,
and that's why we bothering with _units attribute. So, lets drop
capacity* and use more specific terms? I really don't want string
attributes by default (except status).
If we export attributes with predefined units, userspace developers
could just look into include/linux/battery.h and conclude: Ah, great,
if battery reporting energy, then it's in mWh, and if battery reporting
charge it's always in mAh.
* Yup, I've read last discussion regarding batteries, and I've seen
objections against charge term, quoting Shem Multinymous:
And, for the reasons I explained earlier, I strongly suggest not using
the term charge except when referring to the action of charging.
Hence:
s/charge_rate/rate/; s/charge/capacity/
But lets think about it once again? We'll make things much cleaner
if we'll drop capacity at all.
That said, you may need to use uWh and uAh instead of mAh and mWh, though.
Not sure. Is there any existing chip that can report uAh/uWh? That is
great precision.
--
Anton Vorontsov
email: [EMAIL PROTECTED]
backup email: [EMAIL PROTECTED]
irc://irc.freenode.org/bd2
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Kernel-discuss] Re: [PATCH 3/7] [RFC] Battery monitoring class
On Fri, 13 Apr 2007, Anton Vorontsov wrote:
Let's name attributes with mWh units as {min_,max_,design_,}energy,
and attributes with mAh units as {min_,max_,design_,}charge.
[...]
* Yup, I've read last discussion regarding batteries, and I've seen
objections against charge term, quoting Shem Multinymous:
And, for the reasons I explained earlier, I strongly suggest not using
the term charge except when referring to the action of charging.
Hence:
s/charge_rate/rate/; s/charge/capacity/
But lets think about it once again? We'll make things much cleaner
if we'll drop capacity at all.
I stand with Shem on this one. The people behind the SBS specification
seems to agree... that specification is aimed at *engineers* and still
avoids the obvious trap of using charge due to its high potential for
confusion.
I don't even want to know how much of a mess the people writing applets
woudl make of it...
That said, you may need to use uWh and uAh instead of mAh and mWh, though.
Not sure. Is there any existing chip that can report uAh/uWh? That is
great precision.
The way things are going, it should be feasible for small embedded systems
quite soon. Refer to the previous thread.
--
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
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Kernel-discuss] Re: [PATCH 3/7] [RFC] Battery monitoring class
On Thu, Apr 12, 2007 at 09:51:12PM -0300, Henrique de Moraes Holschuh wrote:
On Fri, 13 Apr 2007, Anton Vorontsov wrote:
Let's name attributes with mWh units as {min_,max_,design_,}energy,
and attributes with mAh units as {min_,max_,design_,}charge.
[...]
* Yup, I've read last discussion regarding batteries, and I've seen
objections against charge term, quoting Shem Multinymous:
And, for the reasons I explained earlier, I strongly suggest not using
the term charge except when referring to the action of charging.
Hence:
s/charge_rate/rate/; s/charge/capacity/
But lets think about it once again? We'll make things much cleaner
if we'll drop capacity at all.
I stand with Shem on this one. The people behind the SBS specification
seems to agree... that specification is aimed at *engineers* and still
avoids the obvious trap of using charge due to its high potential for
confusion.
I don't even want to know how much of a mess the people writing applets
woudl make of it...
:-(
Okay, term charge is out of scope, I guess. But can we use capacity
for xAh, and energy for xWh? I just trying to separate these terms
somehow, and avoid _units stuff.
That said, you may need to use uWh and uAh instead of mAh and mWh, though.
Not sure. Is there any existing chip that can report uAh/uWh? That is
great precision.
The way things are going, it should be feasible for small embedded systems
quite soon. Refer to the previous thread.
I see... is it also applicable to currents and voltages? I.e. should we
use uA and uV from the start?
--
Anton Vorontsov
email: [EMAIL PROTECTED]
backup email: [EMAIL PROTECTED]
irc://irc.freenode.org/bd2
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Kernel-discuss] Re: [PATCH 3/7] [RFC] Battery monitoring class
Hi, On 4/12/07, Henrique de Moraes Holschuh [EMAIL PROTECTED] wrote: On Fri, 13 Apr 2007, Anton Vorontsov wrote: * Yup, I've read last discussion regarding batteries, and I've seen objections against charge term, quoting Shem Multinymous: And, for the reasons I explained earlier, I strongly suggest not using the term charge except when referring to the action of charging. Hence: s/charge_rate/rate/; s/charge/capacity/ But lets think about it once again? We'll make things much cleaner if we'll drop capacity at all. I stand with Shem on this one. The people behind the SBS specification seems to agree... that specification is aimed at *engineers* and still avoids the obvious trap of using charge due to its high potential for confusion. I don't even want to know how much of a mess the people writing applets woudl make of it... With fixed-units files, having *_energy and *_capacity isn't too clear either... Nor is it consistent with SBS, since SBS uses capacity to refer to either energy or charge, depending on a units attribute. As a compromise, how about using energy and charge for quantities, and charging (i.e., a verb) when referring to the operation? BTW, tp_smapi uses charge and charging interchangeably; that was a mistake. Shem - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Kernel-discuss] Re: [PATCH 3/7] [RFC] Battery monitoring class
On Thu, Apr 12, 2007 at 10:34:06PM -0400, Shem Multinymous wrote: Hi, On 4/12/07, Henrique de Moraes Holschuh [EMAIL PROTECTED] wrote: On Fri, 13 Apr 2007, Anton Vorontsov wrote: * Yup, I've read last discussion regarding batteries, and I've seen objections against charge term, quoting Shem Multinymous: And, for the reasons I explained earlier, I strongly suggest not using the term charge except when referring to the action of charging. Hence: s/charge_rate/rate/; s/charge/capacity/ But lets think about it once again? We'll make things much cleaner if we'll drop capacity at all. I stand with Shem on this one. The people behind the SBS specification seems to agree... that specification is aimed at *engineers* and still avoids the obvious trap of using charge due to its high potential for confusion. I don't even want to know how much of a mess the people writing applets woudl make of it... With fixed-units files, having *_energy and *_capacity isn't too clear either... Nor is it consistent with SBS, since SBS uses capacity to refer to either energy or charge, depending on a units attribute. As a compromise, how about using energy and charge for quantities, and charging (i.e., a verb) when referring to the operation? It would be great compromise! Please please please! -- Anton Vorontsov email: [EMAIL PROTECTED] backup email: [EMAIL PROTECTED] irc://irc.freenode.org/bd2 - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/7] [RFC] Battery monitoring class
On Thu, Apr 12, 2007 at 03:25:03AM +0400, Anton Vorontsov wrote:
> Here is battery monitor class. According to first copyright string, we're
> maintaining it since 2003. I've took few days and cleaned it up to be
> more suitable for mainline inclusion.
>
> It differs from battery class at git://git.infradead.org/battery-2.6.git:
Why fork from David's work? Does he not like these changes for some
reason?
> +static int battery_create_attrs(struct battery *bat)
> +{
> + int rc;
> +
> + #define create_bat_attr_conditional(name)\
> + if(bat->get_##name) {\
> + rc = device_create_file(bat->dev, _attr_##name); \
> + if (rc) goto name##_failed; \
> + }
> +
> + create_bat_attr_conditional(status);
> + create_bat_attr_conditional(min_voltage);
> + create_bat_attr_conditional(min_current);
> + create_bat_attr_conditional(min_capacity);
> + create_bat_attr_conditional(max_voltage);
> + create_bat_attr_conditional(max_current);
> + create_bat_attr_conditional(max_capacity);
> + create_bat_attr_conditional(temp);
> + create_bat_attr_conditional(voltage);
> + create_bat_attr_conditional(current);
> + create_bat_attr_conditional(capacity);
Use an attribute group please. It's much simpler and will be created at
the proper time so your userspace tools don't have to sit and spin in
order to properly wait for them to show up.
Ok, yes, you want a conditional type of attribute group, like the
new firewire code does. I have no problem adding that if you like.
thanks,
greg k-h
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/7] [RFC] Battery monitoring class
On Thu, 12 Apr 2007 03:25:03 +0400 Anton Vorontsov wrote:
> Here is battery monitor class. According to first copyright string, we're
> maintaining it since 2003. I've took few days and cleaned it up to be
> more suitable for mainline inclusion.
>
> ---
> drivers/Kconfig |2 +
> drivers/Makefile |1 +
> drivers/battery/Kconfig | 11 ++
> drivers/battery/Makefile |1 +
> drivers/battery/battery.c | 303
> +
> include/linux/battery.h | 98 +++
> 6 files changed, 416 insertions(+), 0 deletions(-)
> create mode 100644 drivers/battery/Kconfig
> create mode 100644 drivers/battery/Makefile
> create mode 100644 drivers/battery/battery.c
> create mode 100644 include/linux/battery.h
>
> diff --git a/drivers/battery/battery.c b/drivers/battery/battery.c
> new file mode 100644
> index 000..32b8288
> --- /dev/null
> +++ b/drivers/battery/battery.c
> @@ -0,0 +1,303 @@
> +
> +void battery_status_changed(struct battery *bat)
> +{
> + pr_debug("%s\n", __FUNCTION__);
> + #ifdef CONFIG_LEDS_TRIGGERS
Please don't indent preprocessor controls (ifdef/endif etc.).
> + switch(bat->get_status(bat))
> + {
> + case BATTERY_STATUS_FULL:
> + led_trigger_event(bat->charging_trig, LED_OFF);
> + led_trigger_event(bat->full_trig, LED_FULL);
> + break;
> + case BATTERY_STATUS_CHARGING:
> + led_trigger_event(bat->charging_trig, LED_FULL);
> + led_trigger_event(bat->full_trig, LED_OFF);
> + break;
> + default:
> + led_trigger_event(bat->charging_trig, LED_OFF);
> + led_trigger_event(bat->full_trig, LED_OFF);
> + break;
Place 'switch' and 'case' at the same indent level. This prevents
the "double-indent" for the code statements.
> + }
> + #endif /* CONFIG_LEDS_TRIGGERS */
> + return;
> +}
> +
> +static char *status_text[] = {
> + "Unknown", "Charging", "Discharging", "Not charging", "Full"
> +};
> +
> +static ssize_t battery_show_status(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct battery *bat = dev_get_drvdata(dev);
> + int status = 0;
We usually try to place a blank line between local data and code.
> + if (bat->get_status) {
> + status = bat->get_status(bat);
> + if (status > 4)
> + status = 0;
> + return sprintf(buf, "%s\n", status_text[status]);
> + }
> + return 0;
> +}
> +
> +static int battery_create_attrs(struct battery *bat)
> +{
> + int rc;
> +
> + #define create_bat_attr_conditional(name)\
> + if(bat->get_##name) {\
space after "if"
> + rc = device_create_file(bat->dev, _attr_##name); \
> + if (rc) goto name##_failed; \
> + }
> +
> + create_bat_attr_conditional(status);
> + create_bat_attr_conditional(min_voltage);
> + create_bat_attr_conditional(min_current);
> + create_bat_attr_conditional(min_capacity);
> + create_bat_attr_conditional(max_voltage);
> + create_bat_attr_conditional(max_current);
> + create_bat_attr_conditional(max_capacity);
> + create_bat_attr_conditional(temp);
> + create_bat_attr_conditional(voltage);
> + create_bat_attr_conditional(current);
> + create_bat_attr_conditional(capacity);
> +
> + #define remove_bat_attr_conditional(name) \
> + if(bat->get_##name) \
ditto.
> + device_remove_file(bat->dev, _attr_##name);
> +
> + goto success;
> +
> +capacity_failed: remove_bat_attr_conditional(current);
> +current_failed: remove_bat_attr_conditional(voltage);
> +voltage_failed: remove_bat_attr_conditional(temp);
> +temp_failed: remove_bat_attr_conditional(max_capacity);
> +max_capacity_failed: remove_bat_attr_conditional(max_current);
> +max_current_failed: remove_bat_attr_conditional(max_voltage);
> +max_voltage_failed: remove_bat_attr_conditional(min_capacity);
> +min_capacity_failed: remove_bat_attr_conditional(min_current);
> +min_current_failed: remove_bat_attr_conditional(min_voltage);
> +min_voltage_failed: remove_bat_attr_conditional(status);
I thought there was a class_remove() or something like that?
but I'm not sure of it.
> +status_failed:
> +success:
> + return rc;
> +}
> +
> +static void battery_remove_attrs(struct battery *bat)
> +{
> + remove_bat_attr_conditional(capacity);
> + remove_bat_attr_conditional(current);
> + remove_bat_attr_conditional(voltage);
> + remove_bat_attr_conditional(temp);
> + remove_bat_attr_conditional(max_capacity);
> + remove_bat_attr_conditional(max_current);
> +
[PATCH 3/7] [RFC] Battery monitoring class
Here is battery monitor class. According to first copyright string, we're maintaining it since 2003. I've took few days and cleaned it up to be more suitable for mainline inclusion. It differs from battery class at git://git.infradead.org/battery-2.6.git: * It's using external power kernel interface, i.e. does not fake external powers as batteries. (Same thing David Woodhouse planed last year). * It have predefined set of attributes, this eliminates code duplication by battery drivers. And also gives opportunity to write emulation drivers for legacy stuff (APM emulation driver follow). If driver can't afford some attribute, it will not appear in sysfs. * It insists on reusing its predefined attributes *and* their units. So, userspace getting expected values for any battery. Also common units is required for APM/ACPI emulation. Though our battery class insisting on re-usage, but not forces it. If some battery driver can't convert its own raw values (can't imagine why), then driver is free to implement its own attributes *and* additional _units attribute. Though, this scheme is discouraged. * LEDs support. Each battery register its trigger, and gadgets with LEDs can quickly bind to battery-charging / battery-full triggers. Here how it looks like from user space: # ls /sys/class/battery/main-battery/ capacity max_capacity max_voltage min_current power subsystem uevent current max_current min_capacity min_voltage status temp voltage # cat /sys/class/battery/main-battery/status Full # cat /sys/class/leds/h5400\:green-right/trigger none h5400-radio timer hwtimer main-battery-charging [main-battery-full] # cat /sys/class/leds/h5400\:green-right/brightness 255 --- drivers/Kconfig |2 + drivers/Makefile |1 + drivers/battery/Kconfig | 11 ++ drivers/battery/Makefile |1 + drivers/battery/battery.c | 303 + include/linux/battery.h | 98 +++ 6 files changed, 416 insertions(+), 0 deletions(-) create mode 100644 drivers/battery/Kconfig create mode 100644 drivers/battery/Makefile create mode 100644 drivers/battery/battery.c create mode 100644 include/linux/battery.h diff --git a/drivers/Kconfig b/drivers/Kconfig index c546de3..c3a0038 100644 --- a/drivers/Kconfig +++ b/drivers/Kconfig @@ -56,6 +56,8 @@ source "drivers/w1/Kconfig" source "drivers/power/Kconfig" +source "drivers/battery/Kconfig" + source "drivers/hwmon/Kconfig" source "drivers/mfd/Kconfig" diff --git a/drivers/Makefile b/drivers/Makefile index 2bdaae7..7cbfd37 100644 --- a/drivers/Makefile +++ b/drivers/Makefile @@ -61,6 +61,7 @@ obj-$(CONFIG_RTC_LIB) += rtc/ obj-$(CONFIG_I2C) += i2c/ obj-$(CONFIG_W1) += w1/ obj-$(CONFIG_EXTERNAL_POWER) += power/ +obj-$(CONFIG_BATTERY) += battery/ obj-$(CONFIG_HWMON)+= hwmon/ obj-$(CONFIG_PHONE)+= telephony/ obj-$(CONFIG_MD) += md/ diff --git a/drivers/battery/Kconfig b/drivers/battery/Kconfig new file mode 100644 index 000..c386593 --- /dev/null +++ b/drivers/battery/Kconfig @@ -0,0 +1,11 @@ + +menu "Battery support" + +config BATTERY + tristate "Battery monitoring support" + select EXTERNAL_POWER + help + Say Y here to enable generic battery status reporting in + the /sys filesystem. + +endmenu diff --git a/drivers/battery/Makefile b/drivers/battery/Makefile new file mode 100644 index 000..a2239cb --- /dev/null +++ b/drivers/battery/Makefile @@ -0,0 +1 @@ +obj-$(CONFIG_BATTERY) += battery.o diff --git a/drivers/battery/battery.c b/drivers/battery/battery.c new file mode 100644 index 000..32b8288 --- /dev/null +++ b/drivers/battery/battery.c @@ -0,0 +1,303 @@ +/* + * Universal battery monitor class + * + * Copyright (c) 2007 Anton Vorontsov <[EMAIL PROTECTED]> + * Copyright (c) 2004 Szabolcs Gyurko + * Copyright (c) 2003 Ian Molton <[EMAIL PROTECTED]> + * + * Modified: 2004, Oct Szabolcs Gyurko + * + * You may use this code as per GPL version 2 + * + * All voltages, currents, capacities and temperatures in mV, mA, mAh and + * tenths of a degree unless otherwise stated. It's driver's job to convert + * its raw values to which this class operates. If for some reason driver + * can't afford this requirement, then it have to create its own attributes, + * plus additional "XYZ_units" for each of them. + */ + +#include +#include +#include +#include +#include +#include + +/* If we have hwtimer trigger, then use it to blink charging LED */ +#if defined(CONFIG_LEDS_TRIGGER_HWTIMER) || \ +defined(CONFIG_LEDS_TRIGGER_HWTIMER_MODULE) + #define led_trigger_register_charging led_trigger_register_hwtimer + #define led_trigger_unregister_charging led_trigger_unregister_hwtimer +#else + #define led_trigger_register_charging led_trigger_register_simple + #define
[PATCH 3/7] [RFC] Battery monitoring class
Here is battery monitor class. According to first copyright string, we're maintaining it since 2003. I've took few days and cleaned it up to be more suitable for mainline inclusion. It differs from battery class at git://git.infradead.org/battery-2.6.git: * It's using external power kernel interface, i.e. does not fake external powers as batteries. (Same thing David Woodhouse planed last year). * It have predefined set of attributes, this eliminates code duplication by battery drivers. And also gives opportunity to write emulation drivers for legacy stuff (APM emulation driver follow). If driver can't afford some attribute, it will not appear in sysfs. * It insists on reusing its predefined attributes *and* their units. So, userspace getting expected values for any battery. Also common units is required for APM/ACPI emulation. Though our battery class insisting on re-usage, but not forces it. If some battery driver can't convert its own raw values (can't imagine why), then driver is free to implement its own attributes *and* additional _units attribute. Though, this scheme is discouraged. * LEDs support. Each battery register its trigger, and gadgets with LEDs can quickly bind to battery-charging / battery-full triggers. Here how it looks like from user space: # ls /sys/class/battery/main-battery/ capacity max_capacity max_voltage min_current power subsystem uevent current max_current min_capacity min_voltage status temp voltage # cat /sys/class/battery/main-battery/status Full # cat /sys/class/leds/h5400\:green-right/trigger none h5400-radio timer hwtimer main-battery-charging [main-battery-full] # cat /sys/class/leds/h5400\:green-right/brightness 255 --- drivers/Kconfig |2 + drivers/Makefile |1 + drivers/battery/Kconfig | 11 ++ drivers/battery/Makefile |1 + drivers/battery/battery.c | 303 + include/linux/battery.h | 98 +++ 6 files changed, 416 insertions(+), 0 deletions(-) create mode 100644 drivers/battery/Kconfig create mode 100644 drivers/battery/Makefile create mode 100644 drivers/battery/battery.c create mode 100644 include/linux/battery.h diff --git a/drivers/Kconfig b/drivers/Kconfig index c546de3..c3a0038 100644 --- a/drivers/Kconfig +++ b/drivers/Kconfig @@ -56,6 +56,8 @@ source drivers/w1/Kconfig source drivers/power/Kconfig +source drivers/battery/Kconfig + source drivers/hwmon/Kconfig source drivers/mfd/Kconfig diff --git a/drivers/Makefile b/drivers/Makefile index 2bdaae7..7cbfd37 100644 --- a/drivers/Makefile +++ b/drivers/Makefile @@ -61,6 +61,7 @@ obj-$(CONFIG_RTC_LIB) += rtc/ obj-$(CONFIG_I2C) += i2c/ obj-$(CONFIG_W1) += w1/ obj-$(CONFIG_EXTERNAL_POWER) += power/ +obj-$(CONFIG_BATTERY) += battery/ obj-$(CONFIG_HWMON)+= hwmon/ obj-$(CONFIG_PHONE)+= telephony/ obj-$(CONFIG_MD) += md/ diff --git a/drivers/battery/Kconfig b/drivers/battery/Kconfig new file mode 100644 index 000..c386593 --- /dev/null +++ b/drivers/battery/Kconfig @@ -0,0 +1,11 @@ + +menu Battery support + +config BATTERY + tristate Battery monitoring support + select EXTERNAL_POWER + help + Say Y here to enable generic battery status reporting in + the /sys filesystem. + +endmenu diff --git a/drivers/battery/Makefile b/drivers/battery/Makefile new file mode 100644 index 000..a2239cb --- /dev/null +++ b/drivers/battery/Makefile @@ -0,0 +1 @@ +obj-$(CONFIG_BATTERY) += battery.o diff --git a/drivers/battery/battery.c b/drivers/battery/battery.c new file mode 100644 index 000..32b8288 --- /dev/null +++ b/drivers/battery/battery.c @@ -0,0 +1,303 @@ +/* + * Universal battery monitor class + * + * Copyright (c) 2007 Anton Vorontsov [EMAIL PROTECTED] + * Copyright (c) 2004 Szabolcs Gyurko + * Copyright (c) 2003 Ian Molton [EMAIL PROTECTED] + * + * Modified: 2004, Oct Szabolcs Gyurko + * + * You may use this code as per GPL version 2 + * + * All voltages, currents, capacities and temperatures in mV, mA, mAh and + * tenths of a degree unless otherwise stated. It's driver's job to convert + * its raw values to which this class operates. If for some reason driver + * can't afford this requirement, then it have to create its own attributes, + * plus additional XYZ_units for each of them. + */ + +#include linux/module.h +#include linux/types.h +#include linux/init.h +#include linux/device.h +#include linux/err.h +#include linux/battery.h + +/* If we have hwtimer trigger, then use it to blink charging LED */ +#if defined(CONFIG_LEDS_TRIGGER_HWTIMER) || \ +defined(CONFIG_LEDS_TRIGGER_HWTIMER_MODULE) + #define led_trigger_register_charging led_trigger_register_hwtimer + #define led_trigger_unregister_charging led_trigger_unregister_hwtimer +#else + #define led_trigger_register_charging
Re: [PATCH 3/7] [RFC] Battery monitoring class
On Thu, 12 Apr 2007 03:25:03 +0400 Anton Vorontsov wrote:
Here is battery monitor class. According to first copyright string, we're
maintaining it since 2003. I've took few days and cleaned it up to be
more suitable for mainline inclusion.
---
drivers/Kconfig |2 +
drivers/Makefile |1 +
drivers/battery/Kconfig | 11 ++
drivers/battery/Makefile |1 +
drivers/battery/battery.c | 303
+
include/linux/battery.h | 98 +++
6 files changed, 416 insertions(+), 0 deletions(-)
create mode 100644 drivers/battery/Kconfig
create mode 100644 drivers/battery/Makefile
create mode 100644 drivers/battery/battery.c
create mode 100644 include/linux/battery.h
diff --git a/drivers/battery/battery.c b/drivers/battery/battery.c
new file mode 100644
index 000..32b8288
--- /dev/null
+++ b/drivers/battery/battery.c
@@ -0,0 +1,303 @@
+
+void battery_status_changed(struct battery *bat)
+{
+ pr_debug(%s\n, __FUNCTION__);
+ #ifdef CONFIG_LEDS_TRIGGERS
Please don't indent preprocessor controls (ifdef/endif etc.).
+ switch(bat-get_status(bat))
+ {
+ case BATTERY_STATUS_FULL:
+ led_trigger_event(bat-charging_trig, LED_OFF);
+ led_trigger_event(bat-full_trig, LED_FULL);
+ break;
+ case BATTERY_STATUS_CHARGING:
+ led_trigger_event(bat-charging_trig, LED_FULL);
+ led_trigger_event(bat-full_trig, LED_OFF);
+ break;
+ default:
+ led_trigger_event(bat-charging_trig, LED_OFF);
+ led_trigger_event(bat-full_trig, LED_OFF);
+ break;
Place 'switch' and 'case' at the same indent level. This prevents
the double-indent for the code statements.
+ }
+ #endif /* CONFIG_LEDS_TRIGGERS */
+ return;
+}
+
+static char *status_text[] = {
+ Unknown, Charging, Discharging, Not charging, Full
+};
+
+static ssize_t battery_show_status(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct battery *bat = dev_get_drvdata(dev);
+ int status = 0;
We usually try to place a blank line between local data and code.
+ if (bat-get_status) {
+ status = bat-get_status(bat);
+ if (status 4)
+ status = 0;
+ return sprintf(buf, %s\n, status_text[status]);
+ }
+ return 0;
+}
+
+static int battery_create_attrs(struct battery *bat)
+{
+ int rc;
+
+ #define create_bat_attr_conditional(name)\
+ if(bat-get_##name) {\
space after if
+ rc = device_create_file(bat-dev, dev_attr_##name); \
+ if (rc) goto name##_failed; \
+ }
+
+ create_bat_attr_conditional(status);
+ create_bat_attr_conditional(min_voltage);
+ create_bat_attr_conditional(min_current);
+ create_bat_attr_conditional(min_capacity);
+ create_bat_attr_conditional(max_voltage);
+ create_bat_attr_conditional(max_current);
+ create_bat_attr_conditional(max_capacity);
+ create_bat_attr_conditional(temp);
+ create_bat_attr_conditional(voltage);
+ create_bat_attr_conditional(current);
+ create_bat_attr_conditional(capacity);
+
+ #define remove_bat_attr_conditional(name) \
+ if(bat-get_##name) \
ditto.
+ device_remove_file(bat-dev, dev_attr_##name);
+
+ goto success;
+
+capacity_failed: remove_bat_attr_conditional(current);
+current_failed: remove_bat_attr_conditional(voltage);
+voltage_failed: remove_bat_attr_conditional(temp);
+temp_failed: remove_bat_attr_conditional(max_capacity);
+max_capacity_failed: remove_bat_attr_conditional(max_current);
+max_current_failed: remove_bat_attr_conditional(max_voltage);
+max_voltage_failed: remove_bat_attr_conditional(min_capacity);
+min_capacity_failed: remove_bat_attr_conditional(min_current);
+min_current_failed: remove_bat_attr_conditional(min_voltage);
+min_voltage_failed: remove_bat_attr_conditional(status);
I thought there was a class_remove() or something like that?
but I'm not sure of it.
+status_failed:
+success:
+ return rc;
+}
+
+static void battery_remove_attrs(struct battery *bat)
+{
+ remove_bat_attr_conditional(capacity);
+ remove_bat_attr_conditional(current);
+ remove_bat_attr_conditional(voltage);
+ remove_bat_attr_conditional(temp);
+ remove_bat_attr_conditional(max_capacity);
+ remove_bat_attr_conditional(max_current);
+ remove_bat_attr_conditional(max_voltage);
+ remove_bat_attr_conditional(min_capacity);
+ remove_bat_attr_conditional(min_current);
+
Re: [PATCH 3/7] [RFC] Battery monitoring class
On Thu, Apr 12, 2007 at 03:25:03AM +0400, Anton Vorontsov wrote:
Here is battery monitor class. According to first copyright string, we're
maintaining it since 2003. I've took few days and cleaned it up to be
more suitable for mainline inclusion.
It differs from battery class at git://git.infradead.org/battery-2.6.git:
Why fork from David's work? Does he not like these changes for some
reason?
+static int battery_create_attrs(struct battery *bat)
+{
+ int rc;
+
+ #define create_bat_attr_conditional(name)\
+ if(bat-get_##name) {\
+ rc = device_create_file(bat-dev, dev_attr_##name); \
+ if (rc) goto name##_failed; \
+ }
+
+ create_bat_attr_conditional(status);
+ create_bat_attr_conditional(min_voltage);
+ create_bat_attr_conditional(min_current);
+ create_bat_attr_conditional(min_capacity);
+ create_bat_attr_conditional(max_voltage);
+ create_bat_attr_conditional(max_current);
+ create_bat_attr_conditional(max_capacity);
+ create_bat_attr_conditional(temp);
+ create_bat_attr_conditional(voltage);
+ create_bat_attr_conditional(current);
+ create_bat_attr_conditional(capacity);
Use an attribute group please. It's much simpler and will be created at
the proper time so your userspace tools don't have to sit and spin in
order to properly wait for them to show up.
Ok, yes, you want a conditional type of attribute group, like the
new firewire code does. I have no problem adding that if you like.
thanks,
greg k-h
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
