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

[Hans de Goede <hdegoede@redhat.com>]

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@vger.kernel.org
>> 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