[PATCH v2 0/4] leds: Add a virtual LED driver for groups of

[PATCH v2 0/4] leds: Add a virtual LED driver for groups of

[Jonathan Brophy]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=y, Size: 3803 bytes --]

From: Jonathan Brophy <professor_jonny@hotmail.com>

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

Existing multi-LED drivers are primarily intended to group monochrome LEDs
into multicolor LEDs, allowing per-channel intensity control of hue and
brightness. However, they are not designed to manage grouped LEDs with
independent functional roles or shared behavior.

The leds-group-virtualcolor driver allows arbitrary LEDs to be grouped
and exposed as a single logical LED representing a fixed color, status, or
function. Properties such as triggers and brightness are applied to the group
as a whole, rather than to individual LED elements.

This is particularly useful in consumer devices (e.g., routers), where a
single status LED must reflect multiple device states via color or blink
patterns. In such cases, simple device tree bindings are insufficient,
as multiple triggers may activate simultaneously, resulting in color mixing
and ambiguous status indication.

To avoid this problem, the driver implements a priority mechanism that allows
higher-priority LEDs to assume control of the group. When multiple LEDs of the
same priority are active concurrently, the most recently activated LED takes
precedence over earlier ones. If a higher-priority LED is extinguished, a lower-
priority LED will become active.
If an LED is set to blink, or is controlled with an on or off delay, any time
the LED is inactive but still triggered, it will be the only LED in control and
will be extinguished during this time for best contrast.

leds-group-virtualcolor can also enable decomposition of multi-element multicolor
LEDs into individual virtual groups that can provide individual virtual color
LEDs, supporting flexible trigger assignments and precise status representation.

leds-group-virtualcolor can also expose a singular LED as multiple virtual LEDs,
each having individual triggers, timings, or other properties.

Additionally, traditional bindings can only control individual LED elements,
making it impossible to represent composite colors formed by combinations
of primary RGB components but this is also made possible.

Originally intended to be used with OpenWrt for controlling RGB status LEDs
so control of power, reboot and system upgrade cam mimic the manufactures status
LED mixed colors.
Often when a device ported to OpenWrt RGB status LEDs usually became a glorified
power-led instead of a status led because user scripts or binaries would have to
be implemented as additional packages to control logic.

leds-group-virtualcolor is designed to allow LED behavior to be
fully described and logically controlled in the device tree, enabling early
status indication during system initialization—without relying on user-space
scripts or custom binaries.

Jonathan Brophy (4):
  dt-bindings: leds: Add YAML bindings for Virtual Color LED Group
    driver
  ABI: sysfs-class-leds-virtualcolor: Document sysfs entries for Virtual
    Color LEDs
  dt-bindings: led: add virtual LED bindings
  leds: Add Virtual Color LED Group driver

 .../ABI/sysfs-class-leds-virtualcolor         |   43 +
 .../leds/leds-group-virtualcolor.yaml         |  100 ++
 drivers/leds/rgb/Kconfig                      |   17 +
 drivers/leds/rgb/Makefile                     |   1 +
 drivers/leds/rgb/leds-group-virtualcolor.c    |  439 +++++++
 include/dt-bindings/leds/common.h             |    4 +
 6 files changed, 604 insertions(+),
 create mode 100644 Documentation/ABI/sysfs-class-leds-virtualcolor
 create mode 100644 Documentation/devicetree/bindings/leds/leds-group-virtualcolor.yaml
 create mode 100644 drivers/leds/rgb/leds-group-virtualcolor.c

--
2.43.0

