Re: [PATCH v4 05/13] power: supply: add inhibit-charge-s0 to charge_behaviour

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

Hi Antheas,

Thanks you for your work on this.

On 16-Mar-25 17:46, Antheas Kapenekakis wrote:
> On Sun, 16 Mar 2025 at 14:56, Guenter Roeck <linux@roeck-us.net> wrote:
>>
>> On 3/16/25 04:40, Antheas Kapenekakis wrote:
>>> On Tue, 11 Mar 2025 at 17:54, Antheas Kapenekakis <lkml@antheas.dev> wrote:
>>>>
>>>> OneXPlayer devices have a charge bypass 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
>>>> in the s0 state.
>>>>
>>>> Reviewed-by: Derek J. Clark <derekjohn.clark@gmail.com>
>>>> Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
>>>> ---
>>>>   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
>>>> +                       force-discharge:   Force discharge while AC is attached
>>>> +                       ================== =====================================
>>>>
>>>>   What:          /sys/class/power_supply/<supply_name>/technology
>>>>   Date:          May 2007
>>>> diff --git a/drivers/power/supply/power_supply_sysfs.c b/drivers/power/supply/power_supply_sysfs.c
>>>> index edb058c19c9c..1a98fc26ce96 100644
>>>> --- a/drivers/power/supply/power_supply_sysfs.c
>>>> +++ b/drivers/power/supply/power_supply_sysfs.c
>>>> @@ -140,6 +140,7 @@ static const char * const POWER_SUPPLY_SCOPE_TEXT[] = {
>>>>   static const char * const POWER_SUPPLY_CHARGE_BEHAVIOUR_TEXT[] = {
>>>>          [POWER_SUPPLY_CHARGE_BEHAVIOUR_AUTO]            = "auto",
>>>>          [POWER_SUPPLY_CHARGE_BEHAVIOUR_INHIBIT_CHARGE]  = "inhibit-charge",
>>>> +       [POWER_SUPPLY_CHARGE_BEHAVIOUR_INHIBIT_CHARGE_S0]       = "inhibit-charge-s0",
>>>>          [POWER_SUPPLY_CHARGE_BEHAVIOUR_FORCE_DISCHARGE] = "force-discharge",
>>>>   };
>>>>
>>>> diff --git a/drivers/power/supply/test_power.c b/drivers/power/supply/test_power.c
>>>> index 2a975a110f48..4bc5ab84a9d6 100644
>>>> --- a/drivers/power/supply/test_power.c
>>>> +++ b/drivers/power/supply/test_power.c
>>>> @@ -214,6 +214,7 @@ static const struct power_supply_desc test_power_desc[] = {
>>>>                  .property_is_writeable = test_power_battery_property_is_writeable,
>>>>                  .charge_behaviours = BIT(POWER_SUPPLY_CHARGE_BEHAVIOUR_AUTO)
>>>>                                     | BIT(POWER_SUPPLY_CHARGE_BEHAVIOUR_INHIBIT_CHARGE)
>>>> +                                  | BIT(POWER_SUPPLY_CHARGE_BEHAVIOUR_INHIBIT_CHARGE_S0)
>>>>                                     | BIT(POWER_SUPPLY_CHARGE_BEHAVIOUR_FORCE_DISCHARGE),
>>>>          },
>>>>          [TEST_USB] = {
>>>> diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
>>>> index 6ed53b292162..b1ca5e148759 100644
>>>> --- a/include/linux/power_supply.h
>>>> +++ b/include/linux/power_supply.h
>>>> @@ -212,6 +212,7 @@ enum power_supply_usb_type {
>>>>   enum power_supply_charge_behaviour {
>>>>          POWER_SUPPLY_CHARGE_BEHAVIOUR_AUTO = 0,
>>>>          POWER_SUPPLY_CHARGE_BEHAVIOUR_INHIBIT_CHARGE,
>>>> +       POWER_SUPPLY_CHARGE_BEHAVIOUR_INHIBIT_CHARGE_S0,
>>>>          POWER_SUPPLY_CHARGE_BEHAVIOUR_FORCE_DISCHARGE,
>>>>   };
>>>>
>>>> --
>>>> 2.48.1
>>>>
>>>
>>> Hi Guenter,
>>> I think I need an ack here, and then someone from platform-x86 to
>>> triage the series.
>>>
>>> Do I need to cc anyone extra?
>>>
>>
>> You need to cc the maintainers of affected subsystems. Copying the mailing
>> list is insufficient.
>>
>> Guenter
>>
> 
> Can you tell me who to cc from platform-x86 and linux-pm?

Sebastian Reichel <sre@kernel.org> (maintainer:POWER SUPPLY CLASS/SUBSYSTEM and DRIVERS)
linux-pm@vger.kernel.org (open list:POWER SUPPLY CLASS/SUBSYSTEM and DRIVERS)
linux-kernel@vger.kernel.org (open list)

And something which the tools will not tell you, since charge_behaviour support
was added by Thomas Thomas should be added to the Cc too:

Thomas Weißschuh <linux@weissschuh.net>

Note I also have some generic remarks about this patch, I'll do a top-level
reply to the patch for those.

Regards,

Hans