On Mon, 17 Mar 2025 at 13:27, Hans de Goede <hdegoede@xxxxxxxxxx> wrote: > > Hi Antheas, > > On 11-Mar-25 17:53, Antheas Kapenekakis wrote: > > OneXPlayer devices have a charge bypass > > The term "charge bypass" is typically used for the case where the > external charger gets directly connected to the battery cells, > bypassing the charge-IC inside the device, in making > the external charger directly responsible for battery/charge > management. > > Yet you name the feature inhibit charge, so I guess it simply > disables charging of the battery rather then doing an actual > chaerger-IC bypass ? > > Assuming I have this correct, please stop using the term > charge-bypass as that has a specific (different) meaning. Unfortunately, this is how the feature is called in Windows. On both OneXPlayer and Ayaneo. Manufacturers are centralizing around that term. Under the hood, it should be bypassing the charger circuitry, but it is not obvious during use. The user behavior mirrors `inhibit-charge`, as the battery just stops charging, so the endpoint is appropriate. The previous versions of this patch used the charge_type endpoint. However, it does not have autodetection, so it needs to be hardcoded [1]. There is also an ayaneo-platform out-of-tree driver with a PR for that endpoint, which is where I found out about it [2]. So I thought it would be more appropriate. I can rewrite the commit message to not include the word bypass. [1] https://github.com/hhd-dev/adjustor/blob/6a4a192898c4f9d8495e6554d1f0bc41fef550c7/src/adjustor/drivers/battery/__init__.py#L141-L156 [2] https://github.com/ShadowBlip/ayaneo-platform > > feature > > that allows the user to select between it being > > active always or only when the device is on. > > > > Therefore, add attribute inhibit-charge-s0 to > > charge_behaviour to allow the user to select > > that bypass should only be on when the device is > > Also don't use bypass here please. > > > in the s0 state. > > > > Reviewed-by: Derek J. Clark <derekjohn.clark@xxxxxxxxx> > > Signed-off-by: Antheas Kapenekakis <lkml@xxxxxxxxxxx> > > --- > > Documentation/ABI/testing/sysfs-class-power | 11 ++++++----- > > drivers/power/supply/power_supply_sysfs.c | 1 + > > drivers/power/supply/test_power.c | 1 + > > include/linux/power_supply.h | 1 + > > 4 files changed, 9 insertions(+), 5 deletions(-) > > > > diff --git a/Documentation/ABI/testing/sysfs-class-power b/Documentation/ABI/testing/sysfs-class-power > > index 2a5c1a09a28f..4a187ca11f92 100644 > > --- a/Documentation/ABI/testing/sysfs-class-power > > +++ b/Documentation/ABI/testing/sysfs-class-power > > @@ -508,11 +508,12 @@ Description: > > Access: Read, Write > > > > Valid values: > > - ================ ==================================== > > - auto: Charge normally, respect thresholds > > - inhibit-charge: Do not charge while AC is attached > > - force-discharge: Force discharge while AC is attached > > - ================ ==================================== > > + ================== ===================================== > > + auto: Charge normally, respect thresholds > > + inhibit-charge: Do not charge while AC is attached > > + inhibit-charge-s0: same as inhibit-charge but only in S0 > > Only in S0 suggests that charging gets disabled when the device is on / in-use, > I guess this is intended to avoid generating extra heat while the device is on? > > What about when the device is suspended, should the battery charge then ? > > On x86 we've 2 sorts of suspends S3, and the current name suggests that the > device will charge (no inhibit) then. But modern hw almost always uses > s0i3 / suspend to idle suspend and the name suggests charging would then > still be inhibited? > > Also s0 is an ACPI specific term, so basically 2 remarks here: > > 1. The name should probably be "inhibit-charge-when-on" since the power_supply > calls is platform agnositic and "S0" is not. I tried to be minimal. If we want to make the name longer, I vote for "inhibit-charge-awake". I can spin a v5 with that. The device does not charge while asleep. Only when it is off. > 2. We need to clearly define what happens when the device is suspended and then > make sure that the driver matches this (e.g. if we want to *not* inhibit during > suspend we may need to turn this feature off during suspend). This is handled by the device when it comes to OneXPlayer. No driver changes are needed. > Regards, > > Hans > > > snip > > > Best, Antheas