[PATCH v2 1/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-developed-by: Radoslav Tsvetkov <rtsvetkov@gradotech.eu>
Signed-off-by: Radoslav Tsvetkov <rtsvetkov@gradotech.eu>
Signed-off-by: Jonathan Brophy <professor_jonny@hotmail.com>
---
 .../leds/leds-group-virtualcolor.yaml         | 100 ++++++++++++++++++
 1 file changed, 100 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 000000000000..bafdd8fb9557
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/leds-group-virtualcolor.yaml
@@ -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#
+
+title: Common properties for virtualcolor led class
+
+maintainers:
+  - Radoslav Tsvetkov <rtsvetkov@gradotech.eu>
+
+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.
+  Common LED nodes and properties are inherited from the common.yaml
+  within this documentation directory
+
+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
+
+      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)
+
+      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.
+
+    required:
+      - reg
+      - monochromatic-leds
+
+additionalProperties: false
+
+examples:
+  - |
+    led-controller {
+        compatible = "leds-group-virtualcolor";
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        led@0 {
+            reg = <0>;
+            function = LED_FUNCTION_VIRTUAL_STATUS;
+            color = <LED_COLOR_ID_RED>;
+            leds = <&led_red>;
+            priority = <2>;
+            blink-delay-on = <500>;
+            blink-delay-off = <500>;
+        };
+
+        led@1 {
+            reg = <1>;
+            function = LED_FUNCTION_VIRTUAL_STATUS;
+            color = <LED_COLOR_ID_YELLOW>;
+            leds = <&led_green>, <&led_red>;
+            priority = <1>;
+        };
+    };
--
2.43.0

[PATCH v2 2/4] ABI: sysfs-class-leds-virtualcolor: Document sysfs

[Jonathan Brophy]

From: Jonathan Brophy <professor_jonny@hotmail.com>

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

Co-developed-by: Radoslav Tsvetkov <rtsvetkov@gradotech.eu>
Signed-off-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 000000000000..eeebb65604a5
--- /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 v2 3/4] dt-bindings: led: add virtual LED bindings

[Jonathan Brophy]

From: Jonathan Brophy <professor_jonny@hotmail.com>

Add device tree bindings for virtual LED groups.

The use of the virtual status function identifier allows users to distinguish
virtual LEDs from real LEDs in sysfs.

If a virtual LED and a real LED are triggered at the same time, an undefined
state may occur. Therefore, it is assumed that hardware LEDs are blocked when
a LED is under the control of a virtual LED driver.

Having its own function definition helps reduce ambiguity and improves
clarity in LED behavior and trigger resolution.

Co-developed-by: Radoslav Tsvetkov <rtsvetkov@gradotech.eu>
Signed-off-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

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

[Jonathan Brophy]

From: Jonathan Brophy <professor_jonny@hotmail.com>

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 <rtsvetkov@gradotech.eu>
Co Author: Jonathan Brophy <professor_jonny@hotmail.com>
Copyright (C) 2024 Jonathan Brophy <professor_jonny@hotmail.com>

Co-developed-by: Radoslav Tsvetkov <rtsvetkov@gradotech.eu>
Signed-off-by: Radoslav Tsvetkov <rtsvetkov@gradotech.eu>
Tested-by: Jonathan Brophy <professor_jonny@hotmail.com>
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 | 439 +++++++++++++++++++++
 3 files changed, 457 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..e11ad155d3b4
--- /dev/null
+++ b/drivers/leds/rgb/leds-group-virtualcolor.c
@@ -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
+ * monochromatic LEDs. Provides priority-based control for managing
+ * concurrent LED activation requests, ensuring only the highest-priority
+ * LED state is active at any given time.
+ *
+ * Code created 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 its 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 led_init_data init_data = {};
+	u32 blink_interval;
+	u32 phandle_count;
+	u32 max_brightness;
+	int ret;
+	int 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 = 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.
+			 * 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.flags = LED_CORE_SUSPENDRESUME;
+
+	init_data.fwnode = NULL; // Use OF, not 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:
+	mutex_destroy(&vc_data->lock);
+	return ret;
+
+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, j;
+
+	for (i = 0; i < vc_data->num_vleds; i++) {
+		struct virtual_led *vled = &vc_data->vleds[i];
+
+		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");
\ No newline at end of file
--
2.43.0

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

[Thomas Weißschuh]

On 2025-10-14 01:09:48+1300, Jonathan Brophy wrote:
> From: Jonathan Brophy <professor_jonny@hotmail.com>
> 
> 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 <rtsvetkov@gradotech.eu>
> Co Author: Jonathan Brophy <professor_jonny@hotmail.com>
> Copyright (C) 2024 Jonathan Brophy <professor_jonny@hotmail.com>
> 
> Co-developed-by: Radoslav Tsvetkov <rtsvetkov@gradotech.eu>
> Signed-off-by: Radoslav Tsvetkov <rtsvetkov@gradotech.eu>
> Tested-by: Jonathan Brophy <professor_jonny@hotmail.com>
> 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 | 439 +++++++++++++++++++++
>  3 files changed, 457 insertions(+)
>  create mode 100644 drivers/leds/rgb/leds-group-virtualcolor.c
> 
(...)

> diff --git a/drivers/leds/rgb/leds-group-virtualcolor.c b/drivers/leds/rgb/leds-group-virtualcolor.c
> new file mode 100644
> index 000000000000..e11ad155d3b4
> --- /dev/null
> +++ b/drivers/leds/rgb/leds-group-virtualcolor.c
> @@ -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
> + * monochromatic LEDs. Provides priority-based control for managing
> + * concurrent LED activation requests, ensuring only the highest-priority
> + * LED state is active at any given time.
> + *
> + * Code created by Radoslav Tsvetkov <rtsvetkov@gradotech.eu>
> + * Copyright (C) 2024 Jonathan Brophy <professor_jonny@hotmail.com>
> + *
> + */
> +
> +#include <linux/gpio/consumer.h>

Looks unnecessary.

> +#include <linux/leds.h>

#include <linux/list.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))

work_flags don't look they are meant to be used by drivers.

> +			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);

