All of lore.kernel.org
 help / color / mirror / Atom feed
* [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.