lkml.org 
[lkml]   [2017]   [Jul]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 7/7] serial: imx: Fix imx_shutdown procedure
    On Fri, Jun 30, 2017 at 02:04:46PM +0200, Romain Perier wrote:
    > From: Nandor Han <nandor.han@ge.com>
    >
    > In some cases, It looks that interrupts can happen after the dma was
    s/It/it/

    > 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>
    > ---
    > drivers/tty/serial/imx.c | 38 +++++++++++++++++++-------------------
    > 1 file changed, 19 insertions(+), 19 deletions(-)
    >
    > diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
    > index d5b6e09..7dc6f0c 100644
    > --- a/drivers/tty/serial/imx.c
    > +++ b/drivers/tty/serial/imx.c
    > @@ -1404,44 +1404,44 @@ 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);
    > spin_unlock_irqrestore(&sport->port.lock, flags);
    > - imx_uart_dma_exit(sport);
    > }
    >
    > - mctrl_gpio_disable_ms(sport->gpios);
    > + if (sport->dma_is_inited) {
    > + if (sport->dma_is_enabled) {
    > + spin_lock_irqsave(&sport->port.lock, flags);
    > + imx_disable_dma(sport);
    > + spin_unlock_irqrestore(&sport->port.lock, flags);
    > + }
    > + imx_uart_dma_exit(sport);
    > + }
    >
    > 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);

    The function already had two critical blocks protected by
    spin_lock_irqsave on sport->port.lock. With your patch there are three.
    Is it possible to grab the lock only once?

    >
    > - /*
    > - * Stop our timer.
    > - */
    > - del_timer_sync(&sport->timer);
    > + mctrl_gpio_disable_ms(sport->gpios);
    >
    > - /*
    > - * Disable all interrupts, port and break condition.
    > - */
    > + /* Stop our timer. */

    Updating the comment style in such a commit makes the diff unnecessarily
    hard to read.

    > + 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
    >
    > --
    > To unsubscribe from this list: send the line "unsubscribe linux-serial" in
    > the body of a message to majordomo@vger.kernel.org
    > More majordomo info at http://vger.kernel.org/majordomo-info.html
    >

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

    \
     
     \ /
      Last update: 2017-07-03 09:09    [W:3.082 / U:0.032 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site