Re: [PATCH v7 3/3] leds: trigger: implement a tty trigger

[Uwe Kleine-KÃnig]

Hello Johan,

On Tue, Jul 14, 2020 at 09:13:55AM +0200, Johan Hovold wrote:
> On Tue, Jul 07, 2020 at 06:59:58PM +0200, Uwe Kleine-König wrote:
> > +	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);
> > +
> > +		trigger_data->rx = icount.rx;
> > +		trigger_data->tx = icount.tx;
> > +		firstrun = false;
> > +
> > +		ret = tty_get_icount(trigger_data->tty, &icount);
> > +		if (ret)
> > +			return;
> > +	}
> 
> Haven't looked at the latest proposal in detail, but this looks broken
> as you can potentially loop indefinitely in a worker thread, and with no
> way to stop the trigger (delayed work).

I don't think that potentially looping indefinitely is a problem, but
indeed it should drop the lock during each iteration. Will think about
how to adapt.

Thanks
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

Attachment: signature.asc
Description: PGP signature