Re: [PATCH 1/2] i2c: rcar: fix concurrency issue related to ICDMAER

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, Mar 03, 2019 at 04:03:13PM +0100, Wolfram Sang wrote:
> From: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@xxxxxxxxxxx>
> 
> This patch fixes the problem that an interrupt may set up a new I2C
> message and the DMA callback overwrites this setup.
> 
> By disabling the DMA Enable Register(ICDMAER), rcar_i2c_dma_unmap()
> enables interrupts for register settings (such as Master Control
> Register(ICMCR)) and advances the I2C transfer sequence.
> 
> If an interrupt occurs immediately after ICDMAER is disabled, the
> callback handler later continues and overwrites the previous settings
> from the interrupt. So, disable ICDMAER at the end of the callback to
> ensure other interrupts are masked until then.
> 
> Note that this driver needs to work lock-free because there are IP cores
> with a HW race condition which prevent us from using a spinlock in the
> interrupt handler.
> 
> Reproduction test:
> 1. Add udelay(1) after disabling ICDMAER. (It is expected to
> generate an interrupt of rcar_i2c_irq())
> 
>     void rcar_i2c_dma_unmap(struct rcar_i2c_priv *priv)
>     {
>         ...
>         rcar_i2c_write(priv, ICDMAER, 0);
>         udelay(1);                        <-- Add this line
>         ...
>         priv->dma_direction = DMA_NONE;
>     }
> 
> 2. Execute DMA transfer.
> Performs DMA transfer of read or write. In the sample, write was used.
> 
>  $ i2cset -y -f 4 0x6a 0x01 0x01 0x02 0x03 0x04 0x05 0x06 0x07 0x08 i
> 
> 3. A log message of BUG_ON() will be displayed.
> 
> Signed-off-by: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@xxxxxxxxxxx>
> Signed-off-by: Wolfram Sang <wsa+renesas@xxxxxxxxxxxxxxxxxxxx>

Reviewed-by: Simon Horman <horms+renesas@xxxxxxxxxxxx>

> ---
>  drivers/i2c/busses/i2c-rcar.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
> index dd52a068b140..6410896f717b 100644
> --- a/drivers/i2c/busses/i2c-rcar.c
> +++ b/drivers/i2c/busses/i2c-rcar.c
> @@ -363,9 +363,6 @@ static void rcar_i2c_dma_unmap(struct rcar_i2c_priv *priv)
>  	struct dma_chan *chan = priv->dma_direction == DMA_FROM_DEVICE
>  		? priv->dma_rx : priv->dma_tx;
>  
> -	/* Disable DMA Master Received/Transmitted */
> -	rcar_i2c_write(priv, ICDMAER, 0);
> -
>  	dma_unmap_single(chan->device->dev, sg_dma_address(&priv->sg),
>  			 sg_dma_len(&priv->sg), priv->dma_direction);
>  
> @@ -375,6 +372,9 @@ static void rcar_i2c_dma_unmap(struct rcar_i2c_priv *priv)
>  		priv->flags |= ID_P_NO_RXDMA;
>  
>  	priv->dma_direction = DMA_NONE;
> +
> +	/* Disable DMA Master Received/Transmitted */
> +	rcar_i2c_write(priv, ICDMAER, 0);
>  }
>  
>  static void rcar_i2c_cleanup_dma(struct rcar_i2c_priv *priv)
> -- 
> 2.11.0
> 



[Index of Archives]     [Linux GPIO]     [Linux SPI]     [Linux Hardward Monitoring]     [LM Sensors]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux