Re: [RFC 1/2] leds: trigger: input-events: Replace led_on with a flags field

[Lee Jones <lee@kernel.org>]

On Sat, 01 Jun 2024, Hans de Goede wrote:

> Replace the led_on boolean with a flags field, using bit 0 for FL_LED_ON,
> this is a preparation patch for adding further flags.

I'm generally okay with the principle.

> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/leds/trigger/ledtrig-input-events.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/leds/trigger/ledtrig-input-events.c b/drivers/leds/trigger/ledtrig-input-events.c
> index 1de0176799f9..94d5580ea093 100644
> --- a/drivers/leds/trigger/ledtrig-input-events.c
> +++ b/drivers/leds/trigger/ledtrig-input-events.c
> @@ -17,14 +17,16 @@
>  
>  #define DEFAULT_LED_OFF_DELAY_MS			5000
>  
> +/* To avoid repeatedly setting the brightness while there are events */
> +#define FL_LED_ON					0

FL is non-obvious.

FLAGS_LED_ON?

> +
>  struct input_events_data {
>  	struct input_handler handler;
>  	struct delayed_work work;
>  	spinlock_t lock;
>  	struct led_classdev *led_cdev;
>  	int led_cdev_saved_flags;
> -	/* To avoid repeatedly setting the brightness while there are events */
> -	bool led_on;
> +	unsigned long flags;
>  	unsigned long led_off_time;
>  	unsigned long led_off_delay;
>  };
> @@ -42,7 +44,7 @@ static void led_input_events_work(struct work_struct *work)
>  	 */
>  	if (time_after_eq(jiffies, data->led_off_time)) {
>  		led_set_brightness_nosleep(data->led_cdev, LED_OFF);
> -		data->led_on = false;
> +		clear_bit(FL_LED_ON, &data->flags);
>  	}
>  
>  	spin_unlock_irq(&data->lock);
> @@ -95,10 +97,9 @@ static void input_events_event(struct input_handle *handle, unsigned int type,
>  
>  	spin_lock_irqsave(&data->lock, flags);
>  
> -	if (!data->led_on) {
> +	if (!test_and_set_bit(FL_LED_ON, &data->flags))
>  		led_set_brightness_nosleep(led_cdev, led_cdev->blink_brightness);
> -		data->led_on = true;
> -	}
> +
>  	data->led_off_time = jiffies + led_off_delay;
>  
>  	spin_unlock_irqrestore(&data->lock, flags);
> -- 
> 2.45.1
> 

-- 
Lee Jones [李琼斯]