Re: [PATCH] Revert Asus battery_full_discharging_quirk

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Daniel Drake <drake@xxxxxxxxxxxx> wrote:

This reverts 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.

So how does Windows handle this? Does Asus write their own battery driver?


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.

Have you already contacted userspace developers or already filed a bug?
I'll make a patch for UPower if no one else is working on it.
Is there any other distro that uses non-UPower userspace daemon?


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

Signed-off-by: Daniel Drake <drake@xxxxxxxxxxxx>

Since Asus clarified it's intentional,

Acked-by: Kai-Heng Feng <kai.heng.feng@xxxxxxxxxxxxx>

---
 drivers/acpi/battery.c | 48 +++---------------------------------------------
 1 file changed, 3 insertions(+), 45 deletions(-)

diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
index 7128488a3a72..f2eb6c37ea0a 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"),
-		},
-	},
 	{},
 };

--
2.14.1


--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux