[RFC 0/2] leds: Add led_mc_set_brightness() and led_mc_trigger_event() functions

[RFC 0/2] leds: Add led_mc_set_brightness() and led_mc_trigger_event() functions

[Hans de Goede]

Hi All,

Here is a patch-series adding led_mc_set_brightness() and
led_mc_trigger_event() functions for changing multi-color LED colors
from inside the kernel.

This is a preparation series for adding a new trigger to
drivers/power/supply/power_supply_leds.c which changes the color
of a RGB LED depending on if the battery is discharging (LED off)
charging (LED red) or full (LED green)

This is marked as RFC since the power_supply_leds.c changes have not
been written yet and as such this is compile-tested only atm.
The main goal of this RFC is to gather review feedback on the
chosen approach in these 2 patches.

Regards,

Hans


Hans de Goede (2):
  leds: core: Add led_mc_set_brightness() function
  leds: trigger: Add led_mc_trigger_event() function

 drivers/leds/led-class-multicolor.c |  1 +
 drivers/leds/led-core.c             | 31 +++++++++++++++++++++++++++++
 drivers/leds/led-triggers.c         | 20 +++++++++++++++++++
 include/linux/leds.h                | 26 ++++++++++++++++++++++++
 4 files changed, 78 insertions(+)

-- 
2.43.2

[RFC 1/2] leds: core: Add led_mc_set_brightness() function

[Hans de Goede]

Add a new led_mc_set_brightness() function for in kernel color/brightness
changing of multi-color LEDs.

led-class-multicolor can be build as a module and led_mc_set_brightness()
will have builtin callers, so put led_mc_set_brightness() inside led-core
instead, just like how led_set_brightness() is part of the core and not
of the led-class object.

This also adds a new LED_MULTI_COLOR led_classdev flag to allow
led_mc_set_brightness() to verify that it is operating on a multi-color
LED classdev, avoiding casting the passed in LED classdev to a multi-color
LED classdev, when it actually is not a multi-color LED.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/leds/led-class-multicolor.c |  1 +
 drivers/leds/led-core.c             | 31 +++++++++++++++++++++++++++++
 include/linux/leds.h                | 20 +++++++++++++++++++
 3 files changed, 52 insertions(+)

diff --git a/drivers/leds/led-class-multicolor.c b/drivers/leds/led-class-multicolor.c
index ec62a4811613..df01c0e66c8b 100644
--- a/drivers/leds/led-class-multicolor.c
+++ b/drivers/leds/led-class-multicolor.c
@@ -134,6 +134,7 @@ int led_classdev_multicolor_register_ext(struct device *parent,
 		return -EINVAL;
 
 	led_cdev = &mcled_cdev->led_cdev;
+	led_cdev->flags |= LED_MULTI_COLOR;
 	mcled_cdev->led_cdev.groups = led_multicolor_groups;
 
 	return led_classdev_register_ext(parent, led_cdev, init_data);
diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
index 89c9806cc97f..5889753ebc74 100644
--- a/drivers/leds/led-core.c
+++ b/drivers/leds/led-core.c
@@ -8,6 +8,7 @@
  */
 
 #include <linux/kernel.h>
+#include <linux/led-class-multicolor.h>
 #include <linux/leds.h>
 #include <linux/list.h>
 #include <linux/module.h>
@@ -362,6 +363,36 @@ int led_set_brightness_sync(struct led_classdev *led_cdev, unsigned int value)
 }
 EXPORT_SYMBOL_GPL(led_set_brightness_sync);
 
