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
>
