Re: [PATCH 09/10] leds: Support custom sysfs groups on multicolor and flash LEDs
From: Armin Wolf
Date: Tue Oct 20 2026 - 16:57:48 EST
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 useHmm, a bunch of back-and-forth changes. I suppose the is_visible_const
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@xxxxxx>
---
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,
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(