Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 9 Dec 2022 13:40:58 +0100
From:   Uwe =?utf-8?Q?Kleine-K=C3=B6nig?= <u.kleine-koenig@pengutronix.de>
To:     Qingtao Cao <qingtao.cao.au@gmail.com>
Cc:     Qingtao Cao <qingtao.cao@digi.com>, Pavel Machek <pavel@ucw.cz>,
        linux-leds@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/1] leds: ledtrig-tty.c: call led_set_brightness() when
 the blocking callback is not available
Message-ID: <20221209124058.dwez7wohs7dfeiik@pengutronix.de>
References: <20221209001039.7287-1-qingtao.cao@digi.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
        protocol="application/pgp-signature"; boundary="565mh7mmkut75gk2"
Content-Disposition: inline
In-Reply-To: <20221209001039.7287-1-qingtao.cao@digi.com>
Precedence: bulk


--565mh7mmkut75gk2
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

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 ca=
n_sleep
> false, making create_gpio_led() setting up the brightness_set function po=
inter
> instead of the brightness_set_blocking function pointer. In this case the
> nonblocking version led_set_brightness() should be fallen back on by ledt=
rig_tty_work()
>=20
> Signed-off-by: Qingtao Cao <qingtao.cao@digi.com>
> ---
>  drivers/leds/trigger/ledtrig-tty.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
>=20
> diff --git a/drivers/leds/trigger/ledtrig-tty.c b/drivers/leds/trigger/le=
dtrig-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 *wo=
rk)
> =20
>  	if (icount.rx !=3D trigger_data->rx ||
>  	    icount.tx !=3D 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.

Best regards
Uwe

--=20
Pengutronix e.K.                           | Uwe Kleine-K=F6nig            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

--565mh7mmkut75gk2
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQEzBAABCgAdFiEEfnIqFpAYrP8+dKQLwfwUeK3K7AkFAmOTLNcACgkQwfwUeK3K
7AncHgf+MX7UU32R1xOxDn2kHRK45G5QRz1Iog0wvmFwNW3dXft+e8rrIm66X897
xAc6AleuqAM2Hos8NGFAsehr3StUD1yXSvtR3wAxKCJ3USmF26DOMAXsCZe6UoPb
Shu1sFSeZApZQRdvWHJxI1OXanh1CjfEbar4i6YLsqa4saGlQ+3tbVVqX75Z8F1O
J0fWjZk0LO4a0rsaThYj7qPymqm7VsDDuEVfIMiatH3tIOL5PxfUqm/SRenpPDlF
jJw49kf6xiw6GAJCWdHh3ke3Fo8L0tte+iUpYDLOyW/QOpyxr0akm81Ylc/1Wpnp
4QGxcs+BQiVBFxIV27xyy+8h8+N0Pw==
=2hSk
-----END PGP SIGNATURE-----

--565mh7mmkut75gk2--