From fa93854f7a7ed63d054405bf3779247d5300edd3 Mon Sep 17 00:00:00 2001 From: Ognjen Galic Date: Wed, 7 Feb 2018 15:58:13 +0100 Subject: battery: Add the battery hooking API This is a patch that implements a generic hooking API for the generic ACPI battery driver. With this new generic API, drivers can expose platform specific behaviour via sysfs attributes in /sys/class/power_supply/BATn/ in a generic way. A perfect example of the need for this API are Lenovo ThinkPads. Lenovo ThinkPads have a ACPI extension that allows the setting of start and stop charge thresholds in the EC and battery firmware via ACPI. The thinkpad_acpi module can use this API to expose sysfs attributes that it controls inside the ACPI battery driver sysfs tree, under /sys/class/power_supply/BATN/. The file drivers/acpi/battery.h has been moved to include/acpi/battery.h and the includes inside ac.c, sbs.c, and battery.c have been adjusted to reflect that. When drivers hooks into the API, the API calls add_battery() for each battery in the system that passes it a acpi_battery struct. Then, the drivers can use device_create_file() to create new sysfs attributes with that struct and identify the batteries for per-battery attributes. Signed-off-by: Ognjen Galic Signed-off-by: Rafael J. Wysocki --- drivers/acpi/battery.c | 147 ++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 144 insertions(+), 3 deletions(-) (limited to 'drivers/acpi/battery.c') diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c index 7128488a3a72..f14a2bb1f7cd 100644 --- a/drivers/acpi/battery.c +++ b/drivers/acpi/battery.c @@ -21,8 +21,12 @@ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ */ +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt + #include +#include #include +#include #include #include #include @@ -42,7 +46,7 @@ #include #include -#include "battery.h" +#include #define PREFIX "ACPI: " @@ -125,6 +129,7 @@ struct acpi_battery { struct power_supply_desc bat_desc; struct acpi_device *device; struct notifier_block pm_nb; + struct list_head list; unsigned long update_time; int revision; int rate_now; @@ -630,6 +635,139 @@ static const struct device_attribute alarm_attr = { .store = acpi_battery_alarm_store, }; +/* + * The Battery Hooking API + * + * This API is used inside other drivers that need to expose + * platform-specific behaviour within the generic driver in a + * generic way. + * + */ + +static LIST_HEAD(acpi_battery_list); +static LIST_HEAD(battery_hook_list); +static DEFINE_MUTEX(hook_mutex); + +void __battery_hook_unregister(struct acpi_battery_hook *hook, int lock) +{ + struct acpi_battery *battery; + /* + * In order to remove a hook, we first need to + * de-register all the batteries that are registered. + */ + if (lock) + mutex_lock(&hook_mutex); + list_for_each_entry(battery, &acpi_battery_list, list) { + hook->remove_battery(battery->bat); + } + list_del(&hook->list); + if (lock) + mutex_unlock(&hook_mutex); + pr_info("extension unregistered: %s\n", hook->name); +} + +void battery_hook_unregister(struct acpi_battery_hook *hook) +{ + __battery_hook_unregister(hook, 1); +} +EXPORT_SYMBOL_GPL(battery_hook_unregister); + +void battery_hook_register(struct acpi_battery_hook *hook) +{ + struct acpi_battery *battery; + + mutex_lock(&hook_mutex); + INIT_LIST_HEAD(&hook->list); + list_add(&hook->list, &battery_hook_list); + /* + * Now that the driver is registered, we need + * to notify the hook that a battery is available + * for each battery, so that the driver may add + * its attributes. + */ + list_for_each_entry(battery, &acpi_battery_list, list) { + if (hook->add_battery(battery->bat)) { + /* + * If a add-battery returns non-zero, + * the registration of the extension has failed, + * and we will not add it to the list of loaded + * hooks. + */ + pr_err("extension failed to load: %s", hook->name); + __battery_hook_unregister(hook, 0); + return; + } + } + pr_info("new extension: %s\n", hook->name); + mutex_unlock(&hook_mutex); +} +EXPORT_SYMBOL_GPL(battery_hook_register); + +/* + * This function gets called right after the battery sysfs + * attributes have been added, so that the drivers that + * define custom sysfs attributes can add their own. +*/ +static void battery_hook_add_battery(struct acpi_battery *battery) +{ + struct acpi_battery_hook *hook_node; + + mutex_lock(&hook_mutex); + INIT_LIST_HEAD(&battery->list); + list_add(&battery->list, &acpi_battery_list); + /* + * Since we added a new battery to the list, we need to + * iterate over the hooks and call add_battery for each + * hook that was registered. This usually happens + * when a battery gets hotplugged or initialized + * during the battery module initialization. + */ + list_for_each_entry(hook_node, &battery_hook_list, list) { + if (hook_node->add_battery(battery->bat)) { + /* + * The notification of the extensions has failed, to + * prevent further errors we will unload the extension. + */ + __battery_hook_unregister(hook_node, 0); + pr_err("error in extension, unloading: %s", + hook_node->name); + } + } + mutex_unlock(&hook_mutex); +} + +static void battery_hook_remove_battery(struct acpi_battery *battery) +{ + struct acpi_battery_hook *hook; + + mutex_lock(&hook_mutex); + /* + * Before removing the hook, we need to remove all + * custom attributes from the battery. + */ + list_for_each_entry(hook, &battery_hook_list, list) { + hook->remove_battery(battery->bat); + } + /* Then, just remove the battery from the list */ + list_del(&battery->list); + mutex_unlock(&hook_mutex); +} + +static void __exit battery_hook_exit(void) +{ + struct acpi_battery_hook *hook; + struct acpi_battery_hook *ptr; + /* + * At this point, the acpi_bus_unregister_driver() + * has called remove for all batteries. We just + * need to remove the hooks. + */ + list_for_each_entry_safe(hook, ptr, &battery_hook_list, list) { + __battery_hook_unregister(hook, 1); + } + mutex_destroy(&hook_mutex); +} + static int sysfs_add_battery(struct acpi_battery *battery) { struct power_supply_config psy_cfg = { .drv_data = battery, }; @@ -657,6 +795,7 @@ static int sysfs_add_battery(struct acpi_battery *battery) battery->bat = NULL; return result; } + battery_hook_add_battery(battery); return device_create_file(&battery->bat->dev, &alarm_attr); } @@ -667,7 +806,7 @@ static void sysfs_remove_battery(struct acpi_battery *battery) mutex_unlock(&battery->sysfs_lock); return; } - + battery_hook_remove_battery(battery); device_remove_file(&battery->bat->dev, &alarm_attr); power_supply_unregister(battery->bat); battery->bat = NULL; @@ -1399,8 +1538,10 @@ static int __init acpi_battery_init(void) static void __exit acpi_battery_exit(void) { async_synchronize_cookie(async_cookie + 1); - if (battery_driver_registered) + if (battery_driver_registered) { acpi_bus_unregister_driver(&acpi_battery_driver); + battery_hook_exit(); + } #ifdef CONFIG_ACPI_PROCFS_POWER if (acpi_battery_dir) acpi_unlock_battery_dir(acpi_battery_dir); -- cgit From 91eea70e5e5ce12eb1c7cd922e561fab43e201bd Mon Sep 17 00:00:00 2001 From: Ognjen Galic Date: Wed, 7 Feb 2018 15:59:36 +0100 Subject: ACPI: battery: Add the ThinkPad "Not Charging" quirk 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. Reviewed-by: Andy Shevchenko Signed-off-by: Ognjen Galic Signed-off-by: Rafael J. Wysocki --- drivers/acpi/battery.c | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) (limited to 'drivers/acpi/battery.c') diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c index f14a2bb1f7cd..cfdef4c1d097 100644 --- a/drivers/acpi/battery.c +++ b/drivers/acpi/battery.c @@ -74,6 +74,7 @@ static async_cookie_t async_cookie; static bool battery_driver_registered; static int battery_bix_broken_package; static int battery_notification_delay_ms; +static int battery_quirk_notcharging; static int battery_full_discharging; static unsigned int cache_time = 1000; module_param(cache_time, uint, 0644); @@ -229,6 +230,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; @@ -707,7 +710,7 @@ EXPORT_SYMBOL_GPL(battery_hook_register); * This function gets called right after the battery sysfs * attributes have been added, so that the drivers that * define custom sysfs attributes can add their own. -*/ + */ static void battery_hook_add_battery(struct acpi_battery *battery) { struct acpi_battery_hook *hook_node; @@ -1315,6 +1318,12 @@ static int __init battery_full_discharging_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 = { { .callback = battery_bix_broken_package_quirk, @@ -1364,6 +1373,19 @@ static const struct dmi_system_id bat_dmi_table[] __initconst = { DMI_MATCH(DMI_PRODUCT_NAME, "UX410UAK"), }, }, + { + /* + * 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"), + }, + }, {}, }; -- cgit From 514bcc5dfa69d64f7772cb6442721b9d1b83504d Mon Sep 17 00:00:00 2001 From: Colin Ian King Date: Fri, 23 Feb 2018 16:32:55 +0000 Subject: ACPI: battery: make function __battery_hook_unregister() static The function __battery_hook_unregister is local to the source and does not need to be in global scope, so make it static. Cleans up sparse warning: drivers/acpi/battery.c:654:6: warning: symbol '__battery_hook_unregister' was not declared. Should it be static? Signed-off-by: Colin Ian King Signed-off-by: Rafael J. Wysocki --- drivers/acpi/battery.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/acpi/battery.c') diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c index cfdef4c1d097..b4580b2e8706 100644 --- a/drivers/acpi/battery.c +++ b/drivers/acpi/battery.c @@ -651,7 +651,7 @@ static LIST_HEAD(acpi_battery_list); static LIST_HEAD(battery_hook_list); static DEFINE_MUTEX(hook_mutex); -void __battery_hook_unregister(struct acpi_battery_hook *hook, int lock) +static void __battery_hook_unregister(struct acpi_battery_hook *hook, int lock) { struct acpi_battery *battery; /* -- cgit From a20136a67a995cd5b74e8c0fcb3b2f2e5b2848dd Mon Sep 17 00:00:00 2001 From: Laszlo Toth Date: Sat, 24 Feb 2018 10:20:15 +0100 Subject: ACPI: battery: do not export degraded capacity values over 100 With a degraded battery, full_charge_capacity can be less than design_capacity, however it's not sure that capacity_now's max will follow. Example from an affected machine: /sys/class/power_supply/BAT0/charge_full -> 4290000 /sys/class/power_supply/BAT0/charge_full_design -> 5900000 /sys/class/power_supply/BAT0/charge_now -> 5900000 /sys/class/power_supply/BAT0/capacity -> 137 The battery is a degraded one with a full charge, and charge_now is the value of charge_full_design instead of charge_full. Added a new quirk to test and correct this, and a new function to check if the battery is a degraded one or not. This keeps the possibility to be over 100 if it's really the case. Signed-off-by: Laszlo Toth Signed-off-by: Rafael J. Wysocki --- drivers/acpi/battery.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) (limited to 'drivers/acpi/battery.c') diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c index b4580b2e8706..96ed134bacf8 100644 --- a/drivers/acpi/battery.c +++ b/drivers/acpi/battery.c @@ -121,6 +121,10 @@ enum { post-1.29 BIOS), but as of Nov. 2012, no such update is available for the 2010 models. */ ACPI_BATTERY_QUIRK_THINKPAD_MAH, + /* for batteries reporting current capacity with design capacity + * on a full charge, but showing degradation in full charge cap. + */ + ACPI_BATTERY_QUIRK_DEGRADED_FULL_CHARGE, }; struct acpi_battery { @@ -207,6 +211,12 @@ static int acpi_battery_is_charged(struct acpi_battery *battery) return 0; } +static bool acpi_battery_is_degraded(struct acpi_battery *battery) +{ + return battery->full_charge_capacity && battery->design_capacity && + battery->full_charge_capacity < battery->design_capacity; +} + static int acpi_battery_get_property(struct power_supply *psy, enum power_supply_property psp, union power_supply_propval *val) @@ -483,6 +493,10 @@ static int extract_battery_info(const int use_bix, it's impossible to tell if they would need an adjustment or not if their values were higher. */ } + if (test_bit(ACPI_BATTERY_QUIRK_DEGRADED_FULL_CHARGE, &battery->flags) && + battery->capacity_now > battery->full_charge_capacity) + battery->capacity_now = battery->full_charge_capacity; + return result; } @@ -575,6 +589,10 @@ static int acpi_battery_get_state(struct acpi_battery *battery) battery->capacity_now = battery->capacity_now * 10000 / battery->design_voltage; } + if (test_bit(ACPI_BATTERY_QUIRK_DEGRADED_FULL_CHARGE, &battery->flags) && + battery->capacity_now > battery->full_charge_capacity) + battery->capacity_now = battery->full_charge_capacity; + return result; } @@ -885,6 +903,15 @@ static void acpi_battery_quirks(struct acpi_battery *battery) } } } + + if (test_bit(ACPI_BATTERY_QUIRK_DEGRADED_FULL_CHARGE, &battery->flags)) + return; + + if (acpi_battery_is_degraded(battery) && + battery->capacity_now > battery->full_charge_capacity) { + set_bit(ACPI_BATTERY_QUIRK_DEGRADED_FULL_CHARGE, &battery->flags); + battery->capacity_now = battery->full_charge_capacity; + } } static int acpi_battery_update(struct acpi_battery *battery, bool resume) -- cgit From 7a4ea10c01cdda3d66aa98af258f150d863aee24 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Tue, 13 Mar 2018 10:07:49 +0100 Subject: Revert "ACPI: battery: Add the ThinkPad "Not Charging" quirk" Revert commit 91eea70e5e5c (ACPI: battery: Add the ThinkPad "Not Charging" quirk) as it is reported to cause user space to misbehave. That appears to be due to bugs in user space, so this commit will go in again after the bugs have been fixed and the fixes have been delivered to users. Link: https://marc.info/?l=linux-acpi&m=152089585129589&w=2 Reported-by: Pavel Machek Signed-off-by: Rafael J. Wysocki --- drivers/acpi/battery.c | 24 +----------------------- 1 file changed, 1 insertion(+), 23 deletions(-) (limited to 'drivers/acpi/battery.c') diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c index 96ed134bacf8..c88d41b04789 100644 --- a/drivers/acpi/battery.c +++ b/drivers/acpi/battery.c @@ -74,7 +74,6 @@ static async_cookie_t async_cookie; static bool battery_driver_registered; static int battery_bix_broken_package; static int battery_notification_delay_ms; -static int battery_quirk_notcharging; static int battery_full_discharging; static unsigned int cache_time = 1000; module_param(cache_time, uint, 0644); @@ -240,8 +239,6 @@ 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; @@ -728,7 +725,7 @@ EXPORT_SYMBOL_GPL(battery_hook_register); * This function gets called right after the battery sysfs * attributes have been added, so that the drivers that * define custom sysfs attributes can add their own. - */ +*/ static void battery_hook_add_battery(struct acpi_battery *battery) { struct acpi_battery_hook *hook_node; @@ -1345,12 +1342,6 @@ static int __init battery_full_discharging_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 = { { .callback = battery_bix_broken_package_quirk, @@ -1400,19 +1391,6 @@ static const struct dmi_system_id bat_dmi_table[] __initconst = { DMI_MATCH(DMI_PRODUCT_NAME, "UX410UAK"), }, }, - { - /* - * 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"), - }, - }, {}, }; -- cgit