leds: Add a virtual LED driver for groups of

From: Jonathan Brophy <[email protected]>

Document Virtual Color device tree bindings.

Co-developed-by Radoslav Tsvetkov <[email protected]>

Signed-off-by Radoslav Tsvetkov <[email protected]>

Signed-off-by Jonathan Brophy <[email protected]>

---
 .../leds/leds-group-virtualcolor.yaml         | 100 ++++++++++++++++++
 1 file changed, 100 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/leds-group-virtualcolor.yaml

Documentation/devicetree/bindings/leds/leds-group-virtualcolor.yaml new file +5 -0

new file mode 100644
index 000000000000..bafdd8fb9557
@@ -0,0 +1,100 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/leds/leds-group-virtualcolor.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#

Documentation/devicetree/bindings/leds/leds-group-virtualcolor.yaml new file +11 -0

+
+allOf:
+  - $ref: common.yaml#
+
+properties:
+  compatible:
+    const: leds-group-virtualcolor
+
+  '#address-cells':
+    const: 1
+

Documentation/devicetree/bindings/leds/leds-group-virtualcolor.yaml new file +20 -0

+    properties:
+      reg:
+        maxItems: 1
+        description: Virtual LED number
+
+      leds:
+        $ref: /schemas/types.yaml#/definitions/phandle-array
+        description: List of phandles to the monochromatic LEDs to group
+
+      function:
+        description: |
+          For virtualcolor LEDs this property should be defined as
+          LED_FUNCTION_VIRTUAL_STATUS as outlined in:
+          include/dt-bindings/leds/common.h.
+
+      priority:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        description: Priority level for LED activation
+          (higher value means higher priority)
+

Quoted 3 lines

From: Jonathan Brophy <[email protected]>

Document Virtual Color device tree bindings.

I don't see how you answered my comment about missing justification.

Rob's questions also were not answered.

Few minor things follow up, but considering missing reasoning I did not perform full review.

A nit, subject: drop second/last, redundant "YAML bindings for". The "dt-bindings" prefix is already stating that these are bindings. See also:
https://elixir.bootlin.com/linux/v6.17-rc3/source/Documentation/devicetree/bindings/submitting-patches.rst#L18

... and driver. Again - explain the hardware. Bindings are not for driver.

Documentation/devicetree/bindings/leds/leds-group-virtualcolor.yaml line 15

Quoted 5 lines

Documentation/devicetree/bindings/leds/leds-group-virtualcolor.yaml new file +4 -0

+description: |
+  Bindings to show how to achieve logically grouped virtual LEDs.
+  The nodes and properties defined in this document are unique to the
+  virtualcolor LED class.

That's completely redundant statement.

Quoted 2 lines

+ Common LED nodes and properties are inherited from the common.yaml
+ within this documentation directory

As well drop. Your description is pretty obvious and does not help at all.

Quoted 21 lines

Documentation/devicetree/bindings/leds/leds-group-virtualcolor.yaml new file +21 -0

+    properties:
+      reg:
+        maxItems: 1
+        description: Virtual LED number
+
+      leds:
+        $ref: /schemas/types.yaml#/definitions/phandle-array
+        description: List of phandles to the monochromatic LEDs to 
+ group
+
+      function:
+        description: |
+          For virtualcolor LEDs this property should be defined as
+          LED_FUNCTION_VIRTUAL_STATUS as outlined in:
+          include/dt-bindings/leds/common.h.
+
+      priority:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        description: Priority level for LED activation
+          (higher value means higher priority)
+

Documentation/devicetree/bindings/leds/leds-group-virtualcolor.yaml line 68

Quoted 10 lines

Documentation/devicetree/bindings/leds/leds-group-virtualcolor.yaml new file +10 -0

+      blink-delay-on:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        description: Time in milliseconds the LED is on during blink
+
+      blink-delay-off:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        description: Time in milliseconds the LED is off during blink
+        note: Setting just one of the blink delays to a valid value while
+          setting the other to null will cause the LED to operate with a one-shot
+          on or off delay instead of a repeat cycle.

And drop all above, except reg and leds. If these are new properties, then you need to use proper unit suffixes.

https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/property-units.yaml

Quoted 7 lines

Few minor things follow up, but considering missing reasoning I did not perform full review.

A nit, subject: drop second/last, redundant "YAML bindings for". The "dt-bindings" prefix is already stating that these are bindings. See also:
https://elixir.bootlin.com/linux/v6.17-rc3/source/Documentation/devicetree/bindings/submitting-patches.rst#L18

... and driver. Again - explain the hardware. Bindings are not for driver.

I'm kind a little bit confused what you mean by this statement.

