Hi, On 17-Mar-25 14:50, Antheas Kapenekakis wrote: > On Mon, 17 Mar 2025 at 14:31, Hans de Goede <hdegoede@xxxxxxxxxx> wrote: >> >> Hi, >> >> On 17-Mar-25 13:38, Antheas Kapenekakis wrote: >>> 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. >> >> Ok, so I just did a quick duckduckgo for this and it looks like >> you are right. >> >>> Under the hood, it should be bypassing the charger circuitry, but it >>> is not obvious during use. >> >> Ack reading up on this it seems the idea is not to connect the external >> charger directly to the battery to allow fast-charging without >> the charge-IC inside the device adding heat, which is the traditional >> bypass mode. >> >> Instead the whole battery + charging-IC are cut out of the circuit >> (so bypassed) and the charger is now directly powering the device >> without the battery acting as a buffer if the power-draw superseeds >> what the external charger can deliver. >> >>> The user behavior mirrors `inhibit-charge`, >>> as the battery just stops charging, so the endpoint is appropriate. >> >> Hmm this new bypass mode indeed does seem to mirror inhibit charge >> from a user pov, but it does more. It reminds me of the battery disconnect >> option which some charge-ICs have which just puts the battery FET in >> high impedance mode effectively disconnecting the battery. Now that >> feature is intended for long term storage of devices with a builtin >> battery and it typically also immediately powers off the device ... >> >> Still I wonder if it would make sense to add a new "disconnect" >> charge_behaviour or charge_types enum value for this ? >> > > The battery is not disconnected. It still provides backup. Unplugging > the charger does not turn off the device. So it is more murky. > > From a userspace perspective it is inhibit-charge 1-1. Ok, lets go with inhibit then. >> <snip> >> >>>>> 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. >> >> Is suspend awake though ? > > Sorry I mispoke. When inhibit-charge-awake, the device only charges > while in s0i0. When inhibit-charge, it never charges. This includes > s0i3, S4, and S5. The devices that support this only support modern > standby. > > I just verified this. Ok that sounds good / sane, it likely just disables charging while in s0i0 to avoid generating extra heat while in s0i0, so inhibit-charge-awake sounds good to me. > >>>> 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. >> >> Well you say no charging is done when suspended, the question also is what >> behavior do we want here? I'm fine with the default behaviour, but a case >> could be made that charging while suspended might be desirable (dependent on >> the use case) in which case we would need to disable the inhibit when >> suspending to get the desired behavior. >> >> Also what if other firmware interfaces with a bypass^W inhibit option work >> differently and do charge during suspend ? >> >> It is important that we clearly define the expected behavior now so that >> future devices can be made to behave the same. > > Sorry I mispoke. Charging happens under modern standby under -awake. > > So -awake would mean awake (s0i0) here. > > If other devices charge during sleep and awake, another option could be added. ack, as mentioned above inhibit-charge-awake sounds good to me, thank you for clarifying. Regards, Hans