sysfs_emit();

> +}
> +
> +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;

No locking?

> +	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);
> +}

Why does this have a custom blinking UAPI?
Shouldn't it be generic?

(...)

> +static int leds_virtualcolor_init_vled(struct device *dev, struct device_node *child,
> +				       struct virtual_led *vled, struct leds_virtualcolor *vc_data)
> +{
> +	struct led_init_data init_data = {};
> +	u32 blink_interval;
> +	u32 phandle_count;
> +	u32 max_brightness;
> +	int ret;
> +	int i;

INIT_LIST_HEAD(&vled->list);

> +
> +	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.
> +			 * 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;
> +		}

Just use dev_err_probe(), it will properly handle -EPROBE_DEFER.

> +
> +		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.flags = LED_CORE_SUSPENDRESUME;
> +
> +	init_data.fwnode = NULL; // Use OF, not fwnode

Why?

> +	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;
> +	}

if (ret)
	return dev_err_probe() ...

> +
> +	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;
> +	}

Use 'struct platform_driver::driver::dev_groups' to let the driver core
manage your sysfs attributes instead of doing it manually.

> +
> +	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);

This mutex looks unnecessary.

> +		led_sysfs_disable(led_cdev);
> +		mutex_unlock(&led_cdev->led_access);
> +
> +		devm_add_action_or_reset(dev, restore_sysfs_write_access, led_cdev);

Check for errors.

> +	}
> +
> +	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);

Use devm_mutex_init(), then you can get rid of all the mutex_destroy()
calls below.

> +	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");

return dev_err_probe();

> +		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) {

for_each_child_of_node_scoped() should be nicer.
Also I think you should check for available, or better yet use
for_each_available_child_of_node_scoped().

> +		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")) {

The property should be documented.

> +		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:
> +	mutex_destroy(&vc_data->lock);
> +	return ret;
> +
> +err_mutex_destroy:

Both labels have the same code behind them.

> +	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, j;
> +
> +	for (i = 0; i < vc_data->num_vleds; i++) {
> +		struct virtual_led *vled = &vc_data->vleds[i];
> +
> +		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);

No need to clean up sysfs files, they will be cleaned up automatically.

> +
> +		for (j = 0; j < vled->num_monochromatics; j++) {
> +			if (vled->monochromatics[j]) {
> +				led_put(vled->monochromatics[j]);

This is dropping the reference acquired by devm_of_led_get_optional(),
correct? Then it is unnecessary, as the reference will be dropped
automatically by the devres framework.

> +				vled->monochromatics[j] = NULL;

This looks unnecessary, the memory is going to be freed anyways.

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

(...)

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

[Krzysztof Kozlowski]

On 13/10/2025 14:09, Jonathan Brophy wrote:
> From: Jonathan Brophy <professor_jonny@hotmail.com>
> 
> 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.

> 
> Co-developed-by: Radoslav Tsvetkov <rtsvetkov@gradotech.eu>
> Signed-off-by: Radoslav Tsvetkov <rtsvetkov@gradotech.eu>
> Signed-off-by: Jonathan Brophy <professor_jonny@hotmail.com>
> ---
>  .../leds/leds-group-virtualcolor.yaml         | 100 ++++++++++++++++++
>  1 file changed, 100 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 000000000000..bafdd8fb9557
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/leds-group-virtualcolor.yaml
> @@ -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#
> +
> +title: Common properties for virtualcolor led class
> +
> +maintainers:
> +  - Radoslav Tsvetkov <rtsvetkov@gradotech.eu>
> +
> +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.

> +  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.

> +
> +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#

Missing unevaluatedProperties: false.

> +    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)
> +
> +      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

Best regards,
Krzysztof

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

[Jonathan Brophy]

on  14/10/ 12:42, Krzysztof Kozlowski wrote:

>> From: Jonathan Brophy <professor_jonny@hotmail.com>
>> 
>> 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.

Ok I kind of justified the inclusion of the driver in the cover letter commit message, as for the multi-led binding I have done
the same thing I will make changes.

Sorry It’s a big learning curve for me and I may have put my justification in the wrong place.


>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

>> 
>> +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.

>> +  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

>> +    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)
>> +
>> +      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

Thanks for pointing this out I guessed there was a definition's somewhere,
At the moment the blink settings are unique to this driver I went this way as we were trying to get specific behaviour as the
native timing functions did not work as intended, but I'm looking at replacing it with standard functions if I can get it to
work.


Best regards,
Jonathan

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

[Krzysztof Kozlowski]

On 14/10/2025 05:08, Jonathan Brophy wrote:
>> 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?


I speak about the subject.

> 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
> 
>>>
>>> +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.
> 
>>> +  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
> 
>>> +    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)
>>> +
>>> +      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
> 
> Thanks for pointing this out I guessed there was a definition's somewhere,


