2023-01-02 08:23:31

by Jean-Jacques Hiblot

[permalink] [raw]
Subject: [PATCH v7 0/6] Add a multicolor LED driver for groups of monochromatic LEDs


Some HW design implement multicolor LEDs with several monochromatic LEDs.
Grouping the monochromatic LEDs allows to configure them in sync and use
the triggers.
The PWM multicolor LED driver implements such grouping but only for
PWM-based LEDs. As this feature is also desirable for the other types of
LEDs, this series implements it for any kind of LED device.

changes v6->v7:
- in led_mcg_probe() increment the counter at the end of the loop for
clarity.

changes v5->v6:
- restore sysfs access to the leds when the device is removed

changes v4->v5:
- Use "depends on COMPILE_TEST || OF" in Kconfig to indicate that OF
is a functional requirement, not just a requirement for the
compilation.
- in led_mcg_probe() check if devm_of_led_get_optional() returns an
error before testing for the end of the list.
- use sysfs_emit() instead of sprintf() in color_show().
- some grammar fixes in the comments and the commit logs.

changes v2->v3, only minor changes:
- rephrased the Kconfig descritpion
- make the sysfs interface of underlying LEDs read-only only if the probe
is successful.
- sanitize the header files
- removed the useless call to dev_set_drvdata()
- use dev_fwnode() to get the fwnode to the device.

changes v1->v2:
- Followed Rob Herrings's suggestion to make the dt binding much simpler.
- Added a patch to store the color property of a LED in its class
structure (struct led_classdev).


Jean-Jacques Hiblot (6):
devres: provide devm_krealloc_array()
leds: class: simplify the implementation of devm_of_led_get()
leds: provide devm_of_led_get_optional()
leds: class: store the color index in struct led_classdev
dt-bindings: leds: Add binding for a multicolor group of LEDs
leds: Add a multicolor LED driver to group monochromatic LEDs

Documentation/ABI/testing/sysfs-class-led | 9 +
.../bindings/leds/leds-group-multicolor.yaml | 64 +++++++
drivers/leds/led-class.c | 65 +++++--
drivers/leds/rgb/Kconfig | 10 ++
drivers/leds/rgb/Makefile | 1 +
drivers/leds/rgb/leds-group-multicolor.c | 166 ++++++++++++++++++
include/linux/device.h | 13 ++
include/linux/leds.h | 3 +
8 files changed, 317 insertions(+), 14 deletions(-)
create mode 100644 Documentation/devicetree/bindings/leds/leds-group-multicolor.yaml
create mode 100644 drivers/leds/rgb/leds-group-multicolor.c

--
2.25.1


2023-01-02 08:28:19

by Jean-Jacques Hiblot

[permalink] [raw]
Subject: [PATCH v7 2/6] leds: class: simplify the implementation of devm_of_led_get()

Use the devm_add_action_or_reset() helper.

Signed-off-by: Jean-Jacques Hiblot <[email protected]>
Reviewed-by: Andy Shevchenko <[email protected]>
---
drivers/leds/led-class.c | 20 ++++++--------------
1 file changed, 6 insertions(+), 14 deletions(-)

diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
index 6a8ea94834fa..2c0d979d0c8a 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -258,11 +258,9 @@ void led_put(struct led_classdev *led_cdev)
}
EXPORT_SYMBOL_GPL(led_put);

-static void devm_led_release(struct device *dev, void *res)
+static void devm_led_release(void *cdev)
{
- struct led_classdev **p = res;
-
- led_put(*p);
+ led_put(cdev);
}

