[PATCH 1/4] leds: Add Virtual Color LED Group driver

[PATCH 1/4] leds: Add Virtual Color LED Group driver

[Jonathan Brophy]

From: Jonathan Brophy <professor_jonny@hotmail.com>

This commit introduces a new driver that implements virtual LED groups
by aggregating multiple monochromatic LEDs. The driver provides
priority-based control to manage concurrent LED activation requests,
ensuring that only the highest-priority LED group's state is active at
any given time.

This driver is useful for systems that require coordinated control over
multiple LEDs, such as RGB indicators or status LEDs that reflect
complex system states.

Co-developed-by: Radoslav Tsvetkov <rtsvetkov@gradotech.eu>
Signed-off-by: Jonathan Brophy <professor_jonny@hotmail.com>
---
 drivers/leds/rgb/Kconfig                   |  17 +
 drivers/leds/rgb/Makefile                  |   1 +
 drivers/leds/rgb/leds-group-virtualcolor.c | 440 +++++++++++++++++++++
 3 files changed, 458 insertions(+)
 create mode 100644 drivers/leds/rgb/leds-group-virtualcolor.c

diff --git a/drivers/leds/rgb/Kconfig b/drivers/leds/rgb/Kconfig
index 222d943d826a..70a80fd46b9c 100644
--- a/drivers/leds/rgb/Kconfig
+++ b/drivers/leds/rgb/Kconfig
@@ -75,4 +75,21 @@ config LEDS_MT6370_RGB
 	  This driver can also be built as a module. If so, the module
 	  will be called "leds-mt6370-rgb".
 
+config LEDS_GROUP_VIRTUALCOLOR
+	tristate "Virtual LED Group Driver with Priority Control"
+	depends on OF || COMPILE_TEST
+	help
+	  This option enables support for virtual LED groups that aggregate
+	  multiple monochromatic LEDs with priority-based control. It allows
+	  managing concurrent LED activation requests by ensuring only the
+	  highest-priority LED state is active at any given time.
+
+	  Multiple LEDs can be grouped together and controlled as a single
+	  virtual LED with priority levels and blinking support. This is
+	  useful for systems that need to manage multiple LED indicators
+	  with different priority levels.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called leds-group-virtualcolor.
+
 endif # LEDS_CLASS_MULTICOLOR
diff --git a/drivers/leds/rgb/Makefile b/drivers/leds/rgb/Makefile
index a501fd27f179..693fd300b849 100644
--- a/drivers/leds/rgb/Makefile
+++ b/drivers/leds/rgb/Makefile
@@ -6,3 +6,4 @@ obj-$(CONFIG_LEDS_NCP5623)		+= leds-ncp5623.o
 obj-$(CONFIG_LEDS_PWM_MULTICOLOR)	+= leds-pwm-multicolor.o
 obj-$(CONFIG_LEDS_QCOM_LPG)		+= leds-qcom-lpg.o
 obj-$(CONFIG_LEDS_MT6370_RGB)		+= leds-mt6370-rgb.o
