[PATCH v2 1/3] tty: serial: imx: disable ageing timer interrupt if dma in use

[PATCH v2 1/3] tty: serial: imx: disable ageing timer interrupt if dma in use

[Troy Kisky]

Since commit 4dec2f119e86 ("imx-serial: RX DMA startup latency")
the interrupt routine no longer will start rx dma.
So, we no longer need to enable this interrupt to start dma.

Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>

---
v2: split into a series of 2 patches
---
 drivers/tty/serial/imx.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index dfeff3951f93..bf47b15df2e8 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -1207,10 +1207,6 @@ static void imx_enable_dma(struct imx_port *sport)
 	temp |= UCR1_RDMAEN | UCR1_TDMAEN | UCR1_ATDMAEN;
 	writel(temp, sport->port.membase + UCR1);
 
-	temp = readl(sport->port.membase + UCR2);
-	temp |= UCR2_ATEN;
-	writel(temp, sport->port.membase + UCR2);
-
 	imx_setup_ufcr(sport, TXTL_DMA, RXTL_DMA);
 
 	sport->dma_is_enabled = 1;
-- 
2.11.0

[PATCH v2 1/3] tty: serial: imx: disable ageing timer interrupt if dma in use

[Troy Kisky]

Since commit 4dec2f119e86 ("imx-serial: RX DMA startup latency")
the interrupt routine no longer will start rx dma.
So, we no longer need to enable this interrupt to start dma.

Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>

---
v2: split into a series of 2 patches
---
 drivers/tty/serial/imx.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index dfeff3951f93..bf47b15df2e8 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -1207,10 +1207,6 @@ static void imx_enable_dma(struct imx_port *sport)
 	temp |= UCR1_RDMAEN | UCR1_TDMAEN | UCR1_ATDMAEN;
 	writel(temp, sport->port.membase + UCR1);
 
-	temp = readl(sport->port.membase + UCR2);
-	temp |= UCR2_ATEN;
-	writel(temp, sport->port.membase + UCR2);
-
 	imx_setup_ufcr(sport, TXTL_DMA, RXTL_DMA);
 
 	sport->dma_is_enabled = 1;
-- 
2.11.0

[PATCH v2 2/3] tty: serial: imx: remove dead code imx_dma_rxint

[Troy Kisky]

Since commit 4dec2f119e86 ("imx-serial: RX DMA startup latency")
the interrupt routine no longer will start rx dma.

imx_dma_rxint no longer needs to be called to try and start dma.
It won't start dma because dma_is_rxing is
already true meaning dma is already started.

Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>

---
v2: split into 2 patches
---
 drivers/tty/serial/imx.c | 30 ++----------------------------
 1 file changed, 2 insertions(+), 28 deletions(-)

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index bf47b15df2e8..15b0ecb4cf60 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -732,29 +732,6 @@ static void imx_disable_rx_int(struct imx_port *sport)
 }
 
 static void clear_rx_errors(struct imx_port *sport);
