[RFC][PATCH] ACPI: battery: add power_{now, avg} properties to power_class

[RFC][PATCH] ACPI: battery: add power_{now, avg} properties to power_class

[Alexey Starikovskiy]

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.

Signed-off-by: Alexey Starikovskiy <astarikovskiy@suse.de>
---

 drivers/acpi/battery.c             |    3 ++-
 drivers/acpi/sbs.c                 |    6 ++++--
 drivers/power/power_supply_sysfs.c |    2 ++
 include/linux/power_supply.h       |    2 ++
 4 files changed, 10 insertions(+), 3 deletions(-)


diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
index 1423b0c..88f1fb5 100644
--- a/drivers/acpi/battery.c
+++ b/drivers/acpi/battery.c
@@ -173,6 +173,7 @@ static int acpi_battery_get_property(struct power_supply *psy,
 		val->intval = battery->voltage_now * 1000;
 		break;
 	case POWER_SUPPLY_PROP_CURRENT_NOW:
+	case POWER_SUPPLY_PROP_POWER_NOW:
 		val->intval = battery->current_now * 1000;
 		break;
 	case POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN:
@@ -223,7 +224,7 @@ 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_POWER_NOW,
 	POWER_SUPPLY_PROP_ENERGY_FULL_DESIGN,
 	POWER_SUPPLY_PROP_ENERGY_FULL,
 	POWER_SUPPLY_PROP_ENERGY_NOW,
diff --git a/drivers/acpi/sbs.c b/drivers/acpi/sbs.c
index 6050ce4..994c04e 100644
--- a/drivers/acpi/sbs.c
+++ b/drivers/acpi/sbs.c
@@ -224,10 +224,12 @@ static int acpi_sbs_battery_get_property(struct power_supply *psy,
 				acpi_battery_vscale(battery) * 1000;
 		break;
 	case POWER_SUPPLY_PROP_CURRENT_NOW:
+	case POWER_SUPPLY_PROP_POWER_NOW:
 		val->intval = abs(battery->current_now) *
 				acpi_battery_ipscale(battery) * 1000;
 		break;
 	case POWER_SUPPLY_PROP_CURRENT_AVG:
+	case POWER_SUPPLY_PROP_POWER_AVG:
 		val->intval = abs(battery->current_avg) *
 				acpi_battery_ipscale(battery) * 1000;
 		break;
@@ -291,8 +293,8 @@ 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_POWER_NOW,
+	POWER_SUPPLY_PROP_POWER_AVG,
 	POWER_SUPPLY_PROP_CAPACITY,
 	POWER_SUPPLY_PROP_ENERGY_FULL_DESIGN,
 	POWER_SUPPLY_PROP_ENERGY_FULL,
diff --git a/drivers/power/power_supply_sysfs.c b/drivers/power/power_supply_sysfs.c
index 23ae846..7aa8b4e 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 f9348cb..105a707 100644
--- a/include/linux/power_supply.h
+++ b/include/linux/power_supply.h
@@ -72,6 +72,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,

Re: [RFC][PATCH] ACPI: battery: add power_{now, avg} properties to power_class

[Henrique de Moraes Holschuh]

On Tue, 16 Dec 2008, Alexey Starikovskiy wrote:
> @@ -224,10 +224,12 @@ static int acpi_sbs_battery_get_property(struct power_supply *psy,
>  				acpi_battery_vscale(battery) * 1000;
>  		break;
>  	case POWER_SUPPLY_PROP_CURRENT_NOW:
> +	case POWER_SUPPLY_PROP_POWER_NOW:
>  		val->intval = abs(battery->current_now) *
>  				acpi_battery_ipscale(battery) * 1000;
>  		break;
>  	case POWER_SUPPLY_PROP_CURRENT_AVG:
> +	case POWER_SUPPLY_PROP_POWER_AVG:
>  		val->intval = abs(battery->current_avg) *
>  				acpi_battery_ipscale(battery) * 1000;
>  		break;

Excuse me if I am talking nonsense (I have looked over just the patch,
not the entire file), but how can that be correct?  It is either power
or current, it cannot be both, so the CURRENT case should be dropped.
And if it is power, why have fields named current_now... or is
ipscale() a voltage, and not a scaling factor?

-- 
  "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

Re: [RFC][PATCH] ACPI: battery: add power_{now, avg} properties to power_class

[Alexey Starikovskiy]

Henrique de Moraes Holschuh wrote:
> On Tue, 16 Dec 2008, Alexey Starikovskiy wrote:
>> @@ -224,10 +224,12 @@ static int acpi_sbs_battery_get_property(struct power_supply *psy,
>>  				acpi_battery_vscale(battery) * 1000;
>>  		break;
>>  	case POWER_SUPPLY_PROP_CURRENT_NOW:
>> +	case POWER_SUPPLY_PROP_POWER_NOW:
>>  		val->intval = abs(battery->current_now) *
>>  				acpi_battery_ipscale(battery) * 1000;
>>  		break;
>>  	case POWER_SUPPLY_PROP_CURRENT_AVG:
>> +	case POWER_SUPPLY_PROP_POWER_AVG:
>>  		val->intval = abs(battery->current_avg) *
>>  				acpi_battery_ipscale(battery) * 1000;
>>  		break;
> 
> Excuse me if I am talking nonsense (I have looked over just the patch,
> not the entire file), but how can that be correct?  It is either power
> or current, it cannot be both, so the CURRENT case should be dropped.
file name under /sys depends on property, so if we want some variable to
be named as current_now, it should be returned by case POWER_SUPPLY_PROP_CURRENT_NOW.
Then we register with power_supply, we say which set of properties (either charge or energy) we support,
so for one battery we will receive request for either CURRENT_AVG or POWER_AVG, not both.
> And if it is power, why have fields named current_now... or is
> ipscale() a voltage, and not a scaling factor?
ipscale stands for I/P scaling, as opposed to V scaling -- it depends on units returned by actual battery.
All energy/charge fields are reused, so battery->current_now contains either power_now or current_now from battery.
 

Re: [RFC][PATCH] ACPI: battery: add power_{now, avg} properties to power_class

[Henrique de Moraes Holschuh]

On Tue, 16 Dec 2008, Alexey Starikovskiy wrote:
> Henrique de Moraes Holschuh wrote:
>> On Tue, 16 Dec 2008, Alexey Starikovskiy wrote:
>>> @@ -224,10 +224,12 @@ static int acpi_sbs_battery_get_property(struct power_supply *psy,
>>>  				acpi_battery_vscale(battery) * 1000;
>>>  		break;
>>>  	case POWER_SUPPLY_PROP_CURRENT_NOW:
>>> +	case POWER_SUPPLY_PROP_POWER_NOW:
>>>  		val->intval = abs(battery->current_now) *
>>>  				acpi_battery_ipscale(battery) * 1000;
>>>  		break;
>>>  	case POWER_SUPPLY_PROP_CURRENT_AVG:
>>> +	case POWER_SUPPLY_PROP_POWER_AVG:
>>>  		val->intval = abs(battery->current_avg) *
>>>  				acpi_battery_ipscale(battery) * 1000;
>>>  		break;
>>
>> Excuse me if I am talking nonsense (I have looked over just the patch,
>> not the entire file), but how can that be correct?  It is either power
>> or current, it cannot be both, so the CURRENT case should be dropped.
> file name under /sys depends on property, so if we want some variable to
> be named as current_now, it should be returned by case POWER_SUPPLY_PROP_CURRENT_NOW.
> Then we register with power_supply, we say which set of properties (either charge or energy) we support,
> so for one battery we will receive request for either CURRENT_AVG or POWER_AVG, not both.
>> And if it is power, why have fields named current_now... or is
>> ipscale() a voltage, and not a scaling factor?
> ipscale stands for I/P scaling, as opposed to V scaling -- it depends on units returned by actual battery.
> All energy/charge fields are reused, so battery->current_now contains either power_now or current_now from battery.

I see.  But that's a loaded-spring trap waiting for the unaware.  Can it be
called something else that is neutral re. power or current, pretty please?
even "current_or_power_now" would be less confusing...

-- 
  "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

Re: [RFC][PATCH] ACPI: battery: add power_{now, avg} properties to power_class

[Rafael J. Wysocki]

Hi,

On Monday, 15 of December 2008, 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.
> 
> Signed-off-by: Alexey Starikovskiy <astarikovskiy@suse.de>
> ---
> 
>  drivers/acpi/battery.c             |    3 ++-
>  drivers/acpi/sbs.c                 |    6 ++++--
>  drivers/power/power_supply_sysfs.c |    2 ++
>  include/linux/power_supply.h       |    2 ++
>  4 files changed, 10 insertions(+), 3 deletions(-)
> 
> 
> diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
> index 1423b0c..88f1fb5 100644
> --- a/drivers/acpi/battery.c
> +++ b/drivers/acpi/battery.c
> @@ -173,6 +173,7 @@ static int acpi_battery_get_property(struct power_supply *psy,
>  		val->intval = battery->voltage_now * 1000;
>  		break;
>  	case POWER_SUPPLY_PROP_CURRENT_NOW:
> +	case POWER_SUPPLY_PROP_POWER_NOW:
>  		val->intval = battery->current_now * 1000;
>  		break;
>  	case POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN:
> @@ -223,7 +224,7 @@ 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_POWER_NOW,
>  	POWER_SUPPLY_PROP_ENERGY_FULL_DESIGN,
>  	POWER_SUPPLY_PROP_ENERGY_FULL,
>  	POWER_SUPPLY_PROP_ENERGY_NOW,
> diff --git a/drivers/acpi/sbs.c b/drivers/acpi/sbs.c
> index 6050ce4..994c04e 100644
> --- a/drivers/acpi/sbs.c
> +++ b/drivers/acpi/sbs.c
> @@ -224,10 +224,12 @@ static int acpi_sbs_battery_get_property(struct power_supply *psy,
>  				acpi_battery_vscale(battery) * 1000;
>  		break;
>  	case POWER_SUPPLY_PROP_CURRENT_NOW:
> +	case POWER_SUPPLY_PROP_POWER_NOW:
>  		val->intval = abs(battery->current_now) *
>  				acpi_battery_ipscale(battery) * 1000;

Please introduce another field called 'battery->power_now' for this purpose.
Otherwise, confusion is guaranteed to ensue.

>  		break;
>  	case POWER_SUPPLY_PROP_CURRENT_AVG:
> +	case POWER_SUPPLY_PROP_POWER_AVG:
>  		val->intval = abs(battery->current_avg) *
>  				acpi_battery_ipscale(battery) * 1000;

Same here.

Also, as per our IRC conversation, I'd like 'current_now' to be reported even
if energy units are used, at least for some time, to give a chance to the user
land to switch to 'power_now' and 'power_avg' without pain.

You can put information about that into
Documentation/feature-removal-schedule.txt and say that after 2.6.29
'current_now' will no longer be reported when energy units are used.

Thanks,
Rafael

Re: [RFC][PATCH] ACPI: battery: add power_{now, avg} properties to power_class

[Rafael J. Wysocki]

On Tuesday, 16 of December 2008, Henrique de Moraes Holschuh wrote:
> On Tue, 16 Dec 2008, Alexey Starikovskiy wrote:
> > Henrique de Moraes Holschuh wrote:
> >> On Tue, 16 Dec 2008, Alexey Starikovskiy wrote:
> >>> @@ -224,10 +224,12 @@ static int acpi_sbs_battery_get_property(struct power_supply *psy,
> >>>  				acpi_battery_vscale(battery) * 1000;
> >>>  		break;
> >>>  	case POWER_SUPPLY_PROP_CURRENT_NOW:
> >>> +	case POWER_SUPPLY_PROP_POWER_NOW:
> >>>  		val->intval = abs(battery->current_now) *
> >>>  				acpi_battery_ipscale(battery) * 1000;
> >>>  		break;
> >>>  	case POWER_SUPPLY_PROP_CURRENT_AVG:
> >>> +	case POWER_SUPPLY_PROP_POWER_AVG:
> >>>  		val->intval = abs(battery->current_avg) *
> >>>  				acpi_battery_ipscale(battery) * 1000;
> >>>  		break;
> >>
> >> Excuse me if I am talking nonsense (I have looked over just the patch,
> >> not the entire file), but how can that be correct?  It is either power
> >> or current, it cannot be both, so the CURRENT case should be dropped.
> > file name under /sys depends on property, so if we want some variable to
> > be named as current_now, it should be returned by case POWER_SUPPLY_PROP_CURRENT_NOW.
> > Then we register with power_supply, we say which set of properties (either charge or energy) we support,
> > so for one battery we will receive request for either CURRENT_AVG or POWER_AVG, not both.
> >> And if it is power, why have fields named current_now... or is
> >> ipscale() a voltage, and not a scaling factor?
> > ipscale stands for I/P scaling, as opposed to V scaling -- it depends on units returned by actual battery.
> > All energy/charge fields are reused, so battery->current_now contains either power_now or current_now from battery.
> 
> I see.  But that's a loaded-spring trap waiting for the unaware.  Can it be
> called something else that is neutral re. power or current, pretty please?
> even "current_or_power_now" would be less confusing...

As I said in my recent reply to Alex, I'd prefer it if there were separate
fields called 'power_now' and 'power_avg' under 'battery'.

Then, there won't be any confustion and people reading the source will clearly
understand what is what.

Thanks,
Rafael

Re: [RFC][PATCH] ACPI: battery: add power_{now, avg} properties to power_class

[Alexey Starikovskiy]

Rafael J. Wysocki wrote:
> Hi,
> 
> On Monday, 15 of December 2008, 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.
>>
>> Signed-off-by: Alexey Starikovskiy <astarikovskiy@suse.de>
>> ---
>>
>>  drivers/acpi/battery.c             |    3 ++-
>>  drivers/acpi/sbs.c                 |    6 ++++--
>>  drivers/power/power_supply_sysfs.c |    2 ++
>>  include/linux/power_supply.h       |    2 ++
>>  4 files changed, 10 insertions(+), 3 deletions(-)
>>
>>
>> diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
>> index 1423b0c..88f1fb5 100644
>> --- a/drivers/acpi/battery.c
>> +++ b/drivers/acpi/battery.c
>> @@ -173,6 +173,7 @@ static int acpi_battery_get_property(struct power_supply *psy,
>>  		val->intval = battery->voltage_now * 1000;
>>  		break;
>>  	case POWER_SUPPLY_PROP_CURRENT_NOW:
>> +	case POWER_SUPPLY_PROP_POWER_NOW:
>>  		val->intval = battery->current_now * 1000;
>>  		break;
>>  	case POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN:
>> @@ -223,7 +224,7 @@ 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_POWER_NOW,
>>  	POWER_SUPPLY_PROP_ENERGY_FULL_DESIGN,
>>  	POWER_SUPPLY_PROP_ENERGY_FULL,
>>  	POWER_SUPPLY_PROP_ENERGY_NOW,
>> diff --git a/drivers/acpi/sbs.c b/drivers/acpi/sbs.c
>> index 6050ce4..994c04e 100644
>> --- a/drivers/acpi/sbs.c
>> +++ b/drivers/acpi/sbs.c
>> @@ -224,10 +224,12 @@ static int acpi_sbs_battery_get_property(struct power_supply *psy,
>>  				acpi_battery_vscale(battery) * 1000;
>>  		break;
>>  	case POWER_SUPPLY_PROP_CURRENT_NOW:
>> +	case POWER_SUPPLY_PROP_POWER_NOW:
>>  		val->intval = abs(battery->current_now) *
>>  				acpi_battery_ipscale(battery) * 1000;
> 
> Please introduce another field called 'battery->power_now' for this purpose.
> Otherwise, confusion is guaranteed to ensue.
How about 'battery->rate_now' for both cases? All other fields are re-used as well, and it did not cause any confusion.
Probably no-one ever looked at this code.
> 
>>  		break;
>>  	case POWER_SUPPLY_PROP_CURRENT_AVG:
>> +	case POWER_SUPPLY_PROP_POWER_AVG:
>>  		val->intval = abs(battery->current_avg) *
>>  				acpi_battery_ipscale(battery) * 1000;
> 
> Same here.
> 
> Also, as per our IRC conversation, I'd like 'current_now' to be reported even
> if energy units are used, at least for some time, to give a chance to the user
> land to switch to 'power_now' and 'power_avg' without pain.
To clarify, you suggest that "current_now" will report same value 
as "power_now" in case of energy units?
> 
> You can put information about that into
> Documentation/feature-removal-schedule.txt and say that after 2.6.29
> 'current_now' will no longer be reported when energy units are used.
Sounds good.

Thanks,
Alex.
> 
> Thanks,
> Rafael

Re: [RFC][PATCH] ACPI: battery: add power_{now, avg} properties to power_class

[Rafael J. Wysocki]

On Tuesday, 16 of December 2008, Alexey Starikovskiy wrote:
> Rafael J. Wysocki wrote:
> > Hi,
> > 
> > On Monday, 15 of December 2008, 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.
> >>
> >> Signed-off-by: Alexey Starikovskiy <astarikovskiy@suse.de>
> >> ---
> >>
> >>  drivers/acpi/battery.c             |    3 ++-
> >>  drivers/acpi/sbs.c                 |    6 ++++--
> >>  drivers/power/power_supply_sysfs.c |    2 ++
> >>  include/linux/power_supply.h       |    2 ++
> >>  4 files changed, 10 insertions(+), 3 deletions(-)
> >>
> >>
> >> diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
> >> index 1423b0c..88f1fb5 100644
> >> --- a/drivers/acpi/battery.c
> >> +++ b/drivers/acpi/battery.c
> >> @@ -173,6 +173,7 @@ static int acpi_battery_get_property(struct power_supply *psy,
> >>  		val->intval = battery->voltage_now * 1000;
> >>  		break;
> >>  	case POWER_SUPPLY_PROP_CURRENT_NOW:
> >> +	case POWER_SUPPLY_PROP_POWER_NOW:
> >>  		val->intval = battery->current_now * 1000;
> >>  		break;
> >>  	case POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN:
> >> @@ -223,7 +224,7 @@ 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_POWER_NOW,
> >>  	POWER_SUPPLY_PROP_ENERGY_FULL_DESIGN,
> >>  	POWER_SUPPLY_PROP_ENERGY_FULL,
> >>  	POWER_SUPPLY_PROP_ENERGY_NOW,
> >> diff --git a/drivers/acpi/sbs.c b/drivers/acpi/sbs.c
> >> index 6050ce4..994c04e 100644
> >> --- a/drivers/acpi/sbs.c
> >> +++ b/drivers/acpi/sbs.c
> >> @@ -224,10 +224,12 @@ static int acpi_sbs_battery_get_property(struct power_supply *psy,
> >>  				acpi_battery_vscale(battery) * 1000;
> >>  		break;
> >>  	case POWER_SUPPLY_PROP_CURRENT_NOW:
> >> +	case POWER_SUPPLY_PROP_POWER_NOW:
> >>  		val->intval = abs(battery->current_now) *
> >>  				acpi_battery_ipscale(battery) * 1000;
> > 
> > Please introduce another field called 'battery->power_now' for this purpose.
> > Otherwise, confusion is guaranteed to ensue.
> How about 'battery->rate_now' for both cases? All other fields are re-used
> as well, and it did not cause any confusion.  Probably no-one ever looked at
> this code.

Well, they are called 'capacity_*', which doesn't imply any particular units,
while 'current_*' does.

Anyway, 'battery->rate_now' would be better than 'battery->current_now'.
What about 'battery->capacity_flow'?

> 
> > 
> >>  		break;
> >>  	case POWER_SUPPLY_PROP_CURRENT_AVG:
> >> +	case POWER_SUPPLY_PROP_POWER_AVG:
> >>  		val->intval = abs(battery->current_avg) *
> >>  				acpi_battery_ipscale(battery) * 1000;
> > 
> > Same here.
> > 
> > Also, as per our IRC conversation, I'd like 'current_now' to be reported even
> > if energy units are used, at least for some time, to give a chance to the user
> > land to switch to 'power_now' and 'power_avg' without pain.
> To clarify, you suggest that "current_now" will report same value 
> as "power_now" in case of energy units?

Yes, with a comment why that is so (ie. because of the userland doing stupid
things).

> > You can put information about that into
> > Documentation/feature-removal-schedule.txt and say that after 2.6.29
> > 'current_now' will no longer be reported when energy units are used.
> Sounds good.

OK

Thanks,
Rafael

Re: [RFC][PATCH] ACPI: battery: add power_{now, avg} properties to power_class

[Rafael J. Wysocki]

On Tuesday, 16 of December 2008, Len Brown wrote:
> 
> On Tue, 16 Dec 2008, Alexey Starikovskiy wrote:
> 
> > +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>
> 
> I think that a single kernel release cycle is too quick
> for a user/kernel API change  unless you're trying to fix
> something that is new and you have a pretty good idea
> that nobody is using it yet.  Kernel programmers typically
> want to talk about O(6 months) and Linus comes back and
> talks aboutg O(10 years)...

Well, IMO that could be discussed with the people who work on battery monitors.

If they agree to update their stuff in shorter time, we can remove the
'current_now' thing more quickly.

Thanks,
Rafael

Re: [RFC][PATCH] ACPI: battery: add power_{now, avg} properties to power_class

[Len Brown]

On Tue, 16 Dec 2008, Alexey Starikovskiy wrote:

> +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>

I think that a single kernel release cycle is too quick
for a user/kernel API change  unless you're trying to fix
something that is new and you have a pretty good idea
that nobody is using it yet.  Kernel programmers typically
want to talk about O(6 months) and Linus comes back and
talks aboutg O(10 years)...

-- Len Brown, Intel Open Source Technology Center

[RFC][PATCH] ACPI: battery: add power_{now, avg} properties to power_class

[Alexey Starikovskiy]

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 c28a2ac..f0c1f7b 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 1423b0c..b972939 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},
 };
@@ -581,11 +583,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 23ae846..7aa8b4e 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 f9348cb..105a707 100644
--- a/include/linux/power_supply.h
+++ b/include/linux/power_supply.h
@@ -72,6 +72,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,