Re: [PATCH] leds: pca9532: don't stop blinking for non-zero brightness

[Lee Jones <lee@kernel.org>]

On Sat, 21 Mar 2026, Tobias Deiminger wrote:

> pca9532 unexpectedly stopped blinking when changing brightness to a
> non-zero value. To reproduce:
> 
>  echo timer > /sys/class/leds/led-1/trigger   # blinks
>  echo 255 > /sys/class/leds/led-1/brightness  # blinking stops, light on
>  cat /sys/class/leds/led-1/trigger            # still claims [timer]
> 
> According to Documentation/leds/leds-class.rst, only brightness = 0
> shall be a stop condition:
> 
> > You can change the brightness value of a LED independently of the
> > timer trigger. However, if you set the brightness value to LED_OFF it
> > will also disable the timer trigger.
> 
> Therefore add a guard to continue blinking when brightness != LED_OFF,
> similar to how pca955x does it since 575f10dc64a2 ("leds: pca955x: Add
> HW blink support").
> 
> Signed-off-by: Tobias Deiminger <tobias.deiminger@linutronix.de>
> ---
> 
> Notes:
>     A more advanced solution was to not simply return early on
>     set_brightness, but actually support blinking at arbitrary brightness.
>     This would however require conditional fallback to SW blinking, since
>     PCA 9532 doesn't support routing PWM 0 (dim) and PWM 1 (blink) in
>     series. The bugfix here keeps it simple. Optional SW blinking could be
>     tried in a separate patch.
> 
>  drivers/leds/leds-pca9532.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/leds/leds-pca9532.c b/drivers/leds/leds-pca9532.c
> index 0344189bb991..3de20e087334 100644
> --- a/drivers/leds/leds-pca9532.c
> +++ b/drivers/leds/leds-pca9532.c
> @@ -184,6 +184,8 @@ static int pca9532_set_brightness(struct led_classdev *led_cdev,
>  
>  	if (value == LED_OFF)
>  		led->state = PCA9532_OFF;
> +	else if (led->state == PCA9532_PWM1)
> +		return 0; /* non-zero brightness shall not stop HW blinking */

Comments should start with a capital letter.

Also, as the final 'else' statement uses braces, should we perhaps take the
opportunity to add braces to all branches of this conditional block?

>  	else if (value == LED_FULL)
>  		led->state = PCA9532_ON;
>  	else {
> 
> base-commit: b2c87f5e98cd88095dbc6802197526703d5e4e48
> -- 
> 2.47.3
> 
> 

-- 
Lee Jones [李琼斯]