[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240807180511.436bc1af@5400>
Date: Wed, 7 Aug 2024 18:05:11 -0400
From: Andres Salomon <dilinger@...ued.net>
To: Pali Rohár <pali@...nel.org>
Cc: Armin Wolf <W_Armin@....de>, 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
On Wed, 7 Aug 2024 23:44:13 +0200
Pali Rohár <pali@...nel.org> 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@....de> 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@...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.
> >
> > 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@...r.kernel.org
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.
Powered by blists - more mailing lists