/**
@@ -280,7 +278,7 @@ struct led_classdev *__must_check devm_of_led_get(struct device *dev,
int index)
{
struct led_classdev *led;
- struct led_classdev **dr;
+ int ret;

if (!dev)
return ERR_PTR(-EINVAL);
@@ -289,15 +287,9 @@ struct led_classdev *__must_check devm_of_led_get(struct device *dev,
if (IS_ERR(led))
return led;

- dr = devres_alloc(devm_led_release, sizeof(struct led_classdev *),
- GFP_KERNEL);
- if (!dr) {
- led_put(led);
- return ERR_PTR(-ENOMEM);
- }
-
- *dr = led;
- devres_add(dev, dr);
+ ret = devm_add_action_or_reset(dev, devm_led_release, led);
+ if (ret)
+ return ERR_PTR(ret);

return led;
}
--
2.25.1

2023-01-02 08:31:10

by Jean-Jacques Hiblot

[permalink] [raw]
Subject: [PATCH v7 5/6] dt-bindings: leds: Add binding for a multicolor group of LEDs

This allows to group multiple monochromatic LEDs into a multicolor
LED, e.g. RGB LEDs.

Signed-off-by: Jean-Jacques Hiblot <[email protected]>
Reviewed-by: Rob Herring <[email protected]>
---
.../bindings/leds/leds-group-multicolor.yaml | 64 +++++++++++++++++++
1 file changed, 64 insertions(+)
create mode 100644 Documentation/devicetree/bindings/leds/leds-group-multicolor.yaml

diff --git a/Documentation/devicetree/bindings/leds/leds-group-multicolor.yaml b/Documentation/devicetree/bindings/leds/leds-group-multicolor.yaml
new file mode 100644
index 000000000000..8ed059a5a724
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/leds-group-multicolor.yaml
@@ -0,0 +1,64 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/leds/leds-group-multicolor.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Multi-color LED built with monochromatic LEDs
+
+maintainers:
+ - Jean-Jacques Hiblot <[email protected]>
+
+description: |
+ This driver combines several monochromatic LEDs into one multi-color
+ LED using the multicolor LED class.
+
+properties:
+ compatible:
+ const: leds-group-multicolor
+
+ leds:
+ description:
+ An aray of monochromatic leds
+ $ref: /schemas/types.yaml#/definitions/phandle-array
+
+required:
+ - leds
+
+allOf:
+ - $ref: leds-class-multicolor.yaml#
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+ #include <dt-bindings/leds/common.h>
+
+ monochromatic-leds {
+ compatible = "gpio-leds";
+
+ led0: led-0 {
+ gpios = <&mcu_pio 0 GPIO_ACTIVE_LOW>;
+ color = <LED_COLOR_ID_RED>;
+ };
+
+ led1: led-1 {
+ gpios = <&mcu_pio 1 GPIO_ACTIVE_HIGH>;
+ color = <LED_COLOR_ID_GREEN>;
+ };
+
+ led2: led-2 {
+ gpios = <&mcu_pio 2 GPIO_ACTIVE_HIGH>;
+ color = <LED_COLOR_ID_BLUE>;
+ };
+ };
+
+ multi-led {
+ compatible = "leds-group-multicolor";
+ color = <LED_COLOR_ID_RGB>;
+ function = LED_FUNCTION_INDICATOR;
+ leds = <&led0>, <&led1>, <&led2>;
+ };
+
+...
--
2.25.1

2023-01-02 08:31:24

by Jean-Jacques Hiblot

[permalink] [raw]
Subject: [PATCH v7 3/6] leds: provide devm_of_led_get_optional()

This version of devm_of_led_get() doesn't fail if a LED is not found.
Instead it returns a NULL pointer.

Signed-off-by: Jean-Jacques Hiblot <[email protected]>
Reviewed-by: Andy Shevchenko <[email protected]>
---
drivers/leds/led-class.c | 25 +++++++++++++++++++++++++
include/linux/leds.h | 2 ++
2 files changed, 27 insertions(+)

diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
index 2c0d979d0c8a..9cddcd908bce 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -295,6 +295,31 @@ struct led_classdev *__must_check devm_of_led_get(struct device *dev,
}
EXPORT_SYMBOL_GPL(devm_of_led_get);

+/**
+ * devm_of_led_get_optional - Resource-managed request of an optional LED device
+ * @dev: LED consumer
+ * @index: index of the LED to obtain in the consumer
+ *
+ * The device node of the device is parsed to find the requested LED device.
+ * The LED device returned from this function is automatically released
+ * on driver detach.
+ *
+ * @return a pointer to a LED device, ERR_PTR(errno) on failure and NULL if the
+ * led was not found.
+ */
+struct led_classdev *__must_check devm_of_led_get_optional(struct device *dev,
+ int index)
+{
+ struct led_classdev *led;
+
+ led = devm_of_led_get(dev, index);
+ if (IS_ERR(led) && PTR_ERR(led) == -ENOENT)
+ return NULL;
+
+ return led;
+}
+EXPORT_SYMBOL_GPL(devm_of_led_get_optional);
+
static int led_classdev_next_name(const char *init_name, char *name,
size_t len)
{
diff --git a/include/linux/leds.h b/include/linux/leds.h
index ba4861ec73d3..41df18f42d00 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -215,6 +215,8 @@ extern struct led_classdev *of_led_get(struct device_node *np, int index);
extern void led_put(struct led_classdev *led_cdev);
struct led_classdev *__must_check devm_of_led_get(struct device *dev,
int index);
+struct led_classdev *__must_check devm_of_led_get_optional(struct device *dev,
+ int index);

/**
* led_blink_set - set blinking with software fallback
--
2.25.1

2023-01-02 08:45:35

by Jean-Jacques Hiblot

[permalink] [raw]
Subject: [PATCH v7 4/6] leds: class: store the color index in struct led_classdev

This information might be useful for more than only deriving the led's
name. And since we have this information, we can expose it in the sysfs.

Signed-off-by: Jean-Jacques Hiblot <[email protected]>
---
Documentation/ABI/testing/sysfs-class-led | 9 +++++++++
drivers/leds/led-class.c | 20 ++++++++++++++++++++
include/linux/leds.h | 1 +
3 files changed, 30 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-class-led b/Documentation/ABI/testing/sysfs-class-led
index 2e24ac3bd7ef..6f38edd69d2a 100644
--- a/Documentation/ABI/testing/sysfs-class-led
+++ b/Documentation/ABI/testing/sysfs-class-led
@@ -59,6 +59,15 @@ Description:
brightness. Reading this file when no hw brightness change
event has happened will return an ENODATA error.

+What: /sys/class/leds/<led>/color
+Date: Dec 2022
+KernelVersion: 6.2
+Description:
+ Color of the led.
+
+ This is a read-only file. Reading this file returns the color
+ of the led as a string (ex: "red", "green").
+
What: /sys/class/leds/<led>/trigger
Date: March 2006
KernelVersion: 2.6.17
diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
index 9cddcd908bce..4432e85a5fc6 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -74,6 +74,18 @@ static ssize_t max_brightness_show(struct device *dev,
}
static DEVICE_ATTR_RO(max_brightness);

+static ssize_t color_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ const char *color_text = "invalid";
+ struct led_classdev *led_cdev = dev_get_drvdata(dev);
+
+ if (led_cdev->color < LED_COLOR_ID_MAX)
+ color_text = led_colors[led_cdev->color];
+ return sysfs_emit(buf, "%s\n", color_text);
+}
+static DEVICE_ATTR_RO(color);
+
#ifdef CONFIG_LEDS_TRIGGERS
static BIN_ATTR(trigger, 0644, led_trigger_read, led_trigger_write, 0);
static struct bin_attribute *led_trigger_bin_attrs[] = {
@@ -88,6 +100,7 @@ static const struct attribute_group led_trigger_group = {
static struct attribute *led_class_attrs[] = {
&dev_attr_brightness.attr,
&dev_attr_max_brightness.attr,
+ &dev_attr_color.attr,
NULL,
};

@@ -375,6 +388,10 @@ int led_classdev_register_ext(struct device *parent,
if (fwnode_property_present(init_data->fwnode,
"retain-state-shutdown"))
led_cdev->flags |= LED_RETAIN_AT_SHUTDOWN;
+
+ if (fwnode_property_present(init_data->fwnode, "color"))
+ fwnode_property_read_u32(init_data->fwnode, "color",
+ &led_cdev->color);
}
} else {
proposed_name = led_cdev->name;
@@ -384,6 +401,9 @@ int led_classdev_register_ext(struct device *parent,
if (ret < 0)
return ret;

+ if (led_cdev->color >= LED_COLOR_ID_MAX)
+ dev_warn(parent, "LED %s color identifier out of range\n", final_name);
+
mutex_init(&led_cdev->led_access);
mutex_lock(&led_cdev->led_access);
led_cdev->dev = device_create_with_groups(leds_class, parent, 0,
diff --git a/include/linux/leds.h b/include/linux/leds.h
index 41df18f42d00..9d38cb90331d 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -71,6 +71,7 @@ struct led_classdev {
const char *name;
unsigned int brightness;
unsigned int max_brightness;
+ unsigned int color;
int flags;

/* Lower 16 bits reflect status */
--
2.25.1

2023-01-02 08:55:29

by Jean-Jacques Hiblot

[permalink] [raw]
Subject: [PATCH v7 1/6] devres: provide devm_krealloc_array()

Implement the managed variant of krealloc_array().
This internally uses devm_krealloc() and as such is usable with all memory
allocated by devm_kmalloc() (or devres functions using it implicitly like
devm_kmemdup(), devm_kstrdup() etc.).

Managed realloc'ed chunks can be manually released with devm_kfree().

Signed-off-by: Jean-Jacques Hiblot <[email protected]>
Reviewed-by: Andy Shevchenko <[email protected]>
---
include/linux/device.h | 13 +++++++++++++
1 file changed, 13 insertions(+)

diff --git a/include/linux/device.h b/include/linux/device.h
index 424b55df0272..3b472df6c6cd 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -223,6 +223,19 @@ static inline void *devm_kcalloc(struct device *dev,
{
return devm_kmalloc_array(dev, n, size, flags | __GFP_ZERO);
}
+static inline void *devm_krealloc_array(struct device *dev,
+ void *p,
+ size_t new_n,
+ size_t new_size,
+ gfp_t flags)
+{
+ size_t bytes;
+
+ if (unlikely(check_mul_overflow(new_n, new_size, &bytes)))
+ return NULL;
+
+ return devm_krealloc(dev, p, bytes, flags);
+}
void devm_kfree(struct device *dev, const void *p);
char *devm_kstrdup(struct device *dev, const char *s, gfp_t gfp) __malloc;
const char *devm_kstrdup_const(struct device *dev, const char *s, gfp_t gfp);
--
2.25.1

2023-01-09 17:29:06

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH v7 0/6] Add a multicolor LED driver for groups of monochromatic LEDs

On Mon, 02 Jan 2023, Jean-Jacques Hiblot wrote:
> Some HW design implement multicolor LEDs with several monochromatic LEDs.
> Grouping the monochromatic LEDs allows to configure them in sync and use
> the triggers.
> The PWM multicolor LED driver implements such grouping but only for
> PWM-based LEDs. As this feature is also desirable for the other types of
> LEDs, this series implements it for any kind of LED device.

Another for Pavel.

> changes v6->v7:
> - in led_mcg_probe() increment the counter at the end of the loop for
> clarity.
>
> changes v5->v6:
> - restore sysfs access to the leds when the device is removed
>
> changes v4->v5:
> - Use "depends on COMPILE_TEST || OF" in Kconfig to indicate that OF
> is a functional requirement, not just a requirement for the
> compilation.
> - in led_mcg_probe() check if devm_of_led_get_optional() returns an
> error before testing for the end of the list.
> - use sysfs_emit() instead of sprintf() in color_show().
> - some grammar fixes in the comments and the commit logs.
>
> changes v2->v3, only minor changes:
> - rephrased the Kconfig descritpion
> - make the sysfs interface of underlying LEDs read-only only if the probe
> is successful.
> - sanitize the header files
> - removed the useless call to dev_set_drvdata()
> - use dev_fwnode() to get the fwnode to the device.
>
> changes v1->v2:
> - Followed Rob Herrings's suggestion to make the dt binding much simpler.
> - Added a patch to store the color property of a LED in its class
> structure (struct led_classdev).
>
>
> Jean-Jacques Hiblot (6):
> devres: provide devm_krealloc_array()
> leds: class: simplify the implementation of devm_of_led_get()
> leds: provide devm_of_led_get_optional()
> leds: class: store the color index in struct led_classdev
> dt-bindings: leds: Add binding for a multicolor group of LEDs
> leds: Add a multicolor LED driver to group monochromatic LEDs
>
> Documentation/ABI/testing/sysfs-class-led | 9 +
> .../bindings/leds/leds-group-multicolor.yaml | 64 +++++++
> drivers/leds/led-class.c | 65 +++++--
> drivers/leds/rgb/Kconfig | 10 ++
> drivers/leds/rgb/Makefile | 1 +
> drivers/leds/rgb/leds-group-multicolor.c | 166 ++++++++++++++++++
> include/linux/device.h | 13 ++
> include/linux/leds.h | 3 +
> 8 files changed, 317 insertions(+), 14 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/leds/leds-group-multicolor.yaml
> create mode 100644 drivers/leds/rgb/leds-group-multicolor.c
>
> --
> 2.25.1
>

--
Lee Jones [李琼斯]

2023-02-20 21:28:39

by Jean-Jacques Hiblot

[permalink] [raw]
Subject: Re: [PATCH v7 0/6] Add a multicolor LED driver for groups of monochromatic LEDs

Hi Pavel,

Would you be kind enough and have a look at this series ? It may not be
the prettiest series of all,  but the feature make senses as it provides
an atomic way of handling color leds from user space.

Any feedback would be appreciated.

Thanks,

Jean-Jacques


On 09/01/2023 18:19, Lee Jones wrote:
> On Mon, 02 Jan 2023, Jean-Jacques Hiblot wrote:
>> Some HW design implement multicolor LEDs with several monochromatic LEDs.
>> Grouping the monochromatic LEDs allows to configure them in sync and use
>> the triggers.
>> The PWM multicolor LED driver implements such grouping but only for
>> PWM-based LEDs. As this feature is also desirable for the other types of
>> LEDs, this series implements it for any kind of LED device.
> Another for Pavel.
>
>> changes v6->v7:
>> - in led_mcg_probe() increment the counter at the end of the loop for
>> clarity.
>>
>> changes v5->v6:
>> - restore sysfs access to the leds when the device is removed
>>
>> changes v4->v5:
>> - Use "depends on COMPILE_TEST || OF" in Kconfig to indicate that OF
>> is a functional requirement, not just a requirement for the
>> compilation.
>> - in led_mcg_probe() check if devm_of_led_get_optional() returns an
>> error before testing for the end of the list.
>> - use sysfs_emit() instead of sprintf() in color_show().
>> - some grammar fixes in the comments and the commit logs.
>>
>> changes v2->v3, only minor changes:
>> - rephrased the Kconfig descritpion
>> - make the sysfs interface of underlying LEDs read-only only if the probe
>> is successful.
>> - sanitize the header files
>> - removed the useless call to dev_set_drvdata()
>> - use dev_fwnode() to get the fwnode to the device.
>>
>> changes v1->v2:
>> - Followed Rob Herrings's suggestion to make the dt binding much simpler.
>> - Added a patch to store the color property of a LED in its class
>> structure (struct led_classdev).
>>
>>
>> Jean-Jacques Hiblot (6):
>> devres: provide devm_krealloc_array()
>> leds: class: simplify the implementation of devm_of_led_get()
>> leds: provide devm_of_led_get_optional()
>> leds: class: store the color index in struct led_classdev
>> dt-bindings: leds: Add binding for a multicolor group of LEDs
>> leds: Add a multicolor LED driver to group monochromatic LEDs
>>
>> Documentation/ABI/testing/sysfs-class-led | 9 +
>> .../bindings/leds/leds-group-multicolor.yaml | 64 +++++++
>> drivers/leds/led-class.c | 65 +++++--
>> drivers/leds/rgb/Kconfig | 10 ++
>> drivers/leds/rgb/Makefile | 1 +
>> drivers/leds/rgb/leds-group-multicolor.c | 166 ++++++++++++++++++
>> include/linux/device.h | 13 ++
>> include/linux/leds.h | 3 +
>> 8 files changed, 317 insertions(+), 14 deletions(-)
>> create mode 100644 Documentation/devicetree/bindings/leds/leds-group-multicolor.yaml
>> create mode 100644 drivers/leds/rgb/leds-group-multicolor.c
>>
>> --
>> 2.25.1
>>

2023-02-22 15:05:02

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH v7 0/6] Add a multicolor LED driver for groups of monochromatic LEDs

