[PATCH 00/10] leds: class: Support custom sysfs groups on multicolor and flash LEDs

[PATCH 00/10] leds: class: Support custom sysfs groups on multicolor and flash LEDs

[Armin Wolf]

Until now drivers are unable to atomically register custom sysfs
groups below multicolor and flash LED class devices, because the
led_cdev->sysfs_groups field is being overwritten inside the
associated initialization functions. This patch series tries to
fix this.

The first three patches constify the sysfs attributes of all three
LED flavors, while the next four patches fix various unrelated issues
affecting LED device initialization. The next two patches migrate
the multicolor and flash LEDs to use struct device_type for
registering their sysfs attributes, so that led_cdev->sysfs_groups is
not touched anymore. The last patch contains a minor optimization
for multicolor LEDs.

All patches have been tested on real hardware, with the flash LED
interface being tested using a custom kernel module.

Armin Wolf (10):
  leds: class: Constify sysfs attributes
  leds: multicolor: Constify sysfs attributes
  leds: flash: Constify sysfs attributes
  leds: class: Create "brightness_hw_changed" attribute atomically
  leds: class: Fix race condition when creating the device
  leds: class: Take the LED access lock when reading
    brightness_hw_changed
  leds: class: Use correct type for brightness_hw_changed field
  leds: class: Split led_classdev_register_ext()
  leds: Support custom sysfs groups on multicolor and flash LEDs
  leds: multicolor: Adjust maximum subled intensity during
    initialization

 drivers/leds/led-class-flash.c      | 154 +++++++++--------
 drivers/leds/led-class-multicolor.c |  79 ++++-----
 drivers/leds/led-class.c            | 245 +++++++++++++++++-----------
 drivers/leds/leds.h                 |   3 +
 include/linux/led-class-flash.h     |   5 -
 include/linux/leds.h                |   4 +-
 6 files changed, 268 insertions(+), 222 deletions(-)

-- 
2.56.0

[PATCH 01/10] leds: class: Constify sysfs attributes

[Armin Wolf]

Mark all sysfs attributes as const to ensure that they are placed
in readonly memory to prevent accidental/malicious modification.

Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 drivers/leds/led-class.c | 29 +++++++++++++++--------------
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
index 00be3cb96c00..3775ad13762f 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -27,8 +27,7 @@ static LIST_HEAD(leds_lookup_list);
 
 static struct workqueue_struct *leds_wq;
 
