| Message ID | 20180314084217.9776-1-drake@endlessm.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Delegated to: | Rafael Wysocki |
| Headers | show |
Daniel Drake <drake@endlessm.com> 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@endlessm.com> Since Asus clarified it's intentional, Acked-by: Kai-Heng Feng <kai.heng.feng@canonical.com> > --- > 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Mar 14, 2018 at 4:51 PM, Kai Heng Feng <kai.heng.feng@canonical.com> wrote: > So how does Windows handle this? Does Asus write their own battery driver? Asus does have some windows app that interacts with ACPI/WMI/etc but I don't know if it does anything related to the battery icon. Windows will have the same set of information available: - battery charge amount is full - external power is connected - battery is discharging And it may just be the case that the standard Windows logic to interpret that data results in showing the "battery full, power connected" icon. And GNOME/upower/whatever has different logic and the resulting interpretation makes it choose the "discharging" icon. > 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? Haven't had time yet, so it would be great if you can do this. Thanks Daniel -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wednesday, March 14, 2018 9:42:17 AM CET Daniel Drake 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. > > 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> > --- > 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"), > - }, > - }, > {}, > }; > > Applied, thanks! -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Daniel, I was actually somewhat happy to see the original patch which this reverts as I'm seeing similar problems on various devices. With that said I agree with you that a dmi based quirk is not the solution because that will turn in a game of whack-a-mole. On 14-03-18 09:42, Daniel Drake 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, That is not true, at least not for the troublesome devices which I have. Here is what I see happening: a) The battery code in the ACPI tables is buggy and first checks the charging status bits of the charger-IC, and if those report not charging it will report discharging, without looking at the presence of AC power or at the battery dis(charge) current from the fuel-gauge. b) The bug reproduces after doing the following: 1) Plug in charger while battery is say half full, battery starts charging, charging state bits indicate: pre-charge or fast-charge, ACPI reported battery status is ok 2) When fully charged charging state bits indicate: end-of-charge, ACPI reported battery status is ok 3) unplug the charger, wait 1 minute, replug. Now the battery voltage is still above the start-charging threshold, so the charger will not start charging to avoid wrecking the battery by repeatedly recharging the last 1% capacity. The charger IC charging state bits now are all 0 ("not charging") and the broken ACPI code translate this to "discharging". So we end up in a state where the ACPI code reports "discharging" where it should be reporting "not charging", which is a huge difference and indeed confuses users. TL;DR: the reported information is actually incorrect. Note I'm seeing this on various Asus models: T100TA, T100CHI (same generation) and on the T100HA (newer generation) too. Note the reverted patch checked for current_now == 0, so it would only report FULL if now current was being drawn from the battery, which is what the ACPI code really should be doing (combined with checking for AC presence). > and the quirk approach > taken is inadequate and more thought is needed first. Specifically: > > 1. It only quirks discharging state, not charging See above, the problem is falsely reporting discharging, not falsely reporting 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. I agree we need a generic solution rather then relying on quirks. > 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). Pardon my French: but this is bullshit, the behavior I'm describing can be observed every single charge cycle. I've written multiple full-gauge and charger-ic drivers by now and none actively discharge the battery. What they do is they do not *start* *charging* the battery when it is above a certain threshold, so they charge to 100%, and if you then disconnect it only shortly, so it drops to 99% and then plug in again they do not start a new charge cycle. That is a very different thing from actively discharging the battery and reporting this "not charging" state as discharging to the user will make the user think his adapter/power-brick is broken or not properly plugged in. > 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. Any decent consumer product will have a start-charging threshold below which the charge needs to drop before a new charge cycle is started yes. Active discharging is typically only done by things like smart battery-chargers where you remove the battery-cells from the device using them and directly plug them (e.g. an AA battery) into the charger and even then you typically need to explicitly enable this option in their menus before starting the charge cycle. Devices with sealed in batteries (or phones or laptops with custom battery packs) doing active discharging is something I've never heard of. > 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. Accept that as the current_now == 0 check triggering shows (otherwise the reverted patch would not work) there is no discharging happening. So Asus is simply not telling the entire story here and their ACPI implementation really is buggy. > 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. I disagree, upower which most desktop-environments use to get battery status already has heuristics to deal with the kernel reporting an "unknown" status. The "unknown" status e.g. happens on the T200TA when it gets into a "not charging" state which correctly sets neither the "discharging" nor the "charging" bits in the battery state field when it is on AC power and the battery charge was above the start threshold when the AC adapter got plugged in. The kernel ends up reporting "unknown" in this case due to the ACPI battery spec not having a "not charging" state bit. This is the behavior which I would expect on the other Asus devices which I have too, but their ACPI code is broken. When the kernel reports a state other then "unknown" upower trusts the kernel and that seems like a reasonable thing to do. The kernel is there to abstract hardware. In the past (e.g. with wireless drivers) we have always told the userspace folks that they should not do kludges to work around device specific quirks, but rather that they should file a bug with the kernel and get things fixed there. This IMHO is the same. TL;DR: the kernel reporting a status of "discharging" when the battery is not discharging is a bug (caused by broken ACPI tables) and something which we need to work around at the component which is interpreting those ACPI tables, which is the kernel. ### So with the above hopefully explaining why I believe that we do need a kernel level fix for this, lets discuss how we can fix this, here is what I propose: 1) Add a new acpi_battery_handle_discharging_state() helper which gets called when ACPI_BATTERY_STATE_DISCHARGING is set instead of always reporting POWER_SUPPLY_STATUS_DISCHARGING in this case 2) In this new helper do the following: /* * Some devices wrongly report discharging if the battery's charge level * was above the device's start charging threshold atm the AC adapter * was plugged in and the device thus did not start a new charge cycle. */ if (power_supply_is_system_supplied() && battery->rate_now == 0) { /* * 20180409: Ideally we would use STATUS_NOT_CHARGING here, but * that trips a bug in userspace, so for now we use FULL, see: * https://bugs.freedesktop.org/attachment.cgi?id=138070 * Note that the fix for that bug translates not-charging to * full, so it does not matter much anyways. */ val->intval = POWER_SUPPLY_STATUS_FULL; } else { val->intval = POWER_SUPPLY_STATUS_DISCHARGING; } So what do you think about this as a solution? Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Apr 9, 2018 at 4:03 AM, Hans de Goede <hdegoede@redhat.com> wrote: > Pardon my French: but this is bullshit, the behavior I'm describing can be > observed every single charge cycle. I've written multiple full-gauge and > charger-ic drivers by now and none actively discharge the battery. > > What they do is they do not *start* *charging* the battery when it is above > a certain threshold, so they charge to 100%, and if you then disconnect it > only shortly, so it drops to 99% and then plug in again they do not start > a new charge cycle. That is a very different thing from actively discharging > the battery and reporting this "not charging" state as discharging to the > user will make the user think his adapter/power-brick is broken or not > properly plugged in. I was going mostly on the communications from Asus here - and extrapolating since my UPS is also documented to have discharge cycle behaviour - but I guess I was wrong about "decent consumer products" in general. Thanks for sharing your experience. > So Asus is simply not telling the entire story here and their ACPI > implementation really is buggy. If you have time to write up your analysis against the AML (and any suggested AML changes), I will pass it on to Asus BIOS team in hope that they fix future products. I agree with the approach you are taking in your patches. Thanks for working on this! Daniel -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On 11-04-18 18:28, Daniel Drake wrote: > On Mon, Apr 9, 2018 at 4:03 AM, Hans de Goede <hdegoede@redhat.com> wrote: >> Pardon my French: but this is bullshit, the behavior I'm describing can be >> observed every single charge cycle. I've written multiple full-gauge and >> charger-ic drivers by now and none actively discharge the battery. >> >> What they do is they do not *start* *charging* the battery when it is above >> a certain threshold, so they charge to 100%, and if you then disconnect it >> only shortly, so it drops to 99% and then plug in again they do not start >> a new charge cycle. That is a very different thing from actively discharging >> the battery and reporting this "not charging" state as discharging to the >> user will make the user think his adapter/power-brick is broken or not >> properly plugged in. > > I was going mostly on the communications from Asus here - and > extrapolating since my UPS is also documented to have discharge cycle > behaviour Ah yes I guess UPS-es do have a discharge cycle, as they are always on mains. Laptops are expected to get their discharge from not being plugged in all the time. I know some user do plug them in almost all of the time, which is where the threshold to start a new charge cycle comes in. > - but I guess I was wrong about "decent consumer products" > in general. Thanks for sharing your experience. You're welcome. >> So Asus is simply not telling the entire story here and their ACPI >> implementation really is buggy. > > If you have time to write up your analysis against the AML (and any > suggested AML changes), I will pass it on to Asus BIOS team in hope > that they fix future products. It is sorta hard to analyze a specific AML for this without knowing which brand + model charger-IC and fuel-gauge-IC are used. I think it is best to give them my generic description. What we want from them is to NOT set the discharge bit in the ACPI battery's state field when no discharging is happening (as can be observed by the (dis)charge rate being 0). Basically the problem is "not charging" != "discharging"... > I agree with the approach you are taking in your patches. Thanks for > working on this! You're welcome as I mentioned I've had this on my radar for a while already, so when I first saw Kai-Heng's fix coming in and then you reverting it, it seemed like time to work on this now rather then later. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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"), - }, - }, {}, };
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. 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> --- drivers/acpi/battery.c | 48 +++--------------------------------------------- 1 file changed, 3 insertions(+), 45 deletions(-)