[PATCH] leds: trigger: tty: Fix tty trigger when LED's max_brightness is greater than 1

[PATCH] leds: trigger: tty: Fix tty trigger when LED's max_brightness is greater than 1

[Marek Behún]

The tty LED trigger uses the obsolete LED_ON & LED_OFF constants when
setting LED brightness. This is bad because the LED_ON constant is equal
to 1, and so when activating the tty LED trigger on a LED class device
with max_brightness greater than 1, the LED is dimmer than it can be
(when max_brightness is 255, the LED is very dimm indeed; some devices
translate 1/255 to 0, so the LED is OFF all the time).

Use a mechanism similar to the netdev trigger, wherein on activation
time, the current LED brightness is taken as blinking brightness, and if
it is zero, the max_brightness is taken instead.

Fixes: fd4a641ac88f ("leds: trigger: implement a tty trigger")
Signed-off-by: Marek Behún <kabel@kernel.org>
---
PS: We need to get rid of the LED_ON, LED_OFF, LED_FULL and LED_HALF or
    we're bound to repeat this kind of issues.
---
 drivers/leds/trigger/ledtrig-tty.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/leds/trigger/ledtrig-tty.c b/drivers/leds/trigger/ledtrig-tty.c
index f62db7e520b5..2ff00a0e7ba6 100644
--- a/drivers/leds/trigger/ledtrig-tty.c
+++ b/drivers/leds/trigger/ledtrig-tty.c
@@ -80,6 +80,7 @@ static void ledtrig_tty_work(struct work_struct *work)
 {
 	struct ledtrig_tty_data *trigger_data =
 		container_of(work, struct ledtrig_tty_data, dwork.work);
+	struct led_classdev *led_cdev = trigger_data->led_cdev;
 	struct serial_icounter_struct icount;
 	int ret;
 
@@ -111,6 +112,9 @@ static void ledtrig_tty_work(struct work_struct *work)
 			goto out;
 
 		trigger_data->tty = tty;
+		led_cdev->blink_brightness = led_cdev->brightness;
+		if (!led_cdev->blink_brightness)
+			led_cdev->blink_brightness = led_cdev->max_brightness;
 	}
 
 	ret = tty_get_icount(trigger_data->tty, &icount);
@@ -122,12 +126,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_sync(led_cdev, led_cdev->blink_brightness);
 
 		trigger_data->rx = icount.rx;
 		trigger_data->tx = icount.tx;
 	} else {
-		led_set_brightness_sync(trigger_data->led_cdev, LED_OFF);
+		led_set_brightness_sync(led_cdev, 0);
 	}
 
 out:
-- 
2.41.0

Re: [PATCH] leds: trigger: tty: Fix tty trigger when LED's max_brightness is greater than 1

[Marek Behún]

On Tue,  1 Aug 2023 16:26:42 +0200
Marek Behún <kabel@kernel.org> wrote:

> The tty LED trigger uses the obsolete LED_ON & LED_OFF constants when
> setting LED brightness. This is bad because the LED_ON constant is equal
> to 1, and so when activating the tty LED trigger on a LED class device
> with max_brightness greater than 1, the LED is dimmer than it can be
> (when max_brightness is 255, the LED is very dimm indeed; some devices
> translate 1/255 to 0, so the LED is OFF all the time).
> 
> Use a mechanism similar to the netdev trigger, wherein on activation
> time, the current LED brightness is taken as blinking brightness, and if
> it is zero, the max_brightness is taken instead.
> 
> Fixes: fd4a641ac88f ("leds: trigger: implement a tty trigger")
> Signed-off-by: Marek Behún <kabel@kernel.org>
> ---
> PS: We need to get rid of the LED_ON, LED_OFF, LED_FULL and LED_HALF or
>     we're bound to repeat this kind of issues.

Ignore this, even better fix coming utilizing led_blink_set_oneshot()