Re: [PATCH] leds: trigger/tty: Use led_set_brightness_nosleep() to set brightness

[Florian Eckert <fe@dev.tdt.de>]

Hello Uwe,

On 2023-04-14 18:48, Uwe Kleine-König wrote:
> After commit ba8a86e4dadb ("leds: trigger/tty: Use
> led_set_brightness_sync() from workqueue") this is the second try to
> pick the right function to set the LED brightness from a trigger.
> 
> led_set_brightness_sync() has the problem that it doesn't work for LEDs
> without a .brightness_set_blocking() callback. This is (among others)
> the case for LEDs connected to non-sleeping GPIOs.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
> Hello,
> 
> after a few (non-public and public) reports that the tty trigger 
> doesn't
> work and Jacek pointed out in
> https://lore.kernel.org/all/ad4a1069-72c6-a431-336f-ed78a57a1ba0@gmail.com/#t
> that led_set_brightness_nosleep() is the right function, here comes a
> patch to actually implement that.
> 
> Does this justify a Fixes line? In that case that would be:
> 
> 	Fixes: ba8a86e4dadb ("leds: trigger/tty: Use
> led_set_brightness_sync() from workqueue")
> 
> (As ba8a86e4dadb declares to be a fix for fd4a641ac88f ("leds: trigger:
> implement a tty trigger") I think a further reference to fd4a641ac88f
> isn't necesary.)
> 
> Best regards
> Uwe
> 
>  drivers/leds/trigger/ledtrig-tty.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/leds/trigger/ledtrig-tty.c
> b/drivers/leds/trigger/ledtrig-tty.c
> index f62db7e520b5..c8bbdeac93b9 100644
> --- a/drivers/leds/trigger/ledtrig-tty.c
> +++ b/drivers/leds/trigger/ledtrig-tty.c
> @@ -1,11 +1,11 @@
>  // SPDX-License-Identifier: GPL-2.0
> 
>  #include <linux/delay.h>
> -#include <linux/leds.h>
>  #include <linux/module.h>
>  #include <linux/slab.h>
>  #include <linux/tty.h>
>  #include <uapi/linux/serial.h>
> +#include "../leds.h"
> 
>  struct ledtrig_tty_data {
>  	struct led_classdev *led_cdev;
> @@ -122,12 +122,12 @@ static void ledtrig_tty_work(struct work_struct 
> *work)
> 
>  	if (icount.rx != trigger_data->rx ||
>  	    icount.tx != trigger_data->tx) {
> -		led_set_brightness_sync(trigger_data->led_cdev, LED_ON);
> +		led_set_brightness_nosleep(trigger_data->led_cdev, LED_ON);
> 
>  		trigger_data->rx = icount.rx;
>  		trigger_data->tx = icount.tx;
>  	} else {
> -		led_set_brightness_sync(trigger_data->led_cdev, LED_OFF);
> +		led_set_brightness_nosleep(trigger_data->led_cdev, LED_OFF);
>  	}
> 
>  out:
> 
> base-commit: fe15c26ee26efa11741a7b632e9f23b01aca4cc6

Tested-By: Florian Eckert <fe@dev.tdt.de>