-static ssize_t brightness_show(struct device *dev,
-		struct device_attribute *attr, char *buf)
+static ssize_t brightness_show(struct device *dev, const struct device_attribute *attr, char *buf)
 {
 	struct led_classdev *led_cdev = dev_get_drvdata(dev);
 	unsigned int brightness;
@@ -44,8 +43,8 @@ static ssize_t brightness_show(struct device *dev,
 	return sysfs_emit(buf, "%u\n", brightness);
 }
 
-static ssize_t brightness_store(struct device *dev,
-		struct device_attribute *attr, const char *buf, size_t size)
+static ssize_t brightness_store(struct device *dev, const struct device_attribute *attr,
+				const char *buf, size_t size)
 {
 	struct led_classdev *led_cdev = dev_get_drvdata(dev);
 	unsigned long state;
@@ -74,10 +73,11 @@ static ssize_t brightness_store(struct device *dev,
 	mutex_unlock(&led_cdev->led_access);
 	return ret;
 }
-static DEVICE_ATTR_RW(brightness);
 
-static ssize_t max_brightness_show(struct device *dev,
-		struct device_attribute *attr, char *buf)
+static const DEVICE_ATTR_RW(brightness);
+
+static ssize_t max_brightness_show(struct device *dev, const struct device_attribute *attr,
+				   char *buf)
 {
 	struct led_classdev *led_cdev = dev_get_drvdata(dev);
 	unsigned int max_brightness;
@@ -88,19 +88,20 @@ static ssize_t max_brightness_show(struct device *dev,
 
 	return sysfs_emit(buf, "%u\n", max_brightness);
 }
-static DEVICE_ATTR_RO(max_brightness);
 
-static struct attribute *led_class_attrs[] = {
+static const DEVICE_ATTR_RO(max_brightness);
+
+static const struct attribute *const led_class_attrs[] = {
 	&dev_attr_brightness.attr,
 	&dev_attr_max_brightness.attr,
 	NULL,
 };
 
 static const struct attribute_group led_group = {
-	.attrs = led_class_attrs,
+	.attrs_const = led_class_attrs,
 };
 
-static const struct attribute_group *led_groups[] = {
+static const struct attribute_group *const led_groups[] = {
 	&led_group,
 #ifdef CONFIG_LEDS_TRIGGERS
 	&led_trigger_group,
@@ -109,8 +110,8 @@ static const struct attribute_group *led_groups[] = {
 };
 
 #ifdef CONFIG_LEDS_BRIGHTNESS_HW_CHANGED
-static ssize_t brightness_hw_changed_show(struct device *dev,
-		struct device_attribute *attr, char *buf)
+static ssize_t brightness_hw_changed_show(struct device *dev, const struct device_attribute *attr,
+					  char *buf)
 {
 	struct led_classdev *led_cdev = dev_get_drvdata(dev);
 
@@ -120,7 +121,7 @@ static ssize_t brightness_hw_changed_show(struct device *dev,
 	return sysfs_emit(buf, "%u\n", led_cdev->brightness_hw_changed);
 }
 
-static DEVICE_ATTR_RO(brightness_hw_changed);
+static const DEVICE_ATTR_RO(brightness_hw_changed);
 
 static int led_add_brightness_hw_changed(struct led_classdev *led_cdev)
 {
-- 
2.56.0

[PATCH 02/10] leds: multicolor: Constify sysfs attributes

[Armin Wolf]

Mark all sysfs attributes as const to ensure that they are placed
in readonly memory to prevent accidental/malicious modification.

Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 drivers/leds/led-class-multicolor.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/leds/led-class-multicolor.c b/drivers/leds/led-class-multicolor.c
index 59d46f8a90d5..57df0b2e785b 100644
--- a/drivers/leds/led-class-multicolor.c
+++ b/drivers/leds/led-class-multicolor.c
@@ -10,6 +10,7 @@
 #include <linux/minmax.h>
 #include <linux/module.h>
 #include <linux/slab.h>
+#include <linux/sysfs.h>
 #include <linux/uaccess.h>
 
 static unsigned int led_mc_get_max_intensity(struct led_classdev_mc *mcled_cdev, size_t index)
@@ -47,7 +48,7 @@ int led_mc_calc_color_components(struct led_classdev_mc *mcled_cdev,
 EXPORT_SYMBOL_GPL(led_mc_calc_color_components);
 
 static ssize_t multi_max_intensity_show(struct device *dev,
-					struct device_attribute *intensity_attr, char *buf)
+					const struct device_attribute *intensity_attr, char *buf)
 {
 	struct led_classdev *led_cdev = dev_get_drvdata(dev);
 	struct led_classdev_mc *mcled_cdev = lcdev_to_mccdev(led_cdev);
@@ -64,11 +65,11 @@ static ssize_t multi_max_intensity_show(struct device *dev,
 	buf[len++] = '\n';
 	return len;
 }
-static DEVICE_ATTR_RO(multi_max_intensity);
+static const DEVICE_ATTR_RO(multi_max_intensity);
 
 static ssize_t multi_intensity_store(struct device *dev,
-				struct device_attribute *intensity_attr,
-				const char *buf, size_t size)
+				     const struct device_attribute *intensity_attr, const char *buf,
+				     size_t size)
 {
 	struct led_classdev *led_cdev = dev_get_drvdata(dev);
 	struct led_classdev_mc *mcled_cdev = lcdev_to_mccdev(led_cdev);
@@ -110,8 +111,7 @@ static ssize_t multi_intensity_store(struct device *dev,
 }
 
 static ssize_t multi_intensity_show(struct device *dev,
-			      struct device_attribute *intensity_attr,
-			      char *buf)
+				    const struct device_attribute *intensity_attr, char *buf)
 {
 	struct led_classdev *led_cdev = dev_get_drvdata(dev);
 	struct led_classdev_mc *mcled_cdev = lcdev_to_mccdev(led_cdev);
@@ -128,11 +128,10 @@ static ssize_t multi_intensity_show(struct device *dev,
 	buf[len++] = '\n';
 	return len;
 }
-static DEVICE_ATTR_RW(multi_intensity);
+static const DEVICE_ATTR_RW(multi_intensity);
 
 static ssize_t multi_index_show(struct device *dev,
-			      struct device_attribute *multi_index_attr,
-			      char *buf)
+				const struct device_attribute *multi_index_attr, char *buf)
 {
 	struct led_classdev *led_cdev = dev_get_drvdata(dev);
 	struct led_classdev_mc *mcled_cdev = lcdev_to_mccdev(led_cdev);
@@ -150,9 +149,9 @@ static ssize_t multi_index_show(struct device *dev,
 	buf[len++] = '\n';
 	return len;
 }
-static DEVICE_ATTR_RO(multi_index);
+static const DEVICE_ATTR_RO(multi_index);
 
-static struct attribute *led_multicolor_attrs[] = {
+static const struct attribute *const led_multicolor_attrs[] = {
 	&dev_attr_multi_max_intensity.attr,
 	&dev_attr_multi_intensity.attr,
 	&dev_attr_multi_index.attr,
-- 
2.56.0

[PATCH 03/10] leds: flash: Constify sysfs attributes

[Armin Wolf]

Mark all sysfs attributes as const to ensure that they are placed
in readonly memory to prevent accidental/malicious modification.

Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 drivers/leds/led-class-flash.c | 62 +++++++++++++++++-----------------
 1 file changed, 31 insertions(+), 31 deletions(-)

diff --git a/drivers/leds/led-class-flash.c b/drivers/leds/led-class-flash.c
index 181dfd49c6ad..953265fddb7d 100644
--- a/drivers/leds/led-class-flash.c
+++ b/drivers/leds/led-class-flash.c
@@ -12,6 +12,7 @@
 #include <linux/leds.h>
 #include <linux/module.h>
 #include <linux/slab.h>
+#include <linux/sysfs.h>
 
 #define has_flash_op(fled_cdev, op)				\
 	(fled_cdev && fled_cdev->ops->op)
@@ -34,7 +35,8 @@ static const char * const led_flash_fault_names[] = {
 };
 
 static ssize_t flash_brightness_store(struct device *dev,
-		struct device_attribute *attr, const char *buf, size_t size)
+				      const struct device_attribute *attr, const char *buf,
+				      size_t size)
 {
 	struct led_classdev *led_cdev = dev_get_drvdata(dev);
 	struct led_classdev_flash *fled_cdev = lcdev_to_flcdev(led_cdev);
@@ -62,8 +64,8 @@ static ssize_t flash_brightness_store(struct device *dev,
 	return ret;
 }
 
-static ssize_t flash_brightness_show(struct device *dev,
-		struct device_attribute *attr, char *buf)
+static ssize_t flash_brightness_show(struct device *dev, const struct device_attribute *attr,
+				     char *buf)
 {
 	struct led_classdev *led_cdev = dev_get_drvdata(dev);
 	struct led_classdev_flash *fled_cdev = lcdev_to_flcdev(led_cdev);
@@ -73,20 +75,20 @@ static ssize_t flash_brightness_show(struct device *dev,
 
 	return sprintf(buf, "%u\n", fled_cdev->brightness.val);
 }
-static DEVICE_ATTR_RW(flash_brightness);
+static const DEVICE_ATTR_RW(flash_brightness);
 
-static ssize_t max_flash_brightness_show(struct device *dev,
-		struct device_attribute *attr, char *buf)
+static ssize_t max_flash_brightness_show(struct device *dev, const struct device_attribute *attr,
+					 char *buf)
 {
 	struct led_classdev *led_cdev = dev_get_drvdata(dev);
 	struct led_classdev_flash *fled_cdev = lcdev_to_flcdev(led_cdev);
 
 	return sprintf(buf, "%u\n", fled_cdev->brightness.max);
 }
-static DEVICE_ATTR_RO(max_flash_brightness);
+static const DEVICE_ATTR_RO(max_flash_brightness);
 
-static ssize_t flash_strobe_store(struct device *dev,
-		struct device_attribute *attr, const char *buf, size_t size)
+static ssize_t flash_strobe_store(struct device *dev, const struct device_attribute *attr,
+				  const char *buf, size_t size)
 {
 	struct led_classdev *led_cdev = dev_get_drvdata(dev);
 	struct led_classdev_flash *fled_cdev = lcdev_to_flcdev(led_cdev);
@@ -116,8 +118,7 @@ static ssize_t flash_strobe_store(struct device *dev,
 	return ret;
 }
 
-static ssize_t flash_strobe_show(struct device *dev,
-		struct device_attribute *attr, char *buf)
+static ssize_t flash_strobe_show(struct device *dev, const struct device_attribute *attr, char *buf)
 {
 	struct led_classdev *led_cdev = dev_get_drvdata(dev);
 	struct led_classdev_flash *fled_cdev = lcdev_to_flcdev(led_cdev);
@@ -131,10 +132,10 @@ static ssize_t flash_strobe_show(struct device *dev,
 
 	return sprintf(buf, "%u\n", state);
 }
-static DEVICE_ATTR_RW(flash_strobe);
+static const DEVICE_ATTR_RW(flash_strobe);
 
-static ssize_t flash_timeout_store(struct device *dev,
-		struct device_attribute *attr, const char *buf, size_t size)
+static ssize_t flash_timeout_store(struct device *dev, const struct device_attribute *attr,
+				   const char *buf, size_t size)
 {
 	struct led_classdev *led_cdev = dev_get_drvdata(dev);
 	struct led_classdev_flash *fled_cdev = lcdev_to_flcdev(led_cdev);
@@ -162,28 +163,27 @@ static ssize_t flash_timeout_store(struct device *dev,
 	return ret;
 }
 
-static ssize_t flash_timeout_show(struct device *dev,
-		struct device_attribute *attr, char *buf)
+static ssize_t flash_timeout_show(struct device *dev, const struct device_attribute *attr,
+				  char *buf)
 {
 	struct led_classdev *led_cdev = dev_get_drvdata(dev);
 	struct led_classdev_flash *fled_cdev = lcdev_to_flcdev(led_cdev);
 
 	return sprintf(buf, "%u\n", fled_cdev->timeout.val);
 }
-static DEVICE_ATTR_RW(flash_timeout);
+static const DEVICE_ATTR_RW(flash_timeout);
 
-static ssize_t max_flash_timeout_show(struct device *dev,
-		struct device_attribute *attr, char *buf)
+static ssize_t max_flash_timeout_show(struct device *dev, const struct device_attribute *attr,
+				      char *buf)
 {
 	struct led_classdev *led_cdev = dev_get_drvdata(dev);
 	struct led_classdev_flash *fled_cdev = lcdev_to_flcdev(led_cdev);
 
 	return sprintf(buf, "%u\n", fled_cdev->timeout.max);
 }
-static DEVICE_ATTR_RO(max_flash_timeout);
+static const DEVICE_ATTR_RO(max_flash_timeout);
 
-static ssize_t flash_fault_show(struct device *dev,
-		struct device_attribute *attr, char *buf)
+static ssize_t flash_fault_show(struct device *dev, const struct device_attribute *attr, char *buf)
 {
 	struct led_classdev *led_cdev = dev_get_drvdata(dev);
 	struct led_classdev_flash *fled_cdev = lcdev_to_flcdev(led_cdev);
@@ -205,44 +205,44 @@ static ssize_t flash_fault_show(struct device *dev,
 	ret += sysfs_emit_at(buf, ret, "\n");
 	return ret;
 }
-static DEVICE_ATTR_RO(flash_fault);
+static const DEVICE_ATTR_RO(flash_fault);
 
-static struct attribute *led_flash_strobe_attrs[] = {
+static const struct attribute *const led_flash_strobe_attrs[] = {
 	&dev_attr_flash_strobe.attr,
 	NULL,
 };
 
-static struct attribute *led_flash_timeout_attrs[] = {
+static const struct attribute *const led_flash_timeout_attrs[] = {
 	&dev_attr_flash_timeout.attr,
 	&dev_attr_max_flash_timeout.attr,
 	NULL,
 };
 
-static struct attribute *led_flash_brightness_attrs[] = {
+static const struct attribute *const led_flash_brightness_attrs[] = {
 	&dev_attr_flash_brightness.attr,
 	&dev_attr_max_flash_brightness.attr,
 	NULL,
 };
 
-static struct attribute *led_flash_fault_attrs[] = {
+static const struct attribute *const led_flash_fault_attrs[] = {
 	&dev_attr_flash_fault.attr,
 	NULL,
 };
 
 static const struct attribute_group led_flash_strobe_group = {
-	.attrs = led_flash_strobe_attrs,
+	.attrs_const = led_flash_strobe_attrs,
 };
 
 static const struct attribute_group led_flash_timeout_group = {
-	.attrs = led_flash_timeout_attrs,
+	.attrs_const = led_flash_timeout_attrs,
 };
 
 static const struct attribute_group led_flash_brightness_group = {
-	.attrs = led_flash_brightness_attrs,
+	.attrs_const = led_flash_brightness_attrs,
 };
 
 static const struct attribute_group led_flash_fault_group = {
-	.attrs = led_flash_fault_attrs,
+	.attrs_const = led_flash_fault_attrs,
 };
 
 static void led_flash_resume(struct led_classdev *led_cdev)
-- 
2.56.0

[PATCH 04/10] leds: class: Create "brightness_hw_changed" attribute atomically

[Armin Wolf]

Using device_create_file() might cause userspace applications
listening for uevents to inspect a new LED device before the kernel
has registered all sysfs attributes for it. This might cause those
applications to incorrectly assume that the new LED device does not
support the "brightness_hw_changed" attribute.

Fix this by letting the device core create the sysfs attribute before
the uevent is being send to userspace.

Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 drivers/leds/led-class.c | 85 +++++++++++++++-------------------------
 include/linux/leds.h     |  2 -
 2 files changed, 31 insertions(+), 56 deletions(-)

diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
index 3775ad13762f..d460f041ed86 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -17,6 +17,7 @@
 #include <linux/property.h>
 #include <linux/slab.h>
 #include <linux/spinlock.h>
+#include <linux/sysfs.h>
 #include <linux/timer.h>
 #include <uapi/linux/uleds.h>
 #include <linux/of.h>
@@ -101,14 +102,6 @@ static const struct attribute_group led_group = {
 	.attrs_const = led_class_attrs,
 };
 
-static const struct attribute_group *const led_groups[] = {
-	&led_group,
-#ifdef CONFIG_LEDS_TRIGGERS
-	&led_trigger_group,
-#endif
-	NULL,
-};
-
 #ifdef CONFIG_LEDS_BRIGHTNESS_HW_CHANGED
 static ssize_t brightness_hw_changed_show(struct device *dev, const struct device_attribute *attr,
 					  char *buf)
@@ -123,51 +116,48 @@ static ssize_t brightness_hw_changed_show(struct device *dev, const struct devic
 
 static const DEVICE_ATTR_RO(brightness_hw_changed);
 
-static int led_add_brightness_hw_changed(struct led_classdev *led_cdev)
-{
-	struct device *dev = led_cdev->dev;
-	int ret;
+static const struct attribute *const led_brightness_hw_changed_attrs[] = {
+	&dev_attr_brightness_hw_changed.attr,
+	NULL,
+};
 
-	ret = device_create_file(dev, &dev_attr_brightness_hw_changed);
-	if (ret) {
-		dev_err(dev, "Error creating brightness_hw_changed\n");
-		return ret;
-	}
+static umode_t led_brightness_hw_changed_is_visible(struct kobject *kobj,
+						    const struct attribute *attr, int n)
+{
+	struct device *dev = kobj_to_dev(kobj);
+	struct led_classdev *led_cdev = dev_get_drvdata(dev);
 
-	led_cdev->brightness_hw_changed_kn =
-		sysfs_get_dirent(dev->kobj.sd, "brightness_hw_changed");
-	if (!led_cdev->brightness_hw_changed_kn) {
-		dev_err(dev, "Error getting brightness_hw_changed kn\n");
-		device_remove_file(dev, &dev_attr_brightness_hw_changed);
-		return -ENXIO;
-	}
+	if (attr == &dev_attr_brightness_hw_changed.attr &&
+	    led_cdev->flags & LED_BRIGHT_HW_CHANGED)
+		return attr->mode;
 
 	return 0;
 }
 
-static void led_remove_brightness_hw_changed(struct led_classdev *led_cdev)
-{
-	sysfs_put(led_cdev->brightness_hw_changed_kn);
-	device_remove_file(led_cdev->dev, &dev_attr_brightness_hw_changed);
-}
+static const struct attribute_group led_brightness_hw_changed_group = {
+	.is_visible_const = led_brightness_hw_changed_is_visible,
+	.attrs_const = led_brightness_hw_changed_attrs,
+};
+#endif	/* CONFIG_LEDS_BRIGHTNESS_HW_CHANGED */
 
+static const struct attribute_group *const led_groups[] = {
+	&led_group,
+#ifdef CONFIG_LEDS_BRIGHTNESS_HW_CHANGED
+	&led_brightness_hw_changed_group,
+#endif
+#ifdef CONFIG_LEDS_TRIGGERS
+	&led_trigger_group,
+#endif
+	NULL,
+};
+
+#ifdef CONFIG_LEDS_BRIGHTNESS_HW_CHANGED
 void led_classdev_notify_brightness_hw_changed(struct led_classdev *led_cdev, unsigned int brightness)
 {
-	if (WARN_ON(!led_cdev->brightness_hw_changed_kn))
-		return;
-
 	led_cdev->brightness_hw_changed = brightness;
-	sysfs_notify_dirent(led_cdev->brightness_hw_changed_kn);
+	sysfs_notify(&led_cdev->dev->kobj, NULL, "brightness_hw_changed");
 }
 EXPORT_SYMBOL_GPL(led_classdev_notify_brightness_hw_changed);
-#else
-static int led_add_brightness_hw_changed(struct led_classdev *led_cdev)
-{
-	return 0;
-}
-static void led_remove_brightness_hw_changed(struct led_classdev *led_cdev)
-{
-}
 #endif
 
 /**
@@ -544,16 +534,6 @@ int led_classdev_register_ext(struct device *parent,
 	if (init_data && init_data->fwnode)
 		device_set_node(led_cdev->dev, init_data->fwnode);
 
-	if (led_cdev->flags & LED_BRIGHT_HW_CHANGED) {
-		ret = led_add_brightness_hw_changed(led_cdev);
-		if (ret) {
-			device_unregister(led_cdev->dev);
-			led_cdev->dev = NULL;
-			mutex_unlock(&led_cdev->led_access);
-			return ret;
-		}
-	}
-
 	led_cdev->work_flags = 0;
 #ifdef CONFIG_LEDS_TRIGGERS
 	init_rwsem(&led_cdev->trigger_lock);
@@ -626,9 +606,6 @@ void led_classdev_unregister(struct led_classdev *led_cdev)
 
 	flush_work(&led_cdev->set_brightness_work);
 
-	if (led_cdev->flags & LED_BRIGHT_HW_CHANGED)
-		led_remove_brightness_hw_changed(led_cdev);
-
 	device_unregister(led_cdev->dev);
 
 	down_write(&leds_list_lock);
diff --git a/include/linux/leds.h b/include/linux/leds.h
index beaf23993063..6f975078cc14 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -21,7 +21,6 @@ struct attribute_group;
 struct device_node;
 struct fwnode_handle;
 struct gpio_desc;
-struct kernfs_node;
 struct led_pattern;
 struct platform_device;
 
@@ -237,7 +236,6 @@ struct led_classdev {
 
 #ifdef CONFIG_LEDS_BRIGHTNESS_HW_CHANGED
 	int			 brightness_hw_changed;
-	struct kernfs_node	*brightness_hw_changed_kn;
 #endif
 
 #ifdef CONFIG_LEDS_TRIGGERS_HW_CHANGED
-- 
2.56.0

[PATCH 05/10] leds: class: Fix race condition when creating the device

[Armin Wolf]

Userspace applications can access the sysfs attributes as soon as
device_create_with_groups() has returned. Two of those sysfs
attributes ("brightness_hw_changed" and "max_brightness") access
fields that are not properly initialized at this point, with only
"max_brightness" guarding against this by taking the LED access lock.

Fix the race conditon affecting "brightness_hw_changed" by creating
the device after the associated fields have been properly initialized.
This also allows us to drop the LED access lock inside
max_brightness_show(), as the maximum brightness does not change after
initialization.

Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 drivers/leds/led-class.c | 28 ++++++++++++----------------
 1 file changed, 12 insertions(+), 16 deletions(-)

diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
index d460f041ed86..e1f1b710536b 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -81,13 +81,8 @@ static ssize_t max_brightness_show(struct device *dev, const struct device_attri
 				   char *buf)
 {
 	struct led_classdev *led_cdev = dev_get_drvdata(dev);
-	unsigned int max_brightness;
 
-	mutex_lock(&led_cdev->led_access);
-	max_brightness = led_cdev->max_brightness;
-	mutex_unlock(&led_cdev->led_access);
-
-	return sysfs_emit(buf, "%u\n", max_brightness);
+	return sysfs_emit(buf, "%u\n", led_cdev->max_brightness);
 }
 
 static const DEVICE_ATTR_RO(max_brightness);
@@ -524,16 +519,6 @@ int led_classdev_register_ext(struct device *parent,
 		dev_warn(parent, "LED %s color identifier out of range\n", final_name);
 
 	mutex_init(&led_cdev->led_access);
-	mutex_lock(&led_cdev->led_access);
-	led_cdev->dev = device_create_with_groups(&leds_class, parent, 0,
-						  led_cdev, led_cdev->groups, "%s", final_name);
-	if (IS_ERR(led_cdev->dev)) {
-		mutex_unlock(&led_cdev->led_access);
-		return PTR_ERR(led_cdev->dev);
-	}
-	if (init_data && init_data->fwnode)
-		device_set_node(led_cdev->dev, init_data->fwnode);
-
 	led_cdev->work_flags = 0;
 #ifdef CONFIG_LEDS_TRIGGERS
 	init_rwsem(&led_cdev->trigger_lock);
@@ -549,6 +534,17 @@ int led_classdev_register_ext(struct device *parent,
 	if (!led_cdev->max_brightness)
 		led_cdev->max_brightness = LED_FULL;
 
+	mutex_lock(&led_cdev->led_access);
+	led_cdev->dev = device_create_with_groups(&leds_class, parent, 0, led_cdev,
+						  led_cdev->groups, "%s", final_name);
+	if (IS_ERR(led_cdev->dev)) {
+		mutex_unlock(&led_cdev->led_access);
+		return PTR_ERR(led_cdev->dev);
+	}
+
+	if (init_data && init_data->fwnode)
+		device_set_node(led_cdev->dev, init_data->fwnode);
+
 	led_update_brightness(led_cdev);
 
 	led_cdev->wq = leds_wq;
-- 
2.56.0

[PATCH 06/10] leds: class: Take the LED access lock when reading brightness_hw_changed

[Armin Wolf]

Reading the brightness_hw_changed field needs to be guarded against
concurrent writes by led_classdev_notify_brightness_hw_changed().

Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 drivers/leds/led-class.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
index e1f1b710536b..77a0fce03952 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -6,6 +6,7 @@
  * Copyright (C) 2005-2007 Richard Purdie <rpurdie@openedhand.com>
  */
 
+#include <linux/cleanup.h>
 #include <linux/ctype.h>
 #include <linux/device.h>
 #include <linux/err.h>
@@ -102,11 +103,15 @@ static ssize_t brightness_hw_changed_show(struct device *dev, const struct devic
 					  char *buf)
 {
 	struct led_classdev *led_cdev = dev_get_drvdata(dev);
+	unsigned int brightness_hw_changed;
 
-	if (led_cdev->brightness_hw_changed == -1)
+	scoped_guard(mutex, &led_cdev->led_access)
+		brightness_hw_changed = led_cdev->brightness_hw_changed;
+
+	if (brightness_hw_changed == -1)
 		return -ENODATA;
 
-	return sysfs_emit(buf, "%u\n", led_cdev->brightness_hw_changed);
+	return sysfs_emit(buf, "%u\n", brightness_hw_changed);
 }
 
 static const DEVICE_ATTR_RO(brightness_hw_changed);
-- 
2.56.0

[PATCH 07/10] leds: class: Use correct type for brightness_hw_changed field

[Armin Wolf]

Device drivers can supply a unsigned integer using
led_classdev_notify_brightness_hw_changed(), but the associated
field can only store signed integers, leading to issues for very
large brightness values.

Fix this by using the correct type for the brightness_hw_changed
field.

Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 include/linux/leds.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/leds.h b/include/linux/leds.h
index 6f975078cc14..a057ae623d5a 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -235,7 +235,7 @@ struct led_classdev {
 #endif
 
 #ifdef CONFIG_LEDS_BRIGHTNESS_HW_CHANGED
-	int			 brightness_hw_changed;
+	unsigned int		brightness_hw_changed;
 #endif
 
 #ifdef CONFIG_LEDS_TRIGGERS_HW_CHANGED
-- 
2.56.0

[PATCH 08/10] leds: class: Split led_classdev_register_ext()

[Armin Wolf]

Split led_classdev_register_ext() into two seperate internal functions
to prepare for future changes around LED class device initialization.

Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 drivers/leds/led-class.c | 105 +++++++++++++++++++++++++++++++--------
 drivers/leds/leds.h      |   3 ++
 2 files changed, 88 insertions(+), 20 deletions(-)

diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
index 77a0fce03952..ace417a2388e 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -228,9 +228,15 @@ static struct led_classdev *led_module_get(struct device *led_dev)
 	return led_cdev;
 }
 
+static void led_classdev_release(struct device *dev)
+{
+	kfree(dev);
+}
+
 static const struct class leds_class = {
 	.name = "leds",
 	.dev_groups = led_groups,
+	.dev_release = led_classdev_release,
 	.pm = &leds_class_dev_pm_ops,
 };
 
@@ -465,16 +471,21 @@ static int led_classdev_next_name(const char *init_name, char *name,
 }
 
 /**
- * led_classdev_register_ext - register a new object of led_classdev class
- *			       with init data.
- *
- * @parent: parent of LED device
+ * led_classdev_init - Initialize a new LED class device.
+ * @parent: parent of LED device.
  * @led_cdev: the led_classdev structure for this device.
- * @init_data: LED class device initialization data
+ * @init_data: LED class device initialization data.
+ *
+ * Initializes a new LED class device, but does not register with the
+ * subsystem yet. Useful for performing additional initialization before
+ * adding the LED class device to the subsystem using led_classdev_add().
+ * After calling this function put_device() needs to be called on the dev
+ * field inside the LED class device in order to dispose it.
+ *
+ * Return: Zero on success or a negative error code on failure.
  */
-int led_classdev_register_ext(struct device *parent,
-			      struct led_classdev *led_cdev,
-			      struct led_init_data *init_data)
+int led_classdev_init(struct device *parent, struct led_classdev *led_cdev,
+		      struct led_init_data *init_data)
 {
 	char composed_name[LED_MAX_NAME_SIZE];
 	char final_name[LED_MAX_NAME_SIZE];
@@ -539,23 +550,54 @@ int led_classdev_register_ext(struct device *parent,
 	if (!led_cdev->max_brightness)
 		led_cdev->max_brightness = LED_FULL;
 
-	mutex_lock(&led_cdev->led_access);
-	led_cdev->dev = device_create_with_groups(&leds_class, parent, 0, led_cdev,
-						  led_cdev->groups, "%s", final_name);
-	if (IS_ERR(led_cdev->dev)) {
-		mutex_unlock(&led_cdev->led_access);
-		return PTR_ERR(led_cdev->dev);
+	led_cdev->dev = kzalloc_obj(*led_cdev->dev);
+	if (!led_cdev->dev)
+		return -ENOMEM;
+
+	device_initialize(led_cdev->dev);
+	led_cdev->dev->class = &leds_class;
+	led_cdev->dev->parent = parent;
+	led_cdev->dev->groups = led_cdev->groups;
+	dev_set_drvdata(led_cdev->dev, led_cdev);
+	ret = kobject_set_name(&led_cdev->dev->kobj, "%s", final_name);
+	if (ret < 0)
+	{
+		put_device(led_cdev->dev);
+		return ret;
 	}
 
 	if (init_data && init_data->fwnode)
 		device_set_node(led_cdev->dev, init_data->fwnode);
 
-	led_update_brightness(led_cdev);
-
 	led_cdev->wq = leds_wq;
 
 	led_init_core(led_cdev);
 
+	return 0;
+}
+EXPORT_SYMBOL_GPL(led_classdev_init);
+
+/**
+ * led_classdev_add - Add a new LED class device to the subsystem.
+ * @led_cdev: the led_classdev structure for this device.
+ *
+ * Adds a new LED class device to the subsystem, allowing userspace applications
+ * and/or kernel consumers to access it.
+ *
+ * Returns: Zero on success or a negative error code on failure.
+ */
+int led_classdev_add(struct led_classdev *led_cdev)
+{
+	int ret;
+
+	guard(mutex)(&led_cdev->led_access);
+
+	ret = device_add(led_cdev->dev);
+	if (ret < 0)
+		return ret;
+
+	led_update_brightness(led_cdev);
+
 	/* add to the list of leds */
 	down_write(&leds_list_lock);
 	list_add_tail(&led_cdev->node, &leds_list);
@@ -565,13 +607,36 @@ int led_classdev_register_ext(struct device *parent,
 	led_trigger_set_default(led_cdev);
 #endif
 
-	mutex_unlock(&led_cdev->led_access);
-
-	dev_dbg(parent, "Registered led device: %s\n",
-			led_cdev->name);
+	dev_dbg(led_cdev->dev->parent, "Registered led device: %s\n", led_cdev->name);
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(led_classdev_add);
+
+/**
+ * led_classdev_register_ext - register a new object of led_classdev class
+ *                             with init data.
+ *
+ * @parent: parent of LED device.
+ * @led_cdev: the led_classdev structure for this device.
+ * @init_data: LED class device initialization data.
+ */
+int led_classdev_register_ext(struct device *parent,
+                              struct led_classdev *led_cdev,
+                              struct led_init_data *init_data)
+{
+	int ret;
+
+	ret = led_classdev_init(parent, led_cdev, init_data);
+	if (ret < 0)
+		return ret;
+
+	ret = led_classdev_add(led_cdev);
+	if (ret < 0)
+		put_device(led_cdev->dev);
+
+	return ret;
+}
 EXPORT_SYMBOL_GPL(led_classdev_register_ext);
 
 /**
diff --git a/drivers/leds/leds.h b/drivers/leds/leds.h
index 6d00e6f44126..2d977c5a3520 100644
--- a/drivers/leds/leds.h
+++ b/drivers/leds/leds.h
@@ -18,6 +18,9 @@ static inline int led_get_brightness(struct led_classdev *led_cdev)
 }
 
 void led_init_core(struct led_classdev *led_cdev);
+int led_classdev_init(struct device *parent, struct led_classdev *led_cdev,
+		      struct led_init_data *init_data);
+int led_classdev_add(struct led_classdev *led_cdev);
 void led_stop_software_blink(struct led_classdev *led_cdev);
 void led_set_brightness_nopm(struct led_classdev *led_cdev, unsigned int value);
 void led_set_brightness_nosleep(struct led_classdev *led_cdev, unsigned int value);
-- 
2.56.0

[PATCH 09/10] leds: Support custom sysfs groups on multicolor and flash LEDs

[Armin Wolf]

Drivers registering multicolor/flash LEDs currently cannot use
led_cdev->groups due to said field being overwritten by the
multicolor/flash LED registration functions. Fix this by assigning
each LED type a seperate device type ("multicolor" and "flash")
that carries the sysfs attributes instead.

Also assign a device type for standard LEDs ("standard") for symmetry.

Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 drivers/leds/led-class-flash.c      | 108 ++++++++++++++--------------
 drivers/leds/led-class-multicolor.c |  18 ++++-
 drivers/leds/led-class.c            |   5 ++
 include/linux/led-class-flash.h     |   5 --
 4 files changed, 73 insertions(+), 63 deletions(-)

diff --git a/drivers/leds/led-class-flash.c b/drivers/leds/led-class-flash.c
index 953265fddb7d..e023511cea1c 100644
--- a/drivers/leds/led-class-flash.c
+++ b/drivers/leds/led-class-flash.c
@@ -13,6 +13,7 @@
 #include <linux/module.h>
 #include <linux/slab.h>
 #include <linux/sysfs.h>
+#include "leds.h"
 
 #define has_flash_op(fled_cdev, op)				\
 	(fled_cdev && fled_cdev->ops->op)
@@ -207,42 +208,56 @@ static ssize_t flash_fault_show(struct device *dev, const struct device_attribut
 }
 static const DEVICE_ATTR_RO(flash_fault);
 
-static const struct attribute *const led_flash_strobe_attrs[] = {
+static const struct attribute *const led_flash_attrs[] = {
 	&dev_attr_flash_strobe.attr,
-	NULL,
-};
-
-static const struct attribute *const led_flash_timeout_attrs[] = {
 	&dev_attr_flash_timeout.attr,
 	&dev_attr_max_flash_timeout.attr,
-	NULL,
-};
-
-static const struct attribute *const led_flash_brightness_attrs[] = {
 	&dev_attr_flash_brightness.attr,
 	&dev_attr_max_flash_brightness.attr,
-	NULL,
-};
-
-static const struct attribute *const led_flash_fault_attrs[] = {
 	&dev_attr_flash_fault.attr,
 	NULL,
 };
 
-static const struct attribute_group led_flash_strobe_group = {
-	.attrs_const = led_flash_strobe_attrs,
-};
+static umode_t led_flash_is_visible(struct kobject *kobj, const struct attribute *attr, int n)
+{
+	struct device *dev = kobj_to_dev(kobj);
+	struct led_classdev *led_cdev = dev_get_drvdata(dev);
+	struct led_classdev_flash *fled_cdev = lcdev_to_flcdev(led_cdev);
+
+	if (attr == &dev_attr_flash_strobe.attr)
+		return attr->mode;
+
+        if (attr == &dev_attr_flash_timeout.attr && fled_cdev->ops->timeout_set)
+                return attr->mode;
 
-static const struct attribute_group led_flash_timeout_group = {
-	.attrs_const = led_flash_timeout_attrs,
+	if (attr == &dev_attr_max_flash_timeout.attr && fled_cdev->ops->timeout_set)
+		return attr->mode;
+
+	if (attr == &dev_attr_flash_brightness.attr && fled_cdev->ops->flash_brightness_set)
+		return attr->mode;
+
+	if (attr == &dev_attr_max_flash_brightness.attr && fled_cdev->ops->flash_brightness_set)
+		return attr->mode;
+
+	if (attr == &dev_attr_flash_fault.attr && fled_cdev->ops->fault_get)
+		return attr->mode;
+
+        return 0;
+}
+
+static const struct attribute_group led_flash_group = {
+	.is_visible_const = led_flash_is_visible,
+	.attrs_const = led_flash_attrs,
 };
 
-static const struct attribute_group led_flash_brightness_group = {
-	.attrs_const = led_flash_brightness_attrs,
+static const struct attribute_group *led_flash_groups[] = {
+	&led_flash_group,
+	NULL,
 };
 
-static const struct attribute_group led_flash_fault_group = {
-	.attrs_const = led_flash_fault_attrs,
+static const struct device_type led_flash_device_type = {
+	.name = "flash",
+	.groups = led_flash_groups,
 };
 
 static void led_flash_resume(struct led_classdev *led_cdev)
@@ -254,28 +269,6 @@ static void led_flash_resume(struct led_classdev *led_cdev)
 	call_flash_op(fled_cdev, timeout_set, fled_cdev->timeout.val);
 }
 
-static void led_flash_init_sysfs_groups(struct led_classdev_flash *fled_cdev)
-{
-	struct led_classdev *led_cdev = &fled_cdev->led_cdev;
-	const struct led_flash_ops *ops = fled_cdev->ops;
-	const struct attribute_group **flash_groups = fled_cdev->sysfs_groups;
-
-	int num_sysfs_groups = 0;
-
-	flash_groups[num_sysfs_groups++] = &led_flash_strobe_group;
-
-	if (ops->flash_brightness_set)
-		flash_groups[num_sysfs_groups++] = &led_flash_brightness_group;
-
-	if (ops->timeout_set)
-		flash_groups[num_sysfs_groups++] = &led_flash_timeout_group;
-
-	if (ops->fault_get)
-		flash_groups[num_sysfs_groups++] = &led_flash_fault_group;
-
-	led_cdev->groups = flash_groups;
-}
-
 int led_classdev_flash_register_ext(struct device *parent,
 				    struct led_classdev_flash *fled_cdev,
 				    struct led_init_data *init_data)
@@ -289,26 +282,29 @@ int led_classdev_flash_register_ext(struct device *parent,
 
 	led_cdev = &fled_cdev->led_cdev;
 
-	if (led_cdev->flags & LED_DEV_CAP_FLASH) {
-		if (!led_cdev->brightness_set_blocking)
-			return -EINVAL;
+	if (!(led_cdev->flags & LED_DEV_CAP_FLASH))
+		return led_classdev_register_ext(parent, led_cdev, init_data);
 
-		ops = fled_cdev->ops;
-		if (!ops || !ops->strobe_set)
-			return -EINVAL;
+	if (!led_cdev->brightness_set_blocking)
+		return -EINVAL;
 
-		led_cdev->flash_resume = led_flash_resume;
+	ops = fled_cdev->ops;
+	if (!ops || !ops->strobe_set)
+		return -EINVAL;
 
-		/* Select the sysfs attributes to be created for the device */
-		led_flash_init_sysfs_groups(fled_cdev);
-	}
+	led_cdev->flash_resume = led_flash_resume;
 
 	/* Register led class device */
-	ret = led_classdev_register_ext(parent, led_cdev, init_data);
+	ret = led_classdev_init(parent, led_cdev, init_data);
 	if (ret < 0)
 		return ret;
 
-	return 0;
+	led_cdev->dev->type = &led_flash_device_type;
+	ret = led_classdev_add(led_cdev);
+	if (ret < 0)
+		put_device(led_cdev->dev);
+
+	return ret;
 }
 EXPORT_SYMBOL_GPL(led_classdev_flash_register_ext);
 
diff --git a/drivers/leds/led-class-multicolor.c b/drivers/leds/led-class-multicolor.c
index 57df0b2e785b..9b8c37f7bdf4 100644
--- a/drivers/leds/led-class-multicolor.c
+++ b/drivers/leds/led-class-multicolor.c
@@ -12,6 +12,7 @@
 #include <linux/slab.h>
 #include <linux/sysfs.h>
 #include <linux/uaccess.h>
+#include "leds.h"
 
 static unsigned int led_mc_get_max_intensity(struct led_classdev_mc *mcled_cdev, size_t index)
 {
@@ -159,11 +160,17 @@ static const struct attribute *const led_multicolor_attrs[] = {
 };
 ATTRIBUTE_GROUPS(led_multicolor);
 
+static const struct device_type led_multicolor_device_type = {
+	.name = "multicolor",
+	.groups = led_multicolor_groups,
+};
+
 int led_classdev_multicolor_register_ext(struct device *parent,
 				     struct led_classdev_mc *mcled_cdev,
 				     struct led_init_data *init_data)
 {
 	struct led_classdev *led_cdev;
+	int ret;
 
 	if (!mcled_cdev)
 		return -EINVAL;
@@ -176,9 +183,16 @@ int led_classdev_multicolor_register_ext(struct device *parent,
 
 	led_cdev = &mcled_cdev->led_cdev;
 	led_cdev->flags |= LED_MULTI_COLOR;
-	mcled_cdev->led_cdev.groups = led_multicolor_groups;
+	ret = led_classdev_init(parent, led_cdev, init_data);
+	if (ret < 0)
+		return ret;
+
+	led_cdev->dev->type = &led_multicolor_device_type;
+	ret = led_classdev_add(led_cdev);
+	if (ret < 0)
+		put_device(led_cdev->dev);
 
-	return led_classdev_register_ext(parent, led_cdev, init_data);
+	return ret;
 }
 EXPORT_SYMBOL_GPL(led_classdev_multicolor_register_ext);
 
diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
index ace417a2388e..49b321b3d863 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -613,6 +613,10 @@ int led_classdev_add(struct led_classdev *led_cdev)
 }
 EXPORT_SYMBOL_GPL(led_classdev_add);
 
+static const struct device_type led_device_type = {
+	.name = "standard",
+};
+
 /**
  * led_classdev_register_ext - register a new object of led_classdev class
  *                             with init data.
@@ -631,6 +635,7 @@ int led_classdev_register_ext(struct device *parent,
 	if (ret < 0)
 		return ret;
 
+	led_cdev->dev->type = &led_device_type;
 	ret = led_classdev_add(led_cdev);
 	if (ret < 0)
 		put_device(led_cdev->dev);
diff --git a/include/linux/led-class-flash.h b/include/linux/led-class-flash.h
index 775a96217518..5204ef60dc4b 100644
--- a/include/linux/led-class-flash.h
+++ b/include/linux/led-class-flash.h
@@ -28,8 +28,6 @@ struct led_classdev_flash;
 #define LED_FAULT_LED_OVER_TEMPERATURE	(1 << 8)
 #define LED_NUM_FLASH_FAULTS		9
 
-#define LED_FLASH_SYSFS_GROUPS_SIZE	5
-
 struct led_flash_ops {
 	/* set flash brightness */
 	int (*flash_brightness_set)(struct led_classdev_flash *fled_cdev,
@@ -79,9 +77,6 @@ struct led_classdev_flash {
 
 	/* flash timeout value in microseconds along with its constraints */
 	struct led_flash_setting duration;
-
-	/* LED Flash class sysfs groups */
-	const struct attribute_group *sysfs_groups[LED_FLASH_SYSFS_GROUPS_SIZE];
 };
 
 static inline struct led_classdev_flash *lcdev_to_flcdev(
-- 
2.56.0

[PATCH 10/10] leds: multicolor: Adjust maximum subled intensity during initialization

[Armin Wolf]

The maximum brightness of a LED device is fixed after having called
led_classdev_init(), so performing the adjustment during runtime is
no longer necessary.

Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 drivers/leds/led-class-multicolor.c | 40 ++++++++++++-----------------
 1 file changed, 16 insertions(+), 24 deletions(-)

diff --git a/drivers/leds/led-class-multicolor.c b/drivers/leds/led-class-multicolor.c
index 9b8c37f7bdf4..875c615c33fc 100644
--- a/drivers/leds/led-class-multicolor.c
+++ b/drivers/leds/led-class-multicolor.c
@@ -14,24 +14,6 @@
 #include <linux/uaccess.h>
 #include "leds.h"
 
-static unsigned int led_mc_get_max_intensity(struct led_classdev_mc *mcled_cdev, size_t index)
-{
-	unsigned int max_intensity;
-
-	/*
-	 * The maximum global brightness value might still be changed by
-	 * led_classdev_register_ext() using devicetree properties. This
-	 * prevents us from changing subled_info[X].max_intensity when
-	 * registering a multicolor LED class device, so we have to do
-	 * this during runtime.
-	 */
-	max_intensity = mcled_cdev->subled_info[index].max_intensity;
-	if (max_intensity)
-		return max_intensity;
-
-	return mcled_cdev->led_cdev.max_brightness;
-}
-
 int led_mc_calc_color_components(struct led_classdev_mc *mcled_cdev,
 				 enum led_brightness brightness)
 {
@@ -53,12 +35,10 @@ static ssize_t multi_max_intensity_show(struct device *dev,
 {
 	struct led_classdev *led_cdev = dev_get_drvdata(dev);
 	struct led_classdev_mc *mcled_cdev = lcdev_to_mccdev(led_cdev);
-	unsigned int max_intensity;
 	int len = 0;
 
-	for (int i = 0; i < mcled_cdev->num_colors; i++) {
-		max_intensity = led_mc_get_max_intensity(mcled_cdev, i);
-		len += sysfs_emit_at(buf, len, "%u", max_intensity);
+	for (unsigned int i = 0; i < mcled_cdev->num_colors; i++) {
+		len += sysfs_emit_at(buf, len, "%u", mcled_cdev->subled_info[i].max_intensity);
 		if (i < mcled_cdev->num_colors - 1)
 			len += sprintf(buf + len, " ");
 	}
@@ -98,8 +78,8 @@ static ssize_t multi_intensity_store(struct device *dev,
 		goto err_out;
 	}
 
-	for (int i = 0; i < mcled_cdev->num_colors; i++) {
-		max_intensity = led_mc_get_max_intensity(mcled_cdev, i);
+	for (unsigned int i = 0; i < mcled_cdev->num_colors; i++) {
+		max_intensity = mcled_cdev->subled_info[i].max_intensity;
 		mcled_cdev->subled_info[i].intensity = min(intensity_value[i], max_intensity);
 	}
 
@@ -187,6 +167,18 @@ int led_classdev_multicolor_register_ext(struct device *parent,
 	if (ret < 0)
 		return ret;
 
+	/*
+	 * The maximum global brightness value might still be changed by
+	 * led_classdev_init() using devicetree properties. Because of this
+	 * we can only adjust subled_info[X].max_intensity after said function
+	 * was called.
+	 */
+	for (unsigned int i = 0; i < mcled_cdev->num_colors; i++)
+	{
+		if (!mcled_cdev->subled_info[i].max_intensity)
+			mcled_cdev->subled_info[i].max_intensity = led_cdev->max_brightness;
+	}
+
 	led_cdev->dev->type = &led_multicolor_device_type;
 	ret = led_classdev_add(led_cdev);
 	if (ret < 0)
-- 
2.56.0

Re: [PATCH 04/10] leds: class: Create "brightness_hw_changed" attribute atomically

[Rong Zhang]

Hi Armin,

Nice refactor!

On Sun, 2026-10-18 at 15:46 +0200, Armin Wolf wrote:
> Using device_create_file() might cause userspace applications
> listening for uevents to inspect a new LED device before the kernel
> has registered all sysfs attributes for it. This might cause those
> applications to incorrectly assume that the new LED device does not
> support the "brightness_hw_changed" attribute.
> 
> Fix this by letting the device core create the sysfs attribute before
> the uevent is being send to userspace.
> 
> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
> ---
>  drivers/leds/led-class.c | 85 +++++++++++++++-------------------------
>  include/linux/leds.h     |  2 -
>  2 files changed, 31 insertions(+), 56 deletions(-)
> 
> diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
> index 3775ad13762f..d460f041ed86 100644
> --- a/drivers/leds/led-class.c
> +++ b/drivers/leds/led-class.c
> @@ -17,6 +17,7 @@
>  #include <linux/property.h>
>  #include <linux/slab.h>
>  #include <linux/spinlock.h>
> +#include <linux/sysfs.h>
>  #include <linux/timer.h>
>  #include <uapi/linux/uleds.h>
>  #include <linux/of.h>
> @@ -101,14 +102,6 @@ static const struct attribute_group led_group = {
>  	.attrs_const = led_class_attrs,
>  };
>  
> -static const struct attribute_group *const led_groups[] = {

This is a back-and-forth change. How about moving led_groups to the right
place in PATCH 1 beforehand?

Other than that, FWIW:

   Reviewed-by: Rong Zhang <i@rong.moe>

Thanks,
Rong

> -	&led_group,
> -#ifdef CONFIG_LEDS_TRIGGERS
> -	&led_trigger_group,
> -#endif
> -	NULL,
> -};
> -
>  #ifdef CONFIG_LEDS_BRIGHTNESS_HW_CHANGED
>  static ssize_t brightness_hw_changed_show(struct device *dev, const struct device_attribute *attr,
>  					  char *buf)
> @@ -123,51 +116,48 @@ static ssize_t brightness_hw_changed_show(struct device *dev, const struct devic
>  
>  static const DEVICE_ATTR_RO(brightness_hw_changed);
>  
> -static int led_add_brightness_hw_changed(struct led_classdev *led_cdev)
> -{
> -	struct device *dev = led_cdev->dev;
> -	int ret;
> +static const struct attribute *const led_brightness_hw_changed_attrs[] = {
> +	&dev_attr_brightness_hw_changed.attr,
> +	NULL,
> +};
>  
> -	ret = device_create_file(dev, &dev_attr_brightness_hw_changed);
> -	if (ret) {
> -		dev_err(dev, "Error creating brightness_hw_changed\n");
> -		return ret;
> -	}
> +static umode_t led_brightness_hw_changed_is_visible(struct kobject *kobj,
> +						    const struct attribute *attr, int n)
> +{
> +	struct device *dev = kobj_to_dev(kobj);
> +	struct led_classdev *led_cdev = dev_get_drvdata(dev);
>  
> -	led_cdev->brightness_hw_changed_kn =
> -		sysfs_get_dirent(dev->kobj.sd, "brightness_hw_changed");
> -	if (!led_cdev->brightness_hw_changed_kn) {
> -		dev_err(dev, "Error getting brightness_hw_changed kn\n");
> -		device_remove_file(dev, &dev_attr_brightness_hw_changed);
> -		return -ENXIO;
> -	}
> +	if (attr == &dev_attr_brightness_hw_changed.attr &&
> +	    led_cdev->flags & LED_BRIGHT_HW_CHANGED)
> +		return attr->mode;
>  
>  	return 0;
>  }
>  
> -static void led_remove_brightness_hw_changed(struct led_classdev *led_cdev)
> -{
> -	sysfs_put(led_cdev->brightness_hw_changed_kn);
> -	device_remove_file(led_cdev->dev, &dev_attr_brightness_hw_changed);
> -}
> +static const struct attribute_group led_brightness_hw_changed_group = {
> +	.is_visible_const = led_brightness_hw_changed_is_visible,
> +	.attrs_const = led_brightness_hw_changed_attrs,
> +};
> +#endif	/* CONFIG_LEDS_BRIGHTNESS_HW_CHANGED */
>  
> +static const struct attribute_group *const led_groups[] = {
> +	&led_group,
> +#ifdef CONFIG_LEDS_BRIGHTNESS_HW_CHANGED
> +	&led_brightness_hw_changed_group,
> +#endif
> +#ifdef CONFIG_LEDS_TRIGGERS
> +	&led_trigger_group,
> +#endif
> +	NULL,
> +};
> +
> +#ifdef CONFIG_LEDS_BRIGHTNESS_HW_CHANGED
>  void led_classdev_notify_brightness_hw_changed(struct led_classdev *led_cdev, unsigned int brightness)
>  {
> -	if (WARN_ON(!led_cdev->brightness_hw_changed_kn))
> -		return;
> -
>  	led_cdev->brightness_hw_changed = brightness;
> -	sysfs_notify_dirent(led_cdev->brightness_hw_changed_kn);
> +	sysfs_notify(&led_cdev->dev->kobj, NULL, "brightness_hw_changed");
>  }
>  EXPORT_SYMBOL_GPL(led_classdev_notify_brightness_hw_changed);
> -#else
> -static int led_add_brightness_hw_changed(struct led_classdev *led_cdev)
> -{
> -	return 0;
> -}
> -static void led_remove_brightness_hw_changed(struct led_classdev *led_cdev)
> -{
> -}
>  #endif
>  
>  /**
> @@ -544,16 +534,6 @@ int led_classdev_register_ext(struct device *parent,
>  	if (init_data && init_data->fwnode)
>  		device_set_node(led_cdev->dev, init_data->fwnode);
>  
> -	if (led_cdev->flags & LED_BRIGHT_HW_CHANGED) {
> -		ret = led_add_brightness_hw_changed(led_cdev);
> -		if (ret) {
> -			device_unregister(led_cdev->dev);
> -			led_cdev->dev = NULL;
> -			mutex_unlock(&led_cdev->led_access);
> -			return ret;
> -		}
> -	}
> -
>  	led_cdev->work_flags = 0;
>  #ifdef CONFIG_LEDS_TRIGGERS
>  	init_rwsem(&led_cdev->trigger_lock);
> @@ -626,9 +606,6 @@ void led_classdev_unregister(struct led_classdev *led_cdev)
>  
>  	flush_work(&led_cdev->set_brightness_work);
>  
> -	if (led_cdev->flags & LED_BRIGHT_HW_CHANGED)
> -		led_remove_brightness_hw_changed(led_cdev);
> -
>  	device_unregister(led_cdev->dev);
>  
>  	down_write(&leds_list_lock);
> diff --git a/include/linux/leds.h b/include/linux/leds.h
> index beaf23993063..6f975078cc14 100644
> --- a/include/linux/leds.h
> +++ b/include/linux/leds.h
> @@ -21,7 +21,6 @@ struct attribute_group;
>  struct device_node;
>  struct fwnode_handle;
>  struct gpio_desc;
> -struct kernfs_node;
>  struct led_pattern;
>  struct platform_device;
>  
> @@ -237,7 +236,6 @@ struct led_classdev {
>  
>  #ifdef CONFIG_LEDS_BRIGHTNESS_HW_CHANGED
>  	int			 brightness_hw_changed;
> -	struct kernfs_node	*brightness_hw_changed_kn;
>  #endif
>  
>  #ifdef CONFIG_LEDS_TRIGGERS_HW_CHANGED

Re: [PATCH 09/10] leds: Support custom sysfs groups on multicolor and flash LEDs

[Rong Zhang]

Hi Armin,

Nice refactor!

On Sun, 2026-10-18 at 15:46 +0200, Armin Wolf wrote:
> Drivers registering multicolor/flash LEDs currently cannot use
> led_cdev->groups due to said field being overwritten by the
> multicolor/flash LED registration functions. Fix this by assigning
> each LED type a seperate device type ("multicolor" and "flash")
> that carries the sysfs attributes instead.
> 
> Also assign a device type for standard LEDs ("standard") for symmetry.
> 
> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
> ---
>  drivers/leds/led-class-flash.c      | 108 ++++++++++++++--------------
>  drivers/leds/led-class-multicolor.c |  18 ++++-
>  drivers/leds/led-class.c            |   5 ++
>  include/linux/led-class-flash.h     |   5 --
>  4 files changed, 73 insertions(+), 63 deletions(-)
> 
> diff --git a/drivers/leds/led-class-flash.c b/drivers/leds/led-class-flash.c
> index 953265fddb7d..e023511cea1c 100644
> --- a/drivers/leds/led-class-flash.c
> +++ b/drivers/leds/led-class-flash.c
> @@ -13,6 +13,7 @@
>  #include <linux/module.h>
>  #include <linux/slab.h>
>  #include <linux/sysfs.h>
> +#include "leds.h"
>  
>  #define has_flash_op(fled_cdev, op)				\
>  	(fled_cdev && fled_cdev->ops->op)
> @@ -207,42 +208,56 @@ static ssize_t flash_fault_show(struct device *dev, const struct device_attribut
>  }
>  static const DEVICE_ATTR_RO(flash_fault);
>  
> -static const struct attribute *const led_flash_strobe_attrs[] = {
> +static const struct attribute *const led_flash_attrs[] = {
>  	&dev_attr_flash_strobe.attr,
> -	NULL,
> -};
> -
> -static const struct attribute *const led_flash_timeout_attrs[] = {
>  	&dev_attr_flash_timeout.attr,
>  	&dev_attr_max_flash_timeout.attr,
> -	NULL,
> -};
> -
> -static const struct attribute *const led_flash_brightness_attrs[] = {
>  	&dev_attr_flash_brightness.attr,
>  	&dev_attr_max_flash_brightness.attr,
> -	NULL,
> -};
> -
> -static const struct attribute *const led_flash_fault_attrs[] = {
>  	&dev_attr_flash_fault.attr,
>  	NULL,
>  };
>  
> -static const struct attribute_group led_flash_strobe_group = {
> -	.attrs_const = led_flash_strobe_attrs,
> -};
> +static umode_t led_flash_is_visible(struct kobject *kobj, const struct attribute *attr, int n)
> +{
> +	struct device *dev = kobj_to_dev(kobj);
> +	struct led_classdev *led_cdev = dev_get_drvdata(dev);
> +	struct led_classdev_flash *fled_cdev = lcdev_to_flcdev(led_cdev);
> +
> +	if (attr == &dev_attr_flash_strobe.attr)
> +		return attr->mode;
> +
> +        if (attr == &dev_attr_flash_timeout.attr && fled_cdev->ops->timeout_set)
> +                return attr->mode;
>  
> -static const struct attribute_group led_flash_timeout_group = {
> -	.attrs_const = led_flash_timeout_attrs,
> +	if (attr == &dev_attr_max_flash_timeout.attr && fled_cdev->ops->timeout_set)
> +		return attr->mode;
> +
> +	if (attr == &dev_attr_flash_brightness.attr && fled_cdev->ops->flash_brightness_set)
> +		return attr->mode;
> +
> +	if (attr == &dev_attr_max_flash_brightness.attr && fled_cdev->ops->flash_brightness_set)
> +		return attr->mode;
> +
> +	if (attr == &dev_attr_flash_fault.attr && fled_cdev->ops->fault_get)
> +		return attr->mode;
> +
> +        return 0;
> +}
> +
> +static const struct attribute_group led_flash_group = {
> +	.is_visible_const = led_flash_is_visible,
> +	.attrs_const = led_flash_attrs,
>  };
>  
> -static const struct attribute_group led_flash_brightness_group = {
> -	.attrs_const = led_flash_brightness_attrs,
> +static const struct attribute_group *led_flash_groups[] = {
> +	&led_flash_group,
> +	NULL,
>  };
>  
> -static const struct attribute_group led_flash_fault_group = {
> -	.attrs_const = led_flash_fault_attrs,

Hmm, a bunch of back-and-forth changes. I suppose the is_visible_const
changes can be merged into PATCH 3, right?

Thanks,
Rong

> +static const struct device_type led_flash_device_type = {
> +	.name = "flash",
> +	.groups = led_flash_groups,
>  };
>  
>  static void led_flash_resume(struct led_classdev *led_cdev)
> @@ -254,28 +269,6 @@ static void led_flash_resume(struct led_classdev *led_cdev)
>  	call_flash_op(fled_cdev, timeout_set, fled_cdev->timeout.val);
>  }
>  
> -static void led_flash_init_sysfs_groups(struct led_classdev_flash *fled_cdev)
> -{
> -	struct led_classdev *led_cdev = &fled_cdev->led_cdev;
> -	const struct led_flash_ops *ops = fled_cdev->ops;
> -	const struct attribute_group **flash_groups = fled_cdev->sysfs_groups;
> -
> -	int num_sysfs_groups = 0;
> -
> -	flash_groups[num_sysfs_groups++] = &led_flash_strobe_group;
> -
> -	if (ops->flash_brightness_set)
> -		flash_groups[num_sysfs_groups++] = &led_flash_brightness_group;
> -
> -	if (ops->timeout_set)
> -		flash_groups[num_sysfs_groups++] = &led_flash_timeout_group;
> -
> -	if (ops->fault_get)
> -		flash_groups[num_sysfs_groups++] = &led_flash_fault_group;
> -
> -	led_cdev->groups = flash_groups;
> -}
> -
>  int led_classdev_flash_register_ext(struct device *parent,
>  				    struct led_classdev_flash *fled_cdev,
>  				    struct led_init_data *init_data)
> @@ -289,26 +282,29 @@ int led_classdev_flash_register_ext(struct device *parent,
>  
>  	led_cdev = &fled_cdev->led_cdev;
>  
> -	if (led_cdev->flags & LED_DEV_CAP_FLASH) {
> -		if (!led_cdev->brightness_set_blocking)
> -			return -EINVAL;
> +	if (!(led_cdev->flags & LED_DEV_CAP_FLASH))
> +		return led_classdev_register_ext(parent, led_cdev, init_data);
>  
> -		ops = fled_cdev->ops;
> -		if (!ops || !ops->strobe_set)
> -			return -EINVAL;
> +	if (!led_cdev->brightness_set_blocking)
> +		return -EINVAL;
>  
> -		led_cdev->flash_resume = led_flash_resume;
> +	ops = fled_cdev->ops;
> +	if (!ops || !ops->strobe_set)
> +		return -EINVAL;
>  
> -		/* Select the sysfs attributes to be created for the device */
> -		led_flash_init_sysfs_groups(fled_cdev);
> -	}
> +	led_cdev->flash_resume = led_flash_resume;
>  
>  	/* Register led class device */
> -	ret = led_classdev_register_ext(parent, led_cdev, init_data);
> +	ret = led_classdev_init(parent, led_cdev, init_data);
>  	if (ret < 0)
>  		return ret;
>  
> -	return 0;
> +	led_cdev->dev->type = &led_flash_device_type;
> +	ret = led_classdev_add(led_cdev);
> +	if (ret < 0)
> +		put_device(led_cdev->dev);
> +
> +	return ret;
>  }
>  EXPORT_SYMBOL_GPL(led_classdev_flash_register_ext);
>  
> diff --git a/drivers/leds/led-class-multicolor.c b/drivers/leds/led-class-multicolor.c
> index 57df0b2e785b..9b8c37f7bdf4 100644
> --- a/drivers/leds/led-class-multicolor.c
> +++ b/drivers/leds/led-class-multicolor.c
> @@ -12,6 +12,7 @@
>  #include <linux/slab.h>
>  #include <linux/sysfs.h>
>  #include <linux/uaccess.h>
> +#include "leds.h"
>  
>  static unsigned int led_mc_get_max_intensity(struct led_classdev_mc *mcled_cdev, size_t index)
>  {
> @@ -159,11 +160,17 @@ static const struct attribute *const led_multicolor_attrs[] = {
>  };
>  ATTRIBUTE_GROUPS(led_multicolor);
>  
> +static const struct device_type led_multicolor_device_type = {
> +	.name = "multicolor",
> +	.groups = led_multicolor_groups,
> +};
> +
>  int led_classdev_multicolor_register_ext(struct device *parent,
>  				     struct led_classdev_mc *mcled_cdev,
>  				     struct led_init_data *init_data)
>  {
>  	struct led_classdev *led_cdev;
> +	int ret;
>  
>  	if (!mcled_cdev)
>  		return -EINVAL;
> @@ -176,9 +183,16 @@ int led_classdev_multicolor_register_ext(struct device *parent,
>  
>  	led_cdev = &mcled_cdev->led_cdev;
>  	led_cdev->flags |= LED_MULTI_COLOR;
> -	mcled_cdev->led_cdev.groups = led_multicolor_groups;
> +	ret = led_classdev_init(parent, led_cdev, init_data);
> +	if (ret < 0)
> +		return ret;
> +
> +	led_cdev->dev->type = &led_multicolor_device_type;
> +	ret = led_classdev_add(led_cdev);
> +	if (ret < 0)
> +		put_device(led_cdev->dev);
>  
> -	return led_classdev_register_ext(parent, led_cdev, init_data);
> +	return ret;
>  }
>  EXPORT_SYMBOL_GPL(led_classdev_multicolor_register_ext);
>  
> diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
> index ace417a2388e..49b321b3d863 100644
> --- a/drivers/leds/led-class.c
> +++ b/drivers/leds/led-class.c
> @@ -613,6 +613,10 @@ int led_classdev_add(struct led_classdev *led_cdev)
>  }
>  EXPORT_SYMBOL_GPL(led_classdev_add);
>  
> +static const struct device_type led_device_type = {
> +	.name = "standard",
> +};
> +
>  /**
>   * led_classdev_register_ext - register a new object of led_classdev class
>   *                             with init data.
> @@ -631,6 +635,7 @@ int led_classdev_register_ext(struct device *parent,
>  	if (ret < 0)
>  		return ret;
>  
> +	led_cdev->dev->type = &led_device_type;
>  	ret = led_classdev_add(led_cdev);
>  	if (ret < 0)
>  		put_device(led_cdev->dev);
> diff --git a/include/linux/led-class-flash.h b/include/linux/led-class-flash.h
> index 775a96217518..5204ef60dc4b 100644
> --- a/include/linux/led-class-flash.h
> +++ b/include/linux/led-class-flash.h
> @@ -28,8 +28,6 @@ struct led_classdev_flash;
>  #define LED_FAULT_LED_OVER_TEMPERATURE	(1 << 8)
>  #define LED_NUM_FLASH_FAULTS		9
>  
> -#define LED_FLASH_SYSFS_GROUPS_SIZE	5
> -
>  struct led_flash_ops {
>  	/* set flash brightness */
>  	int (*flash_brightness_set)(struct led_classdev_flash *fled_cdev,
> @@ -79,9 +77,6 @@ struct led_classdev_flash {
>  
>  	/* flash timeout value in microseconds along with its constraints */
>  	struct led_flash_setting duration;
> -
> -	/* LED Flash class sysfs groups */
> -	const struct attribute_group *sysfs_groups[LED_FLASH_SYSFS_GROUPS_SIZE];
>  };
>  
>  static inline struct led_classdev_flash *lcdev_to_flcdev(

Re: [PATCH 10/10] leds: multicolor: Adjust maximum subled intensity during initialization

[Rong Zhang]

Hi Armin,

Nice cleanup!

On Sun, 2026-10-18 at 15:46 +0200, Armin Wolf wrote:
> The maximum brightness of a LED device is fixed after having called
> led_classdev_init(), so performing the adjustment during runtime is
> no longer necessary.
> 
> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
> ---
>  drivers/leds/led-class-multicolor.c | 40 ++++++++++++-----------------
>  1 file changed, 16 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/leds/led-class-multicolor.c b/drivers/leds/led-class-multicolor.c
> index 9b8c37f7bdf4..875c615c33fc 100644
> --- a/drivers/leds/led-class-multicolor.c
> +++ b/drivers/leds/led-class-multicolor.c
> @@ -14,24 +14,6 @@
>  #include <linux/uaccess.h>
>  #include "leds.h"
>  
> -static unsigned int led_mc_get_max_intensity(struct led_classdev_mc *mcled_cdev, size_t index)
> -{
> -	unsigned int max_intensity;
> -
> -	/*
> -	 * The maximum global brightness value might still be changed by
> -	 * led_classdev_register_ext() using devicetree properties. This
> -	 * prevents us from changing subled_info[X].max_intensity when
> -	 * registering a multicolor LED class device, so we have to do
> -	 * this during runtime.
> -	 */
> -	max_intensity = mcled_cdev->subled_info[index].max_intensity;
> -	if (max_intensity)
> -		return max_intensity;
> -
> -	return mcled_cdev->led_cdev.max_brightness;
> -}
> -
>  int led_mc_calc_color_components(struct led_classdev_mc *mcled_cdev,
>  				 enum led_brightness brightness)
>  {
> @@ -53,12 +35,10 @@ static ssize_t multi_max_intensity_show(struct device *dev,
>  {
>  	struct led_classdev *led_cdev = dev_get_drvdata(dev);
>  	struct led_classdev_mc *mcled_cdev = lcdev_to_mccdev(led_cdev);
> -	unsigned int max_intensity;
>  	int len = 0;
>  
> -	for (int i = 0; i < mcled_cdev->num_colors; i++) {
> -		max_intensity = led_mc_get_max_intensity(mcled_cdev, i);
> -		len += sysfs_emit_at(buf, len, "%u", max_intensity);
> +	for (unsigned int i = 0; i < mcled_cdev->num_colors; i++) {
> +		len += sysfs_emit_at(buf, len, "%u", mcled_cdev->subled_info[i].max_intensity);
>  		if (i < mcled_cdev->num_colors - 1)
>  			len += sprintf(buf + len, " ");
>  	}
> @@ -98,8 +78,8 @@ static ssize_t multi_intensity_store(struct device *dev,
>  		goto err_out;
>  	}
>  
> -	for (int i = 0; i < mcled_cdev->num_colors; i++) {
> -		max_intensity = led_mc_get_max_intensity(mcled_cdev, i);
> +	for (unsigned int i = 0; i < mcled_cdev->num_colors; i++) {
> +		max_intensity = mcled_cdev->subled_info[i].max_intensity;
>  		mcled_cdev->subled_info[i].intensity = min(intensity_value[i], max_intensity);
>  	}
>  
> @@ -187,6 +167,18 @@ int led_classdev_multicolor_register_ext(struct device *parent,
>  	if (ret < 0)
>  		return ret;
>  
> +	/*
> +	 * The maximum global brightness value might still be changed by

s/still be/have been/ ?

Thanks,
Rong

> +	 * led_classdev_init() using devicetree properties. Because of this
> +	 * we can only adjust subled_info[X].max_intensity after said function
> +	 * was called.
> +	 */
> +	for (unsigned int i = 0; i < mcled_cdev->num_colors; i++)
> +	{
> +		if (!mcled_cdev->subled_info[i].max_intensity)
> +			mcled_cdev->subled_info[i].max_intensity = led_cdev->max_brightness;
> +	}
> +
>  	led_cdev->dev->type = &led_multicolor_device_type;
>  	ret = led_classdev_add(led_cdev);
>  	if (ret < 0)

Re: [PATCH 04/10] leds: class: Create "brightness_hw_changed" attribute atomically

[Armin Wolf]

Am 19.10.26 um 13:51 schrieb Rong Zhang:

> Hi Armin,
>
> Nice refactor!
>
> On Sun, 2026-10-18 at 15:46 +0200, Armin Wolf wrote:
>> Using device_create_file() might cause userspace applications
>> listening for uevents to inspect a new LED device before the kernel
>> has registered all sysfs attributes for it. This might cause those
>> applications to incorrectly assume that the new LED device does not
>> support the "brightness_hw_changed" attribute.
>>
>> Fix this by letting the device core create the sysfs attribute before
>> the uevent is being send to userspace.
>>
>> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
>> ---
>>   drivers/leds/led-class.c | 85 +++++++++++++++-------------------------
>>   include/linux/leds.h     |  2 -
>>   2 files changed, 31 insertions(+), 56 deletions(-)
>>
>> diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
>> index 3775ad13762f..d460f041ed86 100644
>> --- a/drivers/leds/led-class.c
>> +++ b/drivers/leds/led-class.c
>> @@ -17,6 +17,7 @@
>>   #include <linux/property.h>
>>   #include <linux/slab.h>
>>   #include <linux/spinlock.h>
>> +#include <linux/sysfs.h>
>>   #include <linux/timer.h>
>>   #include <uapi/linux/uleds.h>
>>   #include <linux/of.h>
>> @@ -101,14 +102,6 @@ static const struct attribute_group led_group = {
>>   	.attrs_const = led_class_attrs,
>>   };
>>   
>> -static const struct attribute_group *const led_groups[] = {
> This is a back-and-forth change. How about moving led_groups to the right
> place in PATCH 1 beforehand?
>
> Other than that, FWIW:
>
>     Reviewed-by: Rong Zhang <i@rong.moe>
>
> Thanks,
> Rong

I would prefer moving led_groups[] in patch 4, because patch 1 deals with an entirely
different topic.

Sashiko has also found a major issue in this patch, so i will have to rework it
quite a bit for the v2 revision. I will therefore not include your Reviewed-by tag
just yet, because the changes are quite substantial.

Thanks,
Armin Wolf

>> -	&led_group,
>> -#ifdef CONFIG_LEDS_TRIGGERS
>> -	&led_trigger_group,
>> -#endif
>> -	NULL,
>> -};
>> -
>>   #ifdef CONFIG_LEDS_BRIGHTNESS_HW_CHANGED
>>   static ssize_t brightness_hw_changed_show(struct device *dev, const struct device_attribute *attr,
>>   					  char *buf)
>> @@ -123,51 +116,48 @@ static ssize_t brightness_hw_changed_show(struct device *dev, const struct devic
>>   
>>   static const DEVICE_ATTR_RO(brightness_hw_changed);
>>   
>> -static int led_add_brightness_hw_changed(struct led_classdev *led_cdev)
>> -{
>> -	struct device *dev = led_cdev->dev;
>> -	int ret;
>> +static const struct attribute *const led_brightness_hw_changed_attrs[] = {
>> +	&dev_attr_brightness_hw_changed.attr,
>> +	NULL,
>> +};
>>   
>> -	ret = device_create_file(dev, &dev_attr_brightness_hw_changed);
>> -	if (ret) {
>> -		dev_err(dev, "Error creating brightness_hw_changed\n");
>> -		return ret;
>> -	}
>> +static umode_t led_brightness_hw_changed_is_visible(struct kobject *kobj,
>> +						    const struct attribute *attr, int n)
>> +{
>> +	struct device *dev = kobj_to_dev(kobj);
>> +	struct led_classdev *led_cdev = dev_get_drvdata(dev);
>>   
>> -	led_cdev->brightness_hw_changed_kn =
>> -		sysfs_get_dirent(dev->kobj.sd, "brightness_hw_changed");
>> -	if (!led_cdev->brightness_hw_changed_kn) {
>> -		dev_err(dev, "Error getting brightness_hw_changed kn\n");
>> -		device_remove_file(dev, &dev_attr_brightness_hw_changed);
>> -		return -ENXIO;
>> -	}
>> +	if (attr == &dev_attr_brightness_hw_changed.attr &&
>> +	    led_cdev->flags & LED_BRIGHT_HW_CHANGED)
>> +		return attr->mode;
>>   
>>   	return 0;
>>   }
>>   
>> -static void led_remove_brightness_hw_changed(struct led_classdev *led_cdev)
>> -{
>> -	sysfs_put(led_cdev->brightness_hw_changed_kn);
>> -	device_remove_file(led_cdev->dev, &dev_attr_brightness_hw_changed);
>> -}
>> +static const struct attribute_group led_brightness_hw_changed_group = {
>> +	.is_visible_const = led_brightness_hw_changed_is_visible,
>> +	.attrs_const = led_brightness_hw_changed_attrs,
>> +};
>> +#endif	/* CONFIG_LEDS_BRIGHTNESS_HW_CHANGED */
>>   
>> +static const struct attribute_group *const led_groups[] = {
>> +	&led_group,
>> +#ifdef CONFIG_LEDS_BRIGHTNESS_HW_CHANGED
>> +	&led_brightness_hw_changed_group,
>> +#endif
>> +#ifdef CONFIG_LEDS_TRIGGERS
>> +	&led_trigger_group,
>> +#endif
>> +	NULL,
>> +};
>> +
>> +#ifdef CONFIG_LEDS_BRIGHTNESS_HW_CHANGED
>>   void led_classdev_notify_brightness_hw_changed(struct led_classdev *led_cdev, unsigned int brightness)
>>   {
>> -	if (WARN_ON(!led_cdev->brightness_hw_changed_kn))
>> -		return;
>> -
>>   	led_cdev->brightness_hw_changed = brightness;
>> -	sysfs_notify_dirent(led_cdev->brightness_hw_changed_kn);
>> +	sysfs_notify(&led_cdev->dev->kobj, NULL, "brightness_hw_changed");
>>   }
>>   EXPORT_SYMBOL_GPL(led_classdev_notify_brightness_hw_changed);
>> -#else
>> -static int led_add_brightness_hw_changed(struct led_classdev *led_cdev)
>> -{
>> -	return 0;
>> -}
>> -static void led_remove_brightness_hw_changed(struct led_classdev *led_cdev)
>> -{
>> -}
>>   #endif
>>   
>>   /**
>> @@ -544,16 +534,6 @@ int led_classdev_register_ext(struct device *parent,
>>   	if (init_data && init_data->fwnode)
>>   		device_set_node(led_cdev->dev, init_data->fwnode);
>>   
>> -	if (led_cdev->flags & LED_BRIGHT_HW_CHANGED) {
>> -		ret = led_add_brightness_hw_changed(led_cdev);
>> -		if (ret) {
>> -			device_unregister(led_cdev->dev);
>> -			led_cdev->dev = NULL;
>> -			mutex_unlock(&led_cdev->led_access);
>> -			return ret;
>> -		}
>> -	}
>> -
>>   	led_cdev->work_flags = 0;
>>   #ifdef CONFIG_LEDS_TRIGGERS
>>   	init_rwsem(&led_cdev->trigger_lock);
>> @@ -626,9 +606,6 @@ void led_classdev_unregister(struct led_classdev *led_cdev)
>>   
>>   	flush_work(&led_cdev->set_brightness_work);
>>   
>> -	if (led_cdev->flags & LED_BRIGHT_HW_CHANGED)
>> -		led_remove_brightness_hw_changed(led_cdev);
>> -
>>   	device_unregister(led_cdev->dev);
>>   
>>   	down_write(&leds_list_lock);
>> diff --git a/include/linux/leds.h b/include/linux/leds.h
>> index beaf23993063..6f975078cc14 100644
>> --- a/include/linux/leds.h
>> +++ b/include/linux/leds.h
>> @@ -21,7 +21,6 @@ struct attribute_group;
>>   struct device_node;
>>   struct fwnode_handle;
>>   struct gpio_desc;
>> -struct kernfs_node;
>>   struct led_pattern;
>>   struct platform_device;
>>   
>> @@ -237,7 +236,6 @@ struct led_classdev {
>>   
>>   #ifdef CONFIG_LEDS_BRIGHTNESS_HW_CHANGED
>>   	int			 brightness_hw_changed;
>> -	struct kernfs_node	*brightness_hw_changed_kn;
>>   #endif
>>   
>>   #ifdef CONFIG_LEDS_TRIGGERS_HW_CHANGED

Re: [PATCH 09/10] leds: Support custom sysfs groups on multicolor and flash LEDs

[Armin Wolf]

Am 19.10.26 um 13:53 schrieb Rong Zhang:

> Hi Armin,
>
> Nice refactor!
>
> On Sun, 2026-10-18 at 15:46 +0200, Armin Wolf wrote:
>> Drivers registering multicolor/flash LEDs currently cannot use
>> led_cdev->groups due to said field being overwritten by the
>> multicolor/flash LED registration functions. Fix this by assigning
>> each LED type a seperate device type ("multicolor" and "flash")
>> that carries the sysfs attributes instead.
>>
>> Also assign a device type for standard LEDs ("standard") for symmetry.
>>
>> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
>> ---
>>   drivers/leds/led-class-flash.c      | 108 ++++++++++++++--------------
>>   drivers/leds/led-class-multicolor.c |  18 ++++-
>>   drivers/leds/led-class.c            |   5 ++
>>   include/linux/led-class-flash.h     |   5 --
>>   4 files changed, 73 insertions(+), 63 deletions(-)
>>
>> diff --git a/drivers/leds/led-class-flash.c b/drivers/leds/led-class-flash.c
>> index 953265fddb7d..e023511cea1c 100644
>> --- a/drivers/leds/led-class-flash.c
>> +++ b/drivers/leds/led-class-flash.c
>> @@ -13,6 +13,7 @@
>>   #include <linux/module.h>
>>   #include <linux/slab.h>
>>   #include <linux/sysfs.h>
>> +#include "leds.h"
>>   
>>   #define has_flash_op(fled_cdev, op)				\
>>   	(fled_cdev && fled_cdev->ops->op)
>> @@ -207,42 +208,56 @@ static ssize_t flash_fault_show(struct device *dev, const struct device_attribut
>>   }
>>   static const DEVICE_ATTR_RO(flash_fault);
>>   
>> -static const struct attribute *const led_flash_strobe_attrs[] = {
>> +static const struct attribute *const led_flash_attrs[] = {
>>   	&dev_attr_flash_strobe.attr,
>> -	NULL,
>> -};
>> -
>> -static const struct attribute *const led_flash_timeout_attrs[] = {
>>   	&dev_attr_flash_timeout.attr,
>>   	&dev_attr_max_flash_timeout.attr,
>> -	NULL,
>> -};
>> -
>> -static const struct attribute *const led_flash_brightness_attrs[] = {
>>   	&dev_attr_flash_brightness.attr,
>>   	&dev_attr_max_flash_brightness.attr,
>> -	NULL,
>> -};
>> -
>> -static const struct attribute *const led_flash_fault_attrs[] = {
>>   	&dev_attr_flash_fault.attr,
>>   	NULL,
>>   };
>>   
>> -static const struct attribute_group led_flash_strobe_group = {
>> -	.attrs_const = led_flash_strobe_attrs,
>> -};
>> +static umode_t led_flash_is_visible(struct kobject *kobj, const struct attribute *attr, int n)
>> +{
>> +	struct device *dev = kobj_to_dev(kobj);
>> +	struct led_classdev *led_cdev = dev_get_drvdata(dev);
>> +	struct led_classdev_flash *fled_cdev = lcdev_to_flcdev(led_cdev);
>> +
>> +	if (attr == &dev_attr_flash_strobe.attr)
>> +		return attr->mode;
>> +
>> +        if (attr == &dev_attr_flash_timeout.attr && fled_cdev->ops->timeout_set)
>> +                return attr->mode;
>>   
>> -static const struct attribute_group led_flash_timeout_group = {
>> -	.attrs_const = led_flash_timeout_attrs,
>> +	if (attr == &dev_attr_max_flash_timeout.attr && fled_cdev->ops->timeout_set)
>> +		return attr->mode;
>> +
>> +	if (attr == &dev_attr_flash_brightness.attr && fled_cdev->ops->flash_brightness_set)
>> +		return attr->mode;
>> +
>> +	if (attr == &dev_attr_max_flash_brightness.attr && fled_cdev->ops->flash_brightness_set)
>> +		return attr->mode;
>> +
>> +	if (attr == &dev_attr_flash_fault.attr && fled_cdev->ops->fault_get)
>> +		return attr->mode;
>> +
>> +        return 0;
>> +}
>> +
>> +static const struct attribute_group led_flash_group = {
>> +	.is_visible_const = led_flash_is_visible,
>> +	.attrs_const = led_flash_attrs,
>>   };
>>   
>> -static const struct attribute_group led_flash_brightness_group = {
>> -	.attrs_const = led_flash_brightness_attrs,
>> +static const struct attribute_group *led_flash_groups[] = {
>> +	&led_flash_group,
>> +	NULL,
>>   };
>>   
>> -static const struct attribute_group led_flash_fault_group = {
>> -	.attrs_const = led_flash_fault_attrs,
> Hmm, a bunch of back-and-forth changes. I suppose the is_visible_const
> changes can be merged into PATCH 3, right?
>
> Thanks,
> Rong

If the LED subsystem maintainers are OK with this then i can gladly introduce
the is_visible_const callback already in patch 3.

Thanks,
Armin Wolf

>> +static const struct device_type led_flash_device_type = {
>> +	.name = "flash",
>> +	.groups = led_flash_groups,
>>   };
>>   
>>   static void led_flash_resume(struct led_classdev *led_cdev)
>> @@ -254,28 +269,6 @@ static void led_flash_resume(struct led_classdev *led_cdev)
>>   	call_flash_op(fled_cdev, timeout_set, fled_cdev->timeout.val);
>>   }
>>   
>> -static void led_flash_init_sysfs_groups(struct led_classdev_flash *fled_cdev)
>> -{
>> -	struct led_classdev *led_cdev = &fled_cdev->led_cdev;
>> -	const struct led_flash_ops *ops = fled_cdev->ops;
>> -	const struct attribute_group **flash_groups = fled_cdev->sysfs_groups;
>> -
>> -	int num_sysfs_groups = 0;
>> -
>> -	flash_groups[num_sysfs_groups++] = &led_flash_strobe_group;
>> -
>> -	if (ops->flash_brightness_set)
>> -		flash_groups[num_sysfs_groups++] = &led_flash_brightness_group;
>> -
>> -	if (ops->timeout_set)
>> -		flash_groups[num_sysfs_groups++] = &led_flash_timeout_group;
>> -
>> -	if (ops->fault_get)
>> -		flash_groups[num_sysfs_groups++] = &led_flash_fault_group;
>> -
>> -	led_cdev->groups = flash_groups;
>> -}
>> -
>>   int led_classdev_flash_register_ext(struct device *parent,
>>   				    struct led_classdev_flash *fled_cdev,
>>   				    struct led_init_data *init_data)
>> @@ -289,26 +282,29 @@ int led_classdev_flash_register_ext(struct device *parent,
>>   
>>   	led_cdev = &fled_cdev->led_cdev;
>>   
>> -	if (led_cdev->flags & LED_DEV_CAP_FLASH) {
>> -		if (!led_cdev->brightness_set_blocking)
>> -			return -EINVAL;
>> +	if (!(led_cdev->flags & LED_DEV_CAP_FLASH))
>> +		return led_classdev_register_ext(parent, led_cdev, init_data);
>>   
>> -		ops = fled_cdev->ops;
>> -		if (!ops || !ops->strobe_set)
>> -			return -EINVAL;
>> +	if (!led_cdev->brightness_set_blocking)
>> +		return -EINVAL;
>>   
>> -		led_cdev->flash_resume = led_flash_resume;
>> +	ops = fled_cdev->ops;
>> +	if (!ops || !ops->strobe_set)
>> +		return -EINVAL;
>>   
>> -		/* Select the sysfs attributes to be created for the device */
>> -		led_flash_init_sysfs_groups(fled_cdev);
>> -	}
>> +	led_cdev->flash_resume = led_flash_resume;
>>   
>>   	/* Register led class device */
>> -	ret = led_classdev_register_ext(parent, led_cdev, init_data);
>> +	ret = led_classdev_init(parent, led_cdev, init_data);
>>   	if (ret < 0)
>>   		return ret;
>>   
>> -	return 0;
>> +	led_cdev->dev->type = &led_flash_device_type;
>> +	ret = led_classdev_add(led_cdev);
>> +	if (ret < 0)
>> +		put_device(led_cdev->dev);
>> +
>> +	return ret;
>>   }
>>   EXPORT_SYMBOL_GPL(led_classdev_flash_register_ext);
>>   
>> diff --git a/drivers/leds/led-class-multicolor.c b/drivers/leds/led-class-multicolor.c
>> index 57df0b2e785b..9b8c37f7bdf4 100644
>> --- a/drivers/leds/led-class-multicolor.c
>> +++ b/drivers/leds/led-class-multicolor.c
>> @@ -12,6 +12,7 @@
>>   #include <linux/slab.h>
>>   #include <linux/sysfs.h>
>>   #include <linux/uaccess.h>
>> +#include "leds.h"
>>   
>>   static unsigned int led_mc_get_max_intensity(struct led_classdev_mc *mcled_cdev, size_t index)
>>   {
>> @@ -159,11 +160,17 @@ static const struct attribute *const led_multicolor_attrs[] = {
>>   };
>>   ATTRIBUTE_GROUPS(led_multicolor);
>>   
>> +static const struct device_type led_multicolor_device_type = {
>> +	.name = "multicolor",
>> +	.groups = led_multicolor_groups,
>> +};
>> +
>>   int led_classdev_multicolor_register_ext(struct device *parent,
>>   				     struct led_classdev_mc *mcled_cdev,
>>   				     struct led_init_data *init_data)
>>   {
>>   	struct led_classdev *led_cdev;
>> +	int ret;
>>   
>>   	if (!mcled_cdev)
>>   		return -EINVAL;
>> @@ -176,9 +183,16 @@ int led_classdev_multicolor_register_ext(struct device *parent,
>>   
>>   	led_cdev = &mcled_cdev->led_cdev;
>>   	led_cdev->flags |= LED_MULTI_COLOR;
>> -	mcled_cdev->led_cdev.groups = led_multicolor_groups;
>> +	ret = led_classdev_init(parent, led_cdev, init_data);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	led_cdev->dev->type = &led_multicolor_device_type;
>> +	ret = led_classdev_add(led_cdev);
>> +	if (ret < 0)
>> +		put_device(led_cdev->dev);
>>   
>> -	return led_classdev_register_ext(parent, led_cdev, init_data);
>> +	return ret;
>>   }
>>   EXPORT_SYMBOL_GPL(led_classdev_multicolor_register_ext);
>>   
>> diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
>> index ace417a2388e..49b321b3d863 100644
>> --- a/drivers/leds/led-class.c
>> +++ b/drivers/leds/led-class.c
>> @@ -613,6 +613,10 @@ int led_classdev_add(struct led_classdev *led_cdev)
>>   }
>>   EXPORT_SYMBOL_GPL(led_classdev_add);
>>   
>> +static const struct device_type led_device_type = {
>> +	.name = "standard",
>> +};
>> +
>>   /**
>>    * led_classdev_register_ext - register a new object of led_classdev class
>>    *                             with init data.
>> @@ -631,6 +635,7 @@ int led_classdev_register_ext(struct device *parent,
>>   	if (ret < 0)
>>   		return ret;
>>   
>> +	led_cdev->dev->type = &led_device_type;
>>   	ret = led_classdev_add(led_cdev);
>>   	if (ret < 0)
>>   		put_device(led_cdev->dev);
>> diff --git a/include/linux/led-class-flash.h b/include/linux/led-class-flash.h
>> index 775a96217518..5204ef60dc4b 100644
>> --- a/include/linux/led-class-flash.h
>> +++ b/include/linux/led-class-flash.h
>> @@ -28,8 +28,6 @@ struct led_classdev_flash;
>>   #define LED_FAULT_LED_OVER_TEMPERATURE	(1 << 8)
>>   #define LED_NUM_FLASH_FAULTS		9
>>   
>> -#define LED_FLASH_SYSFS_GROUPS_SIZE	5
>> -
>>   struct led_flash_ops {
>>   	/* set flash brightness */
>>   	int (*flash_brightness_set)(struct led_classdev_flash *fled_cdev,
>> @@ -79,9 +77,6 @@ struct led_classdev_flash {
>>   
>>   	/* flash timeout value in microseconds along with its constraints */
>>   	struct led_flash_setting duration;
>> -
>> -	/* LED Flash class sysfs groups */
>> -	const struct attribute_group *sysfs_groups[LED_FLASH_SYSFS_GROUPS_SIZE];
>>   };
>>   
>>   static inline struct led_classdev_flash *lcdev_to_flcdev(

Re: [PATCH 10/10] leds: multicolor: Adjust maximum subled intensity during initialization

[Armin Wolf]

Am 19.10.26 um 13:54 schrieb Rong Zhang:

> Hi Armin,
>
> Nice cleanup!
>
> On Sun, 2026-10-18 at 15:46 +0200, Armin Wolf wrote:
>> The maximum brightness of a LED device is fixed after having called
>> led_classdev_init(), so performing the adjustment during runtime is
>> no longer necessary.
>>
>> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
>> ---
>>   drivers/leds/led-class-multicolor.c | 40 ++++++++++++-----------------
>>   1 file changed, 16 insertions(+), 24 deletions(-)
>>
>> diff --git a/drivers/leds/led-class-multicolor.c b/drivers/leds/led-class-multicolor.c
>> index 9b8c37f7bdf4..875c615c33fc 100644
>> --- a/drivers/leds/led-class-multicolor.c
>> +++ b/drivers/leds/led-class-multicolor.c
>> @@ -14,24 +14,6 @@
>>   #include <linux/uaccess.h>
>>   #include "leds.h"
>>   
>> -static unsigned int led_mc_get_max_intensity(struct led_classdev_mc *mcled_cdev, size_t index)
>> -{
>> -	unsigned int max_intensity;
>> -
>> -	/*
>> -	 * The maximum global brightness value might still be changed by
>> -	 * led_classdev_register_ext() using devicetree properties. This
>> -	 * prevents us from changing subled_info[X].max_intensity when
>> -	 * registering a multicolor LED class device, so we have to do
>> -	 * this during runtime.
>> -	 */
>> -	max_intensity = mcled_cdev->subled_info[index].max_intensity;
>> -	if (max_intensity)
>> -		return max_intensity;
>> -
>> -	return mcled_cdev->led_cdev.max_brightness;
>> -}
>> -
>>   int led_mc_calc_color_components(struct led_classdev_mc *mcled_cdev,
>>   				 enum led_brightness brightness)
>>   {
>> @@ -53,12 +35,10 @@ static ssize_t multi_max_intensity_show(struct device *dev,
>>   {
>>   	struct led_classdev *led_cdev = dev_get_drvdata(dev);
>>   	struct led_classdev_mc *mcled_cdev = lcdev_to_mccdev(led_cdev);
>> -	unsigned int max_intensity;
>>   	int len = 0;
>>   
>> -	for (int i = 0; i < mcled_cdev->num_colors; i++) {
>> -		max_intensity = led_mc_get_max_intensity(mcled_cdev, i);
>> -		len += sysfs_emit_at(buf, len, "%u", max_intensity);
>> +	for (unsigned int i = 0; i < mcled_cdev->num_colors; i++) {
>> +		len += sysfs_emit_at(buf, len, "%u", mcled_cdev->subled_info[i].max_intensity);
>>   		if (i < mcled_cdev->num_colors - 1)
>>   			len += sprintf(buf + len, " ");
>>   	}
>> @@ -98,8 +78,8 @@ static ssize_t multi_intensity_store(struct device *dev,
>>   		goto err_out;
>>   	}
>>   
>> -	for (int i = 0; i < mcled_cdev->num_colors; i++) {
>> -		max_intensity = led_mc_get_max_intensity(mcled_cdev, i);
>> +	for (unsigned int i = 0; i < mcled_cdev->num_colors; i++) {
>> +		max_intensity = mcled_cdev->subled_info[i].max_intensity;
>>   		mcled_cdev->subled_info[i].intensity = min(intensity_value[i], max_intensity);
>>   	}
>>   
>> @@ -187,6 +167,18 @@ int led_classdev_multicolor_register_ext(struct device *parent,
>>   	if (ret < 0)
>>   		return ret;
>>   
>> +	/*
>> +	 * The maximum global brightness value might still be changed by
> s/still be/have been/ ?
>
> Thanks,
> Rong

Good catch, will be fixed in v2.

Thanks,
Armin Wolf

>> +	 * led_classdev_init() using devicetree properties. Because of this
>> +	 * we can only adjust subled_info[X].max_intensity after said function
>> +	 * was called.
>> +	 */
>> +	for (unsigned int i = 0; i < mcled_cdev->num_colors; i++)
>> +	{
>> +		if (!mcled_cdev->subled_info[i].max_intensity)
>> +			mcled_cdev->subled_info[i].max_intensity = led_cdev->max_brightness;
>> +	}
>> +
>>   	led_cdev->dev->type = &led_multicolor_device_type;
>>   	ret = led_classdev_add(led_cdev);
>>   	if (ret < 0)