| Message ID | 20250311165406.331046-6-lkml@antheas.dev |
|---|---|
| State | Superseded |
| Headers | show |
| Series | hwmon: (oxpsensors) Add devices, features, fix ABI and move to platform/x86 | expand |
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? Thanks, Antheas
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
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? Is it Armin and Rafael? Best, Antheas
On Sun, 16 Mar 2025 at 17:50, Derek John Clark <derekjohn.clark@gmail.com> wrote: > > > > On Sun, Mar 16, 2025, 09:47 Antheas Kapenekakis <lkml@antheas.dev> 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? >> >> Is it Armin and Rafael? > > > Hans, Ilpo, Armin, and Mario > > - Derek Sure. Full series on [1]. Antheas btw, your reply was non-text. [1] https://lore.kernel.org/all/20250311165406.331046-1-lkml@antheas.dev/
On 3/16/25 09:46, Antheas Kapenekakis wrote: [ ... ] >>> 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? > Just use scripts/get_maintainer.pl. Guenter
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
Hi, On 17-Mar-25 14:50, Antheas Kapenekakis wrote: > On Mon, 17 Mar 2025 at 14:31, Hans de Goede <hdegoede@redhat.com> wrote: >> >> Hi, >> >> On 17-Mar-25 13:38, Antheas Kapenekakis wrote: >>> On Mon, 17 Mar 2025 at 13:27, Hans de Goede <hdegoede@redhat.com> 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
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, };