Message ID | 20250312011136.380647-1-craig@mcqueen.au |
---|---|
State | New |
Headers | show |
Series | [v2] leds: led-triggers: Improvements for default trigger | expand |
Hi Craig, Thanks for the update. On 3/12/25 02:11, Craig McQueen wrote: > Accept "default" written to sysfs trigger attr. > If the text "default" is written to the LED's sysfs 'trigger' attr, then > call led_trigger_set_default() to set the LED to its default trigger. > > If the default trigger is set to "none", then led_trigger_set_default() > will remove a trigger. This is in contrast to the default trigger being > unset, in which case led_trigger_set_default() does nothing. > --- > Documentation/ABI/testing/sysfs-class-led | 6 ++++++ > drivers/leds/led-triggers.c | 26 ++++++++++++++++++++++- > 2 files changed, 31 insertions(+), 1 deletion(-) > > diff --git a/Documentation/ABI/testing/sysfs-class-led b/Documentation/ABI/testing/sysfs-class-led > index 2e24ac3bd7ef..0313b82644f2 100644 > --- a/Documentation/ABI/testing/sysfs-class-led > +++ b/Documentation/ABI/testing/sysfs-class-led > @@ -72,6 +72,12 @@ Description: > /sys/class/leds/<led> once a given trigger is selected. For > their documentation see `sysfs-class-led-trigger-*`. > > + Writing "none" removes the trigger for this LED. > + > + Writing "default" sets the trigger to the LED's default trigger > + (which would often be configured in the device tree for the > + hardware). > + > What: /sys/class/leds/<led>/inverted > Date: January 2011 > KernelVersion: 2.6.38 > diff --git a/drivers/leds/led-triggers.c b/drivers/leds/led-triggers.c > index b2d40f87a5ff..00c898af9969 100644 > --- a/drivers/leds/led-triggers.c > +++ b/drivers/leds/led-triggers.c > @@ -49,11 +49,21 @@ ssize_t led_trigger_write(struct file *filp, struct kobject *kobj, > goto unlock; > } > > + /* Empty string. Do nothing. */ > + if (sysfs_streq(buf, "")) { > + goto unlock; > + } > + Do we need this? It seems to be the same case as any other arbitrary string, for which we obviously don't have special handling. > if (sysfs_streq(buf, "none")) { > led_trigger_remove(led_cdev); > goto unlock; > } > > + if (sysfs_streq(buf, "default")) { > + led_trigger_set_default(led_cdev); > + goto unlock; > + } > + > down_read(&triggers_list_lock); > list_for_each_entry(trig, &trigger_list, next_trig) { > if (sysfs_streq(buf, trig->name) && trigger_relevant(led_cdev, trig)) { > @@ -98,6 +108,9 @@ static int led_trigger_format(char *buf, size_t size, > int len = led_trigger_snprintf(buf, size, "%s", > led_cdev->trigger ? "none" : "[none]"); > > + if (led_cdev->default_trigger && led_cdev->default_trigger[0]) > + len += led_trigger_snprintf(buf + len, size - len, " default"); > + > list_for_each_entry(trig, &trigger_list, next_trig) { > bool hit; > > @@ -278,9 +291,20 @@ void led_trigger_set_default(struct led_classdev *led_cdev) > struct led_trigger *trig; > bool found = false; > > - if (!led_cdev->default_trigger) > + /* NULL pointer or empty string. Do nothing. */ > + if (!led_cdev->default_trigger || !led_cdev->default_trigger[0]) > return; > > + /* This case isn't sensible. Do nothing. */ > + if (!strcmp(led_cdev->default_trigger, "default")) > + return; This is rather validation of the default trigger name obtained from DT, which, if at all, should be done after parsing DT property in led_classdev_register_ext(). I'd skip it anyway. > + /* Remove trigger. */ > + if (!strcmp(led_cdev->default_trigger, "none")) { > + led_trigger_remove(led_cdev); This will be handled be led_trigger_set() called from led_match_default_trigger() below. > + return; > + } > + > down_read(&triggers_list_lock); > down_write(&led_cdev->trigger_lock); > list_for_each_entry(trig, &trigger_list, next_trig) {
diff --git a/Documentation/ABI/testing/sysfs-class-led b/Documentation/ABI/testing/sysfs-class-led index 2e24ac3bd7ef..0313b82644f2 100644 --- a/Documentation/ABI/testing/sysfs-class-led +++ b/Documentation/ABI/testing/sysfs-class-led @@ -72,6 +72,12 @@ Description: /sys/class/leds/<led> once a given trigger is selected. For their documentation see `sysfs-class-led-trigger-*`. + Writing "none" removes the trigger for this LED. + + Writing "default" sets the trigger to the LED's default trigger + (which would often be configured in the device tree for the + hardware). + What: /sys/class/leds/<led>/inverted Date: January 2011 KernelVersion: 2.6.38 diff --git a/drivers/leds/led-triggers.c b/drivers/leds/led-triggers.c index b2d40f87a5ff..00c898af9969 100644 --- a/drivers/leds/led-triggers.c +++ b/drivers/leds/led-triggers.c @@ -49,11 +49,21 @@ ssize_t led_trigger_write(struct file *filp, struct kobject *kobj, goto unlock; } + /* Empty string. Do nothing. */ + if (sysfs_streq(buf, "")) { + goto unlock; + } + if (sysfs_streq(buf, "none")) { led_trigger_remove(led_cdev); goto unlock; } + if (sysfs_streq(buf, "default")) { + led_trigger_set_default(led_cdev); + goto unlock; + } + down_read(&triggers_list_lock); list_for_each_entry(trig, &trigger_list, next_trig) { if (sysfs_streq(buf, trig->name) && trigger_relevant(led_cdev, trig)) { @@ -98,6 +108,9 @@ static int led_trigger_format(char *buf, size_t size, int len = led_trigger_snprintf(buf, size, "%s", led_cdev->trigger ? "none" : "[none]"); + if (led_cdev->default_trigger && led_cdev->default_trigger[0]) + len += led_trigger_snprintf(buf + len, size - len, " default"); + list_for_each_entry(trig, &trigger_list, next_trig) { bool hit; @@ -278,9 +291,20 @@ void led_trigger_set_default(struct led_classdev *led_cdev) struct led_trigger *trig; bool found = false; - if (!led_cdev->default_trigger) + /* NULL pointer or empty string. Do nothing. */ + if (!led_cdev->default_trigger || !led_cdev->default_trigger[0]) return; + /* This case isn't sensible. Do nothing. */ + if (!strcmp(led_cdev->default_trigger, "default")) + return; + + /* Remove trigger. */ + if (!strcmp(led_cdev->default_trigger, "none")) { + led_trigger_remove(led_cdev); + return; + } + down_read(&triggers_list_lock); down_write(&led_cdev->trigger_lock); list_for_each_entry(trig, &trigger_list, next_trig) {