+obj-$(CONFIG_LEDS_GROUP_VIRTUALCOLOR)	+= leds-group-virtualcolor.o
diff --git a/drivers/leds/rgb/leds-group-virtualcolor.c b/drivers/leds/rgb/leds-group-virtualcolor.c
new file mode 100644
index 000000000000..f9e13fe255dd
--- /dev/null
+++ b/drivers/leds/rgb/leds-group-virtualcolor.c
@@ -0,0 +1,440 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Virtual LED Group Driver with Priority Control
+ *
+ * This driver implements virtual LED groups by aggregating multiple
+ * monochromatic LEDs. It provides priority-based control for managing
+ * concurrent LED activation requests, ensuring only the highest-priority
+ * LED state is active at any given time.
+ *
+ * Code create by Radoslav Tsvetkov <rtsvetkov@gradotech.eu>
+ * Copyright (C) 2024 Jonathan Brophy <professor_jonny@hotmail.com>
+ *
+ */
+
+#include <linux/gpio/consumer.h>
+#include <linux/leds.h>
+#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;
+
+	if (vled->blink_delay_on || vled->blink_delay_off) {
+		unsigned long blink_mask = (BIT(LED_BLINK_SW) | BIT(LED_BLINK_ONESHOT) |
+					    BIT(LED_SET_BLINK));
+
+		/*
+		 * Make sure the LED is not already blinking.
+		 * We don't want to call led_blink_set multiple times.
+		 */
+		if (!(vled->cdev.work_flags & blink_mask))
+			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;
+		}
+	}
+
+	for (i = 0; i < vled->num_monochromatics; i++)
+		led_set_brightness(vled->monochromatics[i], brightness);
+}
+
+static ssize_t priority_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct virtual_led *vled = dev_get_drvdata(dev);
+
+	return sprintf(buf, "%d\n", vled->priority);
+}
+
+static ssize_t priority_store(struct device *dev, struct device_attribute *attr, const char *buf,
+			      size_t count)
+{
+	struct virtual_led *vled = dev_get_drvdata(dev);
+	int new_priority;
+	int ret;
+
+	ret = kstrtoint(buf, 10, &new_priority);
+	if (ret < 0)
+		return ret;
+
+	vled->priority = new_priority;
+	return count;
+}
+
+static DEVICE_ATTR_RW(priority);
+
+static ssize_t blink_delay_on_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct virtual_led *vled = dev_get_drvdata(dev);
+
+	return sprintf(buf, "%lu\n", vled->blink_delay_on);
+}
+
+static ssize_t blink_delay_on_store(struct device *dev, struct device_attribute *attr,
+				    const char *buf, size_t count)
+{
+	struct virtual_led *vled = dev_get_drvdata(dev);
+	unsigned long new_delay;
+	int ret;
+
+	ret = kstrtoul(buf, 10, &new_delay);
+	if (ret < 0)
+		return ret;
+
+	/* Apply new delay immediately */
+	vled->blink_delay_on = new_delay;
+	virtual_set_monochromatic_brightness(vled, vled->cdev.brightness);
+
+	return count;
+}
+
+static ssize_t blink_delay_off_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct virtual_led *vled = dev_get_drvdata(dev);
+
+	return sprintf(buf, "%lu\n", vled->blink_delay_off);
+}
+
+static ssize_t blink_delay_off_store(struct device *dev, struct device_attribute *attr,
+				     const char *buf, size_t count)
+{
+	struct virtual_led *vled = dev_get_drvdata(dev);
+	unsigned long new_delay;
+	int ret;
+
+	ret = kstrtoul(buf, 10, &new_delay);
+	if (ret < 0)
+		return ret;
+
+	/* Apply new delay immediately */
+	vled->blink_delay_off = new_delay;
+	virtual_set_monochromatic_brightness(vled, vled->cdev.brightness);
+
+	return count;
+}
+
+static DEVICE_ATTR_RW(blink_delay_on);
+static DEVICE_ATTR_RW(blink_delay_off);
+
+static void restore_sysfs_write_access(void *data)
+{
+	struct led_classdev *led_cdev = data;
+
+	mutex_lock(&led_cdev->led_access);
+	led_sysfs_enable(led_cdev);
+	mutex_unlock(&led_cdev->led_access);
+}
+
+static bool virtual_led_is_active(struct list_head *head, struct virtual_led *vled)
+{
+	struct virtual_led *entry;
+
+	list_for_each_entry(entry, head, list) {
+		if (entry == vled)
+			return true;
+	}
+
+	return false;
+}
+
+static int virtual_led_brightness_set(struct led_classdev *cdev, enum led_brightness brightness)
+{
+	struct virtual_led *vled = container_of(cdev, struct virtual_led, cdev);
+	struct leds_virtualcolor *vc_data = vled->vc_data;
+	struct virtual_led *active;
+
+	mutex_lock(&vc_data->lock);
+
+	active = list_first_entry_or_null(&vc_data->active_leds, struct virtual_led, list);
+	if (active) {
+		/*
+		 * If the currently active LED has a higher priority,
+		 * ignore the new request.
+		 */
+		if (active->priority > vled->priority)
+			goto out_unlock;
+
+		/*
+		 * The currently active LED is going to be replaced,
+		 * turn off it's monochromatic LEDs.
+		 */
+		virtual_set_monochromatic_brightness(active, LED_OFF);
+	}
+
+	if (brightness == LED_OFF) {
+		/*
+		 * If the LED is already active, remove it from the active list
+		 * and update the brightness of the next highest priority LED.
+		 */
+		if (virtual_led_is_active(&vc_data->active_leds, vled))
+			list_del(&vled->list);
+
+		active = list_first_entry_or_null(&vc_data->active_leds, struct virtual_led, list);
+		if (active)
+			virtual_set_monochromatic_brightness(active, active->cdev.brightness);
+	} else {
+		/* Add the LED to the active list and update the brightness */
+		if (!virtual_led_is_active(&vc_data->active_leds, vled))
+			list_add(&vled->list, &vc_data->active_leds);
+
+		active = list_first_entry_or_null(&vc_data->active_leds, struct virtual_led, list);
+		if (active)
+			virtual_set_monochromatic_brightness(active, brightness);
+	}
+
+out_unlock:
+	mutex_unlock(&vc_data->lock);
+
+	return 0;
+}
+
+static int leds_virtualcolor_init_vled(struct device *dev, struct device_node *child,
+				       struct virtual_led *vled, struct leds_virtualcolor *vc_data)
+{
+	struct fwnode_handle *child_fwnode = of_fwnode_handle(child);
+	struct led_init_data init_data = {};
+	u32 blink_interval;
+	u32 phandle_count;
+	u32 max_brightness;
+	int ret, i;
+
+	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 = fwnode_property_count_u32(child_fwnode, "leds");
+	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.
+			 * The probe will be retried when it becomes available.
+			 */
+			if (PTR_ERR(led_cdev) == -EPROBE_DEFER)
+				return -EPROBE_DEFER;
+
+			ret = PTR_ERR(led_cdev);
+			dev_err(dev, "Failed to get monochromatic LED for %s, error %d\n",
+				vled->cdev.name, ret);
+			return ret;
+		}
+
+		vled->monochromatics[i] = led_cdev;
+	}
+
+	ret = of_property_read_u32(child, "max-brightness", &max_brightness);
+	if (ret)
+		vled->cdev.max_brightness = LED_FULL;
+	else
+		vled->cdev.max_brightness = max_brightness;
+
+	vled->cdev.brightness_set_blocking = virtual_led_brightness_set;
+	vled->cdev.max_brightness = LED_FULL;
+	vled->cdev.flags = LED_CORE_SUSPENDRESUME;
+
+	init_data.fwnode = child_fwnode;
+	ret = devm_led_classdev_register_ext(dev, &vled->cdev, &init_data);
+	if (ret) {
+		dev_err(dev, "Failed to register virtual LED %s\n", vled->cdev.name);
+		return ret;
+	}
+
+	ret = device_create_file(vled->cdev.dev, &dev_attr_priority);
+	if (ret) {
+		dev_err(dev, "Failed to create sysfs attribute for priority\n");
+		return ret;
+	}
+
+	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;
+}
+
+static int leds_virtualcolor_disable_sysfs_access(struct device *dev, struct virtual_led *vled)
+{
+	int i;
+
+	for (i = 0; i < vled->num_monochromatics; i++) {
+		struct led_classdev *led_cdev = vled->monochromatics[i];
+
+		mutex_lock(&led_cdev->led_access);
+		led_sysfs_disable(led_cdev);
+		mutex_unlock(&led_cdev->led_access);
+
+		devm_add_action_or_reset(dev, restore_sysfs_write_access, led_cdev);
+	}
+
+	return 0;
+}
+
+static int leds_virtualcolor_probe(struct platform_device *pdev)
+{
+	struct leds_virtualcolor *vc_data;
+	struct device *dev = &pdev->dev;
+	struct device_node *child;
+	int count = 0;
+	int ret;
+
+	vc_data = devm_kzalloc(dev, sizeof(*vc_data), GFP_KERNEL);
+	if (!vc_data)
+		return -ENOMEM;
+
+	mutex_init(&vc_data->lock);
+	INIT_LIST_HEAD(&vc_data->active_leds);
+
+	vc_data->num_vleds = of_get_child_count(dev->of_node);
+	if (vc_data->num_vleds == 0) {
+		dev_err(dev, "No virtual LEDs defined in device tree\n");
+		ret = -EINVAL;
+		goto err_mutex_destroy;
+	}
+
+	vc_data->vleds = devm_kcalloc(dev, vc_data->num_vleds, sizeof(*vc_data->vleds), GFP_KERNEL);
+	if (!vc_data->vleds) {
+		ret = -ENOMEM;
+		goto err_mutex_destroy;
+	}
+
+	for_each_child_of_node(dev->of_node, child) {
+		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);
+
+			of_node_put(child);
+			goto err_node_put;
+		}
+
+		count++;
+	}
+
+	platform_set_drvdata(pdev, vc_data);
+
+	if (of_property_read_bool(dev->of_node, "monochromatics-ro")) {
+		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;
+		}
+	}
+
+	return 0;
+
+err_node_put:
+	of_node_put(child);
+err_mutex_destroy:
+	mutex_destroy(&vc_data->lock);
+
+	return ret;
+}
+
+static void leds_virtualcolor_remove(struct platform_device *pdev)
+{
+	struct leds_virtualcolor *vc_data = platform_get_drvdata(pdev);
+	int i;
+
+	for (i = 0; i < vc_data->num_vleds; i++) {
+		struct virtual_led *vled = &vc_data->vleds[i];
+		int j;
+
+		device_remove_file(vled->cdev.dev, &dev_attr_priority);
+		device_remove_file(vled->cdev.dev, &dev_attr_blink_delay_on);
+		device_remove_file(vled->cdev.dev, &dev_attr_blink_delay_off);
+
+		for (j = 0; j < vled->num_monochromatics; j++) {
+			if (vled->monochromatics[j]) {
+				led_put(vled->monochromatics[j]);
+				vled->monochromatics[j] = NULL;
+			}
+		}
+	}
+
+	mutex_destroy(&vc_data->lock);
+}
+
+static const struct of_device_id leds_virtualcolor_of_match[] = {
+	{ .compatible = "leds-group-virtualcolor" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, leds_virtualcolor_of_match);
+
+static struct platform_driver leds_virtualcolor_driver = {
+	.probe  = leds_virtualcolor_probe,
+	.remove = leds_virtualcolor_remove,
+	.driver = {
+		.name           = "leds_virtualcolor",
+		.of_match_table = leds_virtualcolor_of_match,
+	},
+};
+
+module_platform_driver(leds_virtualcolor_driver);
+
+MODULE_AUTHOR("Radoslav Tsvetkov <rtsvetkov@gradotech.eu>");
+MODULE_DESCRIPTION("LEDs Virtual Color Driver with Priority Handling");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:leds-group-virtualcolor");
-- 
2.43.0

[PATCH 2/4] dt-bindings: leds: Add YAML bindings for Virtual Color LED Group driver

[Jonathan Brophy]

From: Jonathan Brophy <professor_jonny@hotmail.com>

Document Virtual Color device tree bindings.

Co Signed-off-by: Radoslav Tsvetkov <rtsvetkov@gradotech.eu>
Signed-off-by: Jonathan Brophy <professor_jonny@hotmail.com>
---
 .../leds/leds-group-virtualcolor.yaml         | 88 +++++++++++++++++++
 1 file changed, 88 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/leds-group-virtualcolor.yaml

diff --git a/Documentation/devicetree/bindings/leds/leds-group-virtualcolor.yaml b/Documentation/devicetree/bindings/leds/leds-group-virtualcolor.yaml
new file mode 100644
index 000000000..945058415
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/leds-group-virtualcolor.yaml
@@ -0,0 +1,88 @@
+# 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#
+title: Virtual LED Group with Priority Control
+
+maintainers:
+  - Radoslav Tsvetkov <rtsvetkov@gradotech.eu>
+
+description: |
+  Virtual LED group driver that combines multiple monochromatic LEDs into
+  logical groups with priority-based control. The driver ensures only the
+  highest-priority LED state is active at any given time.
+
+allOf:
+  - $ref: common.yaml#
+
+properties:
+  compatible:
+    const: leds-group-virtualcolor
+
+  '#address-cells':
+    const: 1
+
+  '#size-cells':
+    const: 0
+
+required:
+  - compatible
+
+patternProperties:
+  '^led@[0-9a-f]$':
+    type: object
+    $ref: common.yaml#
+    properties:
+      reg:
+        maxItems: 1
+        description: Virtual LED number
+
+      monochromatic-leds:
+        $ref: /schemas/types.yaml#/definitions/phandle
+        description: List of phandles to the monochromatic LEDs to group
+
+      priority:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        description: Priority level for LED activation
+                     (higher value means higher priority)
+
+      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
+
+    required:
+      - reg
+      - monochromatic-leds
+
+additionalProperties: false
+
+examples:
+  - |
+    led-controller {
+        compatible = "leds-group-virtualcolor";
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        led@0 {
+            reg = <0>;
+            label = "status:red";
+            monochromatic-leds = <&led_red>;
+            priority = <2>;
+            blink-delay-on = <500>;
+            blink-delay-off = <500>;
+        };
+
+        led@1 {
+            reg = <1>;
+            label = "status:green";
+            monochromatic-leds = <&led_green>;
+            priority = <1>;
+        };
+    };
+
\ No newline at end of file
--
2.43.0

[PATCH 3/4] ABI: sysfs-class-leds-virtualcolor: Document sysfs entries for Virtual Color LEDs

[Jonathan Brophy]

From: Jonathan Brophy <professor_jonny@hotmail.com>

Add sysfs-class-leds-virtualcolor to document Virtual Color drover sysfs
entries

Co-developed-by: Radoslav Tsvetkov <rtsvetkov@gradotech.eu>
Signed-off-by: Jonathan Brophy <professor_jonny@hotmail.com>
---
 .../ABI/sysfs-class-leds-virtualcolor         | 43 +++++++++++++++++++
 1 file changed, 43 insertions(+)
 create mode 100644 Documentation/ABI/sysfs-class-leds-virtualcolor

diff --git a/Documentation/ABI/sysfs-class-leds-virtualcolor b/Documentation/ABI/sysfs-class-leds-virtualcolor
new file mode 100644
index 000000000..60b878791
--- /dev/null
+++ b/Documentation/ABI/sysfs-class-leds-virtualcolor
@@ -0,0 +1,43 @@
+What:		/sys/class/leds/<led>/priority
+Date:		August 2025
+Contact:	Radoslav Tsvetkov <rtsvetkov@gradotech.eu>
+Description:	(RW) Priority level of the virtual LED group. Higher numbers
+        indicate higher priority. When multiple virtual LED groups are
+        active, only the highest priority group's state will be applied
+        to the physical LEDs.
+
+        Valid values: 0 to INT_MAX
+        Default: 0
+
+What:		/sys/class/leds/<led>/blink_delay_on
+Date:		August 2025
+Contact:	Radoslav Tsvetkov <rtsvetkov@gradotech.eu>
+Description:	(RW) The time in milliseconds that the LED should be on while
+        blinking. Setting both blink_delay_on and blink_delay_off to
+        zero disables blinking.
+
+        Valid values: 0 to ULONG_MAX
+        Default: 0
+
+What:		/sys/class/leds/<led>/blink_delay_off
+Date:		August 2025
+Contact:	Radoslav Tsvetkov <rtsvetkov@gradotech.eu>
+Description:	(RW) The time in milliseconds that the LED should be off while
+        blinking. Setting both blink_delay_on and blink_delay_off to
+        zero disables blinking.
+
+        Valid values: 0 to ULONG_MAX
+        Default: 0
+
+What:		/sys/class/leds/<led>/brightness
+Date:		August 2025
+Contact:	Radoslav Tsvetkov <rtsvetkov@gradotech.eu>
+Description:	(RW) Brightness value for the virtual LED group. This value is
+        applied to all monochromatic LEDs in the group if this group
+        has the highest priority among active groups.
+
+        When read-only mode is enabled via device tree, writes to this
+        attribute will return -EPERM.
+
+        Valid values: 0 to LED_FULL (usually 255)
+        Default: LED_OFF (0)
\ No newline at end of file
--
2.43.0

[PATCH 4/4] dt-bindings: led: add virtual LED bindings

[Jonathan Brophy]

From: Jonathan Brophy <professor_jonny@hotmail.com>

Add device tree binding for virtual LED groups.

Co-developed-by: Radoslav Tsvetkov <rtsvetkov@gradotech.eu>
Signed-off-by: Jonathan Brophy <professor_jonny@hotmail.com>
---
 include/dt-bindings/leds/common.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/dt-bindings/leds/common.h b/include/dt-bindings/leds/common.h
index 4f017bea0123..39c34d585a47 100644
--- a/include/dt-bindings/leds/common.h
+++ b/include/dt-bindings/leds/common.h
@@ -63,6 +63,10 @@
      "lp5523:{r,g,b}" (Nokia N900) */
 #define LED_FUNCTION_STATUS "status"

+/* Virtual system LEDs Used for virtual LED groups, multifunction RGB
+	 indicators or status LEDs that reflect complex system states */
+#define LED_FUNCTION_VIRTUAL_STATUS "virtual-status"
+
 #define LED_FUNCTION_MICMUTE "micmute"
 #define LED_FUNCTION_MUTE "mute"

--
2.43.0

Re: [PATCH 2/4] dt-bindings: leds: Add YAML bindings for Virtual Color LED Group driver

[Krzysztof Kozlowski]

On 09/10/2025 17:43, Jonathan Brophy wrote:
> From: Jonathan Brophy <professor_jonny@hotmail.com>
> 
> Document Virtual Color device tree bindings.
> 
> Co Signed-off-by: Radoslav Tsvetkov <rtsvetkov@gradotech.eu>

Messed SoB chain... plus corrupted tag. Please look at submitting
patches what should be written here.

> Signed-off-by: Jonathan Brophy <professor_jonny@hotmail.com>
> ---
>  .../leds/leds-group-virtualcolor.yaml         | 88 +++++++++++++++++++
>  1 file changed, 88 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/leds/leds-group-virtualcolor.yaml
> 
> diff --git a/Documentation/devicetree/bindings/leds/leds-group-virtualcolor.yaml b/Documentation/devicetree/bindings/leds/leds-group-virtualcolor.yaml
> new file mode 100644
> index 000000000..945058415
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/leds-group-virtualcolor.yaml
> @@ -0,0 +1,88 @@
> +# 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#
> +title: Virtual LED Group with Priority Control


Bindings describe real hardware, not virtual arrangements. At least
usually. You need to make a case in the commit msg why we want exception
from standard rule.

> +
> +maintainers:
> +  - Radoslav Tsvetkov <rtsvetkov@gradotech.eu>
> +
> +description: |
> +  Virtual LED group driver that combines multiple monochromatic LEDs into


For sure we do not describe drivers here. Describe hardware or
system/board-level concept.

> +  logical groups with priority-based control. The driver ensures only the
> +  highest-priority LED state is active at any given time.
> +
> +allOf:
> +  - $ref: common.yaml#
> +
> +properties:
> +  compatible:
> +    const: leds-group-virtualcolor
> +
> +  '#address-cells':
> +    const: 1
> +
> +  '#size-cells':
> +    const: 0
> +
> +required:
> +  - compatible
> +
> +patternProperties:
> +  '^led@[0-9a-f]$':
> +    type: object
> +    $ref: common.yaml#
> +    properties:
> +      reg:
> +        maxItems: 1
> +        description: Virtual LED number
> +
> +      monochromatic-leds:
> +        $ref: /schemas/types.yaml#/definitions/phandle
> +        description: List of phandles to the monochromatic LEDs to group

You allow only one phandle, not list, so this is confusing.

> +
> +      priority:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        description: Priority level for LED activation
> +                     (higher value means higher priority)
> +
> +      blink-delay-on:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        description: Time in milliseconds the LED is on during blink

Time in ms is expressed with proper unit suffix.

Isn't there standard property for this?

> +
> +      blink-delay-off:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        description: Time in milliseconds the LED is off during blink
> +
> +    required:
> +      - reg
> +      - monochromatic-leds
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    led-controller {
> +        compatible = "leds-group-virtualcolor";
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        led@0 {
> +            reg = <0>;
> +            label = "status:red";

Use color and function instead.

> +            monochromatic-leds = <&led_red>;
> +            priority = <2>;
> +            blink-delay-on = <500>;
> +            blink-delay-off = <500>;
> +        };
> +
> +        led@1 {
> +            reg = <1>;
> +            label = "status:green";
> +            monochromatic-leds = <&led_green>;
> +            priority = <1>;
> +        };
> +    };
> +
> \ No newline at end of file

You have patch warnings.

Best regards,
Krzysztof

Re: [PATCH 4/4] dt-bindings: led: add virtual LED bindings

[Krzysztof Kozlowski]

On 09/10/2025 17:43, Jonathan Brophy wrote:
> From: Jonathan Brophy <professor_jonny@hotmail.com>
> 
> Add device tree binding for virtual LED groups.
> 
> Co-developed-by: Radoslav Tsvetkov <rtsvetkov@gradotech.eu>

Incomplete chain. See submitting patches.

> Signed-off-by: Jonathan Brophy <professor_jonny@hotmail.com>
> ---
>  include/dt-bindings/leds/common.h | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/include/dt-bindings/leds/common.h b/include/dt-bindings/leds/common.h
> index 4f017bea0123..39c34d585a47 100644
> --- a/include/dt-bindings/leds/common.h
> +++ b/include/dt-bindings/leds/common.h
> @@ -63,6 +63,10 @@
>       "lp5523:{r,g,b}" (Nokia N900) */
>  #define LED_FUNCTION_STATUS "status"
> 
> +/* Virtual system LEDs Used for virtual LED groups, multifunction RGB
> +	 indicators or status LEDs that reflect complex system states */
> +#define LED_FUNCTION_VIRTUAL_STATUS "virtual-status"

Nothing explains why status and virtual status are supposed to be
different for users. You have entire commit msg to explain WHY you are
doing it, WHY we want this patch. I see no explanation and patch is not
looking like something correct, so please make your case in the commit msg.

> +
>  #define LED_FUNCTION_MICMUTE "micmute"
>  #define LED_FUNCTION_MUTE "mute"
> 
> --
> 2.43.0


Best regards,
Krzysztof

Re: [PATCH 1/4] leds: Add Virtual Color LED Group driver

[Krzysztof Kozlowski]

On 09/10/2025 17:43, Jonathan Brophy wrote:
> From: Jonathan Brophy <professor_jonny@hotmail.com>
> 
> This commit introduces a new driver that implements virtual LED groups


Please do not use "This commit/patch/change", but imperative mood. See
longer explanation here:
https://elixir.bootlin.com/linux/v6.16/source/Documentation/process/submitting-patches.rst#L94

> by aggregating multiple monochromatic LEDs. The driver provides
> priority-based control to manage concurrent LED activation requests,



> +
> +static int leds_virtualcolor_init_vled(struct device *dev, struct device_node *child,
> +				       struct virtual_led *vled, struct leds_virtualcolor *vc_data)
> +{
> +	struct fwnode_handle *child_fwnode = of_fwnode_handle(child);
> +	struct led_init_data init_data = {};
> +	u32 blink_interval;
> +	u32 phandle_count;
> +	u32 max_brightness;
> +	int ret, i;
> +
> +	ret = of_property_read_u32(child, "priority", &vled->priority);
> +	if (ret)
> +		vled->priority = 0;
> +
> +	ret = of_property_read_u32(child, "blink", &blink_interval);


Where is this ABI documented? I do not see.

> +	if (!ret) {
> +		vled->blink_delay_on = blink_interval;
> +		vled->blink_delay_off = blink_interval;
> +	}
> +
> +	phandle_count = fwnode_property_count_u32(child_fwnode, "leds");


No, don't mix OF and fwnode.

> +	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.
> +			 * The probe will be retried when it becomes available.
> +			 */
> +			if (PTR_ERR(led_cdev) == -EPROBE_DEFER)
> +				return -EPROBE_DEFER;


Pointless...

> +
> +			ret = PTR_ERR(led_cdev);
> +			dev_err(dev, "Failed to get monochromatic LED for %s, error %d\n",
> +				vled->cdev.name, ret);
> +			return ret;


...just return dev_err_probe

> +		}
> +
> +		vled->monochromatics[i] = led_cdev;
> +	}
> +
> +	ret = of_property_read_u32(child, "max-brightness", &max_brightness);
> +	if (ret)
> +		vled->cdev.max_brightness = LED_FULL;
> +	else
> +		vled->cdev.max_brightness = max_brightness;
> +
> +	vled->cdev.brightness_set_blocking = virtual_led_brightness_set;
> +	vled->cdev.max_brightness = LED_FULL;
> +	vled->cdev.flags = LED_CORE_SUSPENDRESUME;
> +
> +	init_data.fwnode = child_fwnode;
> +	ret = devm_led_classdev_register_ext(dev, &vled->cdev, &init_data);
> +	if (ret) {
> +		dev_err(dev, "Failed to register virtual LED %s\n", vled->cdev.name);
> +		return ret;
> +	}
> +
> +	ret = device_create_file(vled->cdev.dev, &dev_attr_priority);
> +	if (ret) {
> +		dev_err(dev, "Failed to create sysfs attribute for priority\n");
> +		return ret;
> +	}
> +
> +	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;
> +}
> +
> +static int leds_virtualcolor_disable_sysfs_access(struct device *dev, struct virtual_led *vled)
> +{
> +	int i;
> +
> +	for (i = 0; i < vled->num_monochromatics; i++) {
> +		struct led_classdev *led_cdev = vled->monochromatics[i];
> +
> +		mutex_lock(&led_cdev->led_access);
> +		led_sysfs_disable(led_cdev);
> +		mutex_unlock(&led_cdev->led_access);
> +
> +		devm_add_action_or_reset(dev, restore_sysfs_write_access, led_cdev);
> +	}
> +
> +	return 0;
> +}
> +
> +static int leds_virtualcolor_probe(struct platform_device *pdev)
> +{
> +	struct leds_virtualcolor *vc_data;
> +	struct device *dev = &pdev->dev;
> +	struct device_node *child;
> +	int count = 0;
> +	int ret;
> +
> +	vc_data = devm_kzalloc(dev, sizeof(*vc_data), GFP_KERNEL);
> +	if (!vc_data)
> +		return -ENOMEM;
> +
> +	mutex_init(&vc_data->lock);
> +	INIT_LIST_HEAD(&vc_data->active_leds);
> +
> +	vc_data->num_vleds = of_get_child_count(dev->of_node);
> +	if (vc_data->num_vleds == 0) {
> +		dev_err(dev, "No virtual LEDs defined in device tree\n");
> +		ret = -EINVAL;
> +		goto err_mutex_destroy;
> +	}
> +
> +	vc_data->vleds = devm_kcalloc(dev, vc_data->num_vleds, sizeof(*vc_data->vleds), GFP_KERNEL);
> +	if (!vc_data->vleds) {
> +		ret = -ENOMEM;
> +		goto err_mutex_destroy;
> +	}
> +
> +	for_each_child_of_node(dev->of_node, child) {
> +		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);
> +
> +			of_node_put(child);


Just use scoped loop.

> +			goto err_node_put;
> +		}
> +
> +		count++;
> +	}
> +
> +	platform_set_drvdata(pdev, vc_data);
> +
> +	if (of_property_read_bool(dev->of_node, "monochromatics-ro")) {
> +		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;
> +		}
> +	}
> +
> +	return 0;
> +
> +err_node_put:
> +	of_node_put(child);


Double of node release or your code is just confusing. Each functions
cleans up only pieces it allocates, not some other function resources.

> +err_mutex_destroy:
> +	mutex_destroy(&vc_data->lock);
> +
> +	return ret;
> +}
> +
> +static void leds_virtualcolor_remove(struct platform_device *pdev)
> +{
> +	struct leds_virtualcolor *vc_data = platform_get_drvdata(pdev);
> +	int i;
> +
> +	for (i = 0; i < vc_data->num_vleds; i++) {
> +		struct virtual_led *vled = &vc_data->vleds[i];
> +		int j;
> +
> +		device_remove_file(vled->cdev.dev, &dev_attr_priority);
> +		device_remove_file(vled->cdev.dev, &dev_attr_blink_delay_on);
> +		device_remove_file(vled->cdev.dev, &dev_attr_blink_delay_off);
> +
> +		for (j = 0; j < vled->num_monochromatics; j++) {
> +			if (vled->monochromatics[j]) {
> +				led_put(vled->monochromatics[j]);
> +				vled->monochromatics[j] = NULL;
> +			}
> +		}
> +	}
> +
> +	mutex_destroy(&vc_data->lock);
> +}
> +
> +static const struct of_device_id leds_virtualcolor_of_match[] = {
> +	{ .compatible = "leds-group-virtualcolor" },


Please organize the patch documenting compatible (DT bindings) before
their user.
See also:
https://elixir.bootlin.com/linux/v6.14-rc6/source/Documentation/devicetree/bindings/submitting-patches.rst#L46

> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, leds_virtualcolor_of_match);
> +
> +static struct platform_driver leds_virtualcolor_driver = {
> +	.probe  = leds_virtualcolor_probe,
> +	.remove = leds_virtualcolor_remove,
> +	.driver = {
> +		.name           = "leds_virtualcolor",
> +		.of_match_table = leds_virtualcolor_of_match,
> +	},
> +};
> +
> +module_platform_driver(leds_virtualcolor_driver);
> +
> +MODULE_AUTHOR("Radoslav Tsvetkov <rtsvetkov@gradotech.eu>");
> +MODULE_DESCRIPTION("LEDs Virtual Color Driver with Priority Handling");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("platform:leds-group-virtualcolor");


