To validate the special formatting of the "charge_behaviour" sysfs
property add it to the example driver.
Signed-off-by: Thomas Weißschuh <[email protected]>
---
The original submission of the charge_behaviour property did not
implement proper formatting in the default formatting handler in
power_supply_sysfs.c.
At that time it was not a problem because the only provider of the UAPI,
thinkpad_acpi, did its own formatting.
Now there is an in-tree driver, mm8013, and out-of-tree driver which use
the normal power-supply properties and are affected by the wrong
formatting.
In this revision the handling of CHARGE_BEHAVIOUR in mm8013 is dropped
as it is not the correct API for it to use.
That change was not tested by me as I don't have the hardware.
---
Changes in v3:
- Drop already applied patches
- Validate value in test_power_set_battery_property
- Link to v2: https://lore.kernel.org/r/20240303-power_supply-charge_behaviour_prop-v2-0-8ebb0a7c2409@weissschuh.net
Changes in v2:
- Simplify the backwards-compatibility logic (adds a preparatory patch)
- Extend test-power to also handle writing of charge_behaviour
- Drop incorrect CHARGE_BEHAVIOUR from mm8013 driver
- Replace special CHARGE_BEHAVIOUR_AVAILABLE property with bitmask in
struct power_supply_desc
- Link to v1: https://lore.kernel.org/r/20240204-power_supply-charge_behaviour_prop-v1-0-06a20c958f96@weissschuh.net
---
drivers/power/supply/test_power.c | 36 ++++++++++++++++++++++++++++++++++++
1 file changed, 36 insertions(+)
diff --git a/drivers/power/supply/test_power.c b/drivers/power/supply/test_power.c
index 0d0a77584c5d..442ceb7795e1 100644
--- a/drivers/power/supply/test_power.c
+++ b/drivers/power/supply/test_power.c
@@ -35,6 +35,8 @@ static int battery_capacity = 50;
static int battery_voltage = 3300;
static int battery_charge_counter = -1000;
static int battery_current = -1600;
+static enum power_supply_charge_behaviour battery_charge_behaviour =
+ POWER_SUPPLY_CHARGE_BEHAVIOUR_AUTO;
static bool module_initialized;
@@ -123,6 +125,9 @@ static int test_power_get_battery_property(struct power_supply *psy,
case POWER_SUPPLY_PROP_CURRENT_NOW:
val->intval = battery_current;
break;
+ case POWER_SUPPLY_PROP_CHARGE_BEHAVIOUR:
+ val->intval = battery_charge_behaviour;
+ break;
default:
pr_info("%s: some properties deliberately report errors.\n",
__func__);
@@ -131,6 +136,31 @@ static int test_power_get_battery_property(struct power_supply *psy,
return 0;
}
+static int test_power_battery_property_is_writeable(struct power_supply *psy,
+ enum power_supply_property psp)
+{
+ return psp == POWER_SUPPLY_PROP_CHARGE_BEHAVIOUR;
+}
+
+static int test_power_set_battery_property(struct power_supply *psy,
+ enum power_supply_property psp,
+ const union power_supply_propval *val)
+{
+ switch (psp) {
+ case POWER_SUPPLY_PROP_CHARGE_BEHAVIOUR:
+ if (val->intval < 0 ||
+ val->intval >= BITS_PER_TYPE(typeof(psy->desc->charge_behaviours)) ||
+ !(BIT(val->intval) & psy->desc->charge_behaviours)) {
+ return -EINVAL;
+ }
+ battery_charge_behaviour = val->intval;
+ break;
+ default:
+ return -EINVAL;
+ }
+ return 0;
+}
+
static enum power_supply_property test_power_ac_props[] = {
POWER_SUPPLY_PROP_ONLINE,
};
@@ -156,6 +186,7 @@ static enum power_supply_property test_power_battery_props[] = {
POWER_SUPPLY_PROP_VOLTAGE_NOW,
POWER_SUPPLY_PROP_CURRENT_AVG,
POWER_SUPPLY_PROP_CURRENT_NOW,
+ POWER_SUPPLY_PROP_CHARGE_BEHAVIOUR,
};
static char *test_power_ac_supplied_to[] = {
@@ -178,6 +209,11 @@ static const struct power_supply_desc test_power_desc[] = {
.properties = test_power_battery_props,
.num_properties = ARRAY_SIZE(test_power_battery_props),
.get_property = test_power_get_battery_property,
+ .set_property = test_power_set_battery_property,
+ .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_FORCE_DISCHARGE),
},
[TEST_USB] = {
.name = "test_usb",
---
base-commit: 4e61f1e9d58fb0765f59f47d4d1f318b36c14d95
change-id: 20230929-power_supply-charge_behaviour_prop-10ccfd96a666
Best regards,
--
Thomas Weißschuh <[email protected]>
On Wed, 06 Mar 2024 20:37:04 +0100, Thomas Weißschuh wrote:
> To validate the special formatting of the "charge_behaviour" sysfs
> property add it to the example driver.
>
>
Applied, thanks!
[1/1] power: supply: test-power: implement charge_behaviour property
commit: 070c1470ae24317e7b19bd3882b300b6d69922a4
Best regards,
--
Sebastian Reichel <[email protected]>
Hi,
On 3/27/24 11:36 AM, Hans de Goede wrote:
> Hi Sebastian,
>
> On 3/26/24 8:50 PM, Sebastian Reichel wrote:
>>
>> On Wed, 06 Mar 2024 20:37:04 +0100, Thomas Weißschuh wrote:
>>> To validate the special formatting of the "charge_behaviour" sysfs
>>> property add it to the example driver.
>>>
>>>
>>
>> Applied, thanks!
>>
>> [1/1] power: supply: test-power: implement charge_behaviour property
>> commit: 070c1470ae24317e7b19bd3882b300b6d69922a4
>
> Does this mean that you've also applied patches 1-3 of:
> "[PATCH v2 0/4] power: supply: core: align charge_behaviour format with docs" ?
>
> Because this is a new version of 4/4 of that series and I think
> that the new test may depend on the fixes from patches 1-3
> of that series (which I'm reviewing now).
Ok, I have some not entirely trivial comments on patch 3/4 of that series.
I guess you (Sebastian) could address those while merging, or wait for
a v3 of the series.
Regards,
Hans
Hi Sebastian,
On 3/26/24 8:50 PM, Sebastian Reichel wrote:
>
> On Wed, 06 Mar 2024 20:37:04 +0100, Thomas Weißschuh wrote:
>> To validate the special formatting of the "charge_behaviour" sysfs
>> property add it to the example driver.
>>
>>
>
> Applied, thanks!
>
> [1/1] power: supply: test-power: implement charge_behaviour property
> commit: 070c1470ae24317e7b19bd3882b300b6d69922a4
Does this mean that you've also applied patches 1-3 of:
"[PATCH v2 0/4] power: supply: core: align charge_behaviour format with docs" ?
Because this is a new version of 4/4 of that series and I think
that the new test may depend on the fixes from patches 1-3
of that series (which I'm reviewing now).
Regards,
Hans
Hello Hans,
On Wed, Mar 27, 2024 at 11:44:41AM +0100, Hans de Goede wrote:
> On 3/27/24 11:36 AM, Hans de Goede wrote:
> > On 3/26/24 8:50 PM, Sebastian Reichel wrote:
> >> On Wed, 06 Mar 2024 20:37:04 +0100, Thomas Wei?schuh wrote:
> >>> To validate the special formatting of the "charge_behaviour" sysfs
> >>> property add it to the example driver.
> >>
> >> Applied, thanks!
> >>
> >> [1/1] power: supply: test-power: implement charge_behaviour property
> >> commit: 070c1470ae24317e7b19bd3882b300b6d69922a4
> >
> > Does this mean that you've also applied patches 1-3 of:
> > "[PATCH v2 0/4] power: supply: core: align charge_behaviour format with docs" ?
> >
> > Because this is a new version of 4/4 of that series and I think
> > that the new test may depend on the fixes from patches 1-3
> > of that series (which I'm reviewing now).
>
> Ok, I have some not entirely trivial comments on patch 3/4 of that series.
> I guess you (Sebastian) could address those while merging, or wait for
> a v3 of the series.
I can't. Patches 1-3 are already in 6.9-rc1. It looks you did not
get my replies, but they certainly have been captured by lore and
obviously Thomas got them since he send a v3 with just the last
patch:
https://lore.kernel.org/all/20240303-power_supply-charge_behaviour_prop-v2-0-8ebb0a7c2409@weissschuh.net/
Anyways, I think your suggestions for further simplifications in
patch 3 are sensible. They just require doing an extra patch now
instead of being squashed.
Greetings,
-- Sebastian
Hi,
On 3/27/24 2:25 PM, Sebastian Reichel wrote:
> Hello Hans,
>
> On Wed, Mar 27, 2024 at 11:44:41AM +0100, Hans de Goede wrote:
>> On 3/27/24 11:36 AM, Hans de Goede wrote:
>>> On 3/26/24 8:50 PM, Sebastian Reichel wrote:
>>>> On Wed, 06 Mar 2024 20:37:04 +0100, Thomas Weißschuh wrote:
>>>>> To validate the special formatting of the "charge_behaviour" sysfs
>>>>> property add it to the example driver.
>>>>
>>>> Applied, thanks!
>>>>
>>>> [1/1] power: supply: test-power: implement charge_behaviour property
>>>> commit: 070c1470ae24317e7b19bd3882b300b6d69922a4
>>>
>>> Does this mean that you've also applied patches 1-3 of:
>>> "[PATCH v2 0/4] power: supply: core: align charge_behaviour format with docs" ?
>>>
>>> Because this is a new version of 4/4 of that series and I think
>>> that the new test may depend on the fixes from patches 1-3
>>> of that series (which I'm reviewing now).
>>
>> Ok, I have some not entirely trivial comments on patch 3/4 of that series.
>> I guess you (Sebastian) could address those while merging, or wait for
>> a v3 of the series.
>
> I can't. Patches 1-3 are already in 6.9-rc1. It looks you did not
> get my replies, but they certainly have been captured by lore and
> obviously Thomas got them since he send a v3 with just the last
> patch:
>
> https://lore.kernel.org/all/20240303-power_supply-charge_behaviour_prop-v2-0-8ebb0a7c2409@weissschuh.net/
>
> Anyways, I think your suggestions for further simplifications in
> patch 3 are sensible. They just require doing an extra patch now
> instead of being squashed.
Ah I see that is fine too :)
Thomas, can you do a follow-up patch with the simplifications
which I suggested in my review of patch v2 3/4 ?
Regards,
Hans
Hi,
On 2024-03-27 14:34:00+0100, Hans de Goede wrote:
> On 3/27/24 2:25 PM, Sebastian Reichel wrote:
> > Hello Hans,
> >
> > On Wed, Mar 27, 2024 at 11:44:41AM +0100, Hans de Goede wrote:
> >> On 3/27/24 11:36 AM, Hans de Goede wrote:
> >>> On 3/26/24 8:50 PM, Sebastian Reichel wrote:
> >>>> On Wed, 06 Mar 2024 20:37:04 +0100, Thomas Weißschuh wrote:
> >>>>> To validate the special formatting of the "charge_behaviour" sysfs
> >>>>> property add it to the example driver.
> >>>>
> >>>> Applied, thanks!
> >>>>
> >>>> [1/1] power: supply: test-power: implement charge_behaviour property
> >>>> commit: 070c1470ae24317e7b19bd3882b300b6d69922a4
> >>>
> >>> Does this mean that you've also applied patches 1-3 of:
> >>> "[PATCH v2 0/4] power: supply: core: align charge_behaviour format with docs" ?
> >>>
> >>> Because this is a new version of 4/4 of that series and I think
> >>> that the new test may depend on the fixes from patches 1-3
> >>> of that series (which I'm reviewing now).
> >>
> >> Ok, I have some not entirely trivial comments on patch 3/4 of that series.
> >> I guess you (Sebastian) could address those while merging, or wait for
> >> a v3 of the series.
> >
> > I can't. Patches 1-3 are already in 6.9-rc1. It looks you did not
> > get my replies, but they certainly have been captured by lore and
> > obviously Thomas got them since he send a v3 with just the last
> > patch:
> >
> > https://lore.kernel.org/all/20240303-power_supply-charge_behaviour_prop-v2-0-8ebb0a7c2409@weissschuh.net/
> >
> > Anyways, I think your suggestions for further simplifications in
> > patch 3 are sensible. They just require doing an extra patch now
> > instead of being squashed.
>
> Ah I see that is fine too :)
>
> Thomas, can you do a follow-up patch with the simplifications
> which I suggested in my review of patch v2 3/4 ?
Will do!
Thomas
Hi,
On Wed, Mar 27, 2024 at 07:30:03PM +0100, Thomas Wei?schuh wrote:
> On 2024-03-27 14:34:00+0100, Hans de Goede wrote:
> > Thomas, can you do a follow-up patch with the simplifications
> > which I suggested in my review of patch v2 3/4?
>
> Will do!
Thanks for taking care of it!
-- Sebastian