Commit a35ba2f7 authored by Hiromitsu Yamasaki's avatar Hiromitsu Yamasaki Committed by Wolfram Sang

i2c: rcar: fix concurrency issue related to ICDMAER

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 a delay 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);
        usleep_range(500, 800)
        ...
        priv->dma_direction = DMA_NONE;
    }

2. Execute DMA transfers

 $ i2ctransfer -y 4 w9@0x6a 1 1+ r16

3. A log message of BUG_ON() will be displayed.

Fixes: 73e8b052 ("i2c: rcar: add DMA support")
Signed-off-by: default avatarHiromitsu Yamasaki <hiromitsu.yamasaki.ym@renesas.com>
Signed-off-by: default avatarWolfram Sang <wsa+renesas@sang-engineering.com>
[wsa: updated test case to be more reliable, added note to comment]
Reviewed-by: default avatarSimon Horman <horms+renesas@verge.net.au>
Signed-off-by: default avatarWolfram Sang <wsa@the-dreams.de>
parent 60f7691c
...@@ -363,9 +363,6 @@ static void rcar_i2c_dma_unmap(struct rcar_i2c_priv *priv) ...@@ -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 struct dma_chan *chan = priv->dma_direction == DMA_FROM_DEVICE
? priv->dma_rx : priv->dma_tx; ? 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), dma_unmap_single(chan->device->dev, sg_dma_address(&priv->sg),
sg_dma_len(&priv->sg), priv->dma_direction); sg_dma_len(&priv->sg), priv->dma_direction);
...@@ -375,6 +372,9 @@ static void rcar_i2c_dma_unmap(struct rcar_i2c_priv *priv) ...@@ -375,6 +372,9 @@ static void rcar_i2c_dma_unmap(struct rcar_i2c_priv *priv)
priv->flags |= ID_P_NO_RXDMA; priv->flags |= ID_P_NO_RXDMA;
priv->dma_direction = DMA_NONE; priv->dma_direction = DMA_NONE;
/* Disable DMA Master Received/Transmitted, must be last! */
rcar_i2c_write(priv, ICDMAER, 0);
} }
static void rcar_i2c_cleanup_dma(struct rcar_i2c_priv *priv) static void rcar_i2c_cleanup_dma(struct rcar_i2c_priv *priv)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment