* [PATCH v3 1/6] serial: imx: remove CTSC and CTS handling from imx_disable_dma
2017-09-21 16:18 [PATCH v3 0/6] serial: imx: various improvements Martyn Welch
@ 2017-09-21 16:18 ` Martyn Welch
2017-09-21 18:20 ` Uwe Kleine-König
2017-09-21 16:18 ` [PATCH v3 2/6] serial: imx: only set dma_is_rxing when DMA starts Martyn Welch
` (4 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Martyn Welch @ 2017-09-21 16:18 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Nandor Han, Romain Perier, linux-serial, linux-arm-kernel,
linux-kernel, u.kleine-koenig, Fabio Estevam, Martyn Welch
From: Nandor Han <nandor.han@ge.com>
The CTSC and CTS bits affect operation of the CTS/RTS hardware flow
control signal (depending on whether the device is in DCE or DTE mode) and
are not related to DMA. When in RS-232 mode, the driver is using the
automatic CTSC control based on a rxFIFO fill level unless the state of
the CTS signal is explictly set via an ioctl call.
Previous improvements to the imx serial driver have resulted on
imx_disable_dma() only being called on shutdown, by which point the
serial core has already correctly deasserted CTS.
Testing shows that without this handling in imx_disable_dma() the CTS
signal state is set correctly when the device is open and TIOCM_RTS is
set/cleared via the TIOCMGET ioctl. The CTS signal is also correctly
deasserted when the device file is closed.
When in RS-485 mode, the driver uses the CTS signal very differently and
appears to control it via calls to imx_port_rts_active() and
imx_port_rts_inactive().
This configuration of the CTSC and CTS bits are therefore not needed.
Signed-off-by: Nandor Han <nandor.han@ge.com>
Signed-off-by: Romain Perier <romain.perier@collabora.com>
Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk>
---
drivers/tty/serial/imx.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index fe368a4..d90dae3 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -1225,11 +1225,6 @@ static void imx_disable_dma(struct imx_port *sport)
temp &= ~(UCR1_RDMAEN | UCR1_TDMAEN | UCR1_ATDMAEN);
writel(temp, sport->port.membase + UCR1);
- /* clear UCR2 */
- temp = readl(sport->port.membase + UCR2);
- temp &= ~(UCR2_CTSC | UCR2_CTS | UCR2_ATEN);
- writel(temp, sport->port.membase + UCR2);
-
imx_setup_ufcr(sport, TXTL_DEFAULT, RXTL_DEFAULT);
sport->dma_is_enabled = 0;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v3 1/6] serial: imx: remove CTSC and CTS handling from imx_disable_dma
2017-09-21 16:18 ` [PATCH v3 1/6] serial: imx: remove CTSC and CTS handling from imx_disable_dma Martyn Welch
@ 2017-09-21 18:20 ` Uwe Kleine-König
2017-09-21 19:12 ` Martyn Welch
0 siblings, 1 reply; 12+ messages in thread
From: Uwe Kleine-König @ 2017-09-21 18:20 UTC (permalink / raw)
To: Martyn Welch
Cc: Greg Kroah-Hartman, Nandor Han, Romain Perier, linux-serial,
linux-arm-kernel, linux-kernel, Fabio Estevam
On Thu, Sep 21, 2017 at 05:18:12PM +0100, Martyn Welch wrote:
> From: Nandor Han <nandor.han@ge.com>
>
> The CTSC and CTS bits affect operation of the CTS/RTS hardware flow
> control signal (depending on whether the device is in DCE or DTE mode) and
> are not related to DMA. When in RS-232 mode, the driver is using the
> automatic CTSC control based on a rxFIFO fill level unless the state of
> the CTS signal is explictly set via an ioctl call.
>
> Previous improvements to the imx serial driver have resulted on
> imx_disable_dma() only being called on shutdown, by which point the
> serial core has already correctly deasserted CTS.
>
> Testing shows that without this handling in imx_disable_dma() the CTS
> signal state is set correctly when the device is open and TIOCM_RTS is
> set/cleared via the TIOCMGET ioctl. The CTS signal is also correctly
> deasserted when the device file is closed.
With that block kept CTS set once more to inactive. So the block doesn't
hurt and is "only" superflous, right?
> When in RS-485 mode, the driver uses the CTS signal very differently and
> appears to control it via calls to imx_port_rts_active() and
> imx_port_rts_inactive().
>
> This configuration of the CTSC and CTS bits are therefore not needed.
>
> Signed-off-by: Nandor Han <nandor.han@ge.com>
> Signed-off-by: Romain Perier <romain.perier@collabora.com>
> Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk>
> ---
> drivers/tty/serial/imx.c | 5 -----
> 1 file changed, 5 deletions(-)
>
> diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
> index fe368a4..d90dae3 100644
> --- a/drivers/tty/serial/imx.c
> +++ b/drivers/tty/serial/imx.c
> @@ -1225,11 +1225,6 @@ static void imx_disable_dma(struct imx_port *sport)
> temp &= ~(UCR1_RDMAEN | UCR1_TDMAEN | UCR1_ATDMAEN);
> writel(temp, sport->port.membase + UCR1);
>
> - /* clear UCR2 */
> - temp = readl(sport->port.membase + UCR2);
> - temp &= ~(UCR2_CTSC | UCR2_CTS | UCR2_ATEN);
> - writel(temp, sport->port.membase + UCR2);
The commit log doesn't mention ATEN, I guess that one just doesn't
matter any more at this stage? Would be nice to point out though.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 1/6] serial: imx: remove CTSC and CTS handling from imx_disable_dma
2017-09-21 18:20 ` Uwe Kleine-König
@ 2017-09-21 19:12 ` Martyn Welch
0 siblings, 0 replies; 12+ messages in thread
From: Martyn Welch @ 2017-09-21 19:12 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Greg Kroah-Hartman, Nandor Han, Romain Perier, linux-serial,
linux-arm-kernel, linux-kernel, Fabio Estevam
On Thu, Sep 21, 2017 at 08:20:17PM +0200, Uwe Kleine-König wrote:
> On Thu, Sep 21, 2017 at 05:18:12PM +0100, Martyn Welch wrote:
> > From: Nandor Han <nandor.han@ge.com>
> >
> > The CTSC and CTS bits affect operation of the CTS/RTS hardware flow
> > control signal (depending on whether the device is in DCE or DTE mode) and
> > are not related to DMA. When in RS-232 mode, the driver is using the
> > automatic CTSC control based on a rxFIFO fill level unless the state of
> > the CTS signal is explictly set via an ioctl call.
> >
> > Previous improvements to the imx serial driver have resulted on
> > imx_disable_dma() only being called on shutdown, by which point the
> > serial core has already correctly deasserted CTS.
> >
> > Testing shows that without this handling in imx_disable_dma() the CTS
> > signal state is set correctly when the device is open and TIOCM_RTS is
> > set/cleared via the TIOCMGET ioctl. The CTS signal is also correctly
> > deasserted when the device file is closed.
>
> With that block kept CTS set once more to inactive. So the block doesn't
> hurt and is "only" superflous, right?
>
That's my understanding, yes.
> > When in RS-485 mode, the driver uses the CTS signal very differently and
> > appears to control it via calls to imx_port_rts_active() and
> > imx_port_rts_inactive().
> >
> > This configuration of the CTSC and CTS bits are therefore not needed.
> >
> > Signed-off-by: Nandor Han <nandor.han@ge.com>
> > Signed-off-by: Romain Perier <romain.perier@collabora.com>
> > Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk>
> > ---
> > drivers/tty/serial/imx.c | 5 -----
> > 1 file changed, 5 deletions(-)
> >
> > diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
> > index fe368a4..d90dae3 100644
> > --- a/drivers/tty/serial/imx.c
> > +++ b/drivers/tty/serial/imx.c
> > @@ -1225,11 +1225,6 @@ static void imx_disable_dma(struct imx_port *sport)
> > temp &= ~(UCR1_RDMAEN | UCR1_TDMAEN | UCR1_ATDMAEN);
> > writel(temp, sport->port.membase + UCR1);
> >
> > - /* clear UCR2 */
> > - temp = readl(sport->port.membase + UCR2);
> > - temp &= ~(UCR2_CTSC | UCR2_CTS | UCR2_ATEN);
> > - writel(temp, sport->port.membase + UCR2);
>
> The commit log doesn't mention ATEN, I guess that one just doesn't
> matter any more at this stage? Would be nice to point out though.
>
Hmm, going to need to look at this again...
Patch 5 adds clearing ATEN into imx_stop_rx(), which is sensible given
that it's the aging timer on the rxFIFO, but I don't think we should be
removing that from here before it's in imx_stop_rx().
> Best regards
> Uwe
>
> --
> Pengutronix e.K. | Uwe Kleine-König |
> Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v3 2/6] serial: imx: only set dma_is_rxing when DMA starts
2017-09-21 16:18 [PATCH v3 0/6] serial: imx: various improvements Martyn Welch
2017-09-21 16:18 ` [PATCH v3 1/6] serial: imx: remove CTSC and CTS handling from imx_disable_dma Martyn Welch
@ 2017-09-21 16:18 ` Martyn Welch
2017-09-21 18:15 ` Uwe Kleine-König
2017-09-21 16:18 ` [PATCH v3 3/6] serial: imx: Simplify DMA disablement Martyn Welch
` (3 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Martyn Welch @ 2017-09-21 16:18 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Nandor Han, Romain Perier, linux-serial, linux-arm-kernel,
linux-kernel, u.kleine-koenig, Fabio Estevam, Martyn Welch
From: Romain Perier <romain.perier@collabora.com>
The variable dma_is_rxing is currently set to 1 in imx_disable_rx_int().
This is problematic as:
- whilst imx_disable_rx_int() is currently always called before
start_rx_dma() this dependency isn't obvious.
- start_rx_dma() does error checking and might exit without
enabling DMA. Currently this will result in dma_is_rxing suggesting
that DMA is being used for recieving.
To avoid these issues, move the setting of dma_is_rxing to
start_rx_dma() when appropriate.
Signed-off-by: Romain Perier <romain.perier@collabora.com>
Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk>
---
drivers/tty/serial/imx.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index d90dae3..7835279 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -714,8 +714,6 @@ 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);
@@ -1074,6 +1072,7 @@ static int start_rx_dma(struct imx_port *sport)
desc->callback_param = sport;
dev_dbg(dev, "RX: prepare for the DMA.\n");
+ sport->dma_is_rxing = 1;
sport->rx_cookie = dmaengine_submit(desc);
dma_async_issue_pending(chan);
return 0;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v3 2/6] serial: imx: only set dma_is_rxing when DMA starts
2017-09-21 16:18 ` [PATCH v3 2/6] serial: imx: only set dma_is_rxing when DMA starts Martyn Welch
@ 2017-09-21 18:15 ` Uwe Kleine-König
0 siblings, 0 replies; 12+ messages in thread
From: Uwe Kleine-König @ 2017-09-21 18:15 UTC (permalink / raw)
To: Martyn Welch
Cc: Greg Kroah-Hartman, Nandor Han, Romain Perier, linux-serial,
linux-arm-kernel, linux-kernel, Fabio Estevam
On Thu, Sep 21, 2017 at 05:18:13PM +0100, Martyn Welch wrote:
> From: Romain Perier <romain.perier@collabora.com>
>
> The variable dma_is_rxing is currently set to 1 in imx_disable_rx_int().
> This is problematic as:
>
> - whilst imx_disable_rx_int() is currently always called before
> start_rx_dma() this dependency isn't obvious.
> - start_rx_dma() does error checking and might exit without
> enabling DMA. Currently this will result in dma_is_rxing suggesting
> that DMA is being used for recieving.
>
> To avoid these issues, move the setting of dma_is_rxing to
> start_rx_dma() when appropriate.
>
> Signed-off-by: Romain Perier <romain.perier@collabora.com>
> Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk>
Reviewed-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Thanks
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v3 3/6] serial: imx: Simplify DMA disablement
2017-09-21 16:18 [PATCH v3 0/6] serial: imx: various improvements Martyn Welch
2017-09-21 16:18 ` [PATCH v3 1/6] serial: imx: remove CTSC and CTS handling from imx_disable_dma Martyn Welch
2017-09-21 16:18 ` [PATCH v3 2/6] serial: imx: only set dma_is_rxing when DMA starts Martyn Welch
@ 2017-09-21 16:18 ` Martyn Welch
2017-09-21 16:18 ` [PATCH v3 4/6] serial: imx: unmap sg buffers when DMA channel is released Martyn Welch
` (2 subsequent siblings)
5 siblings, 0 replies; 12+ messages in thread
From: Martyn Welch @ 2017-09-21 16:18 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Nandor Han, Romain Perier, linux-serial, linux-arm-kernel,
linux-kernel, u.kleine-koenig, Fabio Estevam, Martyn Welch
From: Nandor Han <nandor.han@ge.com>
This commits simplify the function imx_disable_dma() by moving
the code for disabling RX and TX DMAs to dedicated functions.
This is a preparation for the next commit.
Signed-off-by: Nandor Han <nandor.han@ge.com>
Signed-off-by: Romain Perier <romain.perier@collabora.com>
Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk>
---
drivers/tty/serial/imx.c | 27 ++++++++++++++++++++-------
1 file changed, 20 insertions(+), 7 deletions(-)
diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 7835279..2fb3210 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -350,6 +350,24 @@ static void imx_port_rts_auto(struct imx_port *sport, unsigned long *ucr2)
*ucr2 |= UCR2_CTSC;
}
+static void imx_stop_tx_dma(struct imx_port *sport)
+{
+ unsigned long temp;
+
+ temp = readl(sport->port.membase + UCR1);
+ temp &= ~UCR1_TDMAEN;
+ writel(temp, sport->port.membase + UCR1);
+}
+
+static void imx_stop_rx_dma(struct imx_port *sport)
+{
+ unsigned long temp;
+
+ temp = readl(sport->port.membase + UCR1);
+ temp &= ~(UCR1_RDMAEN | UCR1_ATDMAEN);
+ writel(temp, sport->port.membase + UCR1);
+}
+
/*
* interrupts disabled on entry
*/
@@ -1217,13 +1235,8 @@ static void imx_enable_dma(struct imx_port *sport)
static void imx_disable_dma(struct imx_port *sport)
{
- unsigned long temp;
-
- /* clear UCR1 */
- temp = readl(sport->port.membase + UCR1);
- temp &= ~(UCR1_RDMAEN | UCR1_TDMAEN | UCR1_ATDMAEN);
- writel(temp, sport->port.membase + UCR1);
-
+ imx_stop_rx_dma(sport);
+ imx_stop_tx_dma(sport);
imx_setup_ufcr(sport, TXTL_DEFAULT, RXTL_DEFAULT);
sport->dma_is_enabled = 0;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v3 4/6] serial: imx: unmap sg buffers when DMA channel is released
2017-09-21 16:18 [PATCH v3 0/6] serial: imx: various improvements Martyn Welch
` (2 preceding siblings ...)
2017-09-21 16:18 ` [PATCH v3 3/6] serial: imx: Simplify DMA disablement Martyn Welch
@ 2017-09-21 16:18 ` Martyn Welch
2017-09-21 18:25 ` Uwe Kleine-König
2017-09-21 16:18 ` [PATCH v3 5/6] serial: imx: update the stop rx,tx procedures Martyn Welch
2017-09-21 16:18 ` [PATCH v3 6/6] serial: imx: Fix imx_shutdown procedure Martyn Welch
5 siblings, 1 reply; 12+ messages in thread
From: Martyn Welch @ 2017-09-21 16:18 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Nandor Han, Romain Perier, linux-serial, linux-arm-kernel,
linux-kernel, u.kleine-koenig, Fabio Estevam, Martyn Welch
From: Nandor Han <nandor.han@ge.com>
This commits unmaps sg buffers when the DMA channel is released. It also
sets to zero `dma_is_rxing` and `dma_is_txing` to state that the
corresponding channels cannot transmit/receive data, as these are
disabled.
Signed-off-by: Nandor Han <nandor.han@ge.com>
Signed-off-by: Romain Perier <romain.perier@collabora.com>
Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk>
---
drivers/tty/serial/imx.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 2fb3210..ed02783 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -357,6 +357,12 @@ static void imx_stop_tx_dma(struct imx_port *sport)
temp = readl(sport->port.membase + UCR1);
temp &= ~UCR1_TDMAEN;
writel(temp, sport->port.membase + UCR1);
+
+ if (sport->dma_is_txing) {
+ dma_unmap_sg(sport->port.dev, &sport->tx_sgl[0],
+ sport->dma_tx_nents, DMA_TO_DEVICE);
+ sport->dma_is_txing = 0;
+ }
}
static void imx_stop_rx_dma(struct imx_port *sport)
@@ -366,6 +372,12 @@ static void imx_stop_rx_dma(struct imx_port *sport)
temp = readl(sport->port.membase + UCR1);
temp &= ~(UCR1_RDMAEN | UCR1_ATDMAEN);
writel(temp, sport->port.membase + UCR1);
+
+ if (sport->dma_is_rxing) {
+ dma_unmap_sg(sport->port.dev, &sport->rx_sgl, 1,
+ DMA_FROM_DEVICE);
+ sport->dma_is_rxing = 0;
+ }
}
/*
--
1.8.3.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v3 4/6] serial: imx: unmap sg buffers when DMA channel is released
2017-09-21 16:18 ` [PATCH v3 4/6] serial: imx: unmap sg buffers when DMA channel is released Martyn Welch
@ 2017-09-21 18:25 ` Uwe Kleine-König
0 siblings, 0 replies; 12+ messages in thread
From: Uwe Kleine-König @ 2017-09-21 18:25 UTC (permalink / raw)
To: Martyn Welch
Cc: Greg Kroah-Hartman, Nandor Han, Romain Perier, linux-serial,
linux-arm-kernel, linux-kernel, Fabio Estevam
On Thu, Sep 21, 2017 at 05:18:15PM +0100, Martyn Welch wrote:
> From: Nandor Han <nandor.han@ge.com>
>
> This commits unmaps sg buffers when the DMA channel is released. It also
> sets to zero `dma_is_rxing` and `dma_is_txing` to state that the
> corresponding channels cannot transmit/receive data, as these are
> disabled.
That's a fix for a race, right? The functions being fixed were
introduced just in the patch before and I guess the race was there
already before. It is best if you could resort your series to have fixes
first. This way they can easily be applied during the rc phase while the
rest waits for the merge window.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v3 5/6] serial: imx: update the stop rx,tx procedures
2017-09-21 16:18 [PATCH v3 0/6] serial: imx: various improvements Martyn Welch
` (3 preceding siblings ...)
2017-09-21 16:18 ` [PATCH v3 4/6] serial: imx: unmap sg buffers when DMA channel is released Martyn Welch
@ 2017-09-21 16:18 ` Martyn Welch
2017-09-21 18:28 ` Uwe Kleine-König
2017-09-21 16:18 ` [PATCH v3 6/6] serial: imx: Fix imx_shutdown procedure Martyn Welch
5 siblings, 1 reply; 12+ messages in thread
From: Martyn Welch @ 2017-09-21 16:18 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Nandor Han, Romain Perier, linux-serial, linux-arm-kernel,
linux-kernel, u.kleine-koenig, Fabio Estevam, Martyn Welch
From: Nandor Han <nandor.han@ge.com>
According to "Documentation/serial/driver" both procedures should stop
receiving or sending data. Based on this the procedures should stop the
activity regardless if DMA is enabled or not.
This commit updates both imx_stop_{rx|tx} procedures to stop the
activity and disable the interrupts related to that.
Signed-off-by: Nandor Han <nandor.han@ge.com>
Signed-off-by: Romain Perier <romain.perier@collabora.com>
Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk>
---
drivers/tty/serial/imx.c | 36 +++++++++++++++++-------------------
1 file changed, 17 insertions(+), 19 deletions(-)
diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index ed02783..256b128 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -388,15 +388,14 @@ static void imx_stop_tx(struct uart_port *port)
struct imx_port *sport = (struct imx_port *)port;
unsigned long temp;
- /*
- * We are maybe in the SMP context, so if the DMA TX thread is running
- * on other cpu, we have to wait for it to finish.
- */
- if (sport->dma_is_enabled && sport->dma_is_txing)
- return;
+ sport->tx_bytes = 0;
+
+ if (sport->dma_is_enabled)
+ imx_stop_tx_dma(sport);
temp = readl(port->membase + UCR1);
- writel(temp & ~UCR1_TXMPTYEN, port->membase + UCR1);
+ temp &= ~(UCR1_TXMPTYEN | UCR1_TRDYEN | UCR1_RTSDEN);
+ writel(temp, port->membase + UCR1);
/* in rs485 mode disable transmitter if shifter is empty */
if (port->rs485.flags & SER_RS485_ENABLED &&
@@ -423,21 +422,20 @@ static void imx_stop_rx(struct uart_port *port)
struct imx_port *sport = (struct imx_port *)port;
unsigned long temp;
- if (sport->dma_is_enabled && sport->dma_is_rxing) {
- if (sport->port.suspended) {
- dmaengine_terminate_all(sport->dma_chan_rx);
- sport->dma_is_rxing = 0;
- } else {
- return;
- }
- }
+ if (sport->dma_is_enabled)
+ imx_stop_rx_dma(sport);
+
+ temp = readl(sport->port.membase + UCR1);
+ temp &= ~UCR1_RRDYEN;
+ writel(temp, sport->port.membase + UCR1);
temp = readl(sport->port.membase + UCR2);
- writel(temp & ~UCR2_RXEN, sport->port.membase + UCR2);
+ temp &= ~UCR2_ATEN;
+ writel(temp, sport->port.membase + UCR2);
- /* disable the `Receiver Ready Interrrupt` */
- temp = readl(sport->port.membase + UCR1);
- writel(temp & ~UCR1_RRDYEN, sport->port.membase + UCR1);
+ temp = readl(sport->port.membase + UCR3);
+ temp &= ~UCR3_AWAKEN;
+ writel(temp, sport->port.membase + UCR3);
}
/*
--
1.8.3.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v3 5/6] serial: imx: update the stop rx,tx procedures
2017-09-21 16:18 ` [PATCH v3 5/6] serial: imx: update the stop rx,tx procedures Martyn Welch
@ 2017-09-21 18:28 ` Uwe Kleine-König
0 siblings, 0 replies; 12+ messages in thread
From: Uwe Kleine-König @ 2017-09-21 18:28 UTC (permalink / raw)
To: Martyn Welch
Cc: Greg Kroah-Hartman, Nandor Han, Romain Perier, linux-serial,
linux-arm-kernel, linux-kernel, Fabio Estevam
On Thu, Sep 21, 2017 at 05:18:16PM +0100, Martyn Welch wrote:
> From: Nandor Han <nandor.han@ge.com>
>
> According to "Documentation/serial/driver" both procedures should stop
> receiving or sending data. Based on this the procedures should stop the
> activity regardless if DMA is enabled or not.
>
> This commit updates both imx_stop_{rx|tx} procedures to stop the
> activity and disable the interrupts related to that.
>
> Signed-off-by: Nandor Han <nandor.han@ge.com>
> Signed-off-by: Romain Perier <romain.perier@collabora.com>
> Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk>
> ---
> drivers/tty/serial/imx.c | 36 +++++++++++++++++-------------------
> 1 file changed, 17 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
> index ed02783..256b128 100644
> --- a/drivers/tty/serial/imx.c
> +++ b/drivers/tty/serial/imx.c
> @@ -388,15 +388,14 @@ static void imx_stop_tx(struct uart_port *port)
> struct imx_port *sport = (struct imx_port *)port;
> unsigned long temp;
>
> - /*
> - * We are maybe in the SMP context, so if the DMA TX thread is running
> - * on other cpu, we have to wait for it to finish.
> - */
did you understand this comment you're removing here? I admit I don't
but unless it is non-sensical you're doing something wrong.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v3 6/6] serial: imx: Fix imx_shutdown procedure
2017-09-21 16:18 [PATCH v3 0/6] serial: imx: various improvements Martyn Welch
` (4 preceding siblings ...)
2017-09-21 16:18 ` [PATCH v3 5/6] serial: imx: update the stop rx,tx procedures Martyn Welch
@ 2017-09-21 16:18 ` Martyn Welch
5 siblings, 0 replies; 12+ messages in thread
From: Martyn Welch @ 2017-09-21 16:18 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Nandor Han, Romain Perier, linux-serial, linux-arm-kernel,
linux-kernel, u.kleine-koenig, Fabio Estevam, Martyn Welch
From: Nandor Han <nandor.han@ge.com>
In some cases, it looks that interrupts can happen after the dma was
disabled and port was not yet shutdown. This will result in interrupts
handled by imx_rxint.
This commits updates the shutdown function to ensure that underlying
components are disabled in the right order. This disables RX and TX
blocks, then it disabled interrupts. In case DMA is enabled, it disables
DMA and free corresponding resources. It disables UART port and stop
clocks.
Signed-off-by: Nandor Han <nandor.han@ge.com>
Signed-off-by: Romain Perier <romain.perier@collabora.com>
Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk>
---
drivers/tty/serial/imx.c | 34 +++++++++++++++-------------------
1 file changed, 15 insertions(+), 19 deletions(-)
diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 256b128..393c1c0 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -1368,44 +1368,40 @@ static void imx_shutdown(struct uart_port *port)
unsigned long temp;
unsigned long flags;
- if (sport->dma_is_enabled) {
- sport->dma_is_rxing = 0;
- sport->dma_is_txing = 0;
- dmaengine_terminate_sync(sport->dma_chan_tx);
- dmaengine_terminate_sync(sport->dma_chan_rx);
-
+ if (!sport->port.suspended) {
spin_lock_irqsave(&sport->port.lock, flags);
imx_stop_tx(port);
imx_stop_rx(port);
- imx_disable_dma(sport);
+
+ if (sport->dma_is_inited && sport->dma_is_enabled)
+ imx_disable_dma(sport);
+
spin_unlock_irqrestore(&sport->port.lock, flags);
imx_uart_dma_exit(sport);
}
- mctrl_gpio_disable_ms(sport->gpios);
-
spin_lock_irqsave(&sport->port.lock, flags);
temp = readl(sport->port.membase + UCR2);
- temp &= ~(UCR2_TXEN);
+ temp &= ~(UCR2_TXEN | UCR2_RXEN);
writel(temp, sport->port.membase + UCR2);
+ temp = readl(sport->port.membase + UCR4);
+ temp &= ~UCR4_OREN;
+ writel(temp, sport->port.membase + UCR4);
spin_unlock_irqrestore(&sport->port.lock, flags);
- /*
- * Stop our timer.
- */
- del_timer_sync(&sport->timer);
+ mctrl_gpio_disable_ms(sport->gpios);
- /*
- * Disable all interrupts, port and break condition.
- */
+ /* Stop our timer. */
+ del_timer_sync(&sport->timer);
+ /* Disable port. */
spin_lock_irqsave(&sport->port.lock, flags);
temp = readl(sport->port.membase + UCR1);
- temp &= ~(UCR1_TXMPTYEN | UCR1_RRDYEN | UCR1_RTSDEN | UCR1_UARTEN);
-
+ temp &= ~UCR1_UARTEN;
writel(temp, sport->port.membase + UCR1);
spin_unlock_irqrestore(&sport->port.lock, flags);
+ /* Disable clocks. */
clk_disable_unprepare(sport->clk_per);
clk_disable_unprepare(sport->clk_ipg);
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 12+ messages in thread