Hi,

On Wed, Oct 06, 2021 at 05:27:22PM +0200, Hans de Goede wrote:
> On 10/6/21 4:49 PM, Thomas Weißschuh wrote:
> > On 2021-10-06T10:10+0200, Hans de Goede wrote:
> >> On 10/6/21 12:06 AM, Sebastian Reichel wrote:
> >>> On Tue, Oct 05, 2021 at 08:01:12PM +0200, Hans de Goede wrote:
> >>>> Right, force-discharge automatically implies charging is
> >>>> being inhibited, so putting this in one file makes sense.
> >>>>
> >>>> Any suggestion for the name of the file?
> >>>
> >>> Maybe like this?
> >>>
> >>> ---------------------------------------------------------------------
> >>> What: /sys/class/power_supply/<supply_name>/charge_behaviour
> >>> Date: October 2021
> >>> Contact: linux-pm@vger.kernel.org
> >>> Description:
> >>>  Configure battery behaviour when a charger is being connected.
> >>>
> >>>  Access: Read, Write
> >>>
> >>>  Valid values:
> >>>
> >>>  0: auto / no override
> >>>     When charger is connected battery should be charged
> >>>  1: force idle
> >>>     When charger is connected the battery should neither be charged
> >>>     nor discharged.
> >>>  2: force discharge
> >>>     When charger is connected the battery should be discharged
> >>>     anyways.
> >>> ---------------------------------------------------------------------
> >>
> >> That looks good to me. Although I just realized that some hw may
> >> only support 1. or 2. maybe explicitly document this and that
> >> EOPNOTSUPP will be reported when the value is not supported
> >> (vs EINVAL for plain invalid values) ?
> > 
> > Would that not force a userspace applications to offer all possibilities to
> > the user only to tell them that it's not supported?
> > If the driver knows what is supported and what not it should make this
> > discoverable without actually performing the operation.
> > 
> > Maybe something along the lines of /sys/power/mem_sleep.
> 
> Good point, but something like /sys/power/mem_sleep works
> very differently then how all the other power_supply properties work.

Actually we already use this format in power-supply for USB
types, implemented in power_supply_show_usb_type().

> In general if something is supported or not on a psy class
> device is communicated by the presence / absence of attributes.
>
> So I think we should move back to having 2 separate attributes
> for this after all; and group the 2 together in the doc and
> document that enabling (setting to 1) one of force_charge /
> inhibit_charge automatically clears the setting of the other.
> 
> Then the availability of the features can simply be probed
> by checking for the presence of the property files.

If it's two files, then somebody needs to come up with proper 
names. Things like 'force_discharge' look sensible in this context,
but on a system with two batteries (like some Thinkpads have) it
is easy to confuse with "I want to discharge this battery before
the other one (while no AC is connected)".

-- Sebastian