Message-ID: <d34ad55c-62c4-6eaf-8f73-19a675264a7c@gmail.com>
Date:   Sun, 22 Jan 2023 23:18:19 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.13.0
Subject: Re: [PATCH 1/1] leds: ledtrig-tty.c: call led_set_brightness() when
 the blocking callback is not available
Content-Language: en-US
To:     Lee Jones <lee@kernel.org>
Cc:     =?UTF-8?Q?Uwe_Kleine-K=c3=b6nig?= <u.kleine-koenig@pengutronix.de>,
        Qingtao Cao <qingtao.cao.au@gmail.com>,
        Qingtao Cao <qingtao.cao@digi.com>,
        Pavel Machek <pavel@ucw.cz>, linux-leds@vger.kernel.org,
        linux-kernel@vger.kernel.org
References: <20221209001039.7287-1-qingtao.cao@digi.com>
 <20221209124058.dwez7wohs7dfeiik@pengutronix.de>
 <Y7WnMISK20Z4hsg1@google.com>
 <ad4a1069-72c6-a431-336f-ed78a57a1ba0@gmail.com>
 <Y8qW1tlDmw6KEIuu@google.com>
From:   Jacek Anaszewski <jacek.anaszewski@gmail.com>
In-Reply-To: <Y8qW1tlDmw6KEIuu@google.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

On 1/20/23 14:27, Lee Jones wrote:
> On Sat, 07 Jan 2023, Jacek Anaszewski wrote:
> 
>> 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@digi.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.
> 
> Thanks for helping to keep an eye on things Jacek.
> 
> Much appreciated.
> 

No problem, I still read the list and can elaborate
on various details of LED subsystem, as needed.

-- 
Best regards,
Jacek Anaszewski