Re: [RFC PATCH 1/9] leds: Load trigger modules on-demand if used as hw control trigger

[Lee Jones <lee@kernel.org>]

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.

> +
> +	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
> 

-- 
Lee Jones [李琼斯]