I'm guessing I should omit hardware info in the class yaml and move it to a group yaml like the multicolor ones as below?

If so that is just a mistake on my part not knowing the file structure well.

https://www.kernel.org/doc/Documentation/devicetree/bindings/leds/leds-class-multicolor.yaml
https://elixir.bootlin.com/linux/v6.17.1/source/Documentation/devicetree/bindings/leds/leds-group-multicolor.yaml

Quoted 7 lines

Documentation/devicetree/bindings/leds/leds-group-virtualcolor.yaml line 15

Quoted 5 lines

Documentation/devicetree/bindings/leds/leds-group-virtualcolor.yaml new file +4 -0

+description: |
+  Bindings to show how to achieve logically grouped virtual LEDs.
+  The nodes and properties defined in this document are unique to the
+  virtualcolor LED class.

That's completely redundant statement.

Ok fair enough, but I basically cloned this comment from the leds-group-multicolor as they have something simular.

Quoted 4 lines

Quoted 2 lines

+ Common LED nodes and properties are inherited from the common.yaml
+ within this documentation directory

As well drop. Your description is pretty obvious and does not help at all.

Ok thanks

Quoted 35 lines

Quoted 21 lines

Documentation/devicetree/bindings/leds/leds-group-virtualcolor.yaml new file +21 -0

+    properties:
+      reg:
+        maxItems: 1
+        description: Virtual LED number
+
+      leds:
+        $ref: /schemas/types.yaml#/definitions/phandle-array
+        description: List of phandles to the monochromatic LEDs to 
+ group
+
+      function:
+        description: |
+          For virtualcolor LEDs this property should be defined as
+          LED_FUNCTION_VIRTUAL_STATUS as outlined in:
+          include/dt-bindings/leds/common.h.
+
+      priority:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        description: Priority level for LED activation
+          (higher value means higher priority)
+

Documentation/devicetree/bindings/leds/leds-group-virtualcolor.yaml line 68

Quoted 10 lines

Documentation/devicetree/bindings/leds/leds-group-virtualcolor.yaml new file +10 -0

+      blink-delay-on:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        description: Time in milliseconds the LED is on during blink
+
+      blink-delay-off:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        description: Time in milliseconds the LED is off during blink
+        note: Setting just one of the blink delays to a valid value while
+          setting the other to null will cause the LED to operate with a one-shot
+          on or off delay instead of a repeat cycle.

And drop all above, except reg and leds. If these are new properties, then you need to use proper unit suffixes.

Quoted 1 line

https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/property-units.yaml

Thanks for pointing this out I guessed there was a definition's somewhere,

From: Jonathan Brophy <[email protected]>

Document Virtual Color device tree bindings.

Co-developed-by Radoslav Tsvetkov <[email protected]>

Signed-off-by Radoslav Tsvetkov <[email protected]>

Signed-off-by Jonathan Brophy <[email protected]>

---
 .../leds/leds-group-virtualcolor.yaml         | 100 ++++++++++++++++++
 1 file changed, 100 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/leds-group-virtualcolor.yaml

From: Jonathan Brophy <[email protected]>

Introduces a new driver that implements virtual LED groups, aggregating multiple monochromatic LEDs into virtual groups and providing priority-based control for concurrent state management.

Author: Radoslav Tsvetkov <[email protected]>
Co Author: Jonathan Brophy <[email protected]> Copyright (C) 2024 Jonathan Brophy <[email protected]>

Co-developed-by Radoslav Tsvetkov <[email protected]>

Signed-off-by Radoslav Tsvetkov <[email protected]>

Tested-by Jonathan Brophy <[email protected]>

Signed-off-by Jonathan Brophy <[email protected]>

---
 drivers/leds/rgb/Kconfig                   |  17 +
 drivers/leds/rgb/Makefile                  |   1 +
 drivers/leds/rgb/leds-group-virtualcolor.c | 439 +++++++++++++++++++++
 3 files changed, 457 insertions(+)
 create mode 100644 drivers/leds/rgb/leds-group-virtualcolor.c

drivers/leds/rgb/leds-group-virtualcolor.c new file +5 -0

new file mode 100644
index 000000000000..e11ad155d3b4
@@ -0,0 +1,439 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Virtual LED Group Driver with Priority Control
+ *
+ * Implements virtual LED groups by aggregating multiple

drivers/leds/rgb/leds-group-virtualcolor.c new file +28 -0