You should not need MODULE_ALIAS() in normal cases. If you need it,
usually it means your device ID table is wrong (e.g. misses either
entries or MODULE_DEVICE_TABLE()). MODULE_ALIAS() is not a substitute
for incomplete ID table.




Best regards,
Krzysztof

Re: [PATCH 1/4] leds: Add Virtual Color LED Group driver

[Lee Jones]

On Thu, 09 Oct 2025, Jonathan Brophy wrote:

> From: Jonathan Brophy <professor_jonny@hotmail.com>
> 
> This commit introduces a new driver that implements virtual LED groups
> by aggregating multiple monochromatic LEDs. The driver provides
> priority-based control to manage concurrent LED activation requests,
> ensuring that only the highest-priority LED group's state is active at
> any given time.
> 
> This driver is useful for systems that require coordinated control over
> multiple LEDs, such as RGB indicators or status LEDs that reflect
> complex system states.
> 
> Co-developed-by: Radoslav Tsvetkov <rtsvetkov@gradotech.eu>
> Signed-off-by: Jonathan Brophy <professor_jonny@hotmail.com>
> ---
>  drivers/leds/rgb/Kconfig                   |  17 +
>  drivers/leds/rgb/Makefile                  |   1 +
>  drivers/leds/rgb/leds-group-virtualcolor.c | 440 +++++++++++++++++++++
>  3 files changed, 458 insertions(+)
>  create mode 100644 drivers/leds/rgb/leds-group-virtualcolor.c
> 
> diff --git a/drivers/leds/rgb/Kconfig b/drivers/leds/rgb/Kconfig
> index 222d943d826a..70a80fd46b9c 100644
> --- a/drivers/leds/rgb/Kconfig
> +++ b/drivers/leds/rgb/Kconfig
> @@ -75,4 +75,21 @@ config LEDS_MT6370_RGB
>  	  This driver can also be built as a module. If so, the module
>  	  will be called "leds-mt6370-rgb".
>  
> +config LEDS_GROUP_VIRTUALCOLOR
> +	tristate "Virtual LED Group Driver with Priority Control"
> +	depends on OF || COMPILE_TEST
> +	help
> +	  This option enables support for virtual LED groups that aggregate
> +	  multiple monochromatic LEDs with priority-based control. It allows
> +	  managing concurrent LED activation requests by ensuring only the
> +	  highest-priority LED state is active at any given time.

