* [PATCH] led-triggers: accept "default" written to sysfs trigger attr
@ 2025-03-06 22:55 Craig McQueen
2025-03-06 23:32 ` Lee Jones
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Craig McQueen @ 2025-03-06 22:55 UTC (permalink / raw)
To: linux-leds; +Cc: Craig McQueen
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.
---
drivers/leds/led-triggers.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/leds/led-triggers.c b/drivers/leds/led-triggers.c
index b2d40f87a5ff..f2bc3bb5062d 100644
--- a/drivers/leds/led-triggers.c
+++ b/drivers/leds/led-triggers.c
@@ -54,6 +54,11 @@ ssize_t led_trigger_write(struct file *filp, struct kobject *kobj,
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)) {
--
2.48.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH] led-triggers: accept "default" written to sysfs trigger attr 2025-03-06 22:55 [PATCH] led-triggers: accept "default" written to sysfs trigger attr Craig McQueen @ 2025-03-06 23:32 ` Lee Jones 2025-03-07 1:42 ` Craig McQueen 2025-03-07 16:50 ` Jacek Anaszewski 2025-03-14 11:11 ` Lee Jones 2 siblings, 1 reply; 11+ messages in thread From: Lee Jones @ 2025-03-06 23:32 UTC (permalink / raw) To: Craig McQueen; +Cc: linux-leds On Fri, 07 Mar 2025, Craig McQueen wrote: > 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. More info please. Please explain why this is useful. Under what circumstances would the default trigger not be set? > --- > drivers/leds/led-triggers.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/leds/led-triggers.c b/drivers/leds/led-triggers.c > index b2d40f87a5ff..f2bc3bb5062d 100644 > --- a/drivers/leds/led-triggers.c > +++ b/drivers/leds/led-triggers.c > @@ -54,6 +54,11 @@ ssize_t led_trigger_write(struct file *filp, struct kobject *kobj, > 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)) { > -- > 2.48.1 > > -- Lee Jones [李琼斯] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] led-triggers: accept "default" written to sysfs trigger attr 2025-03-06 23:32 ` Lee Jones @ 2025-03-07 1:42 ` Craig McQueen 0 siblings, 0 replies; 11+ messages in thread From: Craig McQueen @ 2025-03-07 1:42 UTC (permalink / raw) To: Lee Jones; +Cc: linux-leds On Fri, 07 Mar 2025 10:32:32 +1100 Lee Jones wrote > On Fri, 07 Mar 2025, Craig McQueen wrote: > > > 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. > > More info please. > > Please explain why this is useful. > > Under what circumstances would the default trigger not be set? Example Scenario: Device has a row of LEDs on the front, with several LEDs linked to a trigger (in the device tree, eg linux,default-trigger = "battery";). In normal operation, the LEDs are driven by the default trigger. But, in some unusual scenarios (eg an error condition, or main application starting up or shutting down) the application wants to temporarily take manual control of the LEDs. Eg turn on a FAULT LED and indicate a fault code on several other LEDs. When the exceptional manual control is finished, the application wants to return all LEDs to their default trigger. With this code change, the application can simply iterate over all LEDs and write "default" to /sys/class/leds/<each-led>/trigger. In general, we want to make the functionality of led_trigger_set_default() to be accessible to userspace. -- Craig McQueen ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] led-triggers: accept "default" written to sysfs trigger attr 2025-03-06 22:55 [PATCH] led-triggers: accept "default" written to sysfs trigger attr Craig McQueen 2025-03-06 23:32 ` Lee Jones @ 2025-03-07 16:50 ` Jacek Anaszewski 2025-03-07 17:10 ` Jacek Anaszewski 2025-03-14 11:11 ` Lee Jones 2 siblings, 1 reply; 11+ messages in thread From: Jacek Anaszewski @ 2025-03-07 16:50 UTC (permalink / raw) To: Craig McQueen, linux-leds Hi Craig, On 3/6/25 23:55, Craig McQueen wrote: > 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. > --- > drivers/leds/led-triggers.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/leds/led-triggers.c b/drivers/leds/led-triggers.c > index b2d40f87a5ff..f2bc3bb5062d 100644 > --- a/drivers/leds/led-triggers.c > +++ b/drivers/leds/led-triggers.c > @@ -54,6 +54,11 @@ ssize_t led_trigger_write(struct file *filp, struct kobject *kobj, > 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)) { Makes sense for me, this would be the second half of the feature that is now available only from DT level. Reviewed-by: Jacek Anaszewski <jacek.anaszewski@gmail.com> -- Best regards, Jacek Anaszewski ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] led-triggers: accept "default" written to sysfs trigger attr 2025-03-07 16:50 ` Jacek Anaszewski @ 2025-03-07 17:10 ` Jacek Anaszewski 2025-03-09 11:33 ` Craig McQueen 0 siblings, 1 reply; 11+ messages in thread From: Jacek Anaszewski @ 2025-03-07 17:10 UTC (permalink / raw) To: Craig McQueen, linux-leds On 3/7/25 17:50, Jacek Anaszewski wrote: > Hi Craig, > > On 3/6/25 23:55, Craig McQueen wrote: >> 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. >> --- >> drivers/leds/led-triggers.c | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/drivers/leds/led-triggers.c b/drivers/leds/led-triggers.c >> index b2d40f87a5ff..f2bc3bb5062d 100644 >> --- a/drivers/leds/led-triggers.c >> +++ b/drivers/leds/led-triggers.c >> @@ -54,6 +54,11 @@ ssize_t led_trigger_write(struct file *filp, struct >> kobject *kobj, >> 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)) { > > Makes sense for me, this would be the second half of the feature that is > now available only from DT level. > > Reviewed-by: Jacek Anaszewski <jacek.anaszewski@gmail.com> > But after re-thinking it - we need to return -EINVAL in case LED class device does not define default trigger, so that the user had proper feedback. So, led_trigger_set_default() needs to be extended to return error in case of !led_cdev->default_trigger or !found. -- Best regards, Jacek Anaszewski ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] led-triggers: accept "default" written to sysfs trigger attr 2025-03-07 17:10 ` Jacek Anaszewski @ 2025-03-09 11:33 ` Craig McQueen 2025-03-09 18:50 ` Jacek Anaszewski 0 siblings, 1 reply; 11+ messages in thread From: Craig McQueen @ 2025-03-09 11:33 UTC (permalink / raw) To: Jacek Anaszewski; +Cc: linux-leds On Sat, 08 Mar 2025 04:10:49 +1100 Jacek Anaszewski wrote: > On 3/7/25 17:50, Jacek Anaszewski wrote: > > Hi Craig, > > > > On 3/6/25 23:55, Craig McQueen wrote: > >> 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. > >> --- > >> drivers/leds/led-triggers.c | 5 +++++ > >> 1 file changed, 5 insertions(+) > >> > >> diff --git a/drivers/leds/led-triggers.c b/drivers/leds/led-triggers.c > >> index b2d40f87a5ff..f2bc3bb5062d 100644 > >> --- a/drivers/leds/led-triggers.c > >> +++ b/drivers/leds/led-triggers.c > >> @@ -54,6 +54,11 @@ ssize_t led_trigger_write(struct file *filp, struct > >> kobject *kobj, > >> 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)) { > > > > Makes sense for me, this would be the second half of the feature that is > > now available only from DT level. > > > > Reviewed-by: Jacek Anaszewski jacek.anaszewski@gmail.com> > > > > But after re-thinking it - we need to return -EINVAL in case > LED class device does not define default trigger, so that the user > had proper feedback. > > So, led_trigger_set_default() needs to be extended to return error > in case of !led_cdev->default_trigger or !found. In systems I've worked on, some LEDs have a default trigger, while others don't. I.e. it seems normal for an LED to have a default trigger of "none". I don't think of this as an error condition, but a normal operation to set an LED's trigger back to "none". The not-found case is an interesting corner case. It might be that a kernel module that provides a trigger is presently not loaded, so the trigger is not currently available -- but will be available if the kernel module is loaded again. 1. LED has a default trigger "moduletrigger". 2. Module that provides that trigger "moduletrigger" is unloaded. 3. LED has trigger set to something else, "othertrigger". 4. led_trigger_set_default() is called for that LED. Will the LED's trigger be effectively "none", or stay at "othertrigger"? 5. Module that provides "moduletrigger" is loaded again. Will the LED be connected to its default trigger "moduletrigger", or remain at "none"? -- Craig McQueen ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] led-triggers: accept "default" written to sysfs trigger attr 2025-03-09 11:33 ` Craig McQueen @ 2025-03-09 18:50 ` Jacek Anaszewski 2025-03-10 20:18 ` Jacek Anaszewski 2025-03-11 10:27 ` Craig McQueen 0 siblings, 2 replies; 11+ messages in thread From: Jacek Anaszewski @ 2025-03-09 18:50 UTC (permalink / raw) To: Craig McQueen; +Cc: linux-leds On 3/9/25 12:33, Craig McQueen wrote: > On Sat, 08 Mar 2025 04:10:49 +1100 Jacek Anaszewski wrote: > > On 3/7/25 17:50, Jacek Anaszewski wrote: > > > Hi Craig, > > > > > > On 3/6/25 23:55, Craig McQueen wrote: > > >> 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. > > >> --- > > >> drivers/leds/led-triggers.c | 5 +++++ > > >> 1 file changed, 5 insertions(+) > > >> > > >> diff --git a/drivers/leds/led-triggers.c b/drivers/leds/led-triggers.c > > >> index b2d40f87a5ff..f2bc3bb5062d 100644 > > >> --- a/drivers/leds/led-triggers.c > > >> +++ b/drivers/leds/led-triggers.c > > >> @@ -54,6 +54,11 @@ ssize_t led_trigger_write(struct file *filp, struct > > >> kobject *kobj, > > >> 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)) { > > > > > > Makes sense for me, this would be the second half of the feature that is > > > now available only from DT level. > > > > > > Reviewed-by: Jacek Anaszewski jacek.anaszewski@gmail.com> > > > > > > > But after re-thinking it - we need to return -EINVAL in case > > LED class device does not define default trigger, so that the user > > had proper feedback. > > > > So, led_trigger_set_default() needs to be extended to return error > > in case of !led_cdev->default_trigger or !found. > > In systems I've worked on, some LEDs have a default trigger, while others don't. I.e. it seems normal for an LED to have a default trigger of "none". I don't think of this as an error condition, but a normal operation to set an LED's trigger back to "none". > > The not-found case is an interesting corner case. It might be that a kernel module that provides a trigger is presently not loaded, so the trigger is not currently available -- but will be available if the kernel module is loaded again. Fair enough. It would be good to add this description to the entry related to "trigger" file in Documentation/ABI/testing/sysfs-class-led. > > 1. LED has a default trigger "moduletrigger". > 2. Module that provides that trigger "moduletrigger" is unloaded. > 3. LED has trigger set to something else, "othertrigger". > 4. led_trigger_set_default() is called for that LED. > Will the LED's trigger be effectively "none", or stay at "othertrigger"? > 5. Module that provides "moduletrigger" is loaded again. > Will the LED be connected to its default trigger "moduletrigger", or remain at "none"? > -- Best regards, Jacek Anaszewski ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] led-triggers: accept "default" written to sysfs trigger attr 2025-03-09 18:50 ` Jacek Anaszewski @ 2025-03-10 20:18 ` Jacek Anaszewski 2025-03-11 10:27 ` Craig McQueen 1 sibling, 0 replies; 11+ messages in thread From: Jacek Anaszewski @ 2025-03-10 20:18 UTC (permalink / raw) To: Craig McQueen; +Cc: linux-leds On 3/9/25 19:50, Jacek Anaszewski wrote: > > > On 3/9/25 12:33, Craig McQueen wrote: >> On Sat, 08 Mar 2025 04:10:49 +1100 Jacek Anaszewski wrote: >> > On 3/7/25 17:50, Jacek Anaszewski wrote: >> > > Hi Craig, >> > > >> > > On 3/6/25 23:55, Craig McQueen wrote: >> > >> 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. >> > >> --- >> > >> drivers/leds/led-triggers.c | 5 +++++ >> > >> 1 file changed, 5 insertions(+) >> > >> >> > >> diff --git a/drivers/leds/led-triggers.c b/drivers/leds/led- >> triggers.c >> > >> index b2d40f87a5ff..f2bc3bb5062d 100644 >> > >> --- a/drivers/leds/led-triggers.c >> > >> +++ b/drivers/leds/led-triggers.c >> > >> @@ -54,6 +54,11 @@ ssize_t led_trigger_write(struct file *filp, >> struct >> > >> kobject *kobj, >> > >> 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)) { >> > > >> > > Makes sense for me, this would be the second half of the feature >> that is >> > > now available only from DT level. >> > > >> > > Reviewed-by: Jacek Anaszewski jacek.anaszewski@gmail.com> >> > > >> > >> > But after re-thinking it - we need to return -EINVAL in case >> > LED class device does not define default trigger, so that the user >> > had proper feedback. >> > >> > So, led_trigger_set_default() needs to be extended to return error >> > in case of !led_cdev->default_trigger or !found. >> >> In systems I've worked on, some LEDs have a default trigger, while >> others don't. I.e. it seems normal for an LED to have a default >> trigger of "none". I don't think of this as an error condition, but a >> normal operation to set an LED's trigger back to "none". >> >> The not-found case is an interesting corner case. It might be that a >> kernel module that provides a trigger is presently not loaded, so the >> trigger is not currently available -- but will be available if the >> kernel module is loaded again. > > Fair enough. > It would be good to add this description to the entry related to > "trigger" file in Documentation/ABI/testing/sysfs-class-led. > Forgot to answer to the below sequence. >> 1. LED has a default trigger "moduletrigger". >> 2. Module that provides that trigger "moduletrigger" is unloaded. Now led_trigger_unregister() is called, the trigger is being removed from trigger_list and LED class devices having it set, have their trigger property set to NULL. >> 3. LED has trigger set to something else, "othertrigger". LED class device trigger property is being initialized with a pointer to the related struct led_trigger. >> 4. led_trigger_set_default() is called for that LED. >> Will the LED's trigger be effectively "none", or stay at >> "othertrigger"? Will stay at "othertrigger", since led_trigger_set_default() will end up in !found state. >> 5. Module that provides "moduletrigger" is loaded again. >> Will the LED be connected to its default trigger "moduletrigger", >> or remain at "none"? Will remain at "othertrigger". led_trigger_register() would set default trigger for LED class device only if no trigger is set for the LED, and the name matches LED's default trigger -- Best regards, Jacek Anaszewski ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] led-triggers: accept "default" written to sysfs trigger attr 2025-03-09 18:50 ` Jacek Anaszewski 2025-03-10 20:18 ` Jacek Anaszewski @ 2025-03-11 10:27 ` Craig McQueen 2025-03-11 19:22 ` Jacek Anaszewski 1 sibling, 1 reply; 11+ messages in thread From: Craig McQueen @ 2025-03-11 10:27 UTC (permalink / raw) To: Jacek Anaszewski; +Cc: linux-leds On Mon, 10 Mar 2025 05:50:23 +1100 Jacek Anaszewski wrote: > On 3/9/25 12:33, Craig McQueen wrote: > > On Sat, 08 Mar 2025 04:10:49 +1100 Jacek Anaszewski wrote: > > > On 3/7/25 17:50, Jacek Anaszewski wrote: > > > > Hi Craig, > > > > > > > > On 3/6/25 23:55, Craig McQueen wrote: > > > >> 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. > > > >> --- > > > >> drivers/leds/led-triggers.c | 5 +++++ > > > >> 1 file changed, 5 insertions(+) > > > >> > > > >> diff --git a/drivers/leds/led-triggers.c b/drivers/leds/led-triggers.c > > > >> index b2d40f87a5ff..f2bc3bb5062d 100644 > > > >> --- a/drivers/leds/led-triggers.c > > > >> +++ b/drivers/leds/led-triggers.c > > > >> @@ -54,6 +54,11 @@ ssize_t led_trigger_write(struct file *filp, struct > > > >> kobject *kobj, > > > >> 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)) { > > > > > > > > Makes sense for me, this would be the second half of the feature that is > > > > now available only from DT level. > > > > > > > > Reviewed-by: Jacek Anaszewski jacek.anaszewski@gmail.com> > > > > > > > > > > But after re-thinking it - we need to return -EINVAL in case > > > LED class device does not define default trigger, so that the user > > > had proper feedback. > > > > > > So, led_trigger_set_default() needs to be extended to return error > > > in case of !led_cdev->default_trigger or !found. > > > > In systems I've worked on, some LEDs have a default trigger, while others don't. I.e. it seems normal for an LED to have a default trigger of "none". I don't think of this as an error condition, but a normal operation to set an LED's trigger back to "none". > > > > The not-found case is an interesting corner case. It might be that a kernel module that provides a trigger is presently not loaded, so the trigger is not currently available -- but will be available if the kernel module is loaded again. > > Fair enough. > It would be good to add this description to the entry related to > "trigger" file in Documentation/ABI/testing/sysfs-class-led. I tried to update that document. But I wasn't sure what the required format is, when I'm not adding a new attribute but (slightly) modifying the behaviour of an existing attribute. Should I add a note to the existing /sys/class/leds/<led>/trigger description, or should I add a new /sys/class/leds/<led>/trigger entry at the bottom of the document, describing the modified behaviour? -- Craig McQueen ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] led-triggers: accept "default" written to sysfs trigger attr 2025-03-11 10:27 ` Craig McQueen @ 2025-03-11 19:22 ` Jacek Anaszewski 0 siblings, 0 replies; 11+ messages in thread From: Jacek Anaszewski @ 2025-03-11 19:22 UTC (permalink / raw) To: Craig McQueen; +Cc: linux-leds On 3/11/25 11:27, Craig McQueen wrote: > On Mon, 10 Mar 2025 05:50:23 +1100 Jacek Anaszewski wrote: > > On 3/9/25 12:33, Craig McQueen wrote: > > > On Sat, 08 Mar 2025 04:10:49 +1100 Jacek Anaszewski wrote: > > > > On 3/7/25 17:50, Jacek Anaszewski wrote: > > > > > Hi Craig, > > > > > > > > > > On 3/6/25 23:55, Craig McQueen wrote: > > > > >> 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. > > > > >> --- > > > > >> drivers/leds/led-triggers.c | 5 +++++ > > > > >> 1 file changed, 5 insertions(+) > > > > >> > > > > >> diff --git a/drivers/leds/led-triggers.c b/drivers/leds/led-triggers.c > > > > >> index b2d40f87a5ff..f2bc3bb5062d 100644 > > > > >> --- a/drivers/leds/led-triggers.c > > > > >> +++ b/drivers/leds/led-triggers.c > > > > >> @@ -54,6 +54,11 @@ ssize_t led_trigger_write(struct file *filp, struct > > > > >> kobject *kobj, > > > > >> 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)) { > > > > > > > > > > Makes sense for me, this would be the second half of the feature that is > > > > > now available only from DT level. > > > > > > > > > > Reviewed-by: Jacek Anaszewski jacek.anaszewski@gmail.com> > > > > > > > > > > > > > But after re-thinking it - we need to return -EINVAL in case > > > > LED class device does not define default trigger, so that the user > > > > had proper feedback. > > > > > > > > So, led_trigger_set_default() needs to be extended to return error > > > > in case of !led_cdev->default_trigger or !found. > > > > > > In systems I've worked on, some LEDs have a default trigger, while others don't. I.e. it seems normal for an LED to have a default trigger of "none". I don't think of this as an error condition, but a normal operation to set an LED's trigger back to "none". > > > > > > The not-found case is an interesting corner case. It might be that a kernel module that provides a trigger is presently not loaded, so the trigger is not currently available -- but will be available if the kernel module is loaded again. > > > > Fair enough. > > It would be good to add this description to the entry related to > > "trigger" file in Documentation/ABI/testing/sysfs-class-led. > > > I tried to update that document. But I wasn't sure what the required > format is, when I'm not adding a new attribute but (slightly) modifying > the behaviour of an existing attribute. Should I add a note to the existing > /sys/class/leds/<led>/trigger description, or should I add a new Just extend existing documentation of the /sys/class/leds/<led>/trigger file. The intention is to provide a description of the new use case you're adding for that file. Now I've come to the conclusion, that led_trigger_format() should add also "default" as of the strings accepted by the file on write, similarly as it adds "none". However "default" would never be displayed in brackets, as the selected one of course. Instead the exact default trigger or "none" would be put in brackets, as it is done currently. That way it would be self-documenting from the user perspective. > /sys/class/leds/<led>/trigger entry at the bottom of the document, > describing the modified behaviour? > -- Best regards, Jacek Anaszewski ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] led-triggers: accept "default" written to sysfs trigger attr 2025-03-06 22:55 [PATCH] led-triggers: accept "default" written to sysfs trigger attr Craig McQueen 2025-03-06 23:32 ` Lee Jones 2025-03-07 16:50 ` Jacek Anaszewski @ 2025-03-14 11:11 ` Lee Jones 2 siblings, 0 replies; 11+ messages in thread From: Lee Jones @ 2025-03-14 11:11 UTC (permalink / raw) To: Craig McQueen; +Cc: linux-leds On Fri, 07 Mar 2025, Craig McQueen wrote: > 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. > --- > drivers/leds/led-triggers.c | 5 +++++ > 1 file changed, 5 insertions(+) As before. If you don't know what I'm talking about, check your other mails from me. -- Lee Jones [李琼斯] ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2025-03-14 11:12 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-03-06 22:55 [PATCH] led-triggers: accept "default" written to sysfs trigger attr Craig McQueen 2025-03-06 23:32 ` Lee Jones 2025-03-07 1:42 ` Craig McQueen 2025-03-07 16:50 ` Jacek Anaszewski 2025-03-07 17:10 ` Jacek Anaszewski 2025-03-09 11:33 ` Craig McQueen 2025-03-09 18:50 ` Jacek Anaszewski 2025-03-10 20:18 ` Jacek Anaszewski 2025-03-11 10:27 ` Craig McQueen 2025-03-11 19:22 ` Jacek Anaszewski 2025-03-14 11:11 ` Lee Jones
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox