| Message ID | 20230113184334.287130-4-sorganov@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | serial: imx: work-around for hardware RX flood, and then isr improvements | expand |
On Fri, 13 Jan 2023, Sergey Organov wrote: > Do not call uart_handle_sysrq_char() if we got any receive error along with > the character, as we don't want random junk to be considered a sysrq. > > Signed-off-by: Sergey Organov <sorganov@gmail.com> > --- > drivers/tty/serial/imx.c | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c > index e7fce31e460d..1c950112a598 100644 > --- a/drivers/tty/serial/imx.c > +++ b/drivers/tty/serial/imx.c > @@ -911,9 +911,6 @@ static irqreturn_t __imx_uart_rxint(int irq, void *dev_id) > continue; > } > > - if (uart_handle_sysrq_char(&sport->port, (unsigned char)rx)) > - continue; > - > if (unlikely(rx & URXD_ERR)) { > if (rx & URXD_BRK) > sport->port.icount.brk++; > @@ -942,7 +939,8 @@ static irqreturn_t __imx_uart_rxint(int irq, void *dev_id) > flg = TTY_OVERRUN; > > sport->port.sysrq = 0; > - } > + } else if (uart_handle_sysrq_char(&sport->port, (unsigned char)rx)) > + continue; > > if (sport->port.ignore_status_mask & URXD_DUMMY_READ) > goto out; Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
On Fri, Jan 13, 2023 at 09:43:29PM +0300, Sergey Organov wrote: > Do not call uart_handle_sysrq_char() if we got any receive error along with > the character, as we don't want random junk to be considered a sysrq. > > Signed-off-by: Sergey Organov <sorganov@gmail.com> > --- > drivers/tty/serial/imx.c | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c > index e7fce31e460d..1c950112a598 100644 > --- a/drivers/tty/serial/imx.c > +++ b/drivers/tty/serial/imx.c > @@ -911,9 +911,6 @@ static irqreturn_t __imx_uart_rxint(int irq, void *dev_id) > continue; > } > > - if (uart_handle_sysrq_char(&sport->port, (unsigned char)rx)) > - continue; > - > if (unlikely(rx & URXD_ERR)) { > if (rx & URXD_BRK) > sport->port.icount.brk++; > @@ -942,7 +939,8 @@ static irqreturn_t __imx_uart_rxint(int irq, void *dev_id) > flg = TTY_OVERRUN; > > sport->port.sysrq = 0; > - } > + } else if (uart_handle_sysrq_char(&sport->port, (unsigned char)rx)) > + continue; Nit: missing braces {} Note that you could also place just place this after the block due to the reset of the sysrq time stamp. > > if (sport->port.ignore_status_mask & URXD_DUMMY_READ) > goto out; Johan
Johan Hovold <johan@kernel.org> writes: > On Fri, Jan 13, 2023 at 09:43:29PM +0300, Sergey Organov wrote: >> Do not call uart_handle_sysrq_char() if we got any receive error along with >> the character, as we don't want random junk to be considered a sysrq. >> >> Signed-off-by: Sergey Organov <sorganov@gmail.com> >> --- >> drivers/tty/serial/imx.c | 6 ++---- >> 1 file changed, 2 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c >> index e7fce31e460d..1c950112a598 100644 >> --- a/drivers/tty/serial/imx.c >> +++ b/drivers/tty/serial/imx.c >> @@ -911,9 +911,6 @@ static irqreturn_t __imx_uart_rxint(int irq, void *dev_id) >> continue; >> } >> >> - if (uart_handle_sysrq_char(&sport->port, (unsigned char)rx)) >> - continue; >> - >> if (unlikely(rx & URXD_ERR)) { >> if (rx & URXD_BRK) >> sport->port.icount.brk++; >> @@ -942,7 +939,8 @@ static irqreturn_t __imx_uart_rxint(int irq, void *dev_id) >> flg = TTY_OVERRUN; >> >> sport->port.sysrq = 0; >> - } >> + } else if (uart_handle_sysrq_char(&sport->port, (unsigned char)rx)) >> + continue; > > Nit: missing braces {} > > Note that you could also place just place this after the block due to > the reset of the sysrq time stamp. Thanks, I think I'll opt for adding braces. Relying on the reset of the timestamp feels a bit convoluted.
On Tue, Jan 17, 2023 at 08:35:48PM +0300, Sergey Organov wrote: > Johan Hovold <johan@kernel.org> writes: > > On Fri, Jan 13, 2023 at 09:43:29PM +0300, Sergey Organov wrote: > >> @@ -911,9 +911,6 @@ static irqreturn_t __imx_uart_rxint(int irq, void *dev_id) > >> continue; > >> } > >> > >> - if (uart_handle_sysrq_char(&sport->port, (unsigned char)rx)) > >> - continue; > >> - > >> if (unlikely(rx & URXD_ERR)) { > >> if (rx & URXD_BRK) > >> sport->port.icount.brk++; > >> @@ -942,7 +939,8 @@ static irqreturn_t __imx_uart_rxint(int irq, void *dev_id) > >> flg = TTY_OVERRUN; > >> > >> sport->port.sysrq = 0; > >> - } > >> + } else if (uart_handle_sysrq_char(&sport->port, (unsigned char)rx)) > >> + continue; > > > > Nit: missing braces {} > > > > Note that you could also place just place this after the block due to > > the reset of the sysrq time stamp. > > Thanks, I think I'll opt for adding braces. Relying on the reset of the > timestamp feels a bit convoluted. I agree, it may be a bit too subtle. Johan
diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c index e7fce31e460d..1c950112a598 100644 --- a/drivers/tty/serial/imx.c +++ b/drivers/tty/serial/imx.c @@ -911,9 +911,6 @@ static irqreturn_t __imx_uart_rxint(int irq, void *dev_id) continue; } - if (uart_handle_sysrq_char(&sport->port, (unsigned char)rx)) - continue; - if (unlikely(rx & URXD_ERR)) { if (rx & URXD_BRK) sport->port.icount.brk++; @@ -942,7 +939,8 @@ static irqreturn_t __imx_uart_rxint(int irq, void *dev_id) flg = TTY_OVERRUN; sport->port.sysrq = 0; - } + } else if (uart_handle_sysrq_char(&sport->port, (unsigned char)rx)) + continue; if (sport->port.ignore_status_mask & URXD_DUMMY_READ) goto out;
Do not call uart_handle_sysrq_char() if we got any receive error along with the character, as we don't want random junk to be considered a sysrq. Signed-off-by: Sergey Organov <sorganov@gmail.com> --- drivers/tty/serial/imx.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)