Grep for:

  "This driver groups several monochromatic LED devices in a single multicolor LED device."

Does this scratch your itch?  Is this something worth building on?

> +
> +	  Multiple LEDs can be grouped together and controlled as a single
> +	  virtual LED with priority levels and blinking support. This is
> +	  useful for systems that need to manage multiple LED indicators
> +	  with different priority levels.
> +
> +	  To compile this driver as a module, choose M here: the module
> +	  will be called leds-group-virtualcolor.

-- 
Lee Jones [李琼斯]

RE: [PATCH 1/4] leds: Add Virtual Color LED Group driver

[Jonathan Brophy]

-----Original Message-----
From: Lee Jones <lee@kernel.org> 
Sent: Friday, 10 October 2025 4:19 AM
To: Jonathan Brophy <professorjonny98@gmail.com>
Cc: Pavel Machek <pavel@kernel.org>; Jonathan Brophy <professor_jonny@hotmail.com>; Rob Herring <robh@kernel.org>; Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Radoslav Tsvetkov <rtsvetkov@gradotech.eu>; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-leds@vger.kernel.org
Subject: Re: [PATCH 1/4] leds: Add Virtual Color LED Group driver

On Thu, 09 Oct 2025, Jonathan Brophy wrote:

> From: Jonathan Brophy <professor_jonny@hotmail.com>
> 
> This commit introduces a new driver that implements virtual LED groups 
> by aggregating multiple monochromatic LEDs. The driver provides 
> priority-based control to manage concurrent LED activation requests, 
> ensuring that only the highest-priority LED group's state is active at 
> any given time.
> 
> This driver is useful for systems that require coordinated control 
> over multiple LEDs, such as RGB indicators or status LEDs that reflect 
> complex system states.
> 
> Co-developed-by: Radoslav Tsvetkov <rtsvetkov@gradotech.eu>
> Signed-off-by: Jonathan Brophy <professor_jonny@hotmail.com>
> ---
>  drivers/leds/rgb/Kconfig                   |  17 +
>  drivers/leds/rgb/Makefile                  |   1 +
>  drivers/leds/rgb/leds-group-virtualcolor.c | 440 
> +++++++++++++++++++++
>  3 files changed, 458 insertions(+)
>  create mode 100644 drivers/leds/rgb/leds-group-virtualcolor.c
> 
> diff --git a/drivers/leds/rgb/Kconfig b/drivers/leds/rgb/Kconfig index 
> 222d943d826a..70a80fd46b9c 100644
> --- a/drivers/leds/rgb/Kconfig
> +++ b/drivers/leds/rgb/Kconfig
> @@ -75,4 +75,21 @@ config LEDS_MT6370_RGB
>  	  This driver can also be built as a module. If so, the module
>  	  will be called "leds-mt6370-rgb".
>  
> +config LEDS_GROUP_VIRTUALCOLOR
> +	tristate "Virtual LED Group Driver with Priority Control"
> +	depends on OF || COMPILE_TEST
> +	help
> +	  This option enables support for virtual LED groups that aggregate
> +	  multiple monochromatic LEDs with priority-based control. It allows
> +	  managing concurrent LED activation requests by ensuring only the
> +	  highest-priority LED state is active at any given time.

