linux-kernel - Re: [PATCH v1] battery: Add the ThinkPad battery status quirk

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <3A23A2DC-121E-4DA6-863D-EFEAF420A5FB@gmail.com>
Date:   Tue, 10 Apr 2018 16:17:13 +0200
From:   Ognjen Galic <smclt30p@...il.com>
To:     Sebastian Reichel <sre@...nel.org>
CC:     "Rafael J. Wysocki" <rjw@...ysocki.net>,
        Len Brown <lenb@...nel.org>, linux-acpi@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1] battery: Add the ThinkPad battery status quirk



On April 10, 2018 12:58:41 PM GMT+02:00, Sebastian Reichel <sre@...nel.org> wrote:
>Hi,
>
>On Tue, Apr 10, 2018 at 11:06:43AM +0200, Ognjen Galic 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 fix the "Unknown" state.
>> 
>> The chosen new state when the battery is not charging is "Fully
>Charged"
>> because a status of "Not Charging" breaks almost all applications in
>> userspace that depend on upower, because upower in itself can't
>handle
>> a state of "Not Charging", defaulting to "Unknown".
>> 
>> When we report "Fully Charged" instead of "Unknown", the userspace
>handles
>> everything just fine, displaying for example: "Fully Charged, 59%" in
>Xfce.
>
>This probably breaks dual-battery Thinkpads, that charges batteries
>alternating (so one of them is "not charging" without being "fully
>charged"). The correct fix is to teach upower the "not charging"
>state. It exists since the existance of the power supply kernel API,
>so missing support is a bug in UPower.

I did write a patch for upower, submitted it and tested it. The problem is the bug report has been open for a month now and nothing major is happening. And the userspace is still broken.


>
>-- Sebastian
>
>> This is a re-write of: https://patchwork.kernel.org/patch/10205359/
>> 
>> Signed-off-by: Ognjen Galic <smclt30p@...il.com>
>> ---
>>  drivers/acpi/battery.c | 19 ++++++++++++++++++-
>>  1 file changed, 18 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
>> index bdb24d63..cb4457a7 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_thinkpad_full;
>>  static unsigned int cache_time = 1000;
>>  module_param(cache_time, uint, 0644);
>>  MODULE_PARM_DESC(cache_time, "cache time in milliseconds");
>> @@ -233,7 +234,8 @@ static int acpi_battery_get_property(struct
>power_supply *psy,
>>  			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))
>> +		else if (acpi_battery_is_charged(battery) ||
>> +				battery_quirk_thinkpad_full)
>>  			val->intval = POWER_SUPPLY_STATUS_FULL;
>>  		else
>>  			val->intval = POWER_SUPPLY_STATUS_UNKNOWN;
>> @@ -1332,6 +1334,13 @@ battery_notification_delay_quirk(const struct
>dmi_system_id *d)
>>  	return 0;
>>  }
>>  
>> +static int __init
>> +battery_thinkpad_full_charged_quirk(const struct dmi_system_id *d)
>> +{
>> +	battery_quirk_thinkpad_full = 1;
>> +	return 0;
>> +}
>> +
>>  static const struct dmi_system_id bat_dmi_table[] __initconst = {
>>  	{
>>  		.callback = battery_bix_broken_package_quirk,
>> @@ -1349,6 +1358,14 @@ static const struct dmi_system_id
>bat_dmi_table[] __initconst = {
>>  			DMI_MATCH(DMI_PRODUCT_NAME, "Aspire V5-573G"),
>>  		},
>>  	},
>> +	{
>> +		.callback = battery_thinkpad_full_charged_quirk,
>> +		.ident = "Lenovo ThinkPad",
>> +		.matches = {
>> +			DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
>> +			DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad"),
>> +		},
>> +	},
>>  	{},
>>  };
>>  
>> -- 
>> 2.15.1
>> 

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.