+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+
+struct virtual_led {
+	struct led_classdev cdev;
+	struct led_classdev **monochromatics;
+	struct leds_virtualcolor *vc_data;
+	int num_monochromatics;
+	int priority;
+	unsigned long blink_delay_on;
+	unsigned long blink_delay_off;
+	struct list_head list;
+};
+
+struct leds_virtualcolor {
+	struct virtual_led *vleds;
+	int num_vleds;
+	struct list_head active_leds;
+	struct mutex lock; // Protects access to active LEDs
+};
+
+static void virtual_set_monochromatic_brightness(struct virtual_led *vled,
+						 enum led_brightness brightness)
+{
+	int i;

drivers/leds/rgb/leds-group-virtualcolor.c new file +9 -0

+			led_blink_set(&vled->cdev, &vled->blink_delay_on, &vled->blink_delay_off);
+
+		/* Update the blink delays if they have changed */
+		if (vled->blink_delay_on != vled->cdev.blink_delay_on ||
+		    vled->blink_delay_off != vled->cdev.blink_delay_off) {
+			vled->cdev.blink_delay_on = vled->blink_delay_on;
+			vled->cdev.blink_delay_off = vled->blink_delay_off;
+		}
+	}

drivers/leds/rgb/leds-group-virtualcolor.c new file +4 -0

+}
+
+static ssize_t priority_store(struct device *dev, struct device_attribute *attr, const char *buf,
+			      size_t count)

drivers/leds/rgb/leds-group-virtualcolor.c new file +31 -0

+
+	ret = of_property_read_u32(child, "priority", &vled->priority);
+	if (ret)
+		vled->priority = 0;
+
+	ret = of_property_read_u32(child, "blink", &blink_interval);
+	if (!ret) {
+		vled->blink_delay_on = blink_interval;
+		vled->blink_delay_off = blink_interval;
+	}
+
+	phandle_count = of_property_count_elems_of_size(child, "leds", sizeof(u32));
+	if (phandle_count <= 0) {
+		dev_err(dev, "No monochromatic LEDs specified for virtual LED %s\n",
+			vled->cdev.name);
+		return -EINVAL;
+	}
+
+	vled->num_monochromatics = phandle_count;
+	vled->monochromatics = devm_kcalloc(dev, vled->num_monochromatics,
+					    sizeof(*vled->monochromatics), GFP_KERNEL);
+	if (!vled->monochromatics)
+		return -ENOMEM;
+
+	for (i = 0; i < vled->num_monochromatics; i++) {
+		struct led_classdev *led_cdev;
+
+		led_cdev = devm_of_led_get_optional(dev, i);
+		if (IS_ERR(led_cdev)) {
+			/*
+			 * If the LED is not available yet, defer the probe.

drivers/leds/rgb/leds-group-virtualcolor.c new file +4 -0

+
+		vled->monochromatics[i] = led_cdev;
+	}
+

drivers/leds/rgb/leds-group-virtualcolor.c new file +16 -0

+
+	ret = device_create_file(vled->cdev.dev, &dev_attr_blink_delay_on);
+	if (ret) {
+		dev_err(dev, "Failed to create sysfs attribute for blink_delay_on\n");
+		return ret;
+	}
+
+	ret = device_create_file(vled->cdev.dev, &dev_attr_blink_delay_off);
+	if (ret) {
+		dev_err(dev, "Failed to create sysfs attribute for blink_delay_off\n");
+		return ret;
+	}
+
+	vled->vc_data = vc_data;
+
+	return 0;

drivers/leds/rgb/leds-group-virtualcolor.c new file +8 -0

+	}
+
+	return 0;
+}
+
+static int leds_virtualcolor_probe(struct platform_device *pdev)
+{
+	struct leds_virtualcolor *vc_data;

drivers/leds/rgb/leds-group-virtualcolor.c new file +7 -0

+		struct virtual_led *vled = &vc_data->vleds[count];
+
+		ret = leds_virtualcolor_init_vled(dev, child, vled, vc_data);
+		if (ret) {
+			if (ret != -EPROBE_DEFER)
+				dev_err(dev, "Failed to initialize virtual LED %d\n", count);
+

drivers/leds/rgb/leds-group-virtualcolor.c new file +8 -0

+		int i;
+
+		for (i = 0; i < count; i++) {
+			struct virtual_led *vled = &vc_data->vleds[i];
+
+			ret = leds_virtualcolor_disable_sysfs_access(dev, vled);
+			if (ret)
+				goto err_node_put;

drivers/leds/rgb/leds-group-virtualcolor.c new file +6 -0

+	mutex_destroy(&vc_data->lock);
+
+	return ret;
+}
+
+static void leds_virtualcolor_remove(struct platform_device *pdev)

drivers/leds/rgb/leds-group-virtualcolor.c new file +6 -0

+			}
+		}
+	}
+
+	mutex_destroy(&vc_data->lock);
+}