| Message ID | 20230121153639.15402-4-sorganov@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v1,1/7] serial: imx: factor-out common code to imx_uart_soft_reset() | expand |
Hi Sergey, Am 21.01.23 um 16:36 schrieb Sergey Organov: > 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> this looks like a bugfix to me. Since the relevant code is pretty old, i'm not sure about the fixes tag here: Fixes: 279a9acc9b72 ("2.6.11 import") ? > --- > drivers/tty/serial/imx.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c > index e7fce31e460d..e709118fe85c 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,6 +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)
Hi Stefan, Stefan Wahren <stefan.wahren@i2se.com> writes: > Hi Sergey, > > Am 21.01.23 um 16:36 schrieb Sergey Organov: >> 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> > > this looks like a bugfix to me. Since the relevant code is pretty old, > i'm not sure about the fixes tag here: > > Fixes: 279a9acc9b72 ("2.6.11 import") ? Dunno. I've checked a few drivers, and it seems that they don't care either, e.g., look at atmel_serial.c, or mpc52xx_uart.c. Either it doesn't matter, or a few drivers need similar fix? What's going on here, I wonder? Thanks, -- Sergey Organov
On Sun, 22 Jan 2023, Sergey Organov wrote: > Hi Stefan, > > Stefan Wahren <stefan.wahren@i2se.com> writes: > > > Hi Sergey, > > > > Am 21.01.23 um 16:36 schrieb Sergey Organov: > >> 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> > > > > this looks like a bugfix to me. Since the relevant code is pretty old, > > i'm not sure about the fixes tag here: > > > > Fixes: 279a9acc9b72 ("2.6.11 import") ? > > Dunno. I've checked a few drivers, and it seems that they don't care > either, e.g., look at atmel_serial.c, or mpc52xx_uart.c. > > Either it doesn't matter, or a few drivers need similar fix? What's > going on here, I wonder? Usually when one finds a bug from one of the drivers, the other drivers indeed turn out to have the same/similar bug(s). It's not something uncommon. So just fix them all, it's very much appreciated. :-) I understand it might not be possible to test all such fixes on those other HWs but usually such bugs are simple enough to fix that it isn't be a big problem.
Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> writes: > On Sun, 22 Jan 2023, Sergey Organov wrote: > >> Hi Stefan, >> >> Stefan Wahren <stefan.wahren@i2se.com> writes: >> >> > Hi Sergey, >> > >> > Am 21.01.23 um 16:36 schrieb Sergey Organov: >> >> 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> >> > >> > this looks like a bugfix to me. Since the relevant code is pretty old, >> > i'm not sure about the fixes tag here: >> > >> > Fixes: 279a9acc9b72 ("2.6.11 import") ? >> >> Dunno. I've checked a few drivers, and it seems that they don't care >> either, e.g., look at atmel_serial.c, or mpc52xx_uart.c. >> >> Either it doesn't matter, or a few drivers need similar fix? What's >> going on here, I wonder? > > Usually when one finds a bug from one of the drivers, the other drivers > indeed turn out to have the same/similar bug(s). It's not something > uncommon. Yep, it looks like deriving from the same template, with the same issue. > > So just fix them all, it's very much appreciated. :-) I understand it > might not be possible to test all such fixes on those other HWs but > usually such bugs are simple enough to fix that it isn't be a big problem. I'm not even sure this is really a bug, as nobody seems to confirm it with authority yet. Thanks, -- Sergey Organov
diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c index e7fce31e460d..e709118fe85c 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,6 +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)
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 | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)