-static int start_rx_dma(struct imx_port *sport);
-/*
- * If the RXFIFO is filled with some data, and then we
- * arise a DMA operation to receive them.
- */
-static void imx_dma_rxint(struct imx_port *sport)
-{
-	unsigned long temp;
-	unsigned long flags;
-
-	spin_lock_irqsave(&sport->port.lock, flags);
-
-	temp = readl(sport->port.membase + USR2);
-	if ((temp & USR2_RDR) && !sport->dma_is_rxing) {
-
-		imx_disable_rx_int(sport);
-
-		/* tell the DMA to receive the data. */
-		start_rx_dma(sport);
-	}
-
-	spin_unlock_irqrestore(&sport->port.lock, flags);
-}
 
 /*
  * We have a modem side uart, so the meanings of RTS and CTS are inverted.
@@ -816,11 +793,8 @@ static irqreturn_t imx_int(int irq, void *dev_id)
 	sts = readl(sport->port.membase + USR1);
 	sts2 = readl(sport->port.membase + USR2);
 
-	if (sts & (USR1_RRDY | USR1_AGTIM)) {
-		if (sport->dma_is_enabled)
-			imx_dma_rxint(sport);
-		else
-			imx_rxint(irq, dev_id);
+	if (!sport->dma_is_enabled && (sts & (USR1_RRDY | USR1_AGTIM))) {
+		imx_rxint(irq, dev_id);
 		ret = IRQ_HANDLED;
 	}
 
-- 
2.11.0

[PATCH v2 2/3] tty: serial: imx: remove dead code imx_dma_rxint

[Troy Kisky]

Since commit 4dec2f119e86 ("imx-serial: RX DMA startup latency")
the interrupt routine no longer will start rx dma.

imx_dma_rxint no longer needs to be called to try and start dma.
It won't start dma because dma_is_rxing is
already true meaning dma is already started.

Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>

---
v2: split into 2 patches
---
 drivers/tty/serial/imx.c | 30 ++----------------------------
 1 file changed, 2 insertions(+), 28 deletions(-)

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index bf47b15df2e8..15b0ecb4cf60 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -732,29 +732,6 @@ static void imx_disable_rx_int(struct imx_port *sport)
 }
 
 static void clear_rx_errors(struct imx_port *sport);
-static int start_rx_dma(struct imx_port *sport);
-/*
- * If the RXFIFO is filled with some data, and then we
- * arise a DMA operation to receive them.
- */
-static void imx_dma_rxint(struct imx_port *sport)
-{
-	unsigned long temp;
-	unsigned long flags;
-
-	spin_lock_irqsave(&sport->port.lock, flags);
-
-	temp = readl(sport->port.membase + USR2);
-	if ((temp & USR2_RDR) && !sport->dma_is_rxing) {
-
-		imx_disable_rx_int(sport);
-
-		/* tell the DMA to receive the data. */
-		start_rx_dma(sport);
-	}
-
-	spin_unlock_irqrestore(&sport->port.lock, flags);
-}
 
 /*
  * We have a modem side uart, so the meanings of RTS and CTS are inverted.
@@ -816,11 +793,8 @@ static irqreturn_t imx_int(int irq, void *dev_id)
 	sts = readl(sport->port.membase + USR1);
 	sts2 = readl(sport->port.membase + USR2);
 
-	if (sts & (USR1_RRDY | USR1_AGTIM)) {
-		if (sport->dma_is_enabled)
-			imx_dma_rxint(sport);
-		else
-			imx_rxint(irq, dev_id);
+	if (!sport->dma_is_enabled && (sts & (USR1_RRDY | USR1_AGTIM))) {
+		imx_rxint(irq, dev_id);
 		ret = IRQ_HANDLED;
 	}
 
-- 
2.11.0

[PATCH v2 3/3] tty: serial: imx: remove imx_disable_rx_int

[Troy Kisky]

Since imx_disable_rx_int is only called by imx_startup,
let's integrate it into that function.

Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
---
v2: new patch
---
 drivers/tty/serial/imx.c | 39 ++++++++++-----------------------------
 1 file changed, 10 insertions(+), 29 deletions(-)

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 15b0ecb4cf60..506fcd742b47 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -710,27 +710,6 @@ static irqreturn_t imx_rxint(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
-static void imx_disable_rx_int(struct imx_port *sport)
-{
-	unsigned long temp;
-
-	sport->dma_is_rxing = 1;
-
-	/* disable the receiver ready and aging timer interrupts */
-	temp = readl(sport->port.membase + UCR1);
-	temp &= ~(UCR1_RRDYEN);
-	writel(temp, sport->port.membase + UCR1);
-
-	temp = readl(sport->port.membase + UCR2);
-	temp &= ~(UCR2_ATEN);
-	writel(temp, sport->port.membase + UCR2);
-
-	/* disable the rx errors interrupts */
-	temp = readl(sport->port.membase + UCR4);
-	temp &= ~UCR4_OREN;
-	writel(temp, sport->port.membase + UCR4);
-}
-
 static void clear_rx_errors(struct imx_port *sport);
 
 /*
@@ -1024,6 +1003,7 @@ static int start_rx_dma(struct imx_port *sport)
 	struct dma_async_tx_descriptor *desc;
 	int ret;
 
+	sport->dma_is_rxing = 1;
 	sport->rx_ring.head = 0;
 	sport->rx_ring.tail = 0;
 	sport->rx_periods = RX_DMA_PERIODS;
@@ -1260,18 +1240,21 @@ static int imx_startup(struct uart_port *port)
 	if (sport->dma_is_inited && !sport->dma_is_enabled)
 		imx_enable_dma(sport);
 
-	temp = readl(sport->port.membase + UCR1);
-	temp |= UCR1_RRDYEN | UCR1_UARTEN;
+	temp = readl(sport->port.membase + UCR1) & ~UCR1_RRDYEN;
+	if (!sport->dma_is_enabled)
+		temp |= UCR1_RRDYEN;
+	temp |= UCR1_UARTEN;
 	if (sport->have_rtscts)
 			temp |= UCR1_RTSDEN;
 
 	writel(temp, sport->port.membase + UCR1);
 
-	temp = readl(sport->port.membase + UCR4);
-	temp |= UCR4_OREN;
+	temp = readl(sport->port.membase + UCR4) & ~UCR4_OREN;
+	if (!sport->dma_is_enabled)
+		temp |= UCR4_OREN;
 	writel(temp, sport->port.membase + UCR4);
 
-	temp = readl(sport->port.membase + UCR2);
+	temp = readl(sport->port.membase + UCR2) & ~UCR2_ATEN;
 	temp |= (UCR2_RXEN | UCR2_TXEN);
 	if (!sport->have_rtscts)
 		temp |= UCR2_IRTS;
@@ -1305,10 +1288,8 @@ static int imx_startup(struct uart_port *port)
 	 * In our iMX53 the average delay for the first reception dropped from
 	 * approximately 35000 microseconds to 1000 microseconds.
 	 */
