Re: [PATCH v7 02/13] leds: trigger: Move led_trigger_is_hw_controlled() to the right place

[Lee Jones]

--- checkpatch.pl: clean (0 issues) ---

On Mon, 21 Sep 2026, Rong Zhang wrote:

> Currently led_trigger_is_hw_controlled() is placed at led-class.c, which
> is not an right place as it falls into the triggers namespace and does
> triggers stuff.
> 
> Move it into led-triggers.c, and split it into locked and unlocked
> variant for convenience.
> 
> Fixes: b819dc7d8fb2 ("leds: core: Report ENODATA for brightness of hardware controlled LED")
> Acked-by: Ike Panhc <ikepanhc@xxxxxxxxx>
> Signed-off-by: Rong Zhang <i@xxxxxxxx>
> ---
> Changes in v3:
> - New patch in the series, the dependency of the following patches
> ---
>  drivers/leds/led-class.c    | 10 ----------
>  drivers/leds/led-triggers.c | 19 +++++++++++++++++++
>  include/linux/leds.h        |  8 ++++++++
>  3 files changed, 27 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
> index 4259533dafa1..7110bfbe3b83 100644
> --- a/drivers/leds/led-class.c
> +++ b/drivers/leds/led-class.c
> @@ -27,16 +27,6 @@ static LIST_HEAD(leds_lookup_list);
>  
>  static struct workqueue_struct *leds_wq;
>  
> -static bool led_trigger_is_hw_controlled(struct led_classdev *led_cdev)
> -{
> -#ifdef CONFIG_LEDS_TRIGGERS
> -	guard(rwsem_read)(&led_cdev->trigger_lock);
> -	return led_cdev->trigger && led_cdev->trigger->trigger_type;
> -#else
> -	return false;
> -#endif
> -}
> -
>  static ssize_t brightness_show(struct device *dev,
>  		struct device_attribute *attr, char *buf)
>  {
> diff --git a/drivers/leds/led-triggers.c b/drivers/leds/led-triggers.c
> index b1223218bda1..bf2543538ed0 100644
> --- a/drivers/leds/led-triggers.c
> +++ b/drivers/leds/led-triggers.c
> @@ -7,9 +7,11 @@
>   * Author: Richard Purdie <rpurdie@xxxxxxxxxxxxxx>
>   */
>  
> +#include <linux/cleanup.h>
>  #include <linux/export.h>
>  #include <linux/kernel.h>
>  #include <linux/list.h>
> +#include <linux/lockdep.h>
>  #include <linux/spinlock.h>
>  #include <linux/device.h>
>  #include <linux/timer.h>
> @@ -33,6 +35,23 @@ trigger_relevant(struct led_classdev *led_cdev, struct led_trigger *trig)
>  	return !trig->trigger_type || trig->trigger_type == led_cdev->trigger_type;
>  }
>  
> +static bool __led_trigger_is_hw_controlled(struct led_classdev *led_cdev)
> +{
> +	lockdep_assert_held(&led_cdev->trigger_lock);
> +
> +	if (!led_cdev->trigger)
> +		return false;
> +
> +	return led_cdev->trigger->trigger_type;

How does this not cause a compiler warning?

This needs to be evaluated properly.

> +}
> +
> +bool led_trigger_is_hw_controlled(struct led_classdev *led_cdev)
> +{
> +	guard(rwsem_read)(&led_cdev->trigger_lock);
> +	return __led_trigger_is_hw_controlled(led_cdev);
> +}
> +EXPORT_SYMBOL_GPL(led_trigger_is_hw_controlled);
> +
>  ssize_t led_trigger_write(struct file *filp, struct kobject *kobj,
>  			  const struct bin_attribute *bin_attr, char *buf,
>  			  loff_t pos, size_t count)
> diff --git a/include/linux/leds.h b/include/linux/leds.h
> index a515f075c29a..46ad2e03e1c6 100644
> --- a/include/linux/leds.h
> +++ b/include/linux/leds.h
> @@ -534,6 +534,8 @@ void led_trigger_set_default(struct led_classdev *led_cdev);
>  int led_trigger_set(struct led_classdev *led_cdev, struct led_trigger *trigger);
>  void led_trigger_remove(struct led_classdev *led_cdev);
>  
> +bool led_trigger_is_hw_controlled(struct led_classdev *led_cdev);
> +
>  static inline void led_set_trigger_data(struct led_classdev *led_cdev,
>  					void *trigger_data)
>  {
> @@ -584,6 +586,12 @@ static inline int led_trigger_set(struct led_classdev *led_cdev,
>  }
>  
>  static inline void led_trigger_remove(struct led_classdev *led_cdev) {}
> +
> +static inline bool led_trigger_is_hw_controlled(struct led_classdev *led_cdev)
> +{
> +	return false;
> +}
> +
>  static inline void led_set_trigger_data(struct led_classdev *led_cdev) {}
>  static inline void *led_get_trigger_data(struct led_classdev *led_cdev)
>  {
> 
> -- 
> 2.55.0
> 

-- 
Lee Jones