Re: [PATCH] tty: serial: imx: disable UCR4_OREN in .stop_rx()

[Uwe Kleine-König]

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@xxxxxxx>
> > 
> > Disable UCR4_OREN bit in .stop_rx() to avoid endless
> > interrupt happen while tty port is closing.
> > 
> > Signed-off-by: Fugang Duan <fugang.duan@xxxxxxx>
> > Signed-off-by: Sherry Sun <sherry.sun@xxxxxxx>
> > ---
> >  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: signature.asc
Description: PGP signature