| Message ID | 20211222212014.66971-1-linux@weissschuh.net |
|---|---|
| State | Accepted |
| Commit | e96c1197aca628f7d2480a1cc3214912b40b3414 |
| Headers | show |
| Series | ACPI: battery: Add the ThinkPad "Not Charging" quirk | expand |
Hi Thomas, On 12/22/21 22:20, Thomas Weißschuh wrote: > The EC/ACPI firmware on Lenovo ThinkPads used to report a status > of "Unknown" when the battery is between the charge start and > charge stop thresholds. On Windows, it reports "Not Charging" > so the quirk has been added to also report correctly. > > Now the "status" attribute returns "Not Charging" when the > battery on ThinkPads is not physicaly charging. > > Signed-off-by: Thomas Weißschuh <linux@weissschuh.net> Thanks, patch looks good to me. As for the userspace issues in dealing with the POWER_SUPPLY_STATUS_NOT_CHARGING status, those indeed have long been fixed and this status is already returned acpi//battery.c from the acpi_battery_handle_discharging() function for a while no; and we have had no complaints about that: Reviewed-by: Hans de Goede <hdegoede@redhat.com> Regards, Hans > --- > > This is the same as: https://patchwork.kernel.org/patch/10205359/ > > Previously this patch has been applied[0] but then reverted from -next > because it caused a regression in UPower. > This regression however has been fixed in UPower in late 2018[1], > with the fixed version 0.99.10 released in early 2019 [2]. > So maybe it is now time to reintroduce this change. > > Ognen: > > As the patch was originally developed by you, could send a > Signed-off-by-tag, so I can attribute you as co-developer? > > Or maybe the original patch could just be re-applied? > > The original patch had the following tags, which I'm not sure to handle > for this case: > > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> > Signed-off-by: Ognjen Galic <smclt30p@gmail.com> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > Also Cc-ing the UPower maintainers for their opinion: > > Cc: Bastien Nocera <hadess@hadess.net> > Cc: David Zeuthen <davidz@redhat.com> > Cc: Richard Hughes <richard@hughsie.com> > > [0] Applied as 91eea70e5e5ce12eb1c7cd922e561fab43e201bd > [1] https://gitlab.freedesktop.org/upower/upower/-/merge_requests/19/commits > [2] https://gitlab.freedesktop.org/upower/upower/-/commit/215049e7b80c5f24cb35cd229a445c6cf19bd381 > --- > drivers/acpi/battery.c | 22 ++++++++++++++++++++++ > 1 file changed, 22 insertions(+) > > > base-commit: fa55b7dcdc43c1aa1ba12bca9d2dd4318c2a0dbf > > diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c > index 8afa85d6eb6a..ead0114f27c9 100644 > --- a/drivers/acpi/battery.c > +++ b/drivers/acpi/battery.c > @@ -53,6 +53,7 @@ static int battery_bix_broken_package; > static int battery_notification_delay_ms; > static int battery_ac_is_broken; > static int battery_check_pmic = 1; > +static int battery_quirk_notcharging; > static unsigned int cache_time = 1000; > module_param(cache_time, uint, 0644); > MODULE_PARM_DESC(cache_time, "cache time in milliseconds"); > @@ -217,6 +218,8 @@ static int acpi_battery_get_property(struct power_supply *psy, > val->intval = POWER_SUPPLY_STATUS_CHARGING; > else if (acpi_battery_is_charged(battery)) > val->intval = POWER_SUPPLY_STATUS_FULL; > + else if (battery_quirk_notcharging) > + val->intval = POWER_SUPPLY_STATUS_NOT_CHARGING; > else > val->intval = POWER_SUPPLY_STATUS_UNKNOWN; > break; > @@ -1111,6 +1114,12 @@ battery_do_not_check_pmic_quirk(const struct dmi_system_id *d) > return 0; > } > > +static int __init battery_quirk_not_charging(const struct dmi_system_id *d) > +{ > + battery_quirk_notcharging = 1; > + return 0; > +} > + > static const struct dmi_system_id bat_dmi_table[] __initconst = { > { > /* NEC LZ750/LS */ > @@ -1155,6 +1164,19 @@ static const struct dmi_system_id bat_dmi_table[] __initconst = { > DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo MIIX 320-10ICR"), > }, > }, > + { > + /* > + * On Lenovo ThinkPads the BIOS specification defines > + * a state when the bits for charging and discharging > + * are both set to 0. That state is "Not Charging". > + */ > + .callback = battery_quirk_not_charging, > + .ident = "Lenovo ThinkPad", > + .matches = { > + DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"), > + DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad"), > + }, > + }, > {}, > }; > >
On Thu, Dec 23, 2021 at 5:36 PM Hans de Goede <hdegoede@redhat.com> wrote: > > Hi Thomas, > > On 12/22/21 22:20, Thomas Weißschuh wrote: > > The EC/ACPI firmware on Lenovo ThinkPads used to report a status > > of "Unknown" when the battery is between the charge start and > > charge stop thresholds. On Windows, it reports "Not Charging" > > so the quirk has been added to also report correctly. > > > > Now the "status" attribute returns "Not Charging" when the > > battery on ThinkPads is not physicaly charging. > > > > Signed-off-by: Thomas Weißschuh <linux@weissschuh.net> > > Thanks, patch looks good to me. > > As for the userspace issues in dealing with the > POWER_SUPPLY_STATUS_NOT_CHARGING status, those indeed > have long been fixed and this status is already returned > acpi//battery.c from the acpi_battery_handle_discharging() > function for a while no; and we have had no complaints > about that: > > Reviewed-by: Hans de Goede <hdegoede@redhat.com> Applied as 5.17 material, thanks!
diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c index 8afa85d6eb6a..ead0114f27c9 100644 --- a/drivers/acpi/battery.c +++ b/drivers/acpi/battery.c @@ -53,6 +53,7 @@ static int battery_bix_broken_package; static int battery_notification_delay_ms; static int battery_ac_is_broken; static int battery_check_pmic = 1; +static int battery_quirk_notcharging; static unsigned int cache_time = 1000; module_param(cache_time, uint, 0644); MODULE_PARM_DESC(cache_time, "cache time in milliseconds"); @@ -217,6 +218,8 @@ static int acpi_battery_get_property(struct power_supply *psy, val->intval = POWER_SUPPLY_STATUS_CHARGING; else if (acpi_battery_is_charged(battery)) val->intval = POWER_SUPPLY_STATUS_FULL; + else if (battery_quirk_notcharging) + val->intval = POWER_SUPPLY_STATUS_NOT_CHARGING; else val->intval = POWER_SUPPLY_STATUS_UNKNOWN; break; @@ -1111,6 +1114,12 @@ battery_do_not_check_pmic_quirk(const struct dmi_system_id *d) return 0; } +static int __init battery_quirk_not_charging(const struct dmi_system_id *d) +{ + battery_quirk_notcharging = 1; + return 0; +} + static const struct dmi_system_id bat_dmi_table[] __initconst = { { /* NEC LZ750/LS */ @@ -1155,6 +1164,19 @@ static const struct dmi_system_id bat_dmi_table[] __initconst = { DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo MIIX 320-10ICR"), }, }, + { + /* + * On Lenovo ThinkPads the BIOS specification defines + * a state when the bits for charging and discharging + * are both set to 0. That state is "Not Charging". + */ + .callback = battery_quirk_not_charging, + .ident = "Lenovo ThinkPad", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"), + DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad"), + }, + }, {}, };