Re: [PATCH 10/13] leds: backlight trigger: Maintain global list of led backlight triggers

[Lee Jones <lee@kernel.org>]

On Thu, 06 Feb 2025, Thomas Zimmermann wrote:

> Maintain a list of led backlight triggers. This will replace the
> fbdev notifiers that all backlight triggers currently subscribe to.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>  drivers/leds/trigger/ledtrig-backlight.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/leds/trigger/ledtrig-backlight.c b/drivers/leds/trigger/ledtrig-backlight.c
> index 487577d22cfc..c1c1aa60cf07 100644
> --- a/drivers/leds/trigger/ledtrig-backlight.c
> +++ b/drivers/leds/trigger/ledtrig-backlight.c
> @@ -23,8 +23,13 @@ struct bl_trig_notifier {
>  	int old_status;
>  	struct notifier_block notifier;
>  	unsigned invert;
> +
> +	struct list_head entry;

You don't appear to be doing anything with the list here.

It would be better if you introduced the list when it's first utilised.

>  };
>  
> +static struct list_head ledtrig_backlight_list;
> +static struct mutex ledtrig_backlight_list_mutex;
> +
>  static int fb_notifier_callback(struct notifier_block *p,
>  				unsigned long event, void *data)
>  {
> @@ -118,6 +123,10 @@ static int bl_trig_activate(struct led_classdev *led)
>  	if (ret)
>  		dev_err(led->dev, "unable to register backlight trigger\n");
>  
> +	mutex_lock(&ledtrig_backlight_list_mutex);
> +	list_add(&n->entry, &ledtrig_backlight_list);
> +	mutex_unlock(&ledtrig_backlight_list_mutex);
> +
>  	return 0;
>  }
>  
> @@ -125,6 +134,10 @@ static void bl_trig_deactivate(struct led_classdev *led)
>  {
>  	struct bl_trig_notifier *n = led_get_trigger_data(led);
>  
> +	mutex_lock(&ledtrig_backlight_list_mutex);
> +	list_del(&n->entry);
> +	mutex_unlock(&ledtrig_backlight_list_mutex);
> +
>  	fb_unregister_client(&n->notifier);
>  	kfree(n);
>  }
> -- 
> 2.48.1
> 

-- 
Lee Jones [李琼斯]