* [PATCH] tty: serial: imx: disable UCR4_OREN in .stop_rx() @ 2021-11-23 7:04 Sherry Sun 2021-11-23 7:51 ` Uwe Kleine-König 0 siblings, 1 reply; 4+ messages in thread From: Sherry Sun @ 2021-11-23 7:04 UTC (permalink / raw) To: gregkh, jirislaby; +Cc: linux-serial, linux-kernel, linux-imx From: Fugang Duan <fugang.duan@nxp.com> Disable UCR4_OREN bit in .stop_rx() to avoid endless interrupt happen while tty port is closing. Signed-off-by: Fugang Duan <fugang.duan@nxp.com> Signed-off-by: Sherry Sun <sherry.sun@nxp.com> --- drivers/tty/serial/imx.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c index fb75e3e0d828..9ecaf051249d 100644 --- a/drivers/tty/serial/imx.c +++ b/drivers/tty/serial/imx.c @@ -486,18 +486,21 @@ static void imx_uart_stop_tx(struct uart_port *port) static void imx_uart_stop_rx(struct uart_port *port) { struct imx_port *sport = (struct imx_port *)port; - u32 ucr1, ucr2; + u32 ucr1, ucr2, ucr4; ucr1 = imx_uart_readl(sport, UCR1); ucr2 = imx_uart_readl(sport, UCR2); + ucr4 = imx_uart_readl(sport, UCR4); if (sport->dma_is_enabled) { ucr1 &= ~(UCR1_RXDMAEN | UCR1_ATDMAEN); } else { ucr1 &= ~UCR1_RRDYEN; ucr2 &= ~UCR2_ATEN; + ucr4 &= ~UCR4_OREN; } imx_uart_writel(sport, ucr1, UCR1); + imx_uart_writel(sport, ucr4, UCR4); ucr2 &= ~UCR2_RXEN; imx_uart_writel(sport, ucr2, UCR2); -- 2.17.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] tty: serial: imx: disable UCR4_OREN in .stop_rx() 2021-11-23 7:04 [PATCH] tty: serial: imx: disable UCR4_OREN in .stop_rx() Sherry Sun @ 2021-11-23 7:51 ` Uwe Kleine-König 2021-11-23 8:43 ` Uwe Kleine-König 0 siblings, 1 reply; 4+ messages in thread From: Uwe Kleine-König @ 2021-11-23 7:51 UTC (permalink / raw) To: Sherry Sun; +Cc: gregkh, jirislaby, linux-serial, linux-kernel, linux-imx [-- Attachment #1: Type: text/plain, Size: 1901 bytes --] On Tue, Nov 23, 2021 at 03:04:37PM +0800, Sherry Sun wrote: > From: Fugang Duan <fugang.duan@nxp.com> > > Disable UCR4_OREN bit in .stop_rx() to avoid endless > interrupt happen while tty port is closing. > > Signed-off-by: Fugang Duan <fugang.duan@nxp.com> > Signed-off-by: Sherry Sun <sherry.sun@nxp.com> > --- > drivers/tty/serial/imx.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c > index fb75e3e0d828..9ecaf051249d 100644 > --- a/drivers/tty/serial/imx.c > +++ b/drivers/tty/serial/imx.c > @@ -486,18 +486,21 @@ static void imx_uart_stop_tx(struct uart_port *port) > static void imx_uart_stop_rx(struct uart_port *port) > { > struct imx_port *sport = (struct imx_port *)port; > - u32 ucr1, ucr2; > + u32 ucr1, ucr2, ucr4; > > ucr1 = imx_uart_readl(sport, UCR1); > ucr2 = imx_uart_readl(sport, UCR2); > + ucr4 = imx_uart_readl(sport, UCR4); > > if (sport->dma_is_enabled) { > ucr1 &= ~(UCR1_RXDMAEN | UCR1_ATDMAEN); > } else { > ucr1 &= ~UCR1_RRDYEN; > ucr2 &= ~UCR2_ATEN; > + ucr4 &= ~UCR4_OREN; > } > imx_uart_writel(sport, ucr1, UCR1); > + imx_uart_writel(sport, ucr4, UCR4); I don't understand this yet, but I guess this is just a matter of improving the commit log. How does the endless irq happen? ... some time later after thinking and reading the driver source ... Hmm, when the OR event is triggered the handler does imx_uart_writel(sport, USR2_ORE, USR2); and so clears the event. I would expect that this should be good enough to prevent an irq storm even if the receiver is disabled. Doesn't acking work with the receiver disabled? Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | https://www.pengutronix.de/ | [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] tty: serial: imx: disable UCR4_OREN in .stop_rx() 2021-11-23 7:51 ` Uwe Kleine-König @ 2021-11-23 8:43 ` Uwe Kleine-König 2021-11-23 9:05 ` Sherry Sun 0 siblings, 1 reply; 4+ messages in thread From: Uwe Kleine-König @ 2021-11-23 8:43 UTC (permalink / raw) To: Sherry Sun; +Cc: gregkh, jirislaby, linux-serial, linux-kernel, linux-imx [-- Attachment #1: Type: text/plain, Size: 2224 bytes --] Hello again, On Tue, Nov 23, 2021 at 08:51:22AM +0100, Uwe Kleine-König wrote: > On Tue, Nov 23, 2021 at 03:04:37PM +0800, Sherry Sun wrote: > > From: Fugang Duan <fugang.duan@nxp.com> > > > > Disable UCR4_OREN bit in .stop_rx() to avoid endless > > interrupt happen while tty port is closing. > > > > Signed-off-by: Fugang Duan <fugang.duan@nxp.com> > > Signed-off-by: Sherry Sun <sherry.sun@nxp.com> > > --- > > drivers/tty/serial/imx.c | 5 ++++- > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c > > index fb75e3e0d828..9ecaf051249d 100644 > > --- a/drivers/tty/serial/imx.c > > +++ b/drivers/tty/serial/imx.c > > @@ -486,18 +486,21 @@ static void imx_uart_stop_tx(struct uart_port *port) > > static void imx_uart_stop_rx(struct uart_port *port) > > { > > struct imx_port *sport = (struct imx_port *)port; > > - u32 ucr1, ucr2; > > + u32 ucr1, ucr2, ucr4; > > > > ucr1 = imx_uart_readl(sport, UCR1); > > ucr2 = imx_uart_readl(sport, UCR2); > > + ucr4 = imx_uart_readl(sport, UCR4); > > > > if (sport->dma_is_enabled) { > > ucr1 &= ~(UCR1_RXDMAEN | UCR1_ATDMAEN); > > } else { > > ucr1 &= ~UCR1_RRDYEN; > > ucr2 &= ~UCR2_ATEN; > > + ucr4 &= ~UCR4_OREN; > > } > > imx_uart_writel(sport, ucr1, UCR1); > > + imx_uart_writel(sport, ucr4, UCR4); > > I don't understand this yet, but I guess this is just a matter of > improving the commit log. How does the endless irq happen? > > ... some time later after thinking and reading the driver source ... > > Hmm, when the OR event is triggered the handler does > > imx_uart_writel(sport, USR2_ORE, USR2); > > and so clears the event. I would expect that this should be good enough > to prevent an irq storm even if the receiver is disabled. Doesn't acking > work with the receiver disabled? One thing I forgot to mention: If imx_uart_stop_rx() now clears UCR4_OREN, clearing in imx_uart_shutdown() should probably be dropped. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | https://www.pengutronix.de/ | [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [PATCH] tty: serial: imx: disable UCR4_OREN in .stop_rx() 2021-11-23 8:43 ` Uwe Kleine-König @ 2021-11-23 9:05 ` Sherry Sun 0 siblings, 0 replies; 4+ messages in thread From: Sherry Sun @ 2021-11-23 9:05 UTC (permalink / raw) To: Uwe Kleine-König Cc: gregkh, jirislaby, linux-serial, linux-kernel, dl-linux-imx Hi Uwe, > -----Original Message----- > From: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > Sent: 2021年11月23日 16:43 > To: Sherry Sun <sherry.sun@nxp.com> > Cc: gregkh@linuxfoundation.org; jirislaby@kernel.org; linux- > serial@vger.kernel.org; linux-kernel@vger.kernel.org; dl-linux-imx <linux- > imx@nxp.com> > Subject: Re: [PATCH] tty: serial: imx: disable UCR4_OREN in .stop_rx() > > Hello again, > > On Tue, Nov 23, 2021 at 08:51:22AM +0100, Uwe Kleine-König wrote: > > On Tue, Nov 23, 2021 at 03:04:37PM +0800, Sherry Sun wrote: > > > From: Fugang Duan <fugang.duan@nxp.com> > > > > > > Disable UCR4_OREN bit in .stop_rx() to avoid endless interrupt > > > happen while tty port is closing. > > > > > > Signed-off-by: Fugang Duan <fugang.duan@nxp.com> > > > Signed-off-by: Sherry Sun <sherry.sun@nxp.com> > > > --- > > > drivers/tty/serial/imx.c | 5 ++++- > > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c > > > index fb75e3e0d828..9ecaf051249d 100644 > > > --- a/drivers/tty/serial/imx.c > > > +++ b/drivers/tty/serial/imx.c > > > @@ -486,18 +486,21 @@ static void imx_uart_stop_tx(struct uart_port > > > *port) static void imx_uart_stop_rx(struct uart_port *port) { > > > struct imx_port *sport = (struct imx_port *)port; > > > - u32 ucr1, ucr2; > > > + u32 ucr1, ucr2, ucr4; > > > > > > ucr1 = imx_uart_readl(sport, UCR1); > > > ucr2 = imx_uart_readl(sport, UCR2); > > > + ucr4 = imx_uart_readl(sport, UCR4); > > > > > > if (sport->dma_is_enabled) { > > > ucr1 &= ~(UCR1_RXDMAEN | UCR1_ATDMAEN); > > > } else { > > > ucr1 &= ~UCR1_RRDYEN; > > > ucr2 &= ~UCR2_ATEN; > > > + ucr4 &= ~UCR4_OREN; > > > } > > > imx_uart_writel(sport, ucr1, UCR1); > > > + imx_uart_writel(sport, ucr4, UCR4); > > > > I don't understand this yet, but I guess this is just a matter of > > improving the commit log. How does the endless irq happen? To be frankly, this patch is an old local patch, I also don't know the history about it. But I checked the code logic is fine, the UCR4_OREN should be disabled when stop rx. > > > > ... some time later after thinking and reading the driver source ... > > > > Hmm, when the OR event is triggered the handler does > > > > imx_uart_writel(sport, USR2_ORE, USR2); > > > > and so clears the event. I would expect that this should be good > > enough to prevent an irq storm even if the receiver is disabled. > > Doesn't acking work with the receiver disabled? > > One thing I forgot to mention: > > If imx_uart_stop_rx() now clears UCR4_OREN, clearing in > imx_uart_shutdown() should probably be dropped. You are right, with this patch, clear the UCR4_OREN in imx_uart_shutdown is not necessary any more, I will remove it and send V2. Thank! Best regards Sherry > > Best regards > Uwe > > -- > Pengutronix e.K. | Uwe Kleine-König | > Industrial Linux Solutions | https://www.pengutronix.de/ | ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-11-23 9:05 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-11-23 7:04 [PATCH] tty: serial: imx: disable UCR4_OREN in .stop_rx() Sherry Sun 2021-11-23 7:51 ` Uwe Kleine-König 2021-11-23 8:43 ` Uwe Kleine-König 2021-11-23 9:05 ` Sherry Sun
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.