Messages in this thread Patch in this message |  | | Subject | Re: [RFC PATCH 1/9] leds: Load trigger modules on-demand if used as hw control trigger | | From | Rong Zhang <> | | Date | Thu, 12 Mar 2026 00:18:30 +0800 |
| |
Hi Lee,
On Tue, 2026-03-10 at 12:01 +0000, Lee Jones wrote: > On Sat, 28 Feb 2026, Rong Zhang wrote: > > > In the following patches, we are about to support hardware initiated > > Let's not talk about other commits. Only tell us what's happening here. > > > trigger transitions to/from the device's hw control trigger. In case > > the LED hardware switches itself to hw control mode, hw control trigger > > must be loaded before so that the transition can be processed. > > > > Load the trigger module specified by hw_control_trigger, so that > > hardware initiated trigger transitions can be processed when the hw > > "hardware" > > > control trigger is compiled as a module. > > > > Signed-off-by: Rong Zhang <i@rong.moe> > > --- > > drivers/leds/led-class.c | 1 + > > drivers/leds/led-triggers.c | 33 +++++++++++++++++++++++++++++++++ > > drivers/leds/leds.h | 1 + > > 3 files changed, 35 insertions(+) > > > > diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c > > index d34a194535604..0fa45f22246e3 100644 > > --- a/drivers/leds/led-class.c > > +++ b/drivers/leds/led-class.c > > @@ -576,6 +576,7 @@ int led_classdev_register_ext(struct device *parent, > > > > #ifdef CONFIG_LEDS_TRIGGERS > > led_trigger_set_default(led_cdev); > > + led_load_hw_control_trigger(led_cdev); > > led_trigger_load_hw_control > > > #endif > > > > mutex_unlock(&led_cdev->led_access); > > diff --git a/drivers/leds/led-triggers.c b/drivers/leds/led-triggers.c > > index b1223218bda11..3066bc91a5f94 100644 > > --- a/drivers/leds/led-triggers.c > > +++ b/drivers/leds/led-triggers.c > > @@ -313,6 +313,39 @@ void led_trigger_set_default(struct led_classdev *led_cdev) > > } > > EXPORT_SYMBOL_GPL(led_trigger_set_default); > > > > +static inline bool led_match_hw_control_trigger(struct led_classdev *led_cdev, > > + struct led_trigger *trig) > > +{ > > + return (!strcmp(led_cdev->hw_control_trigger, trig->name) && > > + trigger_relevant(led_cdev, trig)); > > > This is ugly. Break it out and provide some commentary. > > > +} > > + > > +void led_load_hw_control_trigger(struct led_classdev *led_cdev) > > +{ > > + struct led_trigger *trig; > > + bool found = false; > > + > > + if (!led_cdev->hw_control_trigger) > > + return; > > + > > + /* default_trigger is handled by led_trigger_set_default(). */ > > Sentences start with uppercase chars. > > > + if (led_cdev->default_trigger && > > + !strcmp(led_cdev->default_trigger, led_cdev->hw_control_trigger)) > > + return; > > Do you need to check default_trigger? > > strcmp() should be able to handle empty strings.
It checks the NULLity of led_cdev->default_trigger instead of its emptiness. While passing an empty string to strcmp is fine, passing a NULL pointer to strcmp() leads to a NULL dereference.
Thanks for your review. ACK to other comments :)
Thanks, Rong
> > > + > > + down_read(&triggers_list_lock); > > + list_for_each_entry(trig, &trigger_list, next_trig) { > > + found = led_match_hw_control_trigger(led_cdev, trig); > > + if (found) > > + break; > > + } > > + up_read(&triggers_list_lock); > > + > > + if (!found) > > + request_module_nowait("ledtrig:%s", led_cdev->hw_control_trigger); > > +} > > +EXPORT_SYMBOL_GPL(led_load_hw_control_trigger); > > + > > /* LED Trigger Interface */ > > > > int led_trigger_register(struct led_trigger *trig) > > diff --git a/drivers/leds/leds.h b/drivers/leds/leds.h > > index bee46651e068f..e85afd4d04fd0 100644 > > --- a/drivers/leds/leds.h > > +++ b/drivers/leds/leds.h > > @@ -21,6 +21,7 @@ void led_init_core(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); > > +void led_load_hw_control_trigger(struct led_classdev *led_cdev); > > ssize_t led_trigger_read(struct file *filp, struct kobject *kobj, > > const struct bin_attribute *attr, char *buf, > > loff_t pos, size_t count); > > -- > > 2.51.0 > >
|  |