Re: [PATCH 1/3] thinkpad_acpi: add support for force_discharge

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

 



Hi,

>> "inhibit_**discharge**"

>> "stops **charging** of the battery"

>> I'm wondering if it should be "inhibit_charge" or something like that?
> Text and file name also seem to have reverse meaning for me. I

> assume the text is the correct one, since it does not seem to

> make sense inhibiting discharge. That would result in instant

> poweroff on AC loss?


Fortunately that's only a typo in the docs file. The actual sysfs node
implemented by patch 2/3 is

	/sys/class/power_supply/BATx/inhibit_charge


--

Freundliche Grüße / Kind regards,

Thomas Koch



Mail : linrunner@xxxxxxx

Web  : https://linrunner.de/tlp


On 08.04.21 15:51, Sebastian Reichel wrote:
Hi,

On Wed, Apr 07, 2021 at 10:33:41AM +0000, Barnabás Pőcze wrote:
2021. április 7., szerda 12:24 keltezéssel, Hans de Goede írta:
2. If we add support for this to the kernel we should probably
first agree on standardized power-supply class property names for
these, rather then coming up with our own names. ATM we register
2 names for the charge start threshold, the one which the thinkpad_acpi
code invented and the standardized name which was later added.

I've added Sebastian, the power-supply class / driver maintainer to
the Cc. for this. Sebastian Nicolo wants to add support for 2 new
features as power-supply properties:

--- a/Documentation/admin-guide/laptops/thinkpad-acpi.rst
+++ b/Documentation/admin-guide/laptops/thinkpad-acpi.rst
...
+Battery forced discharging
+--------------------------
+
+sysfs attribute:
+/sys/class/power_supply/BATx/force_discharge
+
+Setting this attribute to 1 forces the battery to discharge while AC is attached.
+Setting it to 0 terminates forced discharging.
+
+Battery charge inhibiting
+--------------------------
+
+sysfs attribute:
+/sys/class/power_supply/BATx/inhibit_discharge
+
+Setting this attribute to 1 stops charging of the battery as a manual override
+over the threshold attributes. Setting it to 0 terminates the override.


"inhibit_**discharge**"
"stops **charging** of the battery"

I'm wondering if it should be "inhibit_charge" or something like that?

Text and file name also seem to have reverse meaning for me. I
assume the text is the correct one, since it does not seem to
make sense inhibiting discharge. That would result in instant
poweroff on AC loss?

Sebastian, I believe that this should be changes to instead be documented
in: Documentation/ABI/testing/sysfs-class-power
and besides the rename I was wondering if you have any remarks on the proposed
API before Nicolo sends out a v2 ?

IIUIC you have 'force_discharge', which basically means the system
is running from battery power despite an AC adapter being connected
and 'inhibit_discharge', which inhibits charging, so system does not
charge battery when AC is connected, but uses AC to supply itself
(so battery is idle)?

We already have this kind of features on embedded systems (which
often provide all kind of charger details). Those drivers solve
this by having a writable 'status' property in the charger device:

What:           /sys/class/power_supply/<supply_name>/status
Date:           May 2007
Contact:        linux-pm@xxxxxxxxxxxxxxx
Description:
                 Represents the charging status of the battery. Normally this
                 is read-only reporting although for some supplies this can be
                 used to enable/disable charging to the battery.

                 Access: Read, Write

                 Valid values:
                               "Unknown", "Charging", "Discharging",
                               "Not charging", "Full"

If I do not miss anything writing "Discharging" is the same as forced
discharge and "Not Charging" (AKA Idle) is the same as your inhibit feature.

-- Sebastian







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

  Powered by Linux