[PATCH 0/3] power: supply: power-supply-leds: Add activate() callback to triggers

[PATCH 0/3] power: supply: power-supply-leds: Add activate() callback to triggers

[Hans de Goede]

Hi All,

This series adds an activate callback to the power-supply LED triggers to
ensure that power-supply LEDs get the correct initial value when the LED
gets registered after the power_supply has been registered.

Patches 1-2 do some refactoring / prep work and patch 3 adds the actual
activate callback.

This series applies on top of "[PATCH v9 6/7] power: supply: power-supply-leds:
Add charging_orange_full_green trigger for RGB LED":
https://lore.kernel.org/all/20240504164105.114017-7-hdegoede@redhat.com/

Regards,

Hans


Hans de Goede (3):
  power: supply: power-supply-leds: Add
    power_supply_[un]register_led_trigger()
  power: supply: power-supply-leds: Share trig pointer for online and
    charging_full
  power: supply: power-supply-leds: Add activate() callback to triggers

 drivers/power/supply/power_supply_leds.c | 176 +++++++++++++----------
 include/linux/power_supply.h             |   9 +-
 2 files changed, 101 insertions(+), 84 deletions(-)

-- 
2.44.0

[PATCH 1/3] power: supply: power-supply-leds: Add power_supply_[un]register_led_trigger()

[Hans de Goede]

Add power_supply_[un]register_led_trigger() helper functions.

The primary goal of this is as a preparation patch for adding an activate
callback to the power-supply LED triggers to ensure that power-supply
LEDs get the correct initial value when the LED gets registered after
the power_supply has been registered (this will use the psy back pointer).

There also is quite a lot of code duplication in the existing LED trigger
registration in the form of the kasprintf() for the name-template for each
trigger + related error handling. This duplication is removed by these
new helpers.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/power/supply/power_supply_leds.c | 155 ++++++++++++-----------
 include/linux/power_supply.h             |   6 -
 2 files changed, 84 insertions(+), 77 deletions(-)

diff --git a/drivers/power/supply/power_supply_leds.c b/drivers/power/supply/power_supply_leds.c
index 73935de844d9..3e20b0f30aa3 100644
--- a/drivers/power/supply/power_supply_leds.c
+++ b/drivers/power/supply/power_supply_leds.c
@@ -19,6 +19,57 @@
 
 /* Battery specific LEDs triggers. */
 
+struct power_supply_led_trigger {
+	struct led_trigger trig;
+	struct power_supply *psy;
+};
+
+static int power_supply_register_led_trigger(struct power_supply *psy,
+					     const char *name_template,
+					     struct led_trigger **tp)
+{
+	struct power_supply_led_trigger *psy_trig;
+	int err;
+
+	psy_trig = kzalloc(sizeof(*psy_trig), GFP_KERNEL);
+	if (!psy_trig)
+		return -ENOMEM;
+
+	psy_trig->trig.name = kasprintf(GFP_KERNEL, name_template, psy->desc->name);
+	if (!psy_trig->trig.name) {
+		err = -ENOMEM;
+		goto err_free_trigger;
+	}
+
+	psy_trig->psy = psy;
+
+	err = led_trigger_register(&psy_trig->trig);
+	if (err)
+		goto err_free_name;
+
+	*tp = &psy_trig->trig;
+	return 0;
+
+err_free_name:
+	kfree(psy_trig->trig.name);
+err_free_trigger:
+	kfree(psy_trig);
+	return -ENOMEM;
+}
+
+static void power_supply_unregister_led_trigger(struct led_trigger *trig)
+{
+	struct power_supply_led_trigger *psy_trig;
+
+	if (!trig)
+		return;
+
+	psy_trig = container_of(trig, struct power_supply_led_trigger, trig);
+	led_trigger_unregister(&psy_trig->trig);
+	kfree(psy_trig->trig.name);
+	kfree(psy_trig);
+}
+
 static void power_supply_update_bat_leds(struct power_supply *psy)
 {
 	union power_supply_propval status;
@@ -65,69 +116,39 @@ static void power_supply_update_bat_leds(struct power_supply *psy)
 	}
 }
 
