RE: [PATCH v1] tty: serial: imx: enable IDDMAEN for the last tail data

[Robin Gong]

> -----Original Message-----
> From: Uwe Kleine-KÃnig <u.kleine-koenig@xxxxxxxxxxxxxx>
> Sent: 2018å9æ20æ 15:55
> To: Robin Gong <yibin.gong@xxxxxxx>
> Cc: jslaby@xxxxxxxx; Andy Duan <fugang.duan@xxxxxxx>;
> linux-serial@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; dl-linux-imx
> <linux-imx@xxxxxxx>; kernel@xxxxxxxxxxxxxx
> Subject: Re: [PATCH v1] tty: serial: imx: enable IDDMAEN for the last tail data
> 
> Hello,
> 
> On Thu, Sep 20, 2018 at 11:26:00PM +0800, Robin Gong wrote:
> > enable IDDMAEN in UCR4 to let sdma script has the chance to detect the
> > idle status and transfer the last tail data with the interrupt
> > notifying uart driver.Otherwise, the last dma done interrupt of the
> > tail data in rxfifo whose size is less than watermark may never be
> > received by uart driver.
> 
> Is this a theoretic issue? Or does it fix a real problem?
> 
> If the former I'd say UCR1_ATDMAEN being set should prevent this stall.
It's a real fix, you will see 'timeout We read 0 byte' with the attached stress test
running on i.mx7d or i.mx6 if no this patch applied. 
' while [ 1 ]; do /unit_tests/UART/mxc_uart_stress_test.out /dev/ttymxc5 2000000 D L 100 100 L; done &'
Please make sure dma enabled on ttymxcX for your test, and sdma firmware setup in your rootfs 
/lib/firmware/imx/sdma/sdma-imx6q.bin or /lib/firmware/imx/sdma/sdma-imx7d.bin.
You can get sdma firmware from below:
https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git/tree/imx/sdma
> 
> Also I'd say the SDMA scripts I know don't properly handle the IDDMA request.
> 
> Best regards
> Uwe
> 
> > Signed-off-by: Robin Gong <yibin.gong@xxxxxxx>
> > ---
> >  drivers/tty/serial/imx.c | 12 ++++++++++--
> >  1 file changed, 10 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c index
> > 239c0fa..bbb1693 100644
> > --- a/drivers/tty/serial/imx.c
> > +++ b/drivers/tty/serial/imx.c
> > @@ -1278,7 +1278,7 @@ static int imx_uart_dma_init(struct imx_port
> > *sport)
> >
> >  static void imx_uart_enable_dma(struct imx_port *sport)  {
> > -	u32 ucr1;
> > +	u32 ucr1, ucr4;
> >
> >  	imx_uart_setup_ufcr(sport, TXTL_DMA, RXTL_DMA);
> >
> > @@ -1287,18 +1287,26 @@ static void imx_uart_enable_dma(struct
> imx_port *sport)
> >  	ucr1 |= UCR1_RXDMAEN | UCR1_TXDMAEN | UCR1_ATDMAEN;
> >  	imx_uart_writel(sport, ucr1, UCR1);
> >
> > +	ucr4 = imx_uart_readl(sport, UCR4);
> > +	ucr4 |= UCR4_IDDMAEN;
> > +	imx_uart_writel(sport, ucr4, UCR4);
> > +
> >  	sport->dma_is_enabled = 1;
> >  }
> >
> >  static void imx_uart_disable_dma(struct imx_port *sport)  {
> > -	u32 ucr1;
> > +	u32 ucr1, ucr4;
> >
> >  	/* clear UCR1 */
> >  	ucr1 = imx_uart_readl(sport, UCR1);
> >  	ucr1 &= ~(UCR1_RXDMAEN | UCR1_TXDMAEN | UCR1_ATDMAEN);
> >  	imx_uart_writel(sport, ucr1, UCR1);
> >
> > +	ucr4 = imx_uart_readl(sport, UCR4);
> > +	ucr4 &= ~UCR4_IDDMAEN;
> > +	imx_uart_writel(sport, ucr4, UCR4);
> > +
> >  	imx_uart_setup_ufcr(sport, TXTL_DEFAULT, RXTL_DEFAULT);
> >
> >  	sport->dma_is_enabled = 0;
> > --
> > 2.7.4
> >
> >
> 
> --
> Pengutronix e.K.                           | Uwe Kleine-KÃnig
> |
> Industrial Linux Solutions                 |
> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.
> pengutronix.de%2F&amp;data=02%7C01%7Cyibin.gong%40nxp.com%7Cebbc2
> 26ea12d4f6dd93b08d61ece6add%7C686ea1d3bc2b4c6fa92cd99c5c301635%7
> C0%7C0%7C636730269265048370&amp;sdata=7NstF74mOGOFajEwFjBw7FAh
> h3JdvGAIC9nliwrsf0Y%3D&amp;reserved=0  |

Attachment: mxc_uart_stress_test.out
Description: mxc_uart_stress_test.out