On Mon, 20 Feb 2023, Jean-Jacques Hiblot wrote:

> Hi Pavel,
>
> Would you be kind enough and have a look at this series ? It may not be the
> prettiest series of all,  but the feature make senses as it provides an
> atomic way of handling color leds from user space.
>
> Any feedback would be appreciated.

If Pavel doesn't get back to you soon, I'll do my best to take a look
through.

> On 09/01/2023 18:19, Lee Jones wrote:
> > On Mon, 02 Jan 2023, Jean-Jacques Hiblot wrote:
> > > Some HW design implement multicolor LEDs with several monochromatic LEDs.
> > > Grouping the monochromatic LEDs allows to configure them in sync and use
> > > the triggers.
> > > The PWM multicolor LED driver implements such grouping but only for
> > > PWM-based LEDs. As this feature is also desirable for the other types of
> > > LEDs, this series implements it for any kind of LED device.
> > Another for Pavel.
> > > changes v6->v7:
> > > - in led_mcg_probe() increment the counter at the end of the loop for
> > > clarity.
> > >
> > > changes v5->v6:
> > > - restore sysfs access to the leds when the device is removed
> > >
> > > changes v4->v5:
> > > - Use "depends on COMPILE_TEST || OF" in Kconfig to indicate that OF
> > > is a functional requirement, not just a requirement for the
> > > compilation.
> > > - in led_mcg_probe() check if devm_of_led_get_optional() returns an
> > > error before testing for the end of the list.
> > > - use sysfs_emit() instead of sprintf() in color_show().
> > > - some grammar fixes in the comments and the commit logs.
> > >
> > > changes v2->v3, only minor changes:
> > > - rephrased the Kconfig descritpion
> > > - make the sysfs interface of underlying LEDs read-only only if the probe
> > > is successful.
> > > - sanitize the header files
> > > - removed the useless call to dev_set_drvdata()
> > > - use dev_fwnode() to get the fwnode to the device.
> > >
> > > changes v1->v2:
> > > - Followed Rob Herrings's suggestion to make the dt binding much simpler.
> > > - Added a patch to store the color property of a LED in its class
> > > structure (struct led_classdev).
> > >
> > >
> > > Jean-Jacques Hiblot (6):
> > > devres: provide devm_krealloc_array()
> > > leds: class: simplify the implementation of devm_of_led_get()
> > > leds: provide devm_of_led_get_optional()
> > > leds: class: store the color index in struct led_classdev
> > > dt-bindings: leds: Add binding for a multicolor group of LEDs
> > > leds: Add a multicolor LED driver to group monochromatic LEDs
> > >
> > > Documentation/ABI/testing/sysfs-class-led | 9 +
> > > .../bindings/leds/leds-group-multicolor.yaml | 64 +++++++
> > > drivers/leds/led-class.c | 65 +++++--
> > > drivers/leds/rgb/Kconfig | 10 ++
> > > drivers/leds/rgb/Makefile | 1 +
> > > drivers/leds/rgb/leds-group-multicolor.c | 166 ++++++++++++++++++
> > > include/linux/device.h | 13 ++
> > > include/linux/leds.h | 3 +
> > > 8 files changed, 317 insertions(+), 14 deletions(-)
> > > create mode 100644 Documentation/devicetree/bindings/leds/leds-group-multicolor.yaml
> > > create mode 100644 drivers/leds/rgb/leds-group-multicolor.c
> > >
> > > --
> > > 2.25.1
> > >