-static int power_supply_create_bat_triggers(struct power_supply *psy)
-{
-	psy->charging_full_trig_name = kasprintf(GFP_KERNEL,
-					"%s-charging-or-full", psy->desc->name);
-	if (!psy->charging_full_trig_name)
-		goto charging_full_failed;
-
-	psy->charging_trig_name = kasprintf(GFP_KERNEL,
-					"%s-charging", psy->desc->name);
-	if (!psy->charging_trig_name)
-		goto charging_failed;
-
-	psy->full_trig_name = kasprintf(GFP_KERNEL, "%s-full", psy->desc->name);
-	if (!psy->full_trig_name)
-		goto full_failed;
-
-	psy->charging_blink_full_solid_trig_name = kasprintf(GFP_KERNEL,
-		"%s-charging-blink-full-solid", psy->desc->name);
-	if (!psy->charging_blink_full_solid_trig_name)
-		goto charging_blink_full_solid_failed;
-
-	psy->charging_orange_full_green_trig_name = kasprintf(GFP_KERNEL,
-		"%s-charging-orange-full-green", psy->desc->name);
-	if (!psy->charging_orange_full_green_trig_name)
-		goto charging_red_full_green_failed;
-
-	led_trigger_register_simple(psy->charging_full_trig_name,
-				    &psy->charging_full_trig);
-	led_trigger_register_simple(psy->charging_trig_name,
-				    &psy->charging_trig);
-	led_trigger_register_simple(psy->full_trig_name,
-				    &psy->full_trig);
-	led_trigger_register_simple(psy->charging_blink_full_solid_trig_name,
-				    &psy->charging_blink_full_solid_trig);
-	led_trigger_register_simple(psy->charging_orange_full_green_trig_name,
-				    &psy->charging_orange_full_green_trig);
-
-	return 0;
-
-charging_red_full_green_failed:
-	kfree(psy->charging_blink_full_solid_trig_name);
-charging_blink_full_solid_failed:
-	kfree(psy->full_trig_name);
-full_failed:
-	kfree(psy->charging_trig_name);
-charging_failed:
-	kfree(psy->charging_full_trig_name);
-charging_full_failed:
-	return -ENOMEM;
-}
-
 static void power_supply_remove_bat_triggers(struct power_supply *psy)
 {
-	led_trigger_unregister_simple(psy->charging_full_trig);
-	led_trigger_unregister_simple(psy->charging_trig);
-	led_trigger_unregister_simple(psy->full_trig);
-	led_trigger_unregister_simple(psy->charging_blink_full_solid_trig);
-	led_trigger_unregister_simple(psy->charging_orange_full_green_trig);
-	kfree(psy->charging_blink_full_solid_trig_name);
-	kfree(psy->full_trig_name);
-	kfree(psy->charging_trig_name);
-	kfree(psy->charging_full_trig_name);
-	kfree(psy->charging_orange_full_green_trig_name);
+	power_supply_unregister_led_trigger(psy->charging_full_trig);
+	power_supply_unregister_led_trigger(psy->charging_trig);
+	power_supply_unregister_led_trigger(psy->full_trig);
+	power_supply_unregister_led_trigger(psy->charging_blink_full_solid_trig);
+	power_supply_unregister_led_trigger(psy->charging_orange_full_green_trig);
+}
+
+static int power_supply_create_bat_triggers(struct power_supply *psy)
+{
+	int err = 0;
+
+	err |= power_supply_register_led_trigger(psy, "%s-charging-or-full",
+						 &psy->charging_full_trig);
+	err |= power_supply_register_led_trigger(psy, "%s-charging",
+						 &psy->charging_trig);
+	err |= power_supply_register_led_trigger(psy, "%s-full",
+						 &psy->full_trig);
+	err |= power_supply_register_led_trigger(psy, "%s-charging-blink-full-solid",
+						 &psy->charging_blink_full_solid_trig);
+	err |= power_supply_register_led_trigger(psy, "%s-charging-orange-full-green",
+						 &psy->charging_orange_full_green_trig);
+	if (err) {
+		power_supply_remove_bat_triggers(psy);
+		/*
+		 * led_trigger_register() may also return -EEXIST but that should
+		 * never happen with the dynamically generated psy trigger names.
+		 */
+		return -ENOMEM;
+	}
+
+	return 0;
 }
 
 /* Generated power specific LEDs triggers. */