Grep for:

  "This driver groups several monochromatic LED devices in a single multicolor LED device."

Does this scratch your itch?  Is this something worth building on?

> +
> +	  Multiple LEDs can be grouped together and controlled as a single
> +	  virtual LED with priority levels and blinking support. This is
> +	  useful for systems that need to manage multiple LED indicators
> +	  with different priority levels.
> +
> +	  To compile this driver as a module, choose M here: the module
> +	  will be called leds-group-virtualcolor.

--
Lee Jones [李琼斯]

I'm not entirely sure what you mean about grep for but I can update the message more friendly, if that is what you mean?

As for is this worth something building on....  there Are possibly more features that can be added, but it serves the purpose I intended to use it for currently, I did think it may be a security issue if one could create virtual led instances form userspace but it is a potential feature.

Other features such a cycling, diming sequences, on delay and off delay or more complex logic could be configured in the DTS I guess?.

Originally the driver was intended for OpenWrt so standard triggers and aliases can be attached to virtual LEDs,  the intention was to be able to closely match functions of OEM products status LEDs that often indicate multiple states.
The intention was to eliminate userspace scripting controlling the logic, so these statuses could be realised by just simple bindings.

There is a similar basic driver function in the Qualcomm SDK based on OpenWrt but it does not feature properties such as timing and priority.

