ACPI: SBS: report relative state of charge as CAPACITY

[PATCH] ACPI: SBS: report relative state of charge as CAPACITY

Posted by Itay Shem-tov 3 weeks, 1 day ago

acpi_sbs reads SBS command 0x0e (AbsoluteStateOfCharge) into
battery->state_of_charge, which is exported unmodified as
POWER_SUPPLY_PROP_CAPACITY.

Per the Smart Battery Data Specification 1.1, 0x0e is expressed as a
percentage of DesignCapacity and is explicitly permitted to exceed 100%.
Documentation/ABI/testing/sysfs-class-power specifies the capacity
attribute as "Valid values: 0 - 100 (percent)", so any pack whose
FullChargeCapacity exceeds its DesignCapacity - the normal state of a
new or recently replaced battery - makes the driver report out of range.

The correct source is 0x0d (RelativeStateOfCharge), a percentage of
FullChargeCapacity, which the specification bounds to 0..100.

This is the same defect that was fixed in the i2c SBS driver by commit
b1f092f6480e ("sbs-battery.c: Capacity attr = remaining relative
capacity"), whose reasoning applies verbatim here; drivers/acpi/sbs.c was
not updated at the time. drivers/power/supply/sbs-battery.c has used 0x0d
since, so the two SBS drivers currently disagree about what CAPACITY
means.

Observed on a MacBookPro11,1 with an SMP/bq20z451 pack
(FullChargeCapacity 6775 mAh, DesignCapacity 6400 mAh). Both registers
read back-to-back from the pack at a full charge:

  0x0d RelativeStateOfCharge = 100 %
  0x0e AbsoluteStateOfCharge = 106 %

/sys/class/power_supply/BAT0/capacity reported 106 while upower, which
computes charge_now/charge_full itself rather than trusting the driver,
reported 100.

battery->state_of_charge has no other consumer, so no other property
changes behaviour.

Signed-off-by: Itay Shem-tov <itayst@gmail.com>
---
 drivers/acpi/sbs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/acpi/sbs.c b/drivers/acpi/sbs.c
index 86b7c79..862cb94 100644
--- a/drivers/acpi/sbs.c
+++ b/drivers/acpi/sbs.c
@@ -318,7 +318,7 @@ static struct acpi_battery_reader state_readers[] = {
 	{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)},
+	{0x0d, SMBUS_READ_WORD, offsetof(struct acpi_battery, state_of_charge)},
 	{0x16, SMBUS_READ_WORD, offsetof(struct acpi_battery, state)},
 };

--
2.51.0

Re: [PATCH] ACPI: SBS: report relative state of charge as CAPACITY

Posted by Sebastian Reichel 2 weeks, 6 days ago

Hi,

On Thu, Sep 03, 2026 at 01:42:44PM +0300, Itay Shem-tov wrote:
> acpi_sbs reads SBS command 0x0e (AbsoluteStateOfCharge) into
> battery->state_of_charge, which is exported unmodified as
> POWER_SUPPLY_PROP_CAPACITY.
> 
> Per the Smart Battery Data Specification 1.1, 0x0e is expressed as a
> percentage of DesignCapacity and is explicitly permitted to exceed 100%.
> Documentation/ABI/testing/sysfs-class-power specifies the capacity
> attribute as "Valid values: 0 - 100 (percent)", so any pack whose
> FullChargeCapacity exceeds its DesignCapacity - the normal state of a
> new or recently replaced battery - makes the driver report out of range.
> 
> The correct source is 0x0d (RelativeStateOfCharge), a percentage of
> FullChargeCapacity, which the specification bounds to 0..100.
> 
> This is the same defect that was fixed in the i2c SBS driver by commit
> b1f092f6480e ("sbs-battery.c: Capacity attr = remaining relative
> capacity"), whose reasoning applies verbatim here; drivers/acpi/sbs.c was
> not updated at the time. drivers/power/supply/sbs-battery.c has used 0x0d
> since, so the two SBS drivers currently disagree about what CAPACITY
> means.
> 
> Observed on a MacBookPro11,1 with an SMP/bq20z451 pack
> (FullChargeCapacity 6775 mAh, DesignCapacity 6400 mAh). Both registers
> read back-to-back from the pack at a full charge:
> 
>   0x0d RelativeStateOfCharge = 100 %
>   0x0e AbsoluteStateOfCharge = 106 %
> 
> /sys/class/power_supply/BAT0/capacity reported 106 while upower, which
> computes charge_now/charge_full itself rather than trusting the driver,
> reported 100.
> 
> battery->state_of_charge has no other consumer, so no other property
> changes behaviour.
> 
> Signed-off-by: Itay Shem-tov <itayst@gmail.com>
> ---

That also matches the sbs-battery driver (drivers/power/supply/sbs-battery.c):

Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com>

Greetings,

-- Sebastian

>  drivers/acpi/sbs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/acpi/sbs.c b/drivers/acpi/sbs.c
> index 86b7c79..862cb94 100644
> --- a/drivers/acpi/sbs.c
> +++ b/drivers/acpi/sbs.c
> @@ -318,7 +318,7 @@ static struct acpi_battery_reader state_readers[] = {
>  	{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)},
> +	{0x0d, SMBUS_READ_WORD, offsetof(struct acpi_battery, state_of_charge)},
>  	{0x16, SMBUS_READ_WORD, offsetof(struct acpi_battery, state)},
>  };
> 
> --
> 2.51.0