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@xxxxxxxxx>
> > ---
> > 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 [李琼斯]
