Messages in this thread | | | Date | Mon, 13 Jul 2020 12:26:00 +0200 | From | Uwe Kleine-König <> | Subject | Re: [PATCH v7 3/3] leds: trigger: implement a tty trigger |
| |
Hello Pavel,
On Sun, Jul 12, 2020 at 10:24:53AM +0200, Pavel Machek wrote: > > +++ b/drivers/leds/trigger/ledtrig-tty.c > > @@ -0,0 +1,192 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > 2.0+ is preffered.
My employer requests GPL-2.0-only for kernel code.
> > + while (firstrun || > > + icount.rx != trigger_data->rx || > > + icount.tx != trigger_data->tx) { > > + > > + led_set_brightness(trigger_data->led_cdev, LED_ON); > > + > > + msleep(100); > > + > > + led_set_brightness(trigger_data->led_cdev, LED_OFF); > > Is this good idea inside workqueue?
What is "this"? The msleep? Calling led_set_brightness? What would you recommend instead? Maybe led_set_brightness_nosleep()?
> > + trigger_data->rx = icount.rx; > > + trigger_data->tx = icount.tx; > > + firstrun = false; > > + > > + ret = tty_get_icount(trigger_data->tty, &icount); > > + if (ret) > > + return; > > Unbalanced locking.
indeed, will fix and resend after the above issues are resolved.
Best regards Uwe
-- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | https://www.pengutronix.de/ | [unhandled content-type:application/pgp-signature] | |