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
| ||
|
Date: Mon, 11 Apr 2022 12:25:40 +0200 From: Johan Hovold <johan@...nel.org> To: Tony Lindgren <tony@...mide.com> Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>, Andy Shevchenko <andriy.shevchenko@...el.com>, Jiri Slaby <jirislaby@...nel.org>, Sebastian Andrzej Siewior <bigeasy@...utronix.de>, Vignesh Raghavendra <vigneshr@...com>, linux-serial@...r.kernel.org, linux-omap@...r.kernel.org, linux-kernel@...r.kernel.org, "Matwey V . Kornilov" <matwey@....msu.ru>, Steffen Trumtrar <s.trumtrar@...gutronix.de>, Uwe Kleine-König <u.kleine-koenig@...gutronix.de> Subject: Re: [PATCH 1/2] serial: 8250: Fix runtime PM for start_tx() for RS485 On Mon, Apr 11, 2022 at 01:10:34PM +0300, Tony Lindgren wrote: > * Johan Hovold <johan@...nel.org> [220411 09:54]: > > On Mon, Apr 11, 2022 at 12:48:04PM +0300, Tony Lindgren wrote: > > > diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c > > > --- a/drivers/tty/serial/8250/8250_port.c > > > +++ b/drivers/tty/serial/8250/8250_port.c > > > @@ -1681,8 +1681,10 @@ static void serial8250_start_tx(struct uart_port *port) > > > return; > > > > > > if (em485 && > > > - em485->active_timer == &em485->start_tx_timer) > > > + em485->active_timer == &em485->start_tx_timer) { > > > + serial8250_rpm_put_tx(up); > > > return; > > > + } > > > > I was just taking a quick look at your report about this and also > > noticed this return statement. > > > > The runtime PM implementation is a bit of mess as we've discussed > > elsewhere, but the change you propose here doesn't look right. > > Frankly "a bit of mess" applies "a bit more" than just the serial runtime > PM :) Heh. I'm afraid that's all too true. :) > > start_tx() can be deferred in the rs485 case, but that doesn't mean you > > should suspend the device here. In fact, that look like it would just > > break runtime PM (the parts that may work to some extent). > > AFAIK there's currently nothing paired with the serial8250_rpm_get_tx(up) > call at the beginning of serial8250_start_tx() for the early exit cases > if start_tx_rs485() or __start_tx() won't get called. > > Care to clarify a bit more what you have in mind? The problem is that that serial8250_rpm_put_tx() you're adding may suspend the device unconditionally (i.e. regardless of any previous calls to serial8250_rpm_get_tx()). If rs485 tx is just being deferred you mustn't suspend the device before it has had a chance to start transmitting. Johan
Powered by blists - more mailing lists