One thing is that the led dt bindings property's function and colour are quite restrictive in naming as you can't stack the functions when you are trying to describe properties of a complex led, or if you have multiple virtual LEDs of the same color, it can get confusing as it prioritises this as opposed to the node name or label.
The label property makes it easy to describe it but this format is deprecated.

EG:

function = LED_FUNCTION_STATUS;
color = < LED_COLOR_ID_YELLOW>;

label =<YELLOW_SYSTEM_FLASHING_VIRTUAL_STAUS_LED> ;

could we stack the property function in an array to make it more descriptive:

function = < LED_FUNCTION_STATUS >, < LED_FUNCTION_FLASH>, < LED_FUNCTION_VIRTUAL_STATUS> ;

One thing I did also notice the Colour Magenta is not an available option in the DT bindings which is technically correct for an RGB LED  but violet and purple is there as a close option but I don't know where he best place is to comment on this.

Jonathan Brophy

Re: [PATCH 1/4] leds: Add Virtual Color LED Group driver

[Lee Jones]

On Fri, 10 Oct 2025, Jonathan Brophy wrote:

> 
> 
> -----Original Message-----
> From: Lee Jones <lee@kernel.org> 
> Sent: Friday, 10 October 2025 4:19 AM
> To: Jonathan Brophy <professorjonny98@gmail.com>
> Cc: Pavel Machek <pavel@kernel.org>; Jonathan Brophy <professor_jonny@hotmail.com>; Rob Herring <robh@kernel.org>; Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Radoslav Tsvetkov <rtsvetkov@gradotech.eu>; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-leds@vger.kernel.org
> Subject: Re: [PATCH 1/4] leds: Add Virtual Color LED Group driver
> 
> On Thu, 09 Oct 2025, Jonathan Brophy wrote:
> 
> > From: Jonathan Brophy <professor_jonny@hotmail.com>
> > 
> > This commit introduces a new driver that implements virtual LED groups 
> > by aggregating multiple monochromatic LEDs. The driver provides 
> > priority-based control to manage concurrent LED activation requests, 
> > ensuring that only the highest-priority LED group's state is active at 
> > any given time.
> > 
> > This driver is useful for systems that require coordinated control 
> > over multiple LEDs, such as RGB indicators or status LEDs that reflect 
> > complex system states.
> > 
> > Co-developed-by: Radoslav Tsvetkov <rtsvetkov@gradotech.eu>
> > Signed-off-by: Jonathan Brophy <professor_jonny@hotmail.com>
> > ---
> >  drivers/leds/rgb/Kconfig                   |  17 +
> >  drivers/leds/rgb/Makefile                  |   1 +
> >  drivers/leds/rgb/leds-group-virtualcolor.c | 440 
> > +++++++++++++++++++++
> >  3 files changed, 458 insertions(+)
> >  create mode 100644 drivers/leds/rgb/leds-group-virtualcolor.c
> > 
> > diff --git a/drivers/leds/rgb/Kconfig b/drivers/leds/rgb/Kconfig index 
> > 222d943d826a..70a80fd46b9c 100644
> > --- a/drivers/leds/rgb/Kconfig
> > +++ b/drivers/leds/rgb/Kconfig
> > @@ -75,4 +75,21 @@ config LEDS_MT6370_RGB
> >  	  This driver can also be built as a module. If so, the module
> >  	  will be called "leds-mt6370-rgb".
> >  
> > +config LEDS_GROUP_VIRTUALCOLOR
> > +	tristate "Virtual LED Group Driver with Priority Control"
> > +	depends on OF || COMPILE_TEST
> > +	help
> > +	  This option enables support for virtual LED groups that aggregate
> > +	  multiple monochromatic LEDs with priority-based control. It allows
> > +	  managing concurrent LED activation requests by ensuring only the
> > +	  highest-priority LED state is active at any given time.
> 
> Grep for:
> 
>   "This driver groups several monochromatic LED devices in a single multicolor LED device."
> 
> Does this scratch your itch?  Is this something worth building on?
> 
> > +
> > +	  Multiple LEDs can be grouped together and controlled as a single
> > +	  virtual LED with priority levels and blinking support. This is
> > +	  useful for systems that need to manage multiple LED indicators
> > +	  with different priority levels.
> > +
> > +	  To compile this driver as a module, choose M here: the module
> > +	  will be called leds-group-virtualcolor.
> 
> --
> Lee Jones [李琼斯]

