* [PATCH] leds: trigger/tty: Use led_set_brightness() to support all use cases @ 2021-05-03 9:25 Juergen Borleis 2021-05-03 10:16 ` Pavel Machek 2023-01-09 8:43 ` Uwe Kleine-König 0 siblings, 2 replies; 5+ messages in thread From: Juergen Borleis @ 2021-05-03 9:25 UTC (permalink / raw) To: linux-leds, Pavel Machek Cc: Uwe Kleine-König, linux-kernel, Greg Kroah-Hartman, kernel Using led_set_brightness_sync() only works for LEDs which are connected via some kind of external bus like I²C or SPI. But it doesn't work for the simple use case of directly connected LEDs via GPIOs. Because this function only honors the led_classdev::brightness_set_blocking callback. But the LED-GPIO driver registers the led_classdev::brightness_set member if the GPIO can be modified directly and thus, TTY triggers fail silently with -ENOTSUPP. With the previously used led_set_brightness() it works for both use cases. This function first checks for the simple case where the GPIO can be changed without additional overhead, and if it fails, does the modification via a workqueue. Signed-off-by: Juergen Borleis <jbe@pengutronix.de> --- drivers/leds/trigger/ledtrig-tty.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/leds/trigger/ledtrig-tty.c b/drivers/leds/trigger/ledtrig-tty.c index f62db7e..af61281 100644 --- a/drivers/leds/trigger/ledtrig-tty.c +++ b/drivers/leds/trigger/ledtrig-tty.c @@ -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(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(trigger_data->led_cdev, LED_OFF); } out: -- 2.20.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] leds: trigger/tty: Use led_set_brightness() to support all use cases 2021-05-03 9:25 [PATCH] leds: trigger/tty: Use led_set_brightness() to support all use cases Juergen Borleis @ 2021-05-03 10:16 ` Pavel Machek 2023-01-09 8:43 ` Uwe Kleine-König 1 sibling, 0 replies; 5+ messages in thread From: Pavel Machek @ 2021-05-03 10:16 UTC (permalink / raw) To: Juergen Borleis Cc: linux-leds, Uwe Kleine-König, linux-kernel, Greg Kroah-Hartman, kernel [-- Attachment #1: Type: text/plain, Size: 1078 bytes --] On Mon 2021-05-03 11:25:42, Juergen Borleis wrote: > Using led_set_brightness_sync() only works for LEDs which are connected > via some kind of external bus like I²C or SPI. But it doesn't work for > the simple use case of directly connected LEDs via GPIOs. Really? I'd need to check. > Because this function only honors the led_classdev::brightness_set_blocking > callback. But the LED-GPIO driver registers the > led_classdev::brightness_set member if the GPIO can be modified directly > and thus, TTY triggers fail silently with -ENOTSUPP. > > With the previously used led_set_brightness() it works for both use cases. > This function first checks for the simple case where the GPIO can be changed > without additional overhead, and if it fails, does the modification via a > workqueue. Yeah, but that is not what we want. We are already running in the workqueue. We really should have a API that can be called from process context, and just simply sets the brightness. Best regards, Pavel -- http://www.livejournal.com/~pavelmachek [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 181 bytes --] ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] leds: trigger/tty: Use led_set_brightness() to support all use cases 2021-05-03 9:25 [PATCH] leds: trigger/tty: Use led_set_brightness() to support all use cases Juergen Borleis 2021-05-03 10:16 ` Pavel Machek @ 2023-01-09 8:43 ` Uwe Kleine-König 2024-02-23 14:22 ` Lee Jones 1 sibling, 1 reply; 5+ messages in thread From: Uwe Kleine-König @ 2023-01-09 8:43 UTC (permalink / raw) To: Juergen Borleis Cc: linux-leds, Pavel Machek, Greg Kroah-Hartman, linux-kernel, kernel [-- Attachment #1: Type: text/plain, Size: 2392 bytes --] Hello Jürgen, On Mon, May 03, 2021 at 11:25:42AM +0200, Juergen Borleis wrote: > Using led_set_brightness_sync() only works for LEDs which are connected > via some kind of external bus like I²C or SPI. But it doesn't work for > the simple use case of directly connected LEDs via GPIOs. > Because this function only honors the led_classdev::brightness_set_blocking > callback. But the LED-GPIO driver registers the > led_classdev::brightness_set member if the GPIO can be modified directly > and thus, TTY triggers fail silently with -ENOTSUPP. > > With the previously used led_set_brightness() it works for both use cases. > This function first checks for the simple case where the GPIO can be changed > without additional overhead, and if it fails, does the modification via a > workqueue. > > Signed-off-by: Juergen Borleis <jbe@pengutronix.de> > --- > drivers/leds/trigger/ledtrig-tty.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/leds/trigger/ledtrig-tty.c b/drivers/leds/trigger/ledtrig-tty.c > index f62db7e..af61281 100644 > --- a/drivers/leds/trigger/ledtrig-tty.c > +++ b/drivers/leds/trigger/ledtrig-tty.c > @@ -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(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(trigger_data->led_cdev, LED_OFF); > } This problem still exists, right? I think the right thing here is to call led_set_brightness_nosleep() however. Having said that, I think there are too many variants of led_set_brightness which makes it difficult to pick the right one. (I'm aware of - led_set_brightness_nosleep - led_set_brightness_nopm - led_set_brightness_sync - led_set_brightness and there are a few more static variants in led-core.c (__led_set_brightness, __led_set_brightness_blocking, set_brightness_delayed).) Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | https://www.pengutronix.de/ | [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 484 bytes --] ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] leds: trigger/tty: Use led_set_brightness() to support all use cases 2023-01-09 8:43 ` Uwe Kleine-König @ 2024-02-23 14:22 ` Lee Jones 2024-02-23 14:24 ` Lee Jones 0 siblings, 1 reply; 5+ messages in thread From: Lee Jones @ 2024-02-23 14:22 UTC (permalink / raw) To: Uwe Kleine-König Cc: Juergen Borleis, linux-leds, Pavel Machek, Greg Kroah-Hartman, linux-kernel, kernel On Mon, 09 Jan 2023, Uwe Kleine-König wrote: > Hello Jürgen, > > On Mon, May 03, 2021 at 11:25:42AM +0200, Juergen Borleis wrote: > > Using led_set_brightness_sync() only works for LEDs which are connected > > via some kind of external bus like I²C or SPI. But it doesn't work for > > the simple use case of directly connected LEDs via GPIOs. > > Because this function only honors the led_classdev::brightness_set_blocking > > callback. But the LED-GPIO driver registers the > > led_classdev::brightness_set member if the GPIO can be modified directly > > and thus, TTY triggers fail silently with -ENOTSUPP. > > > > With the previously used led_set_brightness() it works for both use cases. > > This function first checks for the simple case where the GPIO can be changed > > without additional overhead, and if it fails, does the modification via a > > workqueue. > > > > Signed-off-by: Juergen Borleis <jbe@pengutronix.de> > > --- > > drivers/leds/trigger/ledtrig-tty.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/leds/trigger/ledtrig-tty.c b/drivers/leds/trigger/ledtrig-tty.c > > index f62db7e..af61281 100644 > > --- a/drivers/leds/trigger/ledtrig-tty.c > > +++ b/drivers/leds/trigger/ledtrig-tty.c > > @@ -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(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(trigger_data->led_cdev, LED_OFF); > > } > > This problem still exists, right? > > I think the right thing here is to call led_set_brightness_nosleep() > however. > > Having said that, I think there are too many variants of > led_set_brightness which makes it difficult to pick the right one. > > (I'm aware of > - led_set_brightness_nosleep > - led_set_brightness_nopm > - led_set_brightness_sync > - led_set_brightness > > and there are a few more static variants in led-core.c > (__led_set_brightness, __led_set_brightness_blocking, > set_brightness_delayed).) This patch (and this response) was never sent to me. Not a great deal I can do if I'm not aware of it. -- Lee Jones [李琼斯] ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] leds: trigger/tty: Use led_set_brightness() to support all use cases 2024-02-23 14:22 ` Lee Jones @ 2024-02-23 14:24 ` Lee Jones 0 siblings, 0 replies; 5+ messages in thread From: Lee Jones @ 2024-02-23 14:24 UTC (permalink / raw) To: Uwe Kleine-König Cc: Juergen Borleis, linux-leds, Pavel Machek, Greg Kroah-Hartman, linux-kernel, kernel On Fri, 23 Feb 2024, Lee Jones wrote: > On Mon, 09 Jan 2023, Uwe Kleine-König wrote: > > > Hello Jürgen, > > > > On Mon, May 03, 2021 at 11:25:42AM +0200, Juergen Borleis wrote: > > > Using led_set_brightness_sync() only works for LEDs which are connected > > > via some kind of external bus like I²C or SPI. But it doesn't work for > > > the simple use case of directly connected LEDs via GPIOs. > > > Because this function only honors the led_classdev::brightness_set_blocking > > > callback. But the LED-GPIO driver registers the > > > led_classdev::brightness_set member if the GPIO can be modified directly > > > and thus, TTY triggers fail silently with -ENOTSUPP. > > > > > > With the previously used led_set_brightness() it works for both use cases. > > > This function first checks for the simple case where the GPIO can be changed > > > without additional overhead, and if it fails, does the modification via a > > > workqueue. > > > > > > Signed-off-by: Juergen Borleis <jbe@pengutronix.de> > > > --- > > > drivers/leds/trigger/ledtrig-tty.c | 4 ++-- > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/leds/trigger/ledtrig-tty.c b/drivers/leds/trigger/ledtrig-tty.c > > > index f62db7e..af61281 100644 > > > --- a/drivers/leds/trigger/ledtrig-tty.c > > > +++ b/drivers/leds/trigger/ledtrig-tty.c > > > @@ -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(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(trigger_data->led_cdev, LED_OFF); > > > } > > > > This problem still exists, right? > > > > I think the right thing here is to call led_set_brightness_nosleep() > > however. > > > > Having said that, I think there are too many variants of > > led_set_brightness which makes it difficult to pick the right one. > > > > (I'm aware of > > - led_set_brightness_nosleep > > - led_set_brightness_nopm > > - led_set_brightness_sync > > - led_set_brightness > > > > and there are a few more static variants in led-core.c > > (__led_set_brightness, __led_set_brightness_blocking, > > set_brightness_delayed).) > > This patch (and this response) was never sent to me. > > Not a great deal I can do if I'm not aware of it. Starting to think I've gone too far back in the LEDs ML's history? =;-) -- Lee Jones [李琼斯] ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-02-23 14:24 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-05-03 9:25 [PATCH] leds: trigger/tty: Use led_set_brightness() to support all use cases Juergen Borleis 2021-05-03 10:16 ` Pavel Machek 2023-01-09 8:43 ` Uwe Kleine-König 2024-02-23 14:22 ` Lee Jones 2024-02-23 14:24 ` Lee Jones
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).