It seems that some batteries (noticed on DELL JYPJ136) assume
capacity_now = design_capacity when fully charged. This causes
reported capacity to suddenly jump to >full_charge_capacity (and that
means capacity reported to userspace is >100% and incorrect)
values after 99%. This patch attempts to detect this bug and trims
capacity_now properly.
Signed-off-by: Josef Gajdusek <[email protected]>
---
Oops, I forgot to sign the previous one.
diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
index e48fc98..3338dd5 100644
--- a/drivers/acpi/battery.c
+++ b/drivers/acpi/battery.c
@@ -532,6 +532,14 @@ static int acpi_battery_get_state(struct acpi_battery *battery)
" invalid.\n");
}
+ /* When fully charged, some batteries wrongly report
+ * capacity_now = design_capacity instead of = full_charge_capacity
+ */
+ if (battery->capacity_now == battery->design_capacity &&
+ battery->full_charge_capacity != ACPI_BATTERY_VALUE_UNKNOWN)
+ battery->capacity_now = battery->full_charge_capacity;
+
+
if (test_bit(ACPI_BATTERY_QUIRK_PERCENTAGE_CAPACITY, &battery->flags)
&& battery->capacity_now >= 0 && battery->capacity_now <= 100)
battery->capacity_now = (battery->capacity_now *
On Thursday, June 12, 2014 04:58:13 PM Josef Gajdusek wrote:
> It seems that some batteries (noticed on DELL JYPJ136) assume
> capacity_now = design_capacity when fully charged. This causes
> reported capacity to suddenly jump to >full_charge_capacity (and that
> means capacity reported to userspace is >100% and incorrect)
> values after 99%. This patch attempts to detect this bug and trims
> capacity_now properly.
>
> Signed-off-by: Josef Gajdusek <[email protected]>
> ---
> Oops, I forgot to sign the previous one.
>
> diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
> index e48fc98..3338dd5 100644
> --- a/drivers/acpi/battery.c
> +++ b/drivers/acpi/battery.c
> @@ -532,6 +532,14 @@ static int acpi_battery_get_state(struct acpi_battery *battery)
> " invalid.\n");
> }
>
> + /* When fully charged, some batteries wrongly report
> + * capacity_now = design_capacity instead of = full_charge_capacity
> + */
> + if (battery->capacity_now == battery->design_capacity &&
Why don't you use the
battery->capacity_now > battery->full_charge_capacity
check here? That would be more robust, wouldn't it?
> + battery->full_charge_capacity != ACPI_BATTERY_VALUE_UNKNOWN)
> + battery->capacity_now = battery->full_charge_capacity;
> +
> +
> if (test_bit(ACPI_BATTERY_QUIRK_PERCENTAGE_CAPACITY, &battery->flags)
> && battery->capacity_now >= 0 && battery->capacity_now <= 100)
> battery->capacity_now = (battery->capacity_now *
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
It seems that some batteries (noticed on DELL JYPJ136) assume
capacity_now = design_capacity when fully charged. This causes
reported capacity to suddenly jump to >full_charge_capacity (and that
means capacity reported to userspace is >100% and incorrect)
values after 99%. This patch attempts to detect this bug, notifies
userspace and trims the value to full_charge_capacity.
Signed-off-by: Josef Gajdusek <[email protected]>
---
Relaxed the check as you suggested and added printk_once.
> Why don't you use the
>
> battery->capacity_now > battery->full_charge_capacity
>
> check here? That would be more robust, wouldn't it?
To allow userspace to see that there is some unknown bug we do not know yet
with the battery. This is taken care of much more elegantly by the printk
now.
diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
index e48fc98..57a800c 100644
--- a/drivers/acpi/battery.c
+++ b/drivers/acpi/battery.c
@@ -532,6 +532,18 @@ static int acpi_battery_get_state(struct acpi_battery *battery)
" invalid.\n");
}
+ /* When fully charged, some batteries wrongly report
+ * capacity_now = design_capacity instead of = full_charge_capacity
+ */
+ if (battery->capacity_now > battery->full_charge_capacity) {
+ battery->capacity_now = battery->full_charge_capacity;
+ printk_once(KERN_WARNING FW_BUG
+ "battery: reported current charge level (%d) "
+ "is higher than reported maximum charge level (%d)."
+ "This is normal on some systems.\n",
+ battery->capacity_now, battery->full_charge_capacity);
+ }
+
if (test_bit(ACPI_BATTERY_QUIRK_PERCENTAGE_CAPACITY, &battery->flags)
&& battery->capacity_now >= 0 && battery->capacity_now <= 100)
battery->capacity_now = (battery->capacity_now *
On Tuesday, June 17, 2014 06:41:54 PM Josef Gajdusek wrote:
> It seems that some batteries (noticed on DELL JYPJ136) assume
> capacity_now = design_capacity when fully charged. This causes
> reported capacity to suddenly jump to >full_charge_capacity (and that
> means capacity reported to userspace is >100% and incorrect)
> values after 99%. This patch attempts to detect this bug, notifies
> userspace and trims the value to full_charge_capacity.
>
> Signed-off-by: Josef Gajdusek <[email protected]>
> ---
> Relaxed the check as you suggested and added printk_once.
>
> > Why don't you use the
> >
> > battery->capacity_now > battery->full_charge_capacity
> >
> > check here? That would be more robust, wouldn't it?
>
> To allow userspace to see that there is some unknown bug we do not know yet
> with the battery. This is taken care of much more elegantly by the printk
> now.
>
> diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
> index e48fc98..57a800c 100644
> --- a/drivers/acpi/battery.c
> +++ b/drivers/acpi/battery.c
> @@ -532,6 +532,18 @@ static int acpi_battery_get_state(struct acpi_battery *battery)
> " invalid.\n");
> }
>
> + /* When fully charged, some batteries wrongly report
> + * capacity_now = design_capacity instead of = full_charge_capacity
> + */
> + if (battery->capacity_now > battery->full_charge_capacity) {
But it still is necessary to check if battery->full_charge_capacity is
not ACPI_BATTERY_VALUE_UNKNOWN, isn't it?
> + battery->capacity_now = battery->full_charge_capacity;
> + printk_once(KERN_WARNING FW_BUG
> + "battery: reported current charge level (%d) "
> + "is higher than reported maximum charge level (%d)."
> + "This is normal on some systems.\n",
> + battery->capacity_now, battery->full_charge_capacity);
> + }
> +
> if (test_bit(ACPI_BATTERY_QUIRK_PERCENTAGE_CAPACITY, &battery->flags)
> && battery->capacity_now >= 0 && battery->capacity_now <= 100)
> battery->capacity_now = (battery->capacity_now *
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
It seems that some batteries (noticed on DELL JYPJ136) assume
capacity_now = design_capacity when fully charged. This causes
reported capacity to suddenly jump to >full_charge_capacity (and that
means capacity reported to userspace is >100% and incorrect)
values after 99%. This patch attempts to detect this bug, notifies
userspace and trims the value to full_charge_capacity.
Signed-off-by: Josef Gajdusek <[email protected]>
---
>
> But it still is necessary to check if battery->full_charge_capacity is
> not ACPI_BATTERY_VALUE_UNKNOWN, isn't it?
>
Fixed, my mistake. When changing the checks, I somehow missed that the values
are not unsigned.
diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
index e48fc98..2e8f699 100644
--- a/drivers/acpi/battery.c
+++ b/drivers/acpi/battery.c
@@ -532,6 +532,19 @@ static int acpi_battery_get_state(struct acpi_battery *battery)
" invalid.\n");
}
+ /* When fully charged, some batteries wrongly report
+ * capacity_now = design_capacity instead of = full_charge_capacity
+ */
+ if (battery->capacity_now > battery->full_charge_capacity &&
+ battery->full_charge_capacity != ACPI_BATTERY_VALUE_UNKNOWN) {
+ battery->capacity_now = battery->full_charge_capacity;
+ printk_once(KERN_WARNING FW_BUG
+ "battery: reported current charge level (%d) "
+ "is higher than reported maximum charge level (%d)."
+ "This is normal on some systems.\n",
+ battery->capacity_now, battery->full_charge_capacity);
+ }
+
if (test_bit(ACPI_BATTERY_QUIRK_PERCENTAGE_CAPACITY, &battery->flags)
&& battery->capacity_now >= 0 && battery->capacity_now <= 100)
battery->capacity_now = (battery->capacity_now *
On Tuesday, June 17, 2014 10:14:00 PM Josef Gajdusek wrote:
> It seems that some batteries (noticed on DELL JYPJ136) assume
> capacity_now = design_capacity when fully charged. This causes
> reported capacity to suddenly jump to >full_charge_capacity (and that
> means capacity reported to userspace is >100% and incorrect)
> values after 99%. This patch attempts to detect this bug, notifies
> userspace and trims the value to full_charge_capacity.
>
> Signed-off-by: Josef Gajdusek <[email protected]>
> ---
> >
> > But it still is necessary to check if battery->full_charge_capacity is
> > not ACPI_BATTERY_VALUE_UNKNOWN, isn't it?
> >
>
> Fixed, my mistake. When changing the checks, I somehow missed that the values
> are not unsigned.
>
> diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
> index e48fc98..2e8f699 100644
> --- a/drivers/acpi/battery.c
> +++ b/drivers/acpi/battery.c
> @@ -532,6 +532,19 @@ static int acpi_battery_get_state(struct acpi_battery *battery)
> " invalid.\n");
> }
>
> + /* When fully charged, some batteries wrongly report
> + * capacity_now = design_capacity instead of = full_charge_capacity
> + */
> + if (battery->capacity_now > battery->full_charge_capacity &&
> + battery->full_charge_capacity != ACPI_BATTERY_VALUE_UNKNOWN) {
> + battery->capacity_now = battery->full_charge_capacity;
Thinking more about the printk_once(). Perhaps it only needs to be printed
when battery->capacity_now != battery->design_capacity (it's a known issue
otherwise).
> + printk_once(KERN_WARNING FW_BUG
> + "battery: reported current charge level (%d) "
> + "is higher than reported maximum charge level (%d)."
> + "This is normal on some systems.\n",
Also I think that the line above doesn't add any real value. :-)
> + battery->capacity_now, battery->full_charge_capacity);
> + }
> +
> if (test_bit(ACPI_BATTERY_QUIRK_PERCENTAGE_CAPACITY, &battery->flags)
> && battery->capacity_now >= 0 && battery->capacity_now <= 100)
> battery->capacity_now = (battery->capacity_now *
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
It seems that some batteries (noticed on DELL JYPJ136) assume
capacity_now = design_capacity when fully charged. This causes
reported capacity to suddenly jump to >full_charge_capacity (and that
means capacity reported to userspace is >100% and incorrect)
values after 99%. This patch detects capacity_now > full_charge_capacity,
notifies userspace (unless it is the known bug where capacity_now ==
design_capacity) and trims the value to full_charge_capacity.
Signed-off-by: Josef Gajdusek <[email protected]>
---
> Thinking more about the printk_once(). Perhaps it only needs to be printed
> when battery->capacity_now != battery->design_capacity (it's a known issue
> otherwise).
Changed. That was actually what I wanted to do originally, but I was unsure
about the kernel policy on this sort of messages.
>> + printk_once(KERN_WARNING FW_BUG
>> + "battery: reported current charge level (%d) "
>> + "is higher than reported maximum charge level (%d)."
>> + "This is normal on some systems.\n",
>
> Also I think that the line above doesn't add any real value. :-)
Removed, now it really is not needed as there is not any known system for which
capacity_now > full_charge_capacity && capacity_now != design_capacity would be
considered "normal"
diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
index e48fc98..9cacabd 100644
--- a/drivers/acpi/battery.c
+++ b/drivers/acpi/battery.c
@@ -532,6 +532,19 @@ static int acpi_battery_get_state(struct acpi_battery *battery)
" invalid.\n");
}
+ /* When fully charged, some batteries wrongly report
+ * capacity_now = design_capacity instead of = full_charge_capacity
+ */
+ if (battery->capacity_now > battery->full_charge_capacity &&
+ battery->full_charge_capacity != ACPI_BATTERY_VALUE_UNKNOWN) {
+ battery->capacity_now = battery->full_charge_capacity;
+ if (battery->capacity_now != battery->design_capacity)
+ printk_once(KERN_WARNING FW_BUG
+ "battery: reported current charge level (%d) "
+ "is higher than reported maximum charge level (%d).\n",
+ battery->capacity_now, battery->full_charge_capacity);
+ }
+
if (test_bit(ACPI_BATTERY_QUIRK_PERCENTAGE_CAPACITY, &battery->flags)
&& battery->capacity_now >= 0 && battery->capacity_now <= 100)
battery->capacity_now = (battery->capacity_now *