--
Lee Jones [李琼斯]

2023-03-15 15:54:35

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH v7 0/6] Add a multicolor LED driver for groups of monochromatic LEDs

On Mon, 02 Jan 2023, Jean-Jacques Hiblot wrote:

>
> Some HW design implement multicolor LEDs with several monochromatic LEDs.
> Grouping the monochromatic LEDs allows to configure them in sync and use
> the triggers.
> The PWM multicolor LED driver implements such grouping but only for
> PWM-based LEDs. As this feature is also desirable for the other types of
> LEDs, this series implements it for any kind of LED device.
>
> changes v6->v7:
> - in led_mcg_probe() increment the counter at the end of the loop for
> clarity.
>
> changes v5->v6:
> - restore sysfs access to the leds when the device is removed
>
> changes v4->v5:
> - Use "depends on COMPILE_TEST || OF" in Kconfig to indicate that OF
> is a functional requirement, not just a requirement for the
> compilation.
> - in led_mcg_probe() check if devm_of_led_get_optional() returns an
> error before testing for the end of the list.
> - use sysfs_emit() instead of sprintf() in color_show().
> - some grammar fixes in the comments and the commit logs.
>
> changes v2->v3, only minor changes:
> - rephrased the Kconfig descritpion
> - make the sysfs interface of underlying LEDs read-only only if the probe
> is successful.
> - sanitize the header files
> - removed the useless call to dev_set_drvdata()
> - use dev_fwnode() to get the fwnode to the device.
>
> changes v1->v2:
> - Followed Rob Herrings's suggestion to make the dt binding much simpler.
> - Added a patch to store the color property of a LED in its class
> structure (struct led_classdev).
>
>
> Jean-Jacques Hiblot (6):
> devres: provide devm_krealloc_array()
> leds: class: simplify the implementation of devm_of_led_get()
> leds: provide devm_of_led_get_optional()
> leds: class: store the color index in struct led_classdev
> dt-bindings: leds: Add binding for a multicolor group of LEDs
> leds: Add a multicolor LED driver to group monochromatic LEDs
>
> Documentation/ABI/testing/sysfs-class-led | 9 +
> .../bindings/leds/leds-group-multicolor.yaml | 64 +++++++
> drivers/leds/led-class.c | 65 +++++--
> drivers/leds/rgb/Kconfig | 10 ++
> drivers/leds/rgb/Makefile | 1 +
> drivers/leds/rgb/leds-group-multicolor.c | 166 ++++++++++++++++++
> include/linux/device.h | 13 ++
> include/linux/leds.h | 3 +
> 8 files changed, 317 insertions(+), 14 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/leds/leds-group-multicolor.yaml
> create mode 100644 drivers/leds/rgb/leds-group-multicolor.c

Most patches are good to go.

Once you've fixed up patch 6, I'll apply the set.

--
Lee Jones [李琼斯]