[1/5] Revert "ACPI / battery: Add quirk for Asus GL502VSK and UX305LA"

Message ID 20190202153757.26990-2-kai.heng.feng@canonical.com
State New
Headers show
Series
  • [1/5] Revert "ACPI / battery: Add quirk for Asus GL502VSK and UX305LA"
Related show

Commit Message

Kai Heng Feng Feb. 2, 2019, 3:37 p.m.
From: Daniel Drake <drake@endlessm.com>

BugLink: https://bugs.launchpad.net/bugs/1745032

Revert commit c68f0676ef7d ("ACPI / battery: Add quirk for Asus
GL502VSK and UX305LA") and commit 4446823e2573 ("ACPI / battery: Add
quirk for Asus UX360UA and UX410UAK").

On many many Asus products, the battery is sometimes reported as
charging or discharging even when it is full and you are on AC power.
This change quirked the kernel to avoid advertising the discharging
state when this happens on 4 laptop models, under the belief that
this was incorrect information.  I presume it originates from user
reports who are confused that their battery status icon says that it
is discharging.

However, the reported information is indeed correct, and the quirk
approach taken is inadequate and more thought is needed first.
Specifically:

 1. It only quirks discharging state, not charging

 2. There are so many different Asus products and DMI naming variants
    within those product families that behave this way; Linux could
    grow to quirk hundreds of products and still not even be close at
    "winning" this battle.

 3. Asus previously clarified that this behaviour is intentional. The
    platform will periodically do a partial discharge/charge cycle
    when the battery is full, because this is one way to extend the
    lifetime of the battery (leaving a battery at 100% charge and
    unused will decrease its usable capacity over time).

    My understanding is that any decent consumer product will have
    this behaviour, but it appears that Asus is different in that
    they expose this info through ACPI.

    However, the behaviour seems correct. The ACPI spec does not
    suggest in that the platform should hide the truth.  It lets you
    report that the battery is full of charge, and discharging, and
    with external power connected; and Asus does this.

 4. In terms of not confusing the user, this seems like something that
    could/should be handled by userspace, which can also detect these
    same (accurate) conditions in the general case.

Revert this quirk before it gets included in a release, while we look
for better approaches.

Signed-off-by: Daniel Drake <drake@endlessm.com>
Acked-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
(cherry picked from commit 82bf43b291888599b4079244d12195d214086fa4)
Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
---
 drivers/acpi/battery.c | 48 +++---------------------------------------
 1 file changed, 3 insertions(+), 45 deletions(-)

Patch

diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
index a4fd1d8b4fe9..13e7b56e33ae 100644
--- a/drivers/acpi/battery.c
+++ b/drivers/acpi/battery.c
@@ -70,7 +70,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_full_discharging;
 static unsigned int cache_time = 1000;
 module_param(cache_time, uint, 0644);
 MODULE_PARM_DESC(cache_time, "cache time in milliseconds");
@@ -215,12 +214,9 @@  static int acpi_battery_get_property(struct power_supply *psy,
 		return -ENODEV;
 	switch (psp) {
 	case POWER_SUPPLY_PROP_STATUS:
-		if (battery->state & ACPI_BATTERY_STATE_DISCHARGING) {
-			if (battery_full_discharging && battery->rate_now == 0)
-				val->intval = POWER_SUPPLY_STATUS_FULL;
-			else
-				val->intval = POWER_SUPPLY_STATUS_DISCHARGING;
-		} else if (battery->state & ACPI_BATTERY_STATE_CHARGING)
+		if (battery->state & ACPI_BATTERY_STATE_DISCHARGING)
+			val->intval = POWER_SUPPLY_STATUS_DISCHARGING;
+		else if (battery->state & ACPI_BATTERY_STATE_CHARGING)
 			val->intval = POWER_SUPPLY_STATUS_CHARGING;
 		else if (acpi_battery_is_charged(battery))
 			val->intval = POWER_SUPPLY_STATUS_FULL;
@@ -1170,12 +1166,6 @@  battery_notification_delay_quirk(const struct dmi_system_id *d)
 	return 0;
 }
 
-static int __init battery_full_discharging_quirk(const struct dmi_system_id *d)
-{
-	battery_full_discharging = 1;
-	return 0;
-}
-
 static const struct dmi_system_id bat_dmi_table[] __initconst = {
 	{
 		.callback = battery_bix_broken_package_quirk,
@@ -1193,38 +1183,6 @@  static const struct dmi_system_id bat_dmi_table[] __initconst = {
 			DMI_MATCH(DMI_PRODUCT_NAME, "Aspire V5-573G"),
 		},
 	},
-	{
-		.callback = battery_full_discharging_quirk,
-		.ident = "ASUS GL502VSK",
-		.matches = {
-			DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
-			DMI_MATCH(DMI_PRODUCT_NAME, "GL502VSK"),
-		},
-	},
-	{
-		.callback = battery_full_discharging_quirk,
-		.ident = "ASUS UX305LA",
-		.matches = {
-			DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
-			DMI_MATCH(DMI_PRODUCT_NAME, "UX305LA"),
-		},
-	},
-	{
-		.callback = battery_full_discharging_quirk,
-		.ident = "ASUS UX360UA",
-		.matches = {
-			DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
-			DMI_MATCH(DMI_PRODUCT_NAME, "UX360UA"),
-		},
-	},
-	{
-		.callback = battery_full_discharging_quirk,
-		.ident = "ASUS UX410UAK",
-		.matches = {
-			DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
-			DMI_MATCH(DMI_PRODUCT_NAME, "UX410UAK"),
-		},
-	},
 	{},
 };