[PATCH] acpi/battery: fix wrong value of capacity_now reported by few battery types when fully charged (resend)

[PATCH] acpi/battery: fix wrong value of capacity_now reported by few battery types when fully charged (resend)

[Josef Gajdusek]

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 <atx@atx.name>
---
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 *

Re: [PATCH] acpi/battery: fix wrong value of capacity_now reported by few battery types when fully charged (resend)

[Rafael J. Wysocki]

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 <atx@atx.name>
> ---
> 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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH] acpi/battery: fix wrong value of capacity_now reported by few battery types when fully charged (resend)

[Josef Gajdusek]

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 <atx@atx.name>
---
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 *

Re: [PATCH] acpi/battery: fix wrong value of capacity_now reported by few battery types when fully charged (resend)

[Rafael J. Wysocki]

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 <atx@atx.name>
> ---
> 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.

Re: [PATCH] acpi/battery: fix wrong value of capacity_now reported by few battery types when fully charged (resend)

[Josef Gajdusek]

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 <atx@atx.name>
---
> 
> 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 *

Re: [PATCH] acpi/battery: fix wrong value of capacity_now reported by few battery types when fully charged (resend)

[Rafael J. Wysocki]

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 <atx@atx.name>
> ---
> > 
> > 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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH] acpi/battery: fix wrong value of capacity_now reported by few battery types when fully charged (resend)

[Josef Gajdusek]

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 <atx@atx.name>
---
> 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 *