linux-kernel - Re: [PATCH v2 1/2] platform/x86:dell-laptop: Add knobs to change battery charge settings

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <5cfe4c42-a003-4668-8c3a-f18fb6b7fba6@gmx.de>
Date: Fri, 26 Jul 2024 20:46:22 +0200
From: Armin Wolf <W_Armin@....de>
To: Andres Salomon <dilinger@...ued.net>, Pali Rohár
 <pali@...nel.org>
Cc: linux-kernel@...r.kernel.org, platform-driver-x86@...r.kernel.org,
 Matthew Garrett <mjg59@...f.ucam.org>, Sebastian Reichel <sre@...nel.org>,
 Hans de Goede <hdegoede@...hat.com>,
 Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
 linux-pm@...r.kernel.org, Dell.Client.Kernel@...l.com
Subject: Re: [PATCH v2 1/2] platform/x86:dell-laptop: Add knobs to change
 battery charge settings

Am 26.07.24 um 20:42 schrieb Armin Wolf:

> Am 26.07.24 um 06:25 schrieb Andres Salomon:
>
>> On Fri, 26 Jul 2024 02:04:09 +0200
>> Pali Rohár <pali@...nel.org> wrote:
>>
>>> On Friday 26 July 2024 01:48:50 Armin Wolf wrote:
>>>> Am 26.07.24 um 00:15 schrieb Pali Rohár:
>>>>
>>>>> On Thursday 25 July 2024 16:24:57 Andres Salomon wrote:
>>>>>> On Thu, 25 Jul 2024 01:01:58 +0200
>>>>>> Pali Rohár <pali@...nel.org> wrote:
>>>>>>
>>>>>>> On Wednesday 24 July 2024 18:23:18 Andres Salomon wrote:
>> [...]
>>>>>>> The issue here is: how to tell kernel that the particular
>>>>>>> dell_battery_hook has to be bound with the primary battery?
>>>>>>>
>>>>>> So from userspace, we've got the expectation that multiple batteries
>>>>>> would show up as /sys/class/power_supply/BAT0,
>>>>>> /sys/class/power_supply/BAT1,
>>>>>> and so on.
>>>>> Yes, I hope so.
>>>>>
>>>>>> The current BAT0 entry shows things like 'capacity' even without
>>>>>> this
>>>>>> patch, and we're just piggybacking off of that to add charge_type
>>>>>> and
>>>>>> other entries. So there shouldn't be any confusion there, agreed?
>>>>> I have not looked at the battery_hook_register() code yet (seems
>>>>> that I
>>>>> would have to properly read it and understand it). But does it
>>>>> mean that
>>>>> battery_hook_register() is adding hook just for "BAT0"?
>>>>>
>>>>> What I mean: cannot that hook be registered to "BAT1" too? Because if
>>>>> yes then we should prevent it. Otherwise this hook which is for "Dell
>>>>> Primary Battery" could be registered also for secondary battery
>>>>> "BAT1".
>>>>> (I hope that now it is more clear what I mean).
>>>> Hi,
>>>>
>>>> the battery hook is being registered to all ACPI batteries present
>>>> on a given system,
>>>> so you need to do some manual filtering when .add_battery() is called.
>>> Ok. So it means that the filtering based on the primary battery in
>>> add_battery callback is needed.
>>>
>> Thanks for the explanations. Seems simple enough to fix that, as some of
>> the other drivers are checking battery->desc->name for "BAT0".
>>
>>
>> One thing that I keep coming back to, and was reinforced as I looked at
>> include/linux/power_supply.h; the generic power supply charge_type has
>> values that are very close to Dells, but with different names. I could
>> shoehorn them in, though, with the following mappings:
>>
>> POWER_SUPPLY_CHARGE_TYPE_FAST,         => "express" (aka ExpressCharge)
>> POWER_SUPPLY_CHARGE_TYPE_STANDARD,     => "standard"
>> POWER_SUPPLY_CHARGE_TYPE_ADAPTIVE,     => "adaptive"
>> POWER_SUPPLY_CHARGE_TYPE_CUSTOM,       => "custom"
>> POWER_SUPPLY_CHARGE_TYPE_LONGLIFE,     => "primarily_ac"
>>
>> The main difference is that Primarily AC is described and documented as
>> slightly different than Long Life, but I suspect the result is roughly
>> the same thing. And the naming "Fast" and "Long Life" wouldn't match the
>> BIOS naming of "ExpressCharge" and "Primarily AC".
>>
>> Until now I've opted to match the BIOS naming, but I'm curious what
>> others
>> think before I send V3 of the patches.
>
> I agree that POWER_SUPPLY_CHARGE_TYPE_FAST should be mapped the
> ExpressCharge,
> but i think that "primarily_ac" should become a official power supply
> charging mode.
>
> The reason is that for example the wilco-charger driver also supports
> such a charging mode
> (currently reported as POWER_SUPPLY_CHARGE_TYPE_TRICKLE) and the
> charging mode seems to be
> both sufficiently different from
> POWER_SUPPLY_CHARGE_TYPE_LONGLIFE/POWER_SUPPLY_CHARGE_TYPE_TRICKLE
> and sufficiently generic to be supported by a wide array of devices.
>
> Thanks,
> Armin Wolf
>
I just read the documentation regarding the charge_type sysfs attribute and it states that:

Trickle:
	Extends battery lifespan, intended for users who
	primarily use their Chromebook while connected to AC.

So i think that "primarily_ac" should be mapped to POWER_SUPPLY_CHARGE_TYPE_TRICKLE.

Thanks,
Armin Wolf