From: Alexey Starikovskiy <astarikovskiy@suse.de>
To: Len Brown <lenb@kernel.org>
Cc: Linux-acpi@vger.kernel.org
Subject: Re: [PATCH 1/3] ACPI: battery: add power_{now, avg} properties to power_class
Date: Fri, 16 Jan 2009 22:32:06 +0300 [thread overview]
Message-ID: <4970E0B6.5070306@suse.de> (raw)
In-Reply-To: <alpine.LFD.2.00.0901161408450.5705@localhost.localdomain>
2.6.30?
or 2.6.31?
Len Brown wrote:
> How can this patch schedule something for removal in 2.6.29
> when the patch itself isn't in the tree before 2.6.29?
> --
> Len Brown, Intel Open Source Technology Center
>
> On Wed, 14 Jan 2009, Alexey Starikovskiy wrote:
>
>> ACPI has smart batteries, which work in units of energy and measure
>> rate of (dis)charge as power, thus it is not appropriate to export it
>> as a current_now. Current_now will be exported until 2.6.29 to allow
>> for userland applications to match.
>>
>> Signed-off-by: Alexey Starikovskiy <astarikovskiy@suse.de>
>> ---
>>
>> Documentation/feature-removal-schedule.txt | 8 +++++++
>> drivers/acpi/battery.c | 14 +++++++------
>> drivers/acpi/sbs.c | 31 ++++++++++++++++------------
>> drivers/power/power_supply_sysfs.c | 2 ++
>> include/linux/power_supply.h | 2 ++
>> 5 files changed, 38 insertions(+), 19 deletions(-)
>>
>>
>> diff --git a/Documentation/feature-removal-schedule.txt b/Documentation/feature-removal-schedule.txt
>> index 5ddbe35..79fe354 100644
>> --- a/Documentation/feature-removal-schedule.txt
>> +++ b/Documentation/feature-removal-schedule.txt
>> @@ -3,6 +3,14 @@ removed in the kernel source tree. Every entry should contain what
>> exactly is going away, why it is happening, and who is going to be doing
>> the work. When the feature is removed from the kernel, it should also
>> be removed from this file.
>> +---------------------------
>> +
>> +What: current_{now,avg} attributes for batteries, reporting in energy units
>> +When: 2.6.29
>> +Why: Batteries, reporting in energy units, will report (dis)charge rate as
>> + power (Watts), and not as current (Amperes), thus new power_{now,avg}
>> + attributes should be used for such batteries to avoid the confusion.
>> +Who: Alexey Starikovskiy <astarikovskiy@suse.de>
>>
>> ---------------------------
>>
>> diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
>> index 65132f9..7aa9f11 100644
>> --- a/drivers/acpi/battery.c
>> +++ b/drivers/acpi/battery.c
>> @@ -92,7 +92,7 @@ struct acpi_battery {
>> #endif
>> struct acpi_device *device;
>> unsigned long update_time;
>> - int current_now;
>> + int rate_now;
>> int capacity_now;
>> int voltage_now;
>> int design_capacity;
>> @@ -173,7 +173,8 @@ static int acpi_battery_get_property(struct power_supply *psy,
>> val->intval = battery->voltage_now * 1000;
>> break;
>> case POWER_SUPPLY_PROP_CURRENT_NOW:
>> - val->intval = battery->current_now * 1000;
>> + case POWER_SUPPLY_PROP_POWER_NOW:
>> + val->intval = battery->rate_now * 1000;
>> break;
>> case POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN:
>> case POWER_SUPPLY_PROP_ENERGY_FULL_DESIGN:
>> @@ -223,7 +224,8 @@ static enum power_supply_property energy_battery_props[] = {
>> POWER_SUPPLY_PROP_TECHNOLOGY,
>> POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN,
>> POWER_SUPPLY_PROP_VOLTAGE_NOW,
>> - POWER_SUPPLY_PROP_CURRENT_NOW,
>> + POWER_SUPPLY_PROP_CURRENT_NOW, /* to be removed in 2.6.29 */
>> + POWER_SUPPLY_PROP_POWER_NOW,
>> POWER_SUPPLY_PROP_ENERGY_FULL_DESIGN,
>> POWER_SUPPLY_PROP_ENERGY_FULL,
>> POWER_SUPPLY_PROP_ENERGY_NOW,
>> @@ -250,7 +252,7 @@ struct acpi_offsets {
>>
>> static struct acpi_offsets state_offsets[] = {
>> {offsetof(struct acpi_battery, state), 0},
>> - {offsetof(struct acpi_battery, current_now), 0},
>> + {offsetof(struct acpi_battery, rate_now), 0},
>> {offsetof(struct acpi_battery, capacity_now), 0},
>> {offsetof(struct acpi_battery, voltage_now), 0},
>> };
>> @@ -582,11 +584,11 @@ static int acpi_battery_print_state(struct seq_file *seq, int result)
>> else
>> seq_printf(seq, "charging state: charged\n");
>>
>> - if (battery->current_now == ACPI_BATTERY_VALUE_UNKNOWN)
>> + if (battery->rate_now == ACPI_BATTERY_VALUE_UNKNOWN)
>> seq_printf(seq, "present rate: unknown\n");
>> else
>> seq_printf(seq, "present rate: %d %s\n",
>> - battery->current_now, acpi_battery_units(battery));
>> + battery->rate_now, acpi_battery_units(battery));
>>
>> if (battery->capacity_now == ACPI_BATTERY_VALUE_UNKNOWN)
>> seq_printf(seq, "remaining capacity: unknown\n");
>> diff --git a/drivers/acpi/sbs.c b/drivers/acpi/sbs.c
>> index 6050ce4..e3b0087 100644
>> --- a/drivers/acpi/sbs.c
>> +++ b/drivers/acpi/sbs.c
>> @@ -102,8 +102,8 @@ struct acpi_battery {
>> u16 cycle_count;
>> u16 temp_now;
>> u16 voltage_now;
>> - s16 current_now;
>> - s16 current_avg;
>> + s16 rate_now;
>> + s16 rate_avg;
>> u16 capacity_now;
>> u16 state_of_charge;
>> u16 state;
>> @@ -202,9 +202,9 @@ static int acpi_sbs_battery_get_property(struct power_supply *psy,
>> return -ENODEV;
>> switch (psp) {
>> case POWER_SUPPLY_PROP_STATUS:
>> - if (battery->current_now < 0)
>> + if (battery->rate_now < 0)
>> val->intval = POWER_SUPPLY_STATUS_DISCHARGING;
>> - else if (battery->current_now > 0)
>> + else if (battery->rate_now > 0)
>> val->intval = POWER_SUPPLY_STATUS_CHARGING;
>> else
>> val->intval = POWER_SUPPLY_STATUS_FULL;
>> @@ -224,11 +224,13 @@ static int acpi_sbs_battery_get_property(struct power_supply *psy,
>> acpi_battery_vscale(battery) * 1000;
>> break;
>> case POWER_SUPPLY_PROP_CURRENT_NOW:
>> - val->intval = abs(battery->current_now) *
>> + case POWER_SUPPLY_PROP_POWER_NOW:
>> + val->intval = abs(battery->rate_now) *
>> acpi_battery_ipscale(battery) * 1000;
>> break;
>> case POWER_SUPPLY_PROP_CURRENT_AVG:
>> - val->intval = abs(battery->current_avg) *
>> + case POWER_SUPPLY_PROP_POWER_AVG:
>> + val->intval = abs(battery->rate_avg) *
>> acpi_battery_ipscale(battery) * 1000;
>> break;
>> case POWER_SUPPLY_PROP_CAPACITY:
>> @@ -291,8 +293,10 @@ static enum power_supply_property sbs_energy_battery_props[] = {
>> POWER_SUPPLY_PROP_TECHNOLOGY,
>> POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN,
>> POWER_SUPPLY_PROP_VOLTAGE_NOW,
>> - POWER_SUPPLY_PROP_CURRENT_NOW,
>> - POWER_SUPPLY_PROP_CURRENT_AVG,
>> + POWER_SUPPLY_PROP_CURRENT_NOW, /* to be removed in 2.6.29 */
>> + POWER_SUPPLY_PROP_CURRENT_AVG, /* to be removed in 2.6.29 */
>> + POWER_SUPPLY_PROP_POWER_NOW,
>> + POWER_SUPPLY_PROP_POWER_AVG,
>> POWER_SUPPLY_PROP_CAPACITY,
>> POWER_SUPPLY_PROP_ENERGY_FULL_DESIGN,
>> POWER_SUPPLY_PROP_ENERGY_FULL,
>> @@ -301,6 +305,7 @@ static enum power_supply_property sbs_energy_battery_props[] = {
>> POWER_SUPPLY_PROP_MODEL_NAME,
>> POWER_SUPPLY_PROP_MANUFACTURER,
>> };
>> +
>> #endif
>>
>> /* --------------------------------------------------------------------------
>> @@ -330,8 +335,8 @@ static struct acpi_battery_reader info_readers[] = {
>> static struct acpi_battery_reader state_readers[] = {
>> {0x08, SMBUS_READ_WORD, offsetof(struct acpi_battery, temp_now)},
>> {0x09, SMBUS_READ_WORD, offsetof(struct acpi_battery, voltage_now)},
>> - {0x0a, SMBUS_READ_WORD, offsetof(struct acpi_battery, current_now)},
>> - {0x0b, SMBUS_READ_WORD, offsetof(struct acpi_battery, current_avg)},
>> + {0x0a, SMBUS_READ_WORD, offsetof(struct acpi_battery, rate_now)},
>> + {0x0b, SMBUS_READ_WORD, offsetof(struct acpi_battery, rate_avg)},
>> {0x0f, SMBUS_READ_WORD, offsetof(struct acpi_battery, capacity_now)},
>> {0x0e, SMBUS_READ_WORD, offsetof(struct acpi_battery, state_of_charge)},
>> {0x16, SMBUS_READ_WORD, offsetof(struct acpi_battery, state)},
>> @@ -589,9 +594,9 @@ static int acpi_battery_read_state(struct seq_file *seq, void *offset)
>> seq_printf(seq, "capacity state: %s\n",
>> (battery->state & 0x0010) ? "critical" : "ok");
>> seq_printf(seq, "charging state: %s\n",
>> - (battery->current_now < 0) ? "discharging" :
>> - ((battery->current_now > 0) ? "charging" : "charged"));
>> - rate = abs(battery->current_now) * acpi_battery_ipscale(battery);
>> + (battery->rate_now < 0) ? "discharging" :
>> + ((battery->rate_now > 0) ? "charging" : "charged"));
>> + rate = abs(battery->rate_now) * acpi_battery_ipscale(battery);
>> rate *= (acpi_battery_mode(battery))?(battery->voltage_now *
>> acpi_battery_vscale(battery)/1000):1;
>> seq_printf(seq, "present rate: %d%s\n", rate,
>> diff --git a/drivers/power/power_supply_sysfs.c b/drivers/power/power_supply_sysfs.c
>> index ac01e06..da73591 100644
>> --- a/drivers/power/power_supply_sysfs.c
>> +++ b/drivers/power/power_supply_sysfs.c
>> @@ -93,6 +93,8 @@ static struct device_attribute power_supply_attrs[] = {
>> POWER_SUPPLY_ATTR(voltage_avg),
>> POWER_SUPPLY_ATTR(current_now),
>> POWER_SUPPLY_ATTR(current_avg),
>> + POWER_SUPPLY_ATTR(power_now),
>> + POWER_SUPPLY_ATTR(power_avg),
>> POWER_SUPPLY_ATTR(charge_full_design),
>> POWER_SUPPLY_ATTR(charge_empty_design),
>> POWER_SUPPLY_ATTR(charge_full),
>> diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
>> index 8ff25e0..594c494 100644
>> --- a/include/linux/power_supply.h
>> +++ b/include/linux/power_supply.h
>> @@ -73,6 +73,8 @@ enum power_supply_property {
>> POWER_SUPPLY_PROP_VOLTAGE_AVG,
>> POWER_SUPPLY_PROP_CURRENT_NOW,
>> POWER_SUPPLY_PROP_CURRENT_AVG,
>> + POWER_SUPPLY_PROP_POWER_NOW,
>> + POWER_SUPPLY_PROP_POWER_AVG,
>> POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN,
>> POWER_SUPPLY_PROP_CHARGE_EMPTY_DESIGN,
>> POWER_SUPPLY_PROP_CHARGE_FULL,
>>
next prev parent reply other threads:[~2009-01-16 19:32 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-01-13 23:57 [PATCH 1/3] ACPI: battery: add power_{now, avg} properties to power_class Alexey Starikovskiy
2009-01-13 23:57 ` [PATCH 2/3] ACPI: EC: Limit workaround for ASUS notebooks even more Alexey Starikovskiy
2009-01-16 19:08 ` Len Brown
2009-01-13 23:57 ` [PATCH 3/3] ACPI: EC: Don't trust ECDT tables from ASUS Alexey Starikovskiy
2009-01-16 19:08 ` Len Brown
2009-01-16 19:11 ` [PATCH 1/3] ACPI: battery: add power_{now, avg} properties to power_class Len Brown
2009-01-16 19:32 ` Alexey Starikovskiy [this message]
2009-03-17 6:24 ` Len Brown
2009-03-17 14:20 ` Alexey Starikovskiy
2009-03-28 21:59 ` Alexey Starikovskiy
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4970E0B6.5070306@suse.de \
--to=astarikovskiy@suse.de \
--cc=Linux-acpi@vger.kernel.org \
--cc=lenb@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).