-	if (sport->dma_is_enabled) {
-		imx_disable_rx_int(sport);
+	if (sport->dma_is_enabled)
 		start_rx_dma(sport);
-	}
 
 	spin_unlock_irqrestore(&sport->port.lock, flags);
 
-- 
2.11.0

[PATCH v2 3/3] tty: serial: imx: remove imx_disable_rx_int

[Troy Kisky]

Since imx_disable_rx_int is only called by imx_startup,
let's integrate it into that function.

Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
---
v2: new patch
---
 drivers/tty/serial/imx.c | 39 ++++++++++-----------------------------
 1 file changed, 10 insertions(+), 29 deletions(-)

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 15b0ecb4cf60..506fcd742b47 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -710,27 +710,6 @@ static irqreturn_t imx_rxint(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
-static void imx_disable_rx_int(struct imx_port *sport)
-{
-	unsigned long temp;
-
-	sport->dma_is_rxing = 1;
-
-	/* disable the receiver ready and aging timer interrupts */
-	temp = readl(sport->port.membase + UCR1);
-	temp &= ~(UCR1_RRDYEN);
-	writel(temp, sport->port.membase + UCR1);
-
-	temp = readl(sport->port.membase + UCR2);
-	temp &= ~(UCR2_ATEN);
-	writel(temp, sport->port.membase + UCR2);
-
-	/* disable the rx errors interrupts */
-	temp = readl(sport->port.membase + UCR4);
-	temp &= ~UCR4_OREN;
-	writel(temp, sport->port.membase + UCR4);
-}
-
 static void clear_rx_errors(struct imx_port *sport);
 
 /*
@@ -1024,6 +1003,7 @@ static int start_rx_dma(struct imx_port *sport)
 	struct dma_async_tx_descriptor *desc;
 	int ret;
 
+	sport->dma_is_rxing = 1;
 	sport->rx_ring.head = 0;
 	sport->rx_ring.tail = 0;
 	sport->rx_periods = RX_DMA_PERIODS;
@@ -1260,18 +1240,21 @@ static int imx_startup(struct uart_port *port)
 	if (sport->dma_is_inited && !sport->dma_is_enabled)
 		imx_enable_dma(sport);
 
-	temp = readl(sport->port.membase + UCR1);
-	temp |= UCR1_RRDYEN | UCR1_UARTEN;
+	temp = readl(sport->port.membase + UCR1) & ~UCR1_RRDYEN;
+	if (!sport->dma_is_enabled)
+		temp |= UCR1_RRDYEN;
+	temp |= UCR1_UARTEN;
 	if (sport->have_rtscts)
 			temp |= UCR1_RTSDEN;
 
 	writel(temp, sport->port.membase + UCR1);
 
-	temp = readl(sport->port.membase + UCR4);
-	temp |= UCR4_OREN;
+	temp = readl(sport->port.membase + UCR4) & ~UCR4_OREN;
+	if (!sport->dma_is_enabled)
+		temp |= UCR4_OREN;
 	writel(temp, sport->port.membase + UCR4);
 
-	temp = readl(sport->port.membase + UCR2);
+	temp = readl(sport->port.membase + UCR2) & ~UCR2_ATEN;
 	temp |= (UCR2_RXEN | UCR2_TXEN);
 	if (!sport->have_rtscts)
 		temp |= UCR2_IRTS;
@@ -1305,10 +1288,8 @@ static int imx_startup(struct uart_port *port)
 	 * In our iMX53 the average delay for the first reception dropped from
 	 * approximately 35000 microseconds to 1000 microseconds.
 	 */
-	if (sport->dma_is_enabled) {
-		imx_disable_rx_int(sport);
+	if (sport->dma_is_enabled)
 		start_rx_dma(sport);
-	}
 
 	spin_unlock_irqrestore(&sport->port.lock, flags);
 
-- 
2.11.0

Re: [PATCH v2 3/3] tty: serial: imx: remove imx_disable_rx_int

[Troy Kisky]

On 10/20/2017 2:20 PM, Troy Kisky wrote:
> Since imx_disable_rx_int is only called by imx_startup,
> let's integrate it into that function.
> 
> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
> ---
> v2: new patch
> ---
>  drivers/tty/serial/imx.c | 39 ++++++++++-----------------------------
>  1 file changed, 10 insertions(+), 29 deletions(-)

While testing this series, I noticed that sending a break on the serial port caused

imx-uart 2020000.serial: DMA transaction error.

Is that normal ?

[PATCH v2 3/3] tty: serial: imx: remove imx_disable_rx_int

[Troy Kisky]

On 10/20/2017 2:20 PM, Troy Kisky wrote:
> Since imx_disable_rx_int is only called by imx_startup,
> let's integrate it into that function.
> 
> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
> ---
> v2: new patch
> ---
>  drivers/tty/serial/imx.c | 39 ++++++++++-----------------------------
>  1 file changed, 10 insertions(+), 29 deletions(-)

While testing this series, I noticed that sending a break on the serial port caused

imx-uart 2020000.serial: DMA transaction error.

Is that normal ?

Re: [PATCH v2 3/3] tty: serial: imx: remove imx_disable_rx_int

[Uwe Kleine-König]

Hello,

On Fri, Oct 20, 2017 at 02:25:11PM -0700, Troy Kisky wrote:
> On 10/20/2017 2:20 PM, Troy Kisky wrote:
> > Since imx_disable_rx_int is only called by imx_startup,
> > let's integrate it into that function.
> > 
> > Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
> > ---
> > v2: new patch
> > ---
> >  drivers/tty/serial/imx.c | 39 ++++++++++-----------------------------
> >  1 file changed, 10 insertions(+), 29 deletions(-)
> 
> While testing this series, I noticed that sending a break on the serial port caused
> 
> imx-uart 2020000.serial: DMA transaction error.
> 
> Is that normal ?

I remember that Jürgen (added to To explictly) hit this or a similar
problem before, maybe he can commen?

Best regards
Uwe
-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[PATCH v2 3/3] tty: serial: imx: remove imx_disable_rx_int

[Uwe Kleine-König]

Hello,

On Fri, Oct 20, 2017 at 02:25:11PM -0700, Troy Kisky wrote:
> On 10/20/2017 2:20 PM, Troy Kisky wrote:
> > Since imx_disable_rx_int is only called by imx_startup,
> > let's integrate it into that function.
> > 
> > Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
> > ---
> > v2: new patch
> > ---
> >  drivers/tty/serial/imx.c | 39 ++++++++++-----------------------------
> >  1 file changed, 10 insertions(+), 29 deletions(-)
> 
> While testing this series, I noticed that sending a break on the serial port caused
> 
> imx-uart 2020000.serial: DMA transaction error.
> 
> Is that normal ?

I remember that J?rgen (added to To explictly) hit this or a similar
problem before, maybe he can commen?

Best regards
Uwe
-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Re: [PATCH v2 3/3] tty: serial: imx: remove imx_disable_rx_int

[Juergen Borleis]

Hi,

On Sunday 22 October 2017 20:49:56 Uwe Kleine-König wrote:
> On Fri, Oct 20, 2017 at 02:25:11PM -0700, Troy Kisky wrote:
> > On 10/20/2017 2:20 PM, Troy Kisky wrote:
> > > Since imx_disable_rx_int is only called by imx_startup,
> > > let's integrate it into that function.
> > >
> > > Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
> > > ---
> > > v2: new patch
> > > ---
> > >  drivers/tty/serial/imx.c | 39
> > > ++++++++++----------------------------- 1 file changed, 10
> > > insertions(+), 29 deletions(-)
> >
> > While testing this series, I noticed that sending a break on the serial
> > port caused
> >
> > imx-uart 2020000.serial: DMA transaction error.
> >
> > Is that normal ?
>
> I remember that Jürgen (added to To explictly) hit this or a similar
> problem before, maybe he can commen?

Yes, it hit me on i.MX53 after DMA was enabled by default. The driver 
currently doesn't handle the "break" event when DMA is active. In this case 
the DMA callback returns with an error (that's the message you see, but 
it's not an error in this case...), the driver clears the flags and drops 
the (break) event itself.

jb

-- 
Pengutronix e.K.                             | Juergen Borleis             |
Industrial Linux Solutions                   | http://www.pengutronix.de/  |

[PATCH v2 3/3] tty: serial: imx: remove imx_disable_rx_int

[Juergen Borleis]

Hi,

On Sunday 22 October 2017 20:49:56 Uwe Kleine-K?nig wrote:
> On Fri, Oct 20, 2017 at 02:25:11PM -0700, Troy Kisky wrote:
> > On 10/20/2017 2:20 PM, Troy Kisky wrote:
> > > Since imx_disable_rx_int is only called by imx_startup,
> > > let's integrate it into that function.
> > >
> > > Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
> > > ---
> > > v2: new patch
> > > ---
> > >  drivers/tty/serial/imx.c | 39
> > > ++++++++++----------------------------- 1 file changed, 10
> > > insertions(+), 29 deletions(-)
> >
> > While testing this series, I noticed that sending a break on the serial
> > port caused
> >
> > imx-uart 2020000.serial: DMA transaction error.
> >
> > Is that normal ?
>
> I remember that J?rgen (added to To explictly) hit this or a similar
> problem before, maybe he can commen?

Yes, it hit me on i.MX53 after DMA was enabled by default. The driver 
currently doesn't handle the "break" event when DMA is active. In this case 
the DMA callback returns with an error (that's the message you see, but 
it's not an error in this case...), the driver clears the flags and drops 
the (break) event itself.

jb

-- 
Pengutronix e.K. ? ? ? ? ? ? ? ? ? ? ? ? ?? ?| Juergen Borleis ? ? ? ? ? ? |
Industrial Linux Solutions ? ? ? ? ? ? ? ?? ?| http://www.pengutronix.de/ ?|

Re: [PATCH v2 3/3] tty: serial: imx: remove imx_disable_rx_int

[Greg KH]

On Fri, Oct 20, 2017 at 02:20:21PM -0700, Troy Kisky wrote:
> Since imx_disable_rx_int is only called by imx_startup,
> let's integrate it into that function.
> 
> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
> ---
> v2: new patch
> ---
>  drivers/tty/serial/imx.c | 39 ++++++++++-----------------------------
>  1 file changed, 10 insertions(+), 29 deletions(-)

Does not apply :(

[PATCH v2 3/3] tty: serial: imx: remove imx_disable_rx_int

[Greg KH]

On Fri, Oct 20, 2017 at 02:20:21PM -0700, Troy Kisky wrote:
> Since imx_disable_rx_int is only called by imx_startup,
> let's integrate it into that function.
> 
> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
> ---
> v2: new patch
> ---
>  drivers/tty/serial/imx.c | 39 ++++++++++-----------------------------
>  1 file changed, 10 insertions(+), 29 deletions(-)

Does not apply :(