[PATCH] power_supply: support CHARGE_NOW in OLPC battery - Kernel

This is a discussion on [PATCH] power_supply: support CHARGE_NOW in OLPC battery - Kernel ; This is originally based on a patch by David Woodhouse . Add support for PROP_CHARGE_NOW by querying the ACR (accumulated current register) from the EC. Signed-off-by: Andres Salomon --- drivers/power/olpc_battery.c | 10 +++++++++- 1 files changed, 9 insertions(+), 1 deletions(-) ...

+ Reply to Thread
Results 1 to 11 of 11

Thread: [PATCH] power_supply: support CHARGE_NOW in OLPC battery

  1. [PATCH] power_supply: support CHARGE_NOW in OLPC battery


    This is originally based on a patch by
    David Woodhouse . Add support for PROP_CHARGE_NOW
    by querying the ACR (accumulated current register) from the EC.

    Signed-off-by: Andres Salomon
    ---
    drivers/power/olpc_battery.c | 10 +++++++++-
    1 files changed, 9 insertions(+), 1 deletions(-)

    diff --git a/drivers/power/olpc_battery.c b/drivers/power/olpc_battery.c
    index e3f6ec8..517dacd 100644
    --- a/drivers/power/olpc_battery.c
    +++ b/drivers/power/olpc_battery.c
    @@ -19,7 +19,7 @@

    #define EC_BAT_VOLTAGE 0x10 /* uint16_t, *9.76/32, mV */
    #define EC_BAT_CURRENT 0x11 /* int16_t, *15.625/120, mA */
    -#define EC_BAT_ACR 0x12
    +#define EC_BAT_ACR 0x12 /* int16_t, *416.667, µAh */
    #define EC_BAT_TEMP 0x13 /* uint16_t, *100/256, °C */
    #define EC_AMB_TEMP 0x14 /* uint16_t, *100/256, °C */
    #define EC_BAT_STATUS 0x15 /* uint8_t, bitmask */
    @@ -289,6 +289,13 @@ static int olpc_bat_get_property(struct power_supply *psy,
    ec_word = be16_to_cpu(ec_word);
    val->intval = ec_word * 100 / 256;
    break;
    + case POWER_SUPPLY_PROP_CHARGE_NOW:
    + ret = olpc_ec_cmd(EC_BAT_ACR, NULL, 0, (void *)&ec_word, 2);
    + if (ret)
    + return ret;
    +
    + val->intval = be16_to_cpu(ec_word);
    + break;
    case POWER_SUPPLY_PROP_SERIAL_NUMBER:
    ret = olpc_ec_cmd(EC_BAT_SERIAL, NULL, 0, (void *)&ser_buf, 8);
    if (ret)
    @@ -317,6 +324,7 @@ static enum power_supply_property olpc_bat_props[] = {
    POWER_SUPPLY_PROP_TEMP_AMBIENT,
    POWER_SUPPLY_PROP_MANUFACTURER,
    POWER_SUPPLY_PROP_SERIAL_NUMBER,
    + POWER_SUPPLY_PROP_CHARGE_NOW,
    };

    /* EEPROM reading goes completely around the power_supply API, sadly */
    --
    1.5.5.1

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

  2. Re: [PATCH] power_supply: support CHARGE_NOW in OLPC battery

    On Thu, May 08, 2008 at 12:34:54AM -0400, Andres Salomon wrote:
    >
    > This is originally based on a patch by
    > David Woodhouse . Add support for PROP_CHARGE_NOW
    > by querying the ACR (accumulated current register) from the EC.
    >
    > Signed-off-by: Andres Salomon
    > ---


    Hi Andres,

    > drivers/power/olpc_battery.c | 10 +++++++++-
    > 1 files changed, 9 insertions(+), 1 deletions(-)
    >
    > diff --git a/drivers/power/olpc_battery.c b/drivers/power/olpc_battery.c
    > index e3f6ec8..517dacd 100644
    > --- a/drivers/power/olpc_battery.c
    > +++ b/drivers/power/olpc_battery.c
    > @@ -19,7 +19,7 @@
    >
    > #define EC_BAT_VOLTAGE 0x10 /* uint16_t, *9.76/32, mV */
    > #define EC_BAT_CURRENT 0x11 /* int16_t, *15.625/120, mA */
    > -#define EC_BAT_ACR 0x12
    > +#define EC_BAT_ACR 0x12 /* int16_t, *416.667, µAh */


    The hardware reports uAh * 416.667..?

    > #define EC_BAT_TEMP 0x13 /* uint16_t, *100/256, °C */
    > #define EC_AMB_TEMP 0x14 /* uint16_t, *100/256, °C */
    > #define EC_BAT_STATUS 0x15 /* uint8_t, bitmask */
    > @@ -289,6 +289,13 @@ static int olpc_bat_get_property(struct power_supply *psy,
    > ec_word = be16_to_cpu(ec_word);
    > val->intval = ec_word * 100 / 256;
    > break;
    > + case POWER_SUPPLY_PROP_CHARGE_NOW:
    > + ret = olpc_ec_cmd(EC_BAT_ACR, NULL, 0, (void *)&ec_word, 2);
    > + if (ret)
    > + return ret;
    > +
    > + val->intval = be16_to_cpu(ec_word);


    But you didn't convert it to the uAh, I think you should.

    > + break;
    > case POWER_SUPPLY_PROP_SERIAL_NUMBER:
    > ret = olpc_ec_cmd(EC_BAT_SERIAL, NULL, 0, (void *)&ser_buf, 8);
    > if (ret)
    > @@ -317,6 +324,7 @@ static enum power_supply_property olpc_bat_props[] = {
    > POWER_SUPPLY_PROP_TEMP_AMBIENT,
    > POWER_SUPPLY_PROP_MANUFACTURER,
    > POWER_SUPPLY_PROP_SERIAL_NUMBER,
    > + POWER_SUPPLY_PROP_CHARGE_NOW,
    > };
    >
    > /* EEPROM reading goes completely around the power_supply API, sadly */


    --
    Anton Vorontsov
    email: cbouatmailru@gmail.com
    irc://irc.freenode.net/bd2
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  3. Re: [PATCH] power_supply: support CHARGE_NOW in OLPC battery

    On Thu, May 08, 2008 at 01:01:02PM -0400, Andres Salomon wrote:
    > Ok, CC'ing Richard as I should've done in the first place...

    [...]
    > > > + case POWER_SUPPLY_PROP_CHARGE_NOW:
    > > > + ret = olpc_ec_cmd(EC_BAT_ACR, NULL, 0, (void
    > > > *)&ec_word, 2);
    > > > + if (ret)
    > > > + return ret;
    > > > +
    > > > + val->intval = be16_to_cpu(ec_word);

    > >
    > > But you didn't convert it to the uAh, I think you should.

    >
    >
    > We have logic in userspace that converts ACR to a useful number; for
    > our purposes, we'd just like to read the raw ACR values.


    No. The purpose of power supply class is to report values in common
    units. Not some random value that userspace then should guess how to
    compute.

    > Perhaps CHARGE_NOW is the wrong thing to be using, if we can't get
    > uAh and CHARGE_* requires that unit.


    I don't think that converting X * uAh or uAh / Y to uAh is impossible.
    I'd suggest you to convert the ACR value to uAh using fractions, see
    ds2760_battery.c. Even if the final value losing precision a bit, it is
    still useful to do CHARGE_NOW.

    Of course, additionally you can create your own private attribute
    which reports raw value, if this is anyhow useful.

    --
    Anton Vorontsov
    email: cbouatmailru@gmail.com
    irc://irc.freenode.net/bd2
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  4. Re: [PATCH] power_supply: support CHARGE_NOW in OLPC battery

    Anton Vorontsov wrote:
    > On Thu, May 08, 2008 at 01:01:02PM -0400, Andres Salomon wrote:
    >> Ok, CC'ing Richard as I should've done in the first place...

    > [...]
    >>>> + case POWER_SUPPLY_PROP_CHARGE_NOW:
    >>>> + ret = olpc_ec_cmd(EC_BAT_ACR, NULL, 0, (void
    >>>> *)&ec_word, 2);
    >>>> + if (ret)
    >>>> + return ret;
    >>>> +
    >>>> + val->intval = be16_to_cpu(ec_word);
    >>> But you didn't convert it to the uAh, I think you should.

    >>
    >> We have logic in userspace that converts ACR to a useful number; for
    >> our purposes, we'd just like to read the raw ACR values.

    >
    > No. The purpose of power supply class is to report values in common
    > units. Not some random value that userspace then should guess how to
    > compute.


    I'm fine with that. I'll have to make my scripts do some detection on
    what kernel is running and convert or not convert but thats not too hard.

    I remember Dave asking me if I wanted it converted and I told him not to
    bother and just to export the raw value. I would deal with the units.
    At the time I was just trying to get some simple debugging info. Turns
    out that ACR is really useful for debugging battery problems and it stuck.

    A single ACR reading itself does not tell you anything usefull. Its a
    signed 16 bit free running coulomb counter that counts up and down based
    on how much juice goes into or out of the battery. So to extract any
    meaningful info you need 2 readings separated by some instance in time.
    The difference between the 2 tells you the relative energy you have
    consumed or added.

    The units for the ACR register in the DS2765 are:

    6.25 uVh / Rsense
    OLPC Rsense = .015 Ohms

    The integer math conversion for ACR to mAh that I use in my scripts is:

    (ACR2 - ACR1) * 625 / 1500

    >> Perhaps CHARGE_NOW is the wrong thing to be using, if we can't get
    >> uAh and CHARGE_* requires that unit.

    >
    > I don't think that converting X * uAh or uAh / Y to uAh is impossible.
    > I'd suggest you to convert the ACR value to uAh using fractions, see
    > ds2760_battery.c. Even if the final value losing precision a bit, it is
    > still useful to do CHARGE_NOW.


    Is CHARGE_NOW supposed to be an absolute indication of the level of
    charge in the battery? If so then thats not ACR. That would be SOC% *
    battery capacity. The EC uses the relative motion of the ACR to update
    the SOC%.

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

  5. Re: [PATCH] power_supply: support CHARGE_NOW in OLPC battery

    On Thu, May 08, 2008 at 02:53:25PM -0400, Richard A. Smith wrote:
    > Anton Vorontsov wrote:
    >> On Thu, May 08, 2008 at 01:01:02PM -0400, Andres Salomon wrote:
    >>> Ok, CC'ing Richard as I should've done in the first place...

    >> [...]
    >>>>> + case POWER_SUPPLY_PROP_CHARGE_NOW:
    >>>>> + ret = olpc_ec_cmd(EC_BAT_ACR, NULL, 0, (void
    >>>>> *)&ec_word, 2);
    >>>>> + if (ret)
    >>>>> + return ret;
    >>>>> +
    >>>>> + val->intval = be16_to_cpu(ec_word);
    >>>> But you didn't convert it to the uAh, I think you should.
    >>>
    >>> We have logic in userspace that converts ACR to a useful number; for
    >>> our purposes, we'd just like to read the raw ACR values.

    >>
    >> No. The purpose of power supply class is to report values in common
    >> units. Not some random value that userspace then should guess how to
    >> compute.

    >
    > I'm fine with that. I'll have to make my scripts do some detection on
    > what kernel is running and convert or not convert but thats not too hard.
    >
    > I remember Dave asking me if I wanted it converted and I told him not to
    > bother and just to export the raw value. I would deal with the units.
    > At the time I was just trying to get some simple debugging info. Turns
    > out that ACR is really useful for debugging battery problems and it
    > stuck.
    >
    > A single ACR reading itself does not tell you anything usefull. Its a
    > signed 16 bit free running coulomb counter that counts up and down based
    > on how much juice goes into or out of the battery. So to extract any
    > meaningful info you need 2 readings separated by some instance in time.
    > The difference between the 2 tells you the relative energy you have
    > consumed or added.


    Cool, I believe this is exactly same thing as we have on DS2760. ACR
    counts up and down. For example, if there was a current of 10 uA for
    30 minutes, ACR will show 5 uAh. Additional 30 minutes with current
    -10 uA will result in 0 uAh in ACR... Right?

    > The units for the ACR register in the DS2765 are:


    Do you have any specs for the ds2765? I'd like to see one, if
    it isn't restricted matter. google:ds2765 pdf wasn't helpful...

    > 6.25 uVh / Rsense
    > OLPC Rsense = .015 Ohms
    >
    > The integer math conversion for ACR to mAh that I use in my scripts is:
    >
    > (ACR2 - ACR1) * 625 / 1500


    Just curious.. What does the final value mean and how do you use it?
    ACR is the Ah counter. You're subtracting one from another, and that
    value will show you.. difference of Ah between T2 and T1..?

    You'll get something useful (current) if you divide the value by T2-T1
    though... ;-) But I believe OLPC can report current by itself...

    >>> Perhaps CHARGE_NOW is the wrong thing to be using, if we can't get
    >>> uAh and CHARGE_* requires that unit.

    >>
    >> I don't think that converting X * uAh or uAh / Y to uAh is impossible.
    >> I'd suggest you to convert the ACR value to uAh using fractions, see
    >> ds2760_battery.c. Even if the final value losing precision a bit, it is
    >> still useful to do CHARGE_NOW.

    >
    > Is CHARGE_NOW supposed to be an absolute indication of the level of
    > charge in the battery? If so then thats not ACR.


    This depends. On DS2760 ACR is the only way to determine battery's
    "charge" at the given time (thus the capacity in percents, since ds2760
    can't report it).

    It needs calibration though... ds2760_battery.c doing this: resetting the
    ACR to CHARGE_FULL value when battery is in charging state with current
    flow < 10 mA.

    Works quite good here.

    > That would be SOC% *
    > battery capacity. The EC uses the relative motion of the ACR to update
    > the SOC%.


    Since you don't seem to use ACR as CHARGE_NOW, I think maybe we indeed
    should implement PROP_ACR or better PROP_CHARGE_COUNTER? With units of
    uAh... would that work?

    --
    Anton Vorontsov
    email: cbouatmailru@gmail.com
    irc://irc.freenode.net/bd2
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  6. Re: [PATCH] power_supply: support CHARGE_NOW in OLPC battery

    Anton Vorontsov wrote:

    > Cool, I believe this is exactly same thing as we have on DS2760. ACR
    > counts up and down. For example, if there was a current of 10 uA for
    > 30 minutes, ACR will show 5 uAh. Additional 30 minutes with current
    > -10 uA will result in 0 uAh in ACR... Right?


    Yes. Exactly correct.

    >> The units for the ACR register in the DS2765 are:

    >
    > Do you have any specs for the ds2765? I'd like to see one, if
    > it isn't restricted matter. google:ds2765 pdf wasn't helpful...
    >


    Ooop. Sorry my typo. DS2756.

    http://wiki.laptop.org/images/e/e9/DS2756.pdf
    or
    http://www.maxim-ic.com/quick_view2.cfm/qv_pk/5104

    >> 6.25 uVh / Rsense
    >> OLPC Rsense = .015 Ohms
    >>
    >> The integer math conversion for ACR to mAh that I use in my scripts is:
    >>
    >> (ACR2 - ACR1) * 625 / 1500

    >
    > Just curious.. What does the final value mean and how do you use it?
    > ACR is the Ah counter. You're subtracting one from another, and that
    > value will show you.. difference of Ah between T2 and T1..?
    >
    > You'll get something useful (current) if you divide the value by T2-T1
    > though... ;-) But I believe OLPC can report current by itself...


    There is no final value. Only relative values. The EC can detect the
    ends of the spectrum. Battery empty and battery full. At those
    endpoints it sets the SOC% to x% (exact x varies depending on the
    battery type) But lets use full and 90% as the example.

    So you charge the battery until the criteria for a full battery are met
    and then you set the SOC% to 90%. This value is stored in the EEPROM of
    the DS2756. So then when the EC senses that the battery is discharging
    it monitors the relative ACR change. It uses that to compute how much %
    loss and updates the SOC%. If you are partially discharged and Ext
    power is provided so that the battery begins to charge then the reverse
    happens. The absolute value of the ACR is never used. Only the
    difference. The corrections of any errors happen when the full or empty
    conditions are met.

    >> Is CHARGE_NOW supposed to be an absolute indication of the level of
    >> charge in the battery? If so then thats not ACR.

    >
    > This depends. On DS2760 ACR is the only way to determine battery's
    > "charge" at the given time (thus the capacity in percents, since ds2760
    > can't report it).


    Ours is the same way. The EC just does the extra work to keep an SOC%
    value.

    > It needs calibration though... ds2760_battery.c doing this: resetting the
    > ACR to CHARGE_FULL value when battery is in charging state with current
    > flow < 10 mA.
    >
    > Works quite good here.


    Ah.. I see now. You are using the ACR directly as the SOC value.
    Whereas OLPC does not.

    If your battery capacity stays reasonably constant across the operating
    temp range then I see how that would work fine. With NiMh chemistry the
    capacity changes quite a bit wrt temperature. Li chemistries capacity
    are much less temp sensitive.

    I can't say for certain because I didn't do the original code design but
    from what I see in the EC code it tries to compensate for that capacity
    change when using NiMh. So they record the net ACR change and use that
    to try and compute the real SOC% change. I suppose you could still make
    it work with just the ACR register but thats not how Quanta chose to do
    the implementation.

    >> That would be SOC% *
    >> battery capacity. The EC uses the relative motion of the ACR to update
    >> the SOC%.

    >
    > Since you don't seem to use ACR as CHARGE_NOW, I think maybe we indeed
    > should implement PROP_ACR or better PROP_CHARGE_COUNTER? With units of
    > uAh... would that work?


    That would work fine.

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

  7. [PATCH] power_supply: add CHARGE_COUNTER property and olpc_battery support for it

    On Thu, 08 May 2008 16:44:43 -0400
    "Richard A. Smith" wrote:

    > Anton Vorontsov wrote:
    >
    > > Cool, I believe this is exactly same thing as we have on DS2760. ACR
    > > counts up and down. For example, if there was a current of 10 uA for
    > > 30 minutes, ACR will show 5 uAh. Additional 30 minutes with current
    > > -10 uA will result in 0 uAh in ACR... Right?

    >
    > Yes. Exactly correct.
    >
    > >> The units for the ACR register in the DS2765 are:

    > >
    > > Do you have any specs for the ds2765? I'd like to see one, if
    > > it isn't restricted matter. google:ds2765 pdf wasn't helpful...
    > >

    >
    > Ooop. Sorry my typo. DS2756.
    >
    > http://wiki.laptop.org/images/e/e9/DS2756.pdf
    > or
    > http://www.maxim-ic.com/quick_view2.cfm/qv_pk/5104
    >


    Wow, having the data sheet makes things so much clearer.


    [...]
    >
    > >> That would be SOC% *
    > >> battery capacity. The EC uses the relative motion of the ACR to
    > >> update the SOC%.

    > >
    > > Since you don't seem to use ACR as CHARGE_NOW, I think maybe we
    > > indeed should implement PROP_ACR or better PROP_CHARGE_COUNTER?
    > > With units of uAh... would that work?

    >
    > That would work fine.
    >


    Alright folks, how does this look?




    From: Andres Salomon

    This adds PROP_CHARGE_COUNTER to the power supply class (documenting it
    as well). The OLPC battery driver uses this for spitting out its ACR
    values (in uAh). We have some rounding errors (the data sheet claims
    416.7, the math actually works out to 416.666667, so we're forced to
    choose between overflows or precision loss. I chose precision loss,
    and stuck w/ data sheet values), but I don't think anyone will care
    that much.

    Signed-off-by: Andres Salomon
    ---
    Documentation/power/power_supply_class.txt | 4 ++++
    drivers/power/olpc_battery.c | 11 ++++++++++-
    drivers/power/power_supply_sysfs.c | 1 +
    include/linux/power_supply.h | 1 +
    4 files changed, 16 insertions(+), 1 deletions(-)

    diff --git a/Documentation/power/power_supply_class.txt b/Documentation/power/power_supply_class.txt
    index a8686e5..c6cd495 100644
    --- a/Documentation/power/power_supply_class.txt
    +++ b/Documentation/power/power_supply_class.txt
    @@ -101,6 +101,10 @@ of charge when battery became full/empty". It also could mean "value of
    charge when battery considered full/empty at given conditions (temperature,
    age)". I.e. these attributes represents real thresholds, not design values.

    +CHARGE_COUNTER - the current charge counter (in µAh). This could easily
    +be negative; there is no empty or full value. It is only useful for
    +relative, time-based measurements.
    +
    ENERGY_FULL, ENERGY_EMPTY - same as above but for energy.

    CAPACITY - capacity in percents.
    diff --git a/drivers/power/olpc_battery.c b/drivers/power/olpc_battery.c
    index e3f6ec8..a928165 100644
    --- a/drivers/power/olpc_battery.c
    +++ b/drivers/power/olpc_battery.c
    @@ -19,7 +19,7 @@

    #define EC_BAT_VOLTAGE 0x10 /* uint16_t, *9.76/32, mV */
    #define EC_BAT_CURRENT 0x11 /* int16_t, *15.625/120, mA */
    -#define EC_BAT_ACR 0x12
    +#define EC_BAT_ACR 0x12 /* int16_t, *416.7, µAh */
    #define EC_BAT_TEMP 0x13 /* uint16_t, *100/256, °C */
    #define EC_AMB_TEMP 0x14 /* uint16_t, *100/256, °C */
    #define EC_BAT_STATUS 0x15 /* uint8_t, bitmask */
    @@ -289,6 +289,14 @@ static int olpc_bat_get_property(struct power_supply *psy,
    ec_word = be16_to_cpu(ec_word);
    val->intval = ec_word * 100 / 256;
    break;
    + case POWER_SUPPLY_PROP_CHARGE_COUNTER:
    + ret = olpc_ec_cmd(EC_BAT_ACR, NULL, 0, (void *)&ec_word, 2);
    + if (ret)
    + return ret;
    +
    + ec_word = be16_to_cpu(ec_word);
    + val->intval = ec_word * 4167 / 10;
    + break;
    case POWER_SUPPLY_PROP_SERIAL_NUMBER:
    ret = olpc_ec_cmd(EC_BAT_SERIAL, NULL, 0, (void *)&ser_buf, 8);
    if (ret)
    @@ -317,6 +325,7 @@ static enum power_supply_property olpc_bat_props[] = {
    POWER_SUPPLY_PROP_TEMP_AMBIENT,
    POWER_SUPPLY_PROP_MANUFACTURER,
    POWER_SUPPLY_PROP_SERIAL_NUMBER,
    + POWER_SUPPLY_PROP_CHARGE_COUNTER,
    };

    /* EEPROM reading goes completely around the power_supply API, sadly */
    diff --git a/drivers/power/power_supply_sysfs.c b/drivers/power/power_supply_sysfs.c
    index c444d6b..82e1246 100644
    --- a/drivers/power/power_supply_sysfs.c
    +++ b/drivers/power/power_supply_sysfs.c
    @@ -99,6 +99,7 @@ static struct device_attribute power_supply_attrs[] = {
    POWER_SUPPLY_ATTR(charge_empty),
    POWER_SUPPLY_ATTR(charge_now),
    POWER_SUPPLY_ATTR(charge_avg),
    + POWER_SUPPLY_ATTR(charge_counter),
    POWER_SUPPLY_ATTR(energy_full_design),
    POWER_SUPPLY_ATTR(energy_empty_design),
    POWER_SUPPLY_ATTR(energy_full),
    diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
    index 68ed19c..ea96ead 100644
    --- a/include/linux/power_supply.h
    +++ b/include/linux/power_supply.h
    @@ -78,6 +78,7 @@ enum power_supply_property {
    POWER_SUPPLY_PROP_CHARGE_EMPTY,
    POWER_SUPPLY_PROP_CHARGE_NOW,
    POWER_SUPPLY_PROP_CHARGE_AVG,
    + POWER_SUPPLY_PROP_CHARGE_COUNTER,
    POWER_SUPPLY_PROP_ENERGY_FULL_DESIGN,
    POWER_SUPPLY_PROP_ENERGY_EMPTY_DESIGN,
    POWER_SUPPLY_PROP_ENERGY_FULL,
    --
    1.5.5.1

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

  8. Re: [PATCH] power_supply: add CHARGE_COUNTER property and olpc_battery support for it

    On Mon, May 12, 2008 at 09:46:29PM -0400, Andres Salomon wrote:
    [...]
    > From: Andres Salomon
    >
    > This adds PROP_CHARGE_COUNTER to the power supply class (documenting it
    > as well). The OLPC battery driver uses this for spitting out its ACR
    > values (in uAh). We have some rounding errors (the data sheet claims
    > 416.7, the math actually works out to 416.666667, so we're forced to
    > choose between overflows or precision loss. I chose precision loss,
    > and stuck w/ data sheet values), but I don't think anyone will care
    > that much.


    Applied to battery-2.6.git, thanks.

    And another thanks for keeping documentation in sync. :-)

    > Signed-off-by: Andres Salomon
    > ---
    > Documentation/power/power_supply_class.txt | 4 ++++
    > drivers/power/olpc_battery.c | 11 ++++++++++-
    > drivers/power/power_supply_sysfs.c | 1 +
    > include/linux/power_supply.h | 1 +
    > 4 files changed, 16 insertions(+), 1 deletions(-)
    >
    > diff --git a/Documentation/power/power_supply_class.txt b/Documentation/power/power_supply_class.txt
    > index a8686e5..c6cd495 100644
    > --- a/Documentation/power/power_supply_class.txt
    > +++ b/Documentation/power/power_supply_class.txt
    > @@ -101,6 +101,10 @@ of charge when battery became full/empty". It also could mean "value of
    > charge when battery considered full/empty at given conditions (temperature,
    > age)". I.e. these attributes represents real thresholds, not design values.
    >
    > +CHARGE_COUNTER - the current charge counter (in µAh). This could easily
    > +be negative; there is no empty or full value. It is only useful for
    > +relative, time-based measurements.
    > +
    > ENERGY_FULL, ENERGY_EMPTY - same as above but for energy.
    >
    > CAPACITY - capacity in percents.
    > diff --git a/drivers/power/olpc_battery.c b/drivers/power/olpc_battery.c
    > index e3f6ec8..a928165 100644
    > --- a/drivers/power/olpc_battery.c
    > +++ b/drivers/power/olpc_battery.c
    > @@ -19,7 +19,7 @@
    >
    > #define EC_BAT_VOLTAGE 0x10 /* uint16_t, *9.76/32, mV */
    > #define EC_BAT_CURRENT 0x11 /* int16_t, *15.625/120, mA */
    > -#define EC_BAT_ACR 0x12
    > +#define EC_BAT_ACR 0x12 /* int16_t, *416.7, µAh */
    > #define EC_BAT_TEMP 0x13 /* uint16_t, *100/256, °C */
    > #define EC_AMB_TEMP 0x14 /* uint16_t, *100/256, °C */
    > #define EC_BAT_STATUS 0x15 /* uint8_t, bitmask */
    > @@ -289,6 +289,14 @@ static int olpc_bat_get_property(struct power_supply *psy,
    > ec_word = be16_to_cpu(ec_word);
    > val->intval = ec_word * 100 / 256;
    > break;
    > + case POWER_SUPPLY_PROP_CHARGE_COUNTER:
    > + ret = olpc_ec_cmd(EC_BAT_ACR, NULL, 0, (void *)&ec_word, 2);
    > + if (ret)
    > + return ret;
    > +
    > + ec_word = be16_to_cpu(ec_word);
    > + val->intval = ec_word * 4167 / 10;
    > + break;
    > case POWER_SUPPLY_PROP_SERIAL_NUMBER:
    > ret = olpc_ec_cmd(EC_BAT_SERIAL, NULL, 0, (void *)&ser_buf, 8);
    > if (ret)
    > @@ -317,6 +325,7 @@ static enum power_supply_property olpc_bat_props[] = {
    > POWER_SUPPLY_PROP_TEMP_AMBIENT,
    > POWER_SUPPLY_PROP_MANUFACTURER,
    > POWER_SUPPLY_PROP_SERIAL_NUMBER,
    > + POWER_SUPPLY_PROP_CHARGE_COUNTER,
    > };
    >
    > /* EEPROM reading goes completely around the power_supply API, sadly */
    > diff --git a/drivers/power/power_supply_sysfs.c b/drivers/power/power_supply_sysfs.c
    > index c444d6b..82e1246 100644
    > --- a/drivers/power/power_supply_sysfs.c
    > +++ b/drivers/power/power_supply_sysfs.c
    > @@ -99,6 +99,7 @@ static struct device_attribute power_supply_attrs[] = {
    > POWER_SUPPLY_ATTR(charge_empty),
    > POWER_SUPPLY_ATTR(charge_now),
    > POWER_SUPPLY_ATTR(charge_avg),
    > + POWER_SUPPLY_ATTR(charge_counter),
    > POWER_SUPPLY_ATTR(energy_full_design),
    > POWER_SUPPLY_ATTR(energy_empty_design),
    > POWER_SUPPLY_ATTR(energy_full),
    > diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
    > index 68ed19c..ea96ead 100644
    > --- a/include/linux/power_supply.h
    > +++ b/include/linux/power_supply.h
    > @@ -78,6 +78,7 @@ enum power_supply_property {
    > POWER_SUPPLY_PROP_CHARGE_EMPTY,
    > POWER_SUPPLY_PROP_CHARGE_NOW,
    > POWER_SUPPLY_PROP_CHARGE_AVG,
    > + POWER_SUPPLY_PROP_CHARGE_COUNTER,
    > POWER_SUPPLY_PROP_ENERGY_FULL_DESIGN,
    > POWER_SUPPLY_PROP_ENERGY_EMPTY_DESIGN,
    > POWER_SUPPLY_PROP_ENERGY_FULL,
    > --
    > 1.5.5.1
    >


    --
    Anton Vorontsov
    email: cbouatmailru@gmail.com
    irc://irc.freenode.net/bd2
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  9. Re: [PATCH] power_supply: add CHARGE_COUNTER property and olpc_battery support for it

    Andres Salomon wrote:

    > + ec_word = be16_to_cpu(ec_word);
    > + val->intval = ec_word * 4167 / 10;


    Whats wrong with:

    > + val->intval = ((int)ec_word) * 6250 / 15;


    Which does not have overflow problems and keeps more precision.

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

  10. Re: [PATCH] power_supply: add CHARGE_COUNTER property and olpc_battery support for it

    On Tue, 13 May 2008 10:20:15 -0400
    "Richard A. Smith" wrote:

    > Andres Salomon wrote:
    >
    > > + ec_word = be16_to_cpu(ec_word);
    > > + val->intval = ec_word * 4167 / 10;

    >
    > Whats wrong with:
    >
    > > + val->intval = ((int)ec_word) * 6250 / 15;

    >
    > Which does not have overflow problems and keeps more precision.
    >


    You're right, of course. Anton, can you please apply the following?



    Subject: [PATCH] power_supply: fix up CHARGE_COUNTER output to be more precise

    As Richard Smith pointed out, ACR * 6250 / 15 provides for less precision
    loss than ACR * 4167 / 10, _and_ it doesn't overflow. Switch to using
    that equation for CHARGE_COUNTER.

    Signed-off-by: Andres Salomon
    ---
    drivers/power/olpc_battery.c | 4 ++--
    1 files changed, 2 insertions(+), 2 deletions(-)

    diff --git a/drivers/power/olpc_battery.c b/drivers/power/olpc_battery.c
    index a928165..f5d712a 100644
    --- a/drivers/power/olpc_battery.c
    +++ b/drivers/power/olpc_battery.c
    @@ -19,7 +19,7 @@

    #define EC_BAT_VOLTAGE 0x10 /* uint16_t, *9.76/32, mV */
    #define EC_BAT_CURRENT 0x11 /* int16_t, *15.625/120, mA */
    -#define EC_BAT_ACR 0x12 /* int16_t, *416.7, µAh */
    +#define EC_BAT_ACR 0x12 /* int16_t, *6250/15, µAh */
    #define EC_BAT_TEMP 0x13 /* uint16_t, *100/256, °C */
    #define EC_AMB_TEMP 0x14 /* uint16_t, *100/256, °C */
    #define EC_BAT_STATUS 0x15 /* uint8_t, bitmask */
    @@ -295,7 +295,7 @@ static int olpc_bat_get_property(struct power_supply *psy,
    return ret;

    ec_word = be16_to_cpu(ec_word);
    - val->intval = ec_word * 4167 / 10;
    + val->intval = ec_word * 6250 / 15;
    break;
    case POWER_SUPPLY_PROP_SERIAL_NUMBER:
    ret = olpc_ec_cmd(EC_BAT_SERIAL, NULL, 0, (void *)&ser_buf, 8);
    --
    1.5.5.1

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

  11. Re: [PATCH] power_supply: add CHARGE_COUNTER property and olpc_battery support for it

    On Tue, May 13, 2008 at 12:23:30PM -0400, Andres Salomon wrote:
    > On Tue, 13 May 2008 10:20:15 -0400
    > "Richard A. Smith" wrote:
    >
    > > Andres Salomon wrote:
    > >
    > > > + ec_word = be16_to_cpu(ec_word);
    > > > + val->intval = ec_word * 4167 / 10;

    > >
    > > Whats wrong with:
    > >
    > > > + val->intval = ((int)ec_word) * 6250 / 15;

    > >
    > > Which does not have overflow problems and keeps more precision.
    > >

    >
    > You're right, of course. Anton, can you please apply the following?


    Sorry for the delay... Applied, thanks!

    > Subject: [PATCH] power_supply: fix up CHARGE_COUNTER output to be more precise
    >
    > As Richard Smith pointed out, ACR * 6250 / 15 provides for less precision
    > loss than ACR * 4167 / 10, _and_ it doesn't overflow. Switch to using
    > that equation for CHARGE_COUNTER.
    >
    > Signed-off-by: Andres Salomon
    > ---
    > drivers/power/olpc_battery.c | 4 ++--
    > 1 files changed, 2 insertions(+), 2 deletions(-)
    >
    > diff --git a/drivers/power/olpc_battery.c b/drivers/power/olpc_battery.c
    > index a928165..f5d712a 100644
    > --- a/drivers/power/olpc_battery.c
    > +++ b/drivers/power/olpc_battery.c
    > @@ -19,7 +19,7 @@
    >
    > #define EC_BAT_VOLTAGE 0x10 /* uint16_t, *9.76/32, mV */
    > #define EC_BAT_CURRENT 0x11 /* int16_t, *15.625/120, mA */
    > -#define EC_BAT_ACR 0x12 /* int16_t, *416.7, µAh */
    > +#define EC_BAT_ACR 0x12 /* int16_t, *6250/15, µAh */
    > #define EC_BAT_TEMP 0x13 /* uint16_t, *100/256, °C */
    > #define EC_AMB_TEMP 0x14 /* uint16_t, *100/256, °C */
    > #define EC_BAT_STATUS 0x15 /* uint8_t, bitmask */
    > @@ -295,7 +295,7 @@ static int olpc_bat_get_property(struct power_supply *psy,
    > return ret;
    >
    > ec_word = be16_to_cpu(ec_word);
    > - val->intval = ec_word * 4167 / 10;
    > + val->intval = ec_word * 6250 / 15;
    > break;
    > case POWER_SUPPLY_PROP_SERIAL_NUMBER:
    > ret = olpc_ec_cmd(EC_BAT_SERIAL, NULL, 0, (void *)&ser_buf, 8);
    > --
    > 1.5.5.1


    --
    Anton Vorontsov
    email: cbouatmailru@gmail.com
    irc://irc.freenode.net/bd2
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

+ Reply to Thread