linux-kernel - Re: [PATCH 1/1] leds: ledtrig-tty.c: call led_set_brightness() when the blocking callback is not available

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sat, 7 Jan 2023 19:34:34 +0100
From:   Jacek Anaszewski <jacek.anaszewski@...il.com>
To:     Lee Jones <lee@...nel.org>,
        Uwe Kleine-König <u.kleine-koenig@...gutronix.de>
Cc:     Qingtao Cao <qingtao.cao.au@...il.com>,
        Qingtao Cao <qingtao.cao@...i.com>,
        Pavel Machek <pavel@....cz>, linux-leds@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/1] leds: ledtrig-tty.c: call led_set_brightness() when
 the blocking callback is not available

Hi all,

On 1/4/23 17:20, Lee Jones wrote:
> On Fri, 09 Dec 2022, Uwe Kleine-König wrote:
> 
>> On Fri, Dec 09, 2022 at 10:10:38AM +1000, Qingtao Cao wrote:
>>> The Marvell GPIO controller's driver will setup its struct gpio_chip's can_sleep
>>> false, making create_gpio_led() setting up the brightness_set function pointer
>>> instead of the brightness_set_blocking function pointer. In this case the
>>> nonblocking version led_set_brightness() should be fallen back on by ledtrig_tty_work()
>>>
>>> Signed-off-by: Qingtao Cao <qingtao.cao@...i.com>
>>> ---
>>>   drivers/leds/trigger/ledtrig-tty.c | 10 ++++++++--
>>>   1 file changed, 8 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/leds/trigger/ledtrig-tty.c b/drivers/leds/trigger/ledtrig-tty.c
>>> index f62db7e520b5..e43d285b5d06 100644
>>> --- a/drivers/leds/trigger/ledtrig-tty.c
>>> +++ b/drivers/leds/trigger/ledtrig-tty.c
>>> @@ -122,12 +122,18 @@ 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);
>>> +		if (trigger_data->led_cdev->brightness_set_blocking)
>>> +			led_set_brightness_sync(trigger_data->led_cdev, LED_ON);
>>> +		else if (trigger_data->led_cdev->brightness_set)
>>> +			led_set_brightness(trigger_data->led_cdev, LED_ON);
>>
>> I had similar issues in the past where a function call worked for
>> sleeping LEDs but not atomic ones (or the other way around? Don't
>> remember the details.)
>>
>> I wonder if there isn't a function that does the right thing no matter
>> what type the LED is. The other triggers should have the same issue, and
>> doing the above check in all of them just looks wrong.
> 
> Anyone fancy taking a deeper dive into this?
> 
> I'd usually have a go myself, but I'm presently swamped.
> 
> Pavel already has an idea?
> 

There is led_set_brightness_nosleep() and it should be used here
from the beginning. Generally it should be used always in triggers.

-- 
Best regards,
Jacek Anaszewski