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

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

 



Am 26.07.24 um 06:25 schrieb Andres Salomon:

On Fri, 26 Jul 2024 02:04:09 +0200
Pali Rohár <pali@xxxxxxxxxx> 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@xxxxxxxxxx> 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






[Index of Archives]     [Linux Kernel Development]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux