| Message ID | 20191210095752.4211-1-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | Mainlined, archived |
| Headers | show |
| Series | [1/3] ACPI / battery: Deal with design or full capacity being reported as -1 | expand |
On Tue, Dec 10, 2019 at 10:57 AM Hans de Goede <hdegoede@redhat.com> wrote: > > Commit b41901a2cf06 ("ACPI / battery: Do not export energy_full[_design] on > devices without full_charge_capacity") added support for some (broken) > devices which always report 0 for both design- and full_charge-capacity. > > This assumes that if the capacity is not being reported it is 0. The > ThunderSoft TS178 tablet's _BIX implementation falsifies this assumption. > It reports ACPI_BATTERY_VALUE_UNKNOWN (-1) as full_charge_capacity, which > we treat as a valid value which causes several problems. > > This commit fixes this by adding a new ACPI_BATTERY_CAPACITY_VALID() helper > which checks that the value is not 0 and not -1; and using this whenever we > need to test if either design_capacity or full_charge_capacity is valid. > > Fixes: b41901a2cf06 ("ACPI / battery: Do not export energy_full[_design] on devices without full_charge_capacity") > Signed-off-by: Hans de Goede <hdegoede@redhat.com> Applying along with the [2-3/3] as 5.6 material, thanks! > --- > drivers/acpi/battery.c | 19 ++++++++++++------- > 1 file changed, 12 insertions(+), 7 deletions(-) > > diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c > index 558fedf8a7a1..9c0d7c577cb9 100644 > --- a/drivers/acpi/battery.c > +++ b/drivers/acpi/battery.c > @@ -38,6 +38,8 @@ > #define PREFIX "ACPI: " > > #define ACPI_BATTERY_VALUE_UNKNOWN 0xFFFFFFFF > +#define ACPI_BATTERY_CAPACITY_VALID(capacity) \ > + ((capacity) != 0 && (capacity) != ACPI_BATTERY_VALUE_UNKNOWN) > > #define ACPI_BATTERY_DEVICE_NAME "Battery" > > @@ -192,7 +194,8 @@ static int acpi_battery_is_charged(struct acpi_battery *battery) > > static bool acpi_battery_is_degraded(struct acpi_battery *battery) > { > - return battery->full_charge_capacity && battery->design_capacity && > + return ACPI_BATTERY_CAPACITY_VALID(battery->full_charge_capacity) && > + ACPI_BATTERY_CAPACITY_VALID(battery->design_capacity) && > battery->full_charge_capacity < battery->design_capacity; > } > > @@ -263,14 +266,14 @@ static int acpi_battery_get_property(struct power_supply *psy, > break; > case POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN: > case POWER_SUPPLY_PROP_ENERGY_FULL_DESIGN: > - if (battery->design_capacity == ACPI_BATTERY_VALUE_UNKNOWN) > + if (!ACPI_BATTERY_CAPACITY_VALID(battery->design_capacity)) > ret = -ENODEV; > else > val->intval = battery->design_capacity * 1000; > break; > case POWER_SUPPLY_PROP_CHARGE_FULL: > case POWER_SUPPLY_PROP_ENERGY_FULL: > - if (battery->full_charge_capacity == ACPI_BATTERY_VALUE_UNKNOWN) > + if (!ACPI_BATTERY_CAPACITY_VALID(battery->full_charge_capacity)) > ret = -ENODEV; > else > val->intval = battery->full_charge_capacity * 1000; > @@ -283,11 +286,12 @@ static int acpi_battery_get_property(struct power_supply *psy, > val->intval = battery->capacity_now * 1000; > break; > case POWER_SUPPLY_PROP_CAPACITY: > - if (battery->capacity_now && battery->full_charge_capacity) > + if (battery->capacity_now == ACPI_BATTERY_VALUE_UNKNOWN || > + !ACPI_BATTERY_CAPACITY_VALID(battery->full_charge_capacity)) > + ret = -ENODEV; > + else > val->intval = battery->capacity_now * 100/ > battery->full_charge_capacity; > - else > - val->intval = 0; > break; > case POWER_SUPPLY_PROP_CAPACITY_LEVEL: > if (battery->state & ACPI_BATTERY_STATE_CRITICAL) > @@ -799,7 +803,8 @@ static int sysfs_add_battery(struct acpi_battery *battery) > battery->bat_desc.properties = charge_battery_props; > battery->bat_desc.num_properties = > ARRAY_SIZE(charge_battery_props); > - } else if (battery->full_charge_capacity == 0) { > + } else if (!ACPI_BATTERY_CAPACITY_VALID( > + battery->full_charge_capacity)) { > battery->bat_desc.properties = > energy_battery_full_cap_broken_props; > battery->bat_desc.num_properties = > -- > 2.23.0 >
diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c index 558fedf8a7a1..9c0d7c577cb9 100644 --- a/drivers/acpi/battery.c +++ b/drivers/acpi/battery.c @@ -38,6 +38,8 @@ #define PREFIX "ACPI: " #define ACPI_BATTERY_VALUE_UNKNOWN 0xFFFFFFFF +#define ACPI_BATTERY_CAPACITY_VALID(capacity) \ + ((capacity) != 0 && (capacity) != ACPI_BATTERY_VALUE_UNKNOWN) #define ACPI_BATTERY_DEVICE_NAME "Battery" @@ -192,7 +194,8 @@ static int acpi_battery_is_charged(struct acpi_battery *battery) static bool acpi_battery_is_degraded(struct acpi_battery *battery) { - return battery->full_charge_capacity && battery->design_capacity && + return ACPI_BATTERY_CAPACITY_VALID(battery->full_charge_capacity) && + ACPI_BATTERY_CAPACITY_VALID(battery->design_capacity) && battery->full_charge_capacity < battery->design_capacity; } @@ -263,14 +266,14 @@ static int acpi_battery_get_property(struct power_supply *psy, break; case POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN: case POWER_SUPPLY_PROP_ENERGY_FULL_DESIGN: - if (battery->design_capacity == ACPI_BATTERY_VALUE_UNKNOWN) + if (!ACPI_BATTERY_CAPACITY_VALID(battery->design_capacity)) ret = -ENODEV; else val->intval = battery->design_capacity * 1000; break; case POWER_SUPPLY_PROP_CHARGE_FULL: case POWER_SUPPLY_PROP_ENERGY_FULL: - if (battery->full_charge_capacity == ACPI_BATTERY_VALUE_UNKNOWN) + if (!ACPI_BATTERY_CAPACITY_VALID(battery->full_charge_capacity)) ret = -ENODEV; else val->intval = battery->full_charge_capacity * 1000; @@ -283,11 +286,12 @@ static int acpi_battery_get_property(struct power_supply *psy, val->intval = battery->capacity_now * 1000; break; case POWER_SUPPLY_PROP_CAPACITY: - if (battery->capacity_now && battery->full_charge_capacity) + if (battery->capacity_now == ACPI_BATTERY_VALUE_UNKNOWN || + !ACPI_BATTERY_CAPACITY_VALID(battery->full_charge_capacity)) + ret = -ENODEV; + else val->intval = battery->capacity_now * 100/ battery->full_charge_capacity; - else - val->intval = 0; break; case POWER_SUPPLY_PROP_CAPACITY_LEVEL: if (battery->state & ACPI_BATTERY_STATE_CRITICAL) @@ -799,7 +803,8 @@ static int sysfs_add_battery(struct acpi_battery *battery) battery->bat_desc.properties = charge_battery_props; battery->bat_desc.num_properties = ARRAY_SIZE(charge_battery_props); - } else if (battery->full_charge_capacity == 0) { + } else if (!ACPI_BATTERY_CAPACITY_VALID( + battery->full_charge_capacity)) { battery->bat_desc.properties = energy_battery_full_cap_broken_props; battery->bat_desc.num_properties =
Commit b41901a2cf06 ("ACPI / battery: Do not export energy_full[_design] on devices without full_charge_capacity") added support for some (broken) devices which always report 0 for both design- and full_charge-capacity. This assumes that if the capacity is not being reported it is 0. The ThunderSoft TS178 tablet's _BIX implementation falsifies this assumption. It reports ACPI_BATTERY_VALUE_UNKNOWN (-1) as full_charge_capacity, which we treat as a valid value which causes several problems. This commit fixes this by adding a new ACPI_BATTERY_CAPACITY_VALID() helper which checks that the value is not 0 and not -1; and using this whenever we need to test if either design_capacity or full_charge_capacity is valid. Fixes: b41901a2cf06 ("ACPI / battery: Do not export energy_full[_design] on devices without full_charge_capacity") Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- drivers/acpi/battery.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-)