Re: [PATCH] leds: core: Omit set_brightness error message for a LED supporting hw trigger only

[Lee Jones <lee@kernel.org>]

On Thu, 11 Apr 2024, Lee Jones wrote:

> On Fri, 05 Apr 2024, Heiner Kallweit wrote:
> 
> > If both set_brightness functions return -ENOTSUPP, then the LED doesn't
> > support setting a fixed brightness value, and the error message isn't
> > helpful. This can be the case e.g. for LEDs supporting a specific hw
> > trigger only.
> > 
> > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> > ---
> >  drivers/leds/led-core.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
> > index 89c9806cc..9485f799f 100644
> > --- a/drivers/leds/led-core.c
> > +++ b/drivers/leds/led-core.c
> > @@ -127,7 +127,8 @@ static void set_brightness_delayed_set_brightness(struct led_classdev *led_cdev,
> >  	ret = __led_set_brightness(led_cdev, value);
> >  	if (ret == -ENOTSUPP)
> >  		ret = __led_set_brightness_blocking(led_cdev, value);
> > -	if (ret < 0 &&
> > +	/* Don't emit error message if LED supports a hw trigger like netdev only */
> > +	if (ret < 0 && ret != -ENOTSUPP &&
> >  	    /* LED HW might have been unplugged, therefore don't warn */
> >  	    !(ret == -ENODEV && (led_cdev->flags & LED_UNREGISTERING) &&
> >  	    (led_cdev->flags & LED_HW_PLUGGABLE)))
> 
> This function is already pretty messy.
> 
> How about something like:
> 
> static void set_brightness_delayed_set_brightness(struct led_classdev *led_cdev,
>                                                   unsigned int value)
> {
>         int ret;
> 
>         ret = __led_set_brightness(led_cdev, value);
>         if (ret == -ENOTSUPP) {
>                 ret = __led_set_brightness_blocking(led_cdev, value);
>                 if (ret == -ENOTSUPP)
>                         /* No back-end support to set a fixed brightness value */
>                         return;
>         }
> 
>         if (ret == -ENODEV && (led_cdev->flags & LED_UNREGISTERING))
>                 /* LED HW might have been unplugged, therefore don't warn */
>                 return;
> 
>         if (ret < 0 && led_cdev->flags & LED_HW_PLUGGABLE)
>                 dev_err(led_cdev->dev, "Setting an LED's brightness failed (%d)\n", ret);
>  }

If it looks reasonable to you, I'd e happy to submit it as a patch.

-- 
Lee Jones [李琼斯]