| Message ID | 1502086885-8390-1-git-send-email-neeraju@codeaurora.org (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | Andy Gross |
| Headers | show |
On Mon, Aug 07, 2017 at 11:51:25AM +0530, Neeraj Upadhyay wrote: > Move the request_irq() call to the end of the msm_startup(), > so that we don't handle interrupts while msm_startup() is > running. This avoids potential races while initialization > is in progress. For example, consider below scenario > where rx handler reads the intermediate value of dma->chan, > set in msm_request_rx_dma(), and tries to do dma mapping, > which results in data abort. > > uart_port_startup() > msm_startup() > request_irq() > ... > msm_request_rx_dma() > ... > dma->chan = dma_request_slave_channel_reason(dev, "rx"); > <UART RX IRQ> > msm_uart_irq() > msm_handle_rx_dm() > msm_start_rx_dma() > dma->desc = dma_map_single() > <data abort> > > Signed-off-by: Neeraj Upadhyay <neeraju@codeaurora.org> This seems reasonable. Reviewd-by: Andy Gross <andy.gross@linaro.org> -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 08/07, Neeraj Upadhyay wrote: > Move the request_irq() call to the end of the msm_startup(), > so that we don't handle interrupts while msm_startup() is > running. This avoids potential races while initialization > is in progress. For example, consider below scenario > where rx handler reads the intermediate value of dma->chan, > set in msm_request_rx_dma(), and tries to do dma mapping, > which results in data abort. > > uart_port_startup() > msm_startup() > request_irq() > ... > msm_request_rx_dma() > ... > dma->chan = dma_request_slave_channel_reason(dev, "rx"); > <UART RX IRQ> > msm_uart_irq() > msm_handle_rx_dm() > msm_start_rx_dma() > dma->desc = dma_map_single() > <data abort> > > Signed-off-by: Neeraj Upadhyay <neeraju@codeaurora.org> > --- Or we can write the IMR register with 0 to mask all irqs before requesting the irq handler be setup? We will unmask the interrupts we care about anyway when we setup termios. If not, this change is ok, but I would guess it doesn't fix spurious irqs from happening while we keep configuring the hardware. Feel free to take my reviewed-by though. Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
diff --git a/drivers/tty/serial/msm_serial.c b/drivers/tty/serial/msm_serial.c index 6788e75..1db79ee 100644 --- a/drivers/tty/serial/msm_serial.c +++ b/drivers/tty/serial/msm_serial.c @@ -1175,11 +1175,6 @@ static int msm_startup(struct uart_port *port) snprintf(msm_port->name, sizeof(msm_port->name), "msm_serial%d", port->line); - ret = request_irq(port->irq, msm_uart_irq, IRQF_TRIGGER_HIGH, - msm_port->name, port); - if (unlikely(ret)) - return ret; - msm_init_clock(port); if (likely(port->fifosize > 12)) @@ -1206,7 +1201,21 @@ static int msm_startup(struct uart_port *port) msm_request_rx_dma(msm_port, msm_port->uart.mapbase); } + ret = request_irq(port->irq, msm_uart_irq, IRQF_TRIGGER_HIGH, + msm_port->name, port); + if (unlikely(ret)) + goto err_irq; + return 0; + +err_irq: + if (msm_port->is_uartdm) + msm_release_dma(msm_port); + + clk_disable_unprepare(msm_port->pclk); + clk_disable_unprepare(msm_port->clk); + + return ret; } static void msm_shutdown(struct uart_port *port)
Move the request_irq() call to the end of the msm_startup(), so that we don't handle interrupts while msm_startup() is running. This avoids potential races while initialization is in progress. For example, consider below scenario where rx handler reads the intermediate value of dma->chan, set in msm_request_rx_dma(), and tries to do dma mapping, which results in data abort. uart_port_startup() msm_startup() request_irq() ... msm_request_rx_dma() ... dma->chan = dma_request_slave_channel_reason(dev, "rx"); <UART RX IRQ> msm_uart_irq() msm_handle_rx_dm() msm_start_rx_dma() dma->desc = dma_map_single() <data abort> Signed-off-by: Neeraj Upadhyay <neeraju@codeaurora.org> --- drivers/tty/serial/msm_serial.c | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-)