@@ -149,20 +170,12 @@ static void power_supply_update_gen_leds(struct power_supply *psy)
 
 static int power_supply_create_gen_triggers(struct power_supply *psy)
 {
-	psy->online_trig_name = kasprintf(GFP_KERNEL, "%s-online",
-					  psy->desc->name);
-	if (!psy->online_trig_name)
-		return -ENOMEM;
-
-	led_trigger_register_simple(psy->online_trig_name, &psy->online_trig);
-
-	return 0;
+	return power_supply_register_led_trigger(psy, "%s-online", &psy->online_trig);
 }
 
 static void power_supply_remove_gen_triggers(struct power_supply *psy)
 {
-	led_trigger_unregister_simple(psy->online_trig);
-	kfree(psy->online_trig_name);
+	power_supply_unregister_led_trigger(psy->online_trig);
 }
 
 /* Choice what triggers to create&update. */
diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
index c852cc882501..e218a8ab72ee 100644
--- a/include/linux/power_supply.h
+++ b/include/linux/power_supply.h
@@ -310,17 +310,11 @@ struct power_supply {
 
 #ifdef CONFIG_LEDS_TRIGGERS
 	struct led_trigger *charging_full_trig;
-	char *charging_full_trig_name;
 	struct led_trigger *charging_trig;
-	char *charging_trig_name;
 	struct led_trigger *full_trig;
-	char *full_trig_name;
 	struct led_trigger *online_trig;
-	char *online_trig_name;
 	struct led_trigger *charging_blink_full_solid_trig;
-	char *charging_blink_full_solid_trig_name;
 	struct led_trigger *charging_orange_full_green_trig;
-	char *charging_orange_full_green_trig_name;
 #endif
 };
 
-- 
2.44.0

Re: [PATCH 1/3] power: supply: power-supply-leds: Add power_supply_[un]register_led_trigger()

[Andy Shevchenko]

On Fri, May 10, 2024 at 09:40:10PM +0200, Hans de Goede wrote:
> Add power_supply_[un]register_led_trigger() helper functions.
> 
> The primary goal of this is as a preparation patch for adding an activate
> callback to the power-supply LED triggers to ensure that power-supply
> LEDs get the correct initial value when the LED gets registered after
> the power_supply has been registered (this will use the psy back pointer).
> 
> There also is quite a lot of code duplication in the existing LED trigger
> registration in the form of the kasprintf() for the name-template for each
> trigger + related error handling. This duplication is removed by these
> new helpers.

...

> +	err = led_trigger_register(&psy_trig->trig);
> +	if (err)
> +		goto err_free_name;


> +err_free_name:
> +	kfree(psy_trig->trig.name);
> +err_free_trigger:
> +	kfree(psy_trig);
> +	return -ENOMEM;

Why not ret?

...

> +static int power_supply_create_bat_triggers(struct power_supply *psy)
> +{
> +	int err = 0;
> +
> +	err |= power_supply_register_led_trigger(psy, "%s-charging-or-full",
> +						 &psy->charging_full_trig);
> +	err |= power_supply_register_led_trigger(psy, "%s-charging",
> +						 &psy->charging_trig);
> +	err |= power_supply_register_led_trigger(psy, "%s-full",
> +						 &psy->full_trig);
> +	err |= power_supply_register_led_trigger(psy, "%s-charging-blink-full-solid",
> +						 &psy->charging_blink_full_solid_trig);
> +	err |= power_supply_register_led_trigger(psy, "%s-charging-orange-full-green",
> +						 &psy->charging_orange_full_green_trig);

Why not using the similar approach as you have done in v4l2 CCI?

> +	if (err) {
> +		power_supply_remove_bat_triggers(psy);
> +		/*
> +		 * led_trigger_register() may also return -EEXIST but that should
> +		 * never happen with the dynamically generated psy trigger names.
> +		 */

Maybe this comment should be above and here just return err; (but see above remark).

> +		return -ENOMEM;
> +	}
> +
> +	return 0;
>  }

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/3] power: supply: power-supply-leds: Add power_supply_[un]register_led_trigger()

[Hans de Goede]

