From: Nandor Han <redacted>
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 <redacted>
Signed-off-by: Romain Perier <redacted>
Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk>
---
drivers/tty/serial/imx.c | 27 ++++++++++++++++++++-------
1 file changed, 20 insertions(+), 7 deletions(-)
From: Romain Perier <redacted>
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 <redacted>
Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk>
---
drivers/tty/serial/imx.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
@@ -714,8 +714,6 @@ static void imx_disable_rx_int(struct imx_port *sport){unsignedlongtemp;-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);return0;
From: Nandor Han <redacted>
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 <redacted>
Signed-off-by: Romain Perier <redacted>
Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk>
---
drivers/tty/serial/imx.c | 36 +++++++++++++++++-------------------
1 file changed, 17 insertions(+), 19 deletions(-)
From: Nandor Han <redacted>
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 <redacted>
Signed-off-by: Romain Perier <redacted>
Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk>
---
drivers/tty/serial/imx.c | 34 +++++++++++++++-------------------
1 file changed, 15 insertions(+), 19 deletions(-)
From: Nandor Han <redacted>
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 <redacted>
Signed-off-by: Romain Perier <redacted>
Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk>
---
drivers/tty/serial/imx.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
From: Nandor Han <redacted>
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 <redacted>
Signed-off-by: Romain Perier <redacted>
Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk>
---
drivers/tty/serial/imx.c | 5 -----
1 file changed, 5 deletions(-)
On Thu, Sep 21, 2017 at 05:18:13PM +0100, Martyn Welch wrote:
From: Romain Perier <redacted>
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 <redacted>
Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk>
On Thu, Sep 21, 2017 at 05:18:12PM +0100, Martyn Welch wrote:
From: Nandor Han <redacted>
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?
quoted hunk
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 <redacted>
Signed-off-by: Romain Perier <redacted>
Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk>
---
drivers/tty/serial/imx.c | 5 -----
1 file changed, 5 deletions(-)
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/ |
On Thu, Sep 21, 2017 at 05:18:15PM +0100, Martyn Welch wrote:
From: Nandor Han <redacted>
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/ |
On Thu, Sep 21, 2017 at 05:18:16PM +0100, Martyn Welch wrote:
quoted hunk
From: Nandor Han <redacted>
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 <redacted>
Signed-off-by: Romain Perier <redacted>
Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk>
---
drivers/tty/serial/imx.c | 36 +++++++++++++++++-------------------
1 file changed, 17 insertions(+), 19 deletions(-)
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/ |
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:
quoted
From: Nandor Han <redacted>
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.
quoted
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 <redacted>
Signed-off-by: Romain Perier <redacted>
Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk>
---
drivers/tty/serial/imx.c | 5 -----
1 file changed, 5 deletions(-)
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/ |