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

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

 



Hi Hans,

from a userspace perspective, I don't think it's optimal to combine the
two features and the status. For example, how do I find out which one is
available?

I have to test the writability of status and then still don't know which
one is available. Seeing if force_discharge or inhibit_charge are there
is much simpler.

And then enabling that: triggering force_discharge by writing
"Discharging" is ok. But for inhibit_charge we would need a new status,
something like "Charging inhibited". This then causes problems for the
existing userspace, says: upowerd could not handle it. You remember the
"Not charging" patch from Ognen?


--

Freundliche Grüße / Kind regards,

Thomas Koch



Mail : linrunner@xxxxxxx

Web  : https://linrunner.de/tlp


On 13.04.21 10:05, Hans de Goede wrote:
Hi,

On 4/9/21 8:33 PM, Thomas Koch wrote:
Hi,

On 08.04.21 15:51, Sebastian Reichel wrote:

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.

There are ThinkPads with two batteries (BAT0, BAT1) and the hardware
allows to select which one to discharge. An approach through
/sys/class/power_supply/AC/status won't cover this.

The mentioned status strings come from /sys/class/power_supply/VAT#/status,
rather then from /sys/class/power_supply/AC/status.

There is one problem though, which is that the status attribute is being
managed by drivers/acpi/battery.c. There is infra for a driver like
the thinkpad_apci driver to add new attributes to a power_supply but
AFAIK there is no infra to say intercept writes to an attribute where
the reading is handled by another driver.

I guess we could add some special hook to allow another driver to
intercept status writes.

Sebastian, what is your take on this ?

Regards,

Hans





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

  Powered by Linux