Re: [PATCH] serial: atmel: don't enable IRQs prematurely

[Jiri Slaby <jirislaby@kernel.org>]

On 19. 06. 23, 11:45, Dan Carpenter wrote:
> The atmel_complete_tx_dma() function disables IRQs at the start
> of the function by calling spin_lock_irqsave(&port->lock, flags);
> There is no need to disable them a second time using the
> spin_lock_irq() function and, in fact, doing so is a bug because
> it will enable IRQs prematurely when we call spin_unlock_irq().
> 
> Just use spin_lock/unlock() instead without disabling or enabling
> IRQs.
> 
> Fixes: 08f738be88bb ("serial: at91: add tx dma support")
> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> ---
>   drivers/tty/serial/atmel_serial.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
> index 6e9192f122aa..3467a875641a 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -868,11 +868,11 @@ static void atmel_complete_tx_dma(void *arg)
>   		dmaengine_terminate_all(chan);
>   	uart_xmit_advance(port, atmel_port->tx_len);
>   
> -	spin_lock_irq(&atmel_port->lock_tx);
> +	spin_lock(&atmel_port->lock_tx);
>   	async_tx_ack(atmel_port->desc_tx);
>   	atmel_port->cookie_tx = -EINVAL;
>   	atmel_port->desc_tx = NULL;
> -	spin_unlock_irq(&atmel_port->lock_tx);
> +	spin_unlock(&atmel_port->lock_tx);

Hmm, can you ensure the DMA engine code calls this with irqs disabled? 
If so, you should document it in the commit log. If not, you shyuld use 
_irqsave() variant.

thanks,
-- 
js
suse labs