Hi Andy,

Thank you for the review.

On 5/10/24 10:04 PM, Andy Shevchenko wrote:
> On Fri, May 10, 2024 at 09:40:10PM +0200, Hans de Goede wrote:
>> Add power_supply_[un]register_led_trigger() helper functions.
>>
>> The primary goal of this is as a preparation patch for adding an activate
>> callback to the power-supply LED triggers to ensure that power-supply
>> LEDs get the correct initial value when the LED gets registered after
>> the power_supply has been registered (this will use the psy back pointer).
>>
>> There also is quite a lot of code duplication in the existing LED trigger
>> registration in the form of the kasprintf() for the name-template for each
>> trigger + related error handling. This duplication is removed by these
>> new helpers.
> 
> ...
> 
>> +	err = led_trigger_register(&psy_trig->trig);
>> +	if (err)
>> +		goto err_free_name;
> 
> 
>> +err_free_name:
>> +	kfree(psy_trig->trig.name);
>> +err_free_trigger:
>> +	kfree(psy_trig);
>> +	return -ENOMEM;
> 
> Why not ret?

Fixed for v2.


> ...
> 
>> +static int power_supply_create_bat_triggers(struct power_supply *psy)
>> +{
>> +	int err = 0;
>> +
>> +	err |= power_supply_register_led_trigger(psy, "%s-charging-or-full",
>> +						 &psy->charging_full_trig);
>> +	err |= power_supply_register_led_trigger(psy, "%s-charging",
>> +						 &psy->charging_trig);
>> +	err |= power_supply_register_led_trigger(psy, "%s-full",
>> +						 &psy->full_trig);
>> +	err |= power_supply_register_led_trigger(psy, "%s-charging-blink-full-solid",
>> +						 &psy->charging_blink_full_solid_trig);
>> +	err |= power_supply_register_led_trigger(psy, "%s-charging-orange-full-green",
>> +						 &psy->charging_orange_full_green_trig);
> 
> Why not using the similar approach as you have done in v4l2 CCI?

That is a good idea I've done that for v2.

>> +	if (err) {
>> +		power_supply_remove_bat_triggers(psy);
>> +		/*
>> +		 * led_trigger_register() may also return -EEXIST but that should
>> +		 * never happen with the dynamically generated psy trigger names.
>> +		 */
> 
> Maybe this comment should be above and here just return err; (but see above remark).

With the err propagation approach from v4l2 CCI this can now simply return err
since that now correctly reflects the first error (and any following calls
are made no-ops).

Regards,

Hans



> 
>> +		return -ENOMEM;
>> +	}
>> +
>> +	return 0;
>>  }
> 

[PATCH 2/3] power: supply: power-supply-leds: Share trig pointer for online and charging_full

[Hans de Goede]

Either 5 different LED triggers are registered for battery power-supply
devices or a single online LED trigger is used for non battery power-supply
devices.

These 5 / 1 LED trigger(s) are never used at the same time. So there is
no need for a separate LED trigger pointer for the online trigger. Rename
the first battery trigger from charging_full_trig to just trig and use this
for the online trigger too.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/power/supply/power_supply_leds.c | 18 +++++++++---------
 include/linux/power_supply.h             |  3 +--
 2 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/power/supply/power_supply_leds.c b/drivers/power/supply/power_supply_leds.c
index 3e20b0f30aa3..634f7d33afd0 100644
--- a/drivers/power/supply/power_supply_leds.c
+++ b/drivers/power/supply/power_supply_leds.c
@@ -83,7 +83,7 @@ static void power_supply_update_bat_leds(struct power_supply *psy)
 
 	switch (status.intval) {
 	case POWER_SUPPLY_STATUS_FULL:
-		led_trigger_event(psy->charging_full_trig, LED_FULL);
+		led_trigger_event(psy->trig, LED_FULL);
 		led_trigger_event(psy->charging_trig, LED_OFF);
 		led_trigger_event(psy->full_trig, LED_FULL);
 		/* Going from blink to LED on requires a LED_OFF event to stop blink */
@@ -95,7 +95,7 @@ static void power_supply_update_bat_leds(struct power_supply *psy)
 				     LED_FULL);
 		break;
 	case POWER_SUPPLY_STATUS_CHARGING:
-		led_trigger_event(psy->charging_full_trig, LED_FULL);
+		led_trigger_event(psy->trig, LED_FULL);
 		led_trigger_event(psy->charging_trig, LED_FULL);
 		led_trigger_event(psy->full_trig, LED_OFF);
 		led_trigger_blink(psy->charging_blink_full_solid_trig, 0, 0);
@@ -105,7 +105,7 @@ static void power_supply_update_bat_leds(struct power_supply *psy)
 				     LED_FULL);
 		break;
 	default:
-		led_trigger_event(psy->charging_full_trig, LED_OFF);
+		led_trigger_event(psy->trig, LED_OFF);
 		led_trigger_event(psy->charging_trig, LED_OFF);
 		led_trigger_event(psy->full_trig, LED_OFF);
 		led_trigger_event(psy->charging_blink_full_solid_trig,
@@ -118,7 +118,7 @@ static void power_supply_update_bat_leds(struct power_supply *psy)
 
 static void power_supply_remove_bat_triggers(struct power_supply *psy)
 {
-	power_supply_unregister_led_trigger(psy->charging_full_trig);
+	power_supply_unregister_led_trigger(psy->trig);
 	power_supply_unregister_led_trigger(psy->charging_trig);
 	power_supply_unregister_led_trigger(psy->full_trig);
 	power_supply_unregister_led_trigger(psy->charging_blink_full_solid_trig);
@@ -130,7 +130,7 @@ static int power_supply_create_bat_triggers(struct power_supply *psy)
 	int err = 0;
 
 	err |= power_supply_register_led_trigger(psy, "%s-charging-or-full",
-						 &psy->charging_full_trig);
+						 &psy->trig);
 	err |= power_supply_register_led_trigger(psy, "%s-charging",
 						 &psy->charging_trig);
 	err |= power_supply_register_led_trigger(psy, "%s-full",
@@ -163,19 +163,19 @@ static void power_supply_update_gen_leds(struct power_supply *psy)
 	dev_dbg(&psy->dev, "%s %d\n", __func__, online.intval);
 
 	if (online.intval)
-		led_trigger_event(psy->online_trig, LED_FULL);
+		led_trigger_event(psy->trig, LED_FULL);
 	else
-		led_trigger_event(psy->online_trig, LED_OFF);
+		led_trigger_event(psy->trig, LED_OFF);
 }
 
 static int power_supply_create_gen_triggers(struct power_supply *psy)
 {
-	return power_supply_register_led_trigger(psy, "%s-online", &psy->online_trig);
+	return power_supply_register_led_trigger(psy, "%s-online", &psy->trig);
 }
 
 static void power_supply_remove_gen_triggers(struct power_supply *psy)
 {
-	power_supply_unregister_led_trigger(psy->online_trig);
+	power_supply_unregister_led_trigger(psy->trig);
 }
 
 /* Choice what triggers to create&update. */
diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
index e218a8ab72ee..65082ef75692 100644
--- a/include/linux/power_supply.h
+++ b/include/linux/power_supply.h
@@ -309,10 +309,9 @@ struct power_supply {
 #endif
 
 #ifdef CONFIG_LEDS_TRIGGERS
-	struct led_trigger *charging_full_trig;
+	struct led_trigger *trig;
 	struct led_trigger *charging_trig;
 	struct led_trigger *full_trig;
-	struct led_trigger *online_trig;
 	struct led_trigger *charging_blink_full_solid_trig;
 	struct led_trigger *charging_orange_full_green_trig;
 #endif
-- 
2.44.0

[PATCH 3/3] power: supply: power-supply-leds: Add activate() callback to triggers

[Hans de Goede]

Add an activate() callback to the power-supply LED triggers so that
the LED being activated will properly reflect the current power-supply
state for power-supply devices which are already fully registered
when the trigger gets activated.

This fixes e.g. wrong LED state (1) when the LED gets registered
after the power-supply device.

1) Until the psy driver has a reason to call power_supply_changed()
   which may take quite a while

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/power/supply/power_supply_leds.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/power/supply/power_supply_leds.c b/drivers/power/supply/power_supply_leds.c
index 634f7d33afd0..7ff6d41671b9 100644
--- a/drivers/power/supply/power_supply_leds.c
+++ b/drivers/power/supply/power_supply_leds.c
@@ -24,6 +24,16 @@ struct power_supply_led_trigger {
 	struct power_supply *psy;
 };
 
+static int power_supply_led_trigger_activate(struct led_classdev *led_cdev)
+{
+	struct power_supply_led_trigger *psy_trig =
+		container_of(led_cdev->trigger, struct power_supply_led_trigger, trig);
+
+	/* Sync current power-supply state to LED being activated */
+	power_supply_update_leds(psy_trig->psy);
+	return 0;
+}
+
 static int power_supply_register_led_trigger(struct power_supply *psy,
 					     const char *name_template,
 					     struct led_trigger **tp)
@@ -41,6 +51,7 @@ static int power_supply_register_led_trigger(struct power_supply *psy,
 		goto err_free_trigger;
 	}
 
+	psy_trig->trig.activate = power_supply_led_trigger_activate;
 	psy_trig->psy = psy;
 
 	err = led_trigger_register(&psy_trig->trig);
-- 
2.44.0

Re: [PATCH 3/3] power: supply: power-supply-leds: Add activate() callback to triggers

[Andy Shevchenko]

On Fri, May 10, 2024 at 09:40:12PM +0200, Hans de Goede wrote:
> Add an activate() callback to the power-supply LED triggers so that
> the LED being activated will properly reflect the current power-supply
> state for power-supply devices which are already fully registered
> when the trigger gets activated.
> 
> This fixes e.g. wrong LED state (1) when the LED gets registered
> after the power-supply device.
> 
> 1) Until the psy driver has a reason to call power_supply_changed()
>    which may take quite a while