+/*
+ * This is a led-core function because just like led_set_brightness()
+ * it is used in kernel by e.g. triggers.
+ */
+void led_mc_set_brightness(struct led_classdev *led_cdev,
+			   unsigned int *intensity_value, unsigned int num_colors,
+			   unsigned int brightness)
+{
+	struct led_classdev_mc *mcled_cdev;
+	unsigned int i;
+
+	if (!(led_cdev->flags & LED_MULTI_COLOR)) {
+		dev_err_once(led_cdev->dev, "%s: error not a multi-color LED\n",  __func__);
+		return;
+	}
+
+	mcled_cdev = lcdev_to_mccdev(led_cdev);
+	if (num_colors != mcled_cdev->num_colors) {
+		dev_err_once(led_cdev->dev, "%s: error num_colors mismatch %d != %d\n",
+			     __func__, num_colors, mcled_cdev->num_colors);
+		return;
+	}
+
+	for (i = 0; i < mcled_cdev->num_colors; i++)
+		mcled_cdev->subled_info[i].intensity = intensity_value[i];
+
+	led_set_brightness(led_cdev, brightness);
+}
+EXPORT_SYMBOL_GPL(led_mc_set_brightness);
+
 int led_update_brightness(struct led_classdev *led_cdev)
 {
 	int ret;
diff --git a/include/linux/leds.h b/include/linux/leds.h
index 4754b02d3a2c..fed88eb9e170 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -115,6 +115,7 @@ struct led_classdev {
 #define LED_BRIGHT_HW_CHANGED	BIT(21)
 #define LED_RETAIN_AT_SHUTDOWN	BIT(22)
 #define LED_INIT_DEFAULT_TRIGGER BIT(23)
+#define LED_MULTI_COLOR		BIT(24)
 
 	/* set_brightness_work / blink_timer flags, atomic, private. */
 	unsigned long		work_flags;
@@ -392,6 +393,25 @@ void led_set_brightness(struct led_classdev *led_cdev, unsigned int brightness);
  */
 int led_set_brightness_sync(struct led_classdev *led_cdev, unsigned int value);
 
+/**
+ * led_mc_set_brightness - set mc LED color intensity values and brightness
+ * @led_cdev: the LED to set
+ * @intensity_value: array of per color intensity values to set
+ * @num_colors: amount of entries in intensity_value array
+ * @brightness: the brightness to set the LED to
+ *
+ * Set a multi-color LED's per color intensity values and brightness.
+ * If necessary, this cancels the software blink timer. This function is
+ * guaranteed not to sleep.
+ *
+ * Calling this function on a non multi-color led_classdev or with the wrong
+ * num_colors value is an error. In this case an error will be logged once
+ * and the call will do nothing.
+ */
+void led_mc_set_brightness(struct led_classdev *led_cdev,
+			   unsigned int *intensity_value, unsigned int num_colors,
+			   unsigned int brightness);
+
 /**
  * led_update_brightness - update LED brightness
  * @led_cdev: the LED to query
-- 
2.43.2

[RFC 2/2] leds: trigger: Add led_mc_trigger_event() function

[Hans de Goede]

Add a new led_mc_trigger_event() function for triggers which want to
change the color of a multi-color LED based on their trigger conditions.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
This is a preparation patch for adding a new trigger to
drivers/power/supply/power_supply_leds.c which changes the color
of a RGB LED depending on if the battery is discharging (LED off)
charging (LED red) or full (LED green)
---
 drivers/leds/led-triggers.c | 20 ++++++++++++++++++++
 include/linux/leds.h        |  6 ++++++
 2 files changed, 26 insertions(+)

diff --git a/drivers/leds/led-triggers.c b/drivers/leds/led-triggers.c
index bd59a14a4a90..fcc4e7a7b12b 100644
--- a/drivers/leds/led-triggers.c
+++ b/drivers/leds/led-triggers.c
@@ -380,6 +380,26 @@ void led_trigger_event(struct led_trigger *trig,
 }
 EXPORT_SYMBOL_GPL(led_trigger_event);
 
+void led_mc_trigger_event(struct led_trigger *trig,
+			  unsigned int *intensity_value, unsigned int num_colors,
+			  enum led_brightness brightness)
+{
+	struct led_classdev *led_cdev;
+
+	if (!trig)
+		return;
+
+	rcu_read_lock();
+	list_for_each_entry_rcu(led_cdev, &trig->led_cdevs, trig_list) {
+		if (!(led_cdev->flags & LED_MULTI_COLOR))
+			continue;
+
+		led_mc_set_brightness(led_cdev, intensity_value, num_colors, brightness);
+	}
+	rcu_read_unlock();
+}
+EXPORT_SYMBOL_GPL(led_mc_trigger_event);
+
 static void led_trigger_blink_setup(struct led_trigger *trig,
 			     unsigned long delay_on,
 			     unsigned long delay_off,
diff --git a/include/linux/leds.h b/include/linux/leds.h
index fed88eb9e170..5378e4cd03ff 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -526,6 +526,9 @@ void led_trigger_register_simple(const char *name,
 				struct led_trigger **trigger);
 void led_trigger_unregister_simple(struct led_trigger *trigger);
 void led_trigger_event(struct led_trigger *trigger,  enum led_brightness event);
+void led_mc_trigger_event(struct led_trigger *trig,
+			  unsigned int *intensity_value, unsigned int num_colors,
+			  enum led_brightness brightness);
 void led_trigger_blink(struct led_trigger *trigger, unsigned long delay_on,
 		       unsigned long delay_off);
 void led_trigger_blink_oneshot(struct led_trigger *trigger,
@@ -562,6 +565,9 @@ static inline void led_trigger_register_simple(const char *name,
 static inline void led_trigger_unregister_simple(struct led_trigger *trigger) {}
 static inline void led_trigger_event(struct led_trigger *trigger,
 				enum led_brightness event) {}
+static inline void led_mc_trigger_event(struct led_trigger *trig,
+				unsigned int *intensity_value, unsigned int num_colors,
+				enum led_brightness brightness) {}
 static inline void led_trigger_blink(struct led_trigger *trigger,
 				      unsigned long delay_on,
 				      unsigned long delay_off) {}
-- 
2.43.2

Re: [RFC 0/2] leds: Add led_mc_set_brightness() and led_mc_trigger_event() functions

[Jacek Anaszewski]

Hi Hans,

On 3/9/24 20:08, Hans de Goede wrote:
> Hi All,
> 
> Here is a patch-series adding led_mc_set_brightness() and
> led_mc_trigger_event() functions for changing multi-color LED colors
> from inside the kernel.
> 
> This is a preparation series for adding a new trigger to
> drivers/power/supply/power_supply_leds.c which changes the color
> of a RGB LED depending on if the battery is discharging (LED off)
> charging (LED red) or full (LED green)
> 
> This is marked as RFC since the power_supply_leds.c changes have not
> been written yet and as such this is compile-tested only atm.
> The main goal of this RFC is to gather review feedback on the
> chosen approach in these 2 patches.
> 
> Regards,
> 
> Hans
> 
> 
> Hans de Goede (2):
>    leds: core: Add led_mc_set_brightness() function
>    leds: trigger: Add led_mc_trigger_event() function
> 
>   drivers/leds/led-class-multicolor.c |  1 +
>   drivers/leds/led-core.c             | 31 +++++++++++++++++++++++++++++
>   drivers/leds/led-triggers.c         | 20 +++++++++++++++++++
>   include/linux/leds.h                | 26 ++++++++++++++++++++++++
>   4 files changed, 78 insertions(+)
> 

LGTM.

Reviewed-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>

-- 
Best regards,
Jacek Anaszewski

Re: [RFC 0/2] leds: Add led_mc_set_brightness() and led_mc_trigger_event() functions

[Lee Jones]

On Sat, 09 Mar 2024, Hans de Goede wrote:

> Hi All,
> 
> Here is a patch-series adding led_mc_set_brightness() and
> led_mc_trigger_event() functions for changing multi-color LED colors
> from inside the kernel.
> 
> This is a preparation series for adding a new trigger to
> drivers/power/supply/power_supply_leds.c which changes the color
> of a RGB LED depending on if the battery is discharging (LED off)
> charging (LED red) or full (LED green)
> 
> This is marked as RFC since the power_supply_leds.c changes have not
> been written yet and as such this is compile-tested only atm.
> The main goal of this RFC is to gather review feedback on the
> chosen approach in these 2 patches.
> 
> Regards,
> 
> Hans
> 
> 
> Hans de Goede (2):
>   leds: core: Add led_mc_set_brightness() function
>   leds: trigger: Add led_mc_trigger_event() function
> 
>  drivers/leds/led-class-multicolor.c |  1 +
>  drivers/leds/led-core.c             | 31 +++++++++++++++++++++++++++++
>  drivers/leds/led-triggers.c         | 20 +++++++++++++++++++
>  include/linux/leds.h                | 26 ++++++++++++++++++++++++
>  4 files changed, 78 insertions(+)

What is it you want me to do with this RFC patches Hans?

-- 
Lee Jones [李琼斯]

Re: [RFC 0/2] leds: Add led_mc_set_brightness() and led_mc_trigger_event() functions

[Kate Hsuan]

Hi,

On Fri, Mar 22, 2024 at 2:19 AM Lee Jones <lee@kernel.org> wrote:
>
> On Sat, 09 Mar 2024, Hans de Goede wrote:
>
> > Hi All,
> >
> > Here is a patch-series adding led_mc_set_brightness() and
> > led_mc_trigger_event() functions for changing multi-color LED colors
> > from inside the kernel.
> >
> > This is a preparation series for adding a new trigger to
> > drivers/power/supply/power_supply_leds.c which changes the color
> > of a RGB LED depending on if the battery is discharging (LED off)
> > charging (LED red) or full (LED green)
> >
> > This is marked as RFC since the power_supply_leds.c changes have not
> > been written yet and as such this is compile-tested only atm.
> > The main goal of this RFC is to gather review feedback on the
> > chosen approach in these 2 patches.
> >
> > Regards,
> >
> > Hans
> >
> >
> > Hans de Goede (2):
> >   leds: core: Add led_mc_set_brightness() function
> >   leds: trigger: Add led_mc_trigger_event() function
> >
> >  drivers/leds/led-class-multicolor.c |  1 +
> >  drivers/leds/led-core.c             | 31 +++++++++++++++++++++++++++++
> >  drivers/leds/led-triggers.c         | 20 +++++++++++++++++++
> >  include/linux/leds.h                | 26 ++++++++++++++++++++++++
> >  4 files changed, 78 insertions(+)
>
> What is it you want me to do with this RFC patches Hans?
>
> --
> Lee Jones [李琼斯]
>

This is used for the Xiaomi pad2 indicator LED to show the battery
status in a multi-color manner and it can be found in the following
URL.
https://lore.kernel.org/linux-leds/20240322033736.9344-1-hpa@redhat.com/T/#m577efb6549aeb29d2813faf8c51ed38e9e092598

Thank you

-- 
BR,
Kate