It's better to set your mailer up to quote properly, then place your
replies directly under the text you're replying to.

> I'm not entirely sure what you mean about grep for but I can update the message more friendly, if that is what you mean?

And line-wrap too please.

> As for is this worth something building on....  there Are possibly more features that can be added, but it serves the purpose I intended to use it for currently, I did think it may be a security issue if one could create virtual led instances form userspace but it is a potential feature.
> 
> Other features such a cycling, diming sequences, on delay and off delay or more complex logic could be configured in the DTS I guess?.
> 
> Originally the driver was intended for OpenWrt so standard triggers and aliases can be attached to virtual LEDs,  the intention was to be able to closely match functions of OEM products status LEDs that often indicate multiple states.
> The intention was to eliminate userspace scripting controlling the logic, so these statuses could be realised by just simple bindings.
> 
> There is a similar basic driver function in the Qualcomm SDK based on OpenWrt but it does not feature properties such as timing and priority.
> 
> One thing is that the led dt bindings property's function and colour are quite restrictive in naming as you can't stack the functions when you are trying to describe properties of a complex led, or if you have multiple virtual LEDs of the same color, it can get confusing as it prioritises this as opposed to the node name or label.
> The label property makes it easy to describe it but this format is deprecated.
> 
> EG:
> 
> function = LED_FUNCTION_STATUS;
> color = < LED_COLOR_ID_YELLOW>;
> 
> label =<YELLOW_SYSTEM_FLASHING_VIRTUAL_STAUS_LED> ;
> 
> could we stack the property function in an array to make it more descriptive:
> 
> function = < LED_FUNCTION_STATUS >, < LED_FUNCTION_FLASH>, < LED_FUNCTION_VIRTUAL_STATUS> ;
> 
> One thing I did also notice the Colour Magenta is not an available option in the DT bindings which is technically correct for an RGB LED  but violet and purple is there as a close option but I don't know where he best place is to comment on this.
> 
> Jonathan Brophy

Whoa, that was too much text for a reply to a message you didn't understand!

By "Grep for", I really did mean to "physically grep for this line in the kernel".

Doing so would have brought you to here:

https://github.com/torvalds/linux/blob/master/drivers/leds/rgb/leds-group-multicolor.c#L5

This looks similar to what you're trying to achieve.  At least the
documentation line appears to describe something related.

Please read leds-group-multicolor.c and see if it helps you at all.

-- 
Lee Jones [李琼斯]

Re: [PATCH 1/4] leds: Add Virtual Color LED Group driver

[Rob Herring]

On Thu, Oct 09, 2025 at 09:43:36PM +1300, Jonathan Brophy wrote:
> From: Jonathan Brophy <professor_jonny@hotmail.com>
> 
> This commit introduces a new driver that implements virtual LED groups
> by aggregating multiple monochromatic LEDs. The driver provides
> priority-based control to manage concurrent LED activation requests,
> ensuring that only the highest-priority LED group's state is active at
> any given time.
> 
> This driver is useful for systems that require coordinated control over
> multiple LEDs, such as RGB indicators or status LEDs that reflect
> complex system states.

Please version your patches because this is not version 1.

This should start with why the existing multi led support doesn't work 
for you or can't be extended in some way for what you want to do. I 
already raised this and now you have the same comments again.

Rob

RE: [PATCH 1/4] leds: Add Virtual Color LED Group driver

[Jonathan Brophy]

On FRI, 10 October 2025, Krzysztof Kozlowski wrote:


>> +	if (!ret) {
>> +		vled->blink_delay_on = blink_interval;
>> +		vled->blink_delay_off = blink_interval;
>> +	}
>> +
>> +	phandle_count = fwnode_property_count_u32(child_fwnode, "leds");
>
>
>No, don't mix OF and fwnode.

Thanks for the guidance I am working my way through the List if fixes and will offer a new patch set when complete.

Just one question is there a preference to use Device Tree (OF) functions or FWnode functions?
It is my under standing FWnode is newer and  more universal.

 Eg:
 phandle_count = of_property_count_elems_of_size(child, "leds", sizeof(u32));

Regards
Jonathan Brophy

Re: [PATCH 1/4] leds: Add Virtual Color LED Group driver

[Krzysztof Kozlowski]

On 13/10/2025 02:24, Jonathan Brophy wrote:
> On FRI, 10 October 2025, Krzysztof Kozlowski wrote:
> 
> 
>>> +	if (!ret) {
>>> +		vled->blink_delay_on = blink_interval;
>>> +		vled->blink_delay_off = blink_interval;
>>> +	}
>>> +
>>> +	phandle_count = fwnode_property_count_u32(child_fwnode, "leds");
>>
>>
>> No, don't mix OF and fwnode.
> 
> Thanks for the guidance I am working my way through the List if fixes and will offer a new patch set when complete.
> 
> Just one question is there a preference to use Device Tree (OF) functions or FWnode functions?
> It is my under standing FWnode is newer and  more universal.

I think fwnode is better, but my comment here was - choose one.

Best regards,
Krzysztof