...

> +static int power_supply_led_trigger_activate(struct led_classdev *led_cdev)
> +{
> +	struct power_supply_led_trigger *psy_trig =
> +		container_of(led_cdev->trigger, struct power_supply_led_trigger, trig);

Second time same container_of(), perhaps a helper [macro]?

> +	/* Sync current power-supply state to LED being activated */
> +	power_supply_update_leds(psy_trig->psy);
> +	return 0;
> +}

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 3/3] power: supply: power-supply-leds: Add activate() callback to triggers

[Hans de Goede]

Hi,

On 5/10/24 10:07 PM, Andy Shevchenko wrote:
> On Fri, May 10, 2024 at 09:40:12PM +0200, Hans de Goede wrote:
>> Add an activate() callback to the power-supply LED triggers so that
>> the LED being activated will properly reflect the current power-supply
>> state for power-supply devices which are already fully registered
>> when the trigger gets activated.
>>
>> This fixes e.g. wrong LED state (1) when the LED gets registered
>> after the power-supply device.
>>
>> 1) Until the psy driver has a reason to call power_supply_changed()
>>    which may take quite a while
> 
> ...
> 
>> +static int power_supply_led_trigger_activate(struct led_classdev *led_cdev)
>> +{
>> +	struct power_supply_led_trigger *psy_trig =
>> +		container_of(led_cdev->trigger, struct power_supply_led_trigger, trig);
> 
> Second time same container_of(), perhaps a helper [macro]?

Ack, fixed for v2.

Regards,

Hans



> 
>> +	/* Sync current power-supply state to LED being activated */
>> +	power_supply_update_leds(psy_trig->psy);
>> +	return 0;
>> +}
> 

Re: [PATCH 0/3] power: supply: power-supply-leds: Add activate() callback to triggers

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 659 bytes --]

Hi,

On Fri, May 10, 2024 at 09:40:09PM +0200, Hans de Goede wrote:
> This series adds an activate callback to the power-supply LED triggers to
> ensure that power-supply LEDs get the correct initial value when the LED
> gets registered after the power_supply has been registered.
> 
> Patches 1-2 do some refactoring / prep work and patch 3 adds the actual
> activate callback.
> 
> This series applies on top of "[PATCH v9 6/7] power: supply: power-supply-leds:
> Add charging_orange_full_green trigger for RGB LED":
> https://lore.kernel.org/all/20240504164105.114017-7-hdegoede@redhat.com/

Apart from Andy's comments LGTM.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]