linux-acpi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Sitsofe Wheeler <sitsofe@yahoo.com>
To: Alexey Starikovskiy <astarikovskiy@suse.de>,
	Len Brown <lenb@kernel.org>, Zhang Rui <rui.zhang@intel.com>
Cc: Matthew Garrett <mjg@redhat.com>,
	David Zeuthen <davidz@redhat.com>,
	Richard Hughes <richard@hughsie.com>,
	linux-acpi@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Returning ACPI_BATTERY_VALUE_UNKNOWN to userspace
Date: Sat, 16 Oct 2010 15:13:22 +0100	[thread overview]
Message-ID: <20101016141321.GA11054@sucs.org> (raw)

Hi,

I have an EeePC 900 with a battery/BIOS that does not report the rate at
which it charges/discharges. When I look at
/proc/acpi/battery/BAT0/state this is what is reported:

present:                 yes
capacity state:          ok
charging state:          charging
present rate:            unknown
remaining capacity:      3120 mAh
present voltage:         7889 mV

However looking at /sys/class/power_supply/BAT0/current_now reports:

-1000

Why -1000? I think it's because it's -1 * 1000 == -1000! In
drivers/acpi/battery.c, ACPI_BATTERY_VALUE_UNKNOWN is defined as being
0xFFFFFFFF. As rate_now is a signed int variable when it is assigned
ACPI_BATTERY_VALUE_UNKNOWN its value is -1. However, before the value is
returned via sysfs it is multiplied by 1000:
http://lxr.linux.no/linux+v2.6.35.7/drivers/acpi/battery.c#L222
(http://lxr.linux.no/linux+v2.6.35.7/drivers/acpi/battery.c#L524 shows
that acpi_battery_get_property will be called via sysfs).

If the above is a correct interpretation, this behaviour was introduced
when sysfs battery support was added in commit
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=d7380965752505951668e85de59c128d1d6fd21f
so it has effectively been always been this way.

However, looking at the code for the userspace power reporting tool
upower shows that it is not expecting to test against -1000 - it is
trying to test against 0xffff:
http://cgit.freedesktop.org/DeviceKit/upower/tree/src/linux/up-device-supply.c?id=UPOWER_0_9_6#n583
. Unfortunately, it's not clear that testing 0xffff is ever the right
thing to do. I wrote the following test program:

#include <stdio.h>
#include <math.h>
int main(void) {
	double minusone = -1;
	double sysfs = -1000;
	double hex_kernel = (int) 0xffffffff;
	double hex_tested = 0xffff;
	double energy_rate = fabs(sysfs / 1000000.0);
	double energy_rate_minusone = fabs(minusone / 1000000.0);
	printf("%f %f %f %f %f %f\n", minusone, sysfs, hex_kernel, hex_tested, energy_rate, energy_rate_minusone);
	return 0;
}

Which output the following:

-1.000000 -1000.000000 -1.000000 65535.000000 0.001000 0.000001

Given that at least upower (which is already deployed) will need to be
changed, I'm unsure as to where fixes for this should go. Was it really
the intent for userspace to test for -1000 instead of -1 to determine
an unknown rate?

-- 
Sitsofe | http://sucs.org/~sits/

             reply	other threads:[~2010-10-16 14:13 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-10-16 14:13 Sitsofe Wheeler [this message]
2010-10-16 23:05 ` [PATCH] ACPI / Battery: Return -ENODATA for unknown values in get_property() (was: Re: Returning ACPI_BATTERY ...) Rafael J. Wysocki
2010-10-17  5:19   ` Henrique de Moraes Holschuh
2010-10-17  9:59     ` [PATCH] ACPI / Battery: Return -ENODATA for unknown values in get_property() Sitsofe Wheeler
2010-10-17 13:10       ` Henrique de Moraes Holschuh
2010-10-17 14:50         ` Sitsofe Wheeler
2010-10-17 18:32           ` Rafael J. Wysocki
2010-10-21 16:54             ` Sitsofe Wheeler
2010-10-21 19:57               ` Rafael J. Wysocki
2010-10-21 20:46                 ` Sitsofe Wheeler
2010-10-22 22:19                   ` Rafael J. Wysocki
2010-10-23 15:36                     ` Sitsofe Wheeler
2010-10-23 17:31                       ` Rafael J. Wysocki
2010-10-22 12:31               ` Richard Hughes
2010-10-23 15:43                 ` Sitsofe Wheeler
2010-10-25 13:17             ` Pavel Machek
2010-10-25 20:36               ` Rafael J. Wysocki

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=20101016141321.GA11054@sucs.org \
    --to=sitsofe@yahoo.com \
    --cc=astarikovskiy@suse.de \
    --cc=davidz@redhat.com \
    --cc=lenb@kernel.org \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mjg@redhat.com \
    --cc=richard@hughsie.com \
    --cc=rui.zhang@intel.com \
    /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).