You reference common LED bindings, so no need to duplicate properties
from there.


Best regards,
Krzysztof

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

[Rob Herring (Arm)]

On Tue, 14 Oct 2025 01:09:45 +1300, Jonathan Brophy wrote:
> From: Jonathan Brophy <professor_jonny@hotmail.com>
> 
> Document Virtual Color device tree bindings.
> 
> Co-developed-by: Radoslav Tsvetkov <rtsvetkov@gradotech.eu>
> Signed-off-by: Radoslav Tsvetkov <rtsvetkov@gradotech.eu>
> Signed-off-by: Jonathan Brophy <professor_jonny@hotmail.com>
> ---
>  .../leds/leds-group-virtualcolor.yaml         | 100 ++++++++++++++++++
>  1 file changed, 100 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/leds/leds-group-virtualcolor.yaml
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/leds/leds-group-virtualcolor.yaml: patternProperties:^led@[0-9a-f]$:properties:blink-delay-off: 'anyOf' conditional failed, one must be fixed:
	'note' is not one of ['$ref', 'additionalItems', 'additionalProperties', 'allOf', 'anyOf', 'const', 'contains', 'default', 'dependencies', 'dependentRequired', 'dependentSchemas', 'deprecated', 'description', 'else', 'enum', 'exclusiveMaximum', 'exclusiveMinimum', 'items', 'if', 'minItems', 'minimum', 'maxItems', 'maximum', 'multipleOf', 'not', 'oneOf', 'pattern', 'patternProperties', 'properties', 'required', 'then', 'typeSize', 'unevaluatedProperties', 'uniqueItems']
	'type' was expected
	from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
Lexical error: Documentation/devicetree/bindings/leds/leds-group-virtualcolor.example.dts:25.28-55 Unexpected 'LED_FUNCTION_VIRTUAL_STATUS'
Error: Documentation/devicetree/bindings/leds/leds-group-virtualcolor.example.dts:25.28-55 syntax error
FATAL ERROR: Unable to parse input tree
make[2]: *** [scripts/Makefile.dtbs:132: Documentation/devicetree/bindings/leds/leds-group-virtualcolor.example.dtb] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1525: dt_binding_check] Error 2
make: *** [Makefile:248: __sub-make] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20251013120955.227572-2-professorjonny98@gmail.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

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

[Dan Carpenter]

Hi Jonathan,

kernel test robot noticed the following build warnings:

