On Wed, 7 Aug 2024 23:44:13 +0200 Pali Rohár <pali@xxxxxxxxxx> wrote: > On Wednesday 07 August 2024 17:28:32 Andres Salomon wrote: > > On Fri, 26 Jul 2024 20:46:22 +0200 > > Armin Wolf <W_Armin@xxxxxx> wrote: > > > > > 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@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 > > > > > > > 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. > > > > Do you think it's worth keeping around the sysfs-class-power-dell > > file? At this point it's basically just documenting the slight naming > > differences: > > > > > > Standard: > > Fully charge the battery at a moderate rate. > > Fast: > > Quickly charge the battery using fast-charge > > technology. This is harder on the battery than > > standard charging and may lower its lifespan. > > The Dell BIOS calls this ExpressCharge™. > > Trickle: > > Users who primarily operate the system while > > plugged into an external power source can extend > > battery life with this mode. The Dell BIOS calls > > this "Primarily AC Use". > > Adaptive: > > Automatically optimize battery charge rate based > > on typical usage pattern. > > Custom: > > Use the charge_control_* properties to determine > > when to start and stop charging. Advanced users > > can use this to drastically extend battery life. > > > > Access: Read, Write > > Valid values: > > "Standard", "Fast", "Trickle", > > "Adaptive", "Custom" > > Another option could be to extend the description in sysfs-class-power > file that "Trickle" covers "Dell's Primarily AC Use" and "Fast" covers > "Dell's ExpressCharge". > > So if somebody is going to implement charge_type for some other Laptop > vendor then this information can help. How's this? @@ -378,8 +378,10 @@ Date: July 2009 Contact: linux-pm@xxxxxxxxxxxxxxx Description: Represents the type of charging currently being applied to the - battery. "Trickle", "Fast", and "Standard" all mean different - charging speeds. "Adaptive" means that the charger uses some + battery. "Fast" and "Standard" mean different charging speeds. + "Trickle" means a slow charging speed, or (depending on the + hardware) charging optimized for being mostly plugged into + wall power. "Adaptive" means that the charger uses some algorithm to adjust the charge rate dynamically, without any user configuration required. "Custom" means that the charger -- I'm available for contract & employment work, please contact me if interested.