https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Jonathan-Brophy/dt-bindings-leds-Add-YAML-bindings-for-Virtual-Color-LED-Group-driver/20251013-201353
base:   https://git.kernel.org/pub/scm/linux/kernel/git/lee/leds.git for-leds-next
patch link:    https://lore.kernel.org/r/20251013120955.227572-5-professorjonny98%40gmail.com
patch subject: [PATCH v2 4/4] leds: Add Virtual Color LED Group driver
config: loongarch-randconfig-r072-20251019 (https://download.01.org/0day-ci/archive/20251023/202510231653.3V9E5oxE-lkp@intel.com/config)
compiler: loongarch64-linux-gcc (GCC) 15.1.0

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
| Closes: https://lore.kernel.org/r/202510231653.3V9E5oxE-lkp@intel.com/

smatch warnings:
drivers/leds/rgb/leds-group-virtualcolor.c:239 leds_virtualcolor_init_vled() warn: 'phandle_count' unsigned <= 0

vim +/phandle_count +239 drivers/leds/rgb/leds-group-virtualcolor.c

793ec9b4ba92eea Jonathan Brophy 2025-10-14  218  static int leds_virtualcolor_init_vled(struct device *dev, struct device_node *child,
793ec9b4ba92eea Jonathan Brophy 2025-10-14  219  				       struct virtual_led *vled, struct leds_virtualcolor *vc_data)
793ec9b4ba92eea Jonathan Brophy 2025-10-14  220  {
793ec9b4ba92eea Jonathan Brophy 2025-10-14  221  	struct led_init_data init_data = {};
793ec9b4ba92eea Jonathan Brophy 2025-10-14  222  	u32 blink_interval;
793ec9b4ba92eea Jonathan Brophy 2025-10-14  223  	u32 phandle_count;
793ec9b4ba92eea Jonathan Brophy 2025-10-14  224  	u32 max_brightness;
793ec9b4ba92eea Jonathan Brophy 2025-10-14  225  	int ret;
793ec9b4ba92eea Jonathan Brophy 2025-10-14  226  	int i;
793ec9b4ba92eea Jonathan Brophy 2025-10-14  227  
793ec9b4ba92eea Jonathan Brophy 2025-10-14  228  	ret = of_property_read_u32(child, "priority", &vled->priority);
793ec9b4ba92eea Jonathan Brophy 2025-10-14  229  	if (ret)
793ec9b4ba92eea Jonathan Brophy 2025-10-14  230  		vled->priority = 0;
793ec9b4ba92eea Jonathan Brophy 2025-10-14  231  
793ec9b4ba92eea Jonathan Brophy 2025-10-14  232  	ret = of_property_read_u32(child, "blink", &blink_interval);
793ec9b4ba92eea Jonathan Brophy 2025-10-14  233  	if (!ret) {
793ec9b4ba92eea Jonathan Brophy 2025-10-14  234  		vled->blink_delay_on = blink_interval;
793ec9b4ba92eea Jonathan Brophy 2025-10-14  235  		vled->blink_delay_off = blink_interval;
793ec9b4ba92eea Jonathan Brophy 2025-10-14  236  	}
793ec9b4ba92eea Jonathan Brophy 2025-10-14  237  
793ec9b4ba92eea Jonathan Brophy 2025-10-14  238  	phandle_count = of_property_count_elems_of_size(child, "leds", sizeof(u32));
793ec9b4ba92eea Jonathan Brophy 2025-10-14 @239  	if (phandle_count <= 0) {
                                                            ^^^^^^^^^^^^^^^^^^
u32 can't be < 0.

793ec9b4ba92eea Jonathan Brophy 2025-10-14  240  		dev_err(dev, "No monochromatic LEDs specified for virtual LED %s\n",
793ec9b4ba92eea Jonathan Brophy 2025-10-14  241  			vled->cdev.name);
793ec9b4ba92eea Jonathan Brophy 2025-10-14  242  		return -EINVAL;
793ec9b4ba92eea Jonathan Brophy 2025-10-14  243  	}
793ec9b4ba92eea Jonathan Brophy 2025-10-14  244  
793ec9b4ba92eea Jonathan Brophy 2025-10-14  245  	vled->num_monochromatics = phandle_count;
793ec9b4ba92eea Jonathan Brophy 2025-10-14  246  	vled->monochromatics = devm_kcalloc(dev, vled->num_monochromatics,
793ec9b4ba92eea Jonathan Brophy 2025-10-14  247  					    sizeof(*vled->monochromatics), GFP_KERNEL);
793ec9b4ba92eea Jonathan Brophy 2025-10-14  248  	if (!vled->monochromatics)
793ec9b4ba92eea Jonathan Brophy 2025-10-14  249  		return -ENOMEM;

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki