Re: [PATCH v4 1/2] i2c: add DMA support for freescale i2c driver

[sourav <sourav.poddar-l0cyMroinI0@public.gmane.org>]

>> +
>> +    dma->chan_tx = dma_request_slave_channel(dev, "tx");
>> +    return 0;

To be more clear,
return here looks to be some leftover.

> ?? Looks to be some leftover?
>> +    if (!dma->chan_tx) {
>> +        dev_info(dev, "DMA tx channel request failed\n");
>> +        ret = -ENODEV;
>> +        goto fail_al;
>> +    }
>> +
>> +    dma_sconfig.dst_addr = phy_addr +
>> +                (IMX_I2C_I2DR<<  i2c_imx->hwdata->regshift);
>> +    dma_sconfig.dst_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
>> +    dma_sconfig.dst_maxburst = 1;
>> +    dma_sconfig.direction = DMA_MEM_TO_DEV;
>> +    ret = dmaengine_slave_config(dma->chan_tx,&dma_sconfig);
>> +    if (ret<  0) {
>> +        dev_info(dev, "DMA slave config failed, err = %d\n", ret);
>> +        goto fail_tx;
>> +    }
>> +
>> +    dma->chan_rx = dma_request_slave_channel(dev, "rx");
>> +    if (!dma->chan_rx) {
>> +        dev_info(dev, "DMA rx channel request failed\n");
>> +        ret = -ENODEV;
>> +        goto fail_tx;
>> +    }
>> +
>> +    dma_sconfig.src_addr = phy_addr +
>> +                (IMX_I2C_I2DR<<  i2c_imx->hwdata->regshift);
>> +    dma_sconfig.src_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
>> +    dma_sconfig.src_maxburst = 1;
>> +    dma_sconfig.direction = DMA_DEV_TO_MEM;
>> +    ret = dmaengine_slave_config(dma->chan_rx,&dma_sconfig);
>> +    if (ret<  0) {
>> +        dev_info(dev, "DMA slave config failed, err = %d\n", ret);
>> +        goto fail_rx;
>> +    }
>> +
>> +    i2c_imx->dma = dma;
>> +
>> +    init_completion(&dma->cmd_complete);
>> +
>> +    return 0;
>> +
>> +fail_rx:
>> +    dma_release_channel(dma->chan_rx);
>> +fail_tx:
>> +    dma_release_channel(dma->chan_tx);
>> +fail_al:
>> +    devm_kfree(dev, dma);
>> +
>> +    return ret;
>> +}
>> +
>> +static void i2c_imx_dma_callback(void *arg)
>> +{
>> +    struct imx_i2c_struct *i2c_imx = (struct imx_i2c_struct *)arg;
>> +    struct imx_i2c_dma *dma = i2c_imx->dma;
>> +
>> +    dma_unmap_single(dma->chan_using->device->dev, dma->dma_buf,
>> +            dma->dma_len, dma->dma_data_dir);
>> +    complete(&dma->cmd_complete);
>> +}
>> +
>> +static int i2c_imx_dma_xfer(struct imx_i2c_struct *i2c_imx,
>> +                    struct i2c_msg *msgs)
>> +{
>> +    struct imx_i2c_dma *dma = i2c_imx->dma;
>> +    struct dma_async_tx_descriptor *txdesc;
>> +    struct device *dev =&i2c_imx->adapter.dev;
>> +
>> +    dma->dma_buf = dma_map_single(dma->chan_using->device->dev, 
>> msgs->buf,
>> +                    dma->dma_len, dma->dma_data_dir);
>> +    if (dma_mapping_error(dma->chan_using->device->dev, 
>> dma->dma_buf)) {
>> +        dev_err(dev, "DMA mapping failed\n");
>> +        return -EINVAL;
>> +    }
>> +
>> +    txdesc = dmaengine_prep_slave_single(dma->chan_using, dma->dma_buf,
>> +                    dma->dma_len, dma->dma_transfer_dir,
>> +                    DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
>> +    if (!txdesc) {
>> +        dev_err(dev, "Not able to get desc for DMA xfer\n");
>> +        dma_unmap_single(dma->chan_using->device->dev, dma->dma_buf,
>> +                    dma->dma_len, dma->dma_data_dir);
>> +        return -EINVAL;
>> +    }
>> +
>> +    txdesc->callback = i2c_imx_dma_callback;
>> +    txdesc->callback_param = i2c_imx;
>> +    dmaengine_submit(txdesc);
>> +    dma_async_issue_pending(dma->chan_using);
>> +
>> +    return 0;
>> +}
>> +
>> +static void i2c_imx_dma_free(struct imx_i2c_struct *i2c_imx)
>> +{
>> +    struct imx_i2c_dma *dma = i2c_imx->dma;
>> +
>> +    dma->dma_buf = 0;
>> +    dma->dma_len = 0;
>> +
>> +    dma_release_channel(dma->chan_tx);
>> +    dma->chan_tx = NULL;
>> +
>> +    dma_release_channel(dma->chan_rx);
>> +    dma->chan_rx = NULL;
>> +
>> +    dma->chan_using = NULL;
>> +}
>> +
>>   /** Functions for IMX I2C adapter driver 
>> ***************************************
>>   
>> *******************************************************************************/
>>
>> @@ -334,6 +483,11 @@ static int i2c_imx_start(struct imx_i2c_struct 
>> *i2c_imx)
>>
>>       temp |= I2CR_IIEN | I2CR_MTX | I2CR_TXAK;
>>       imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
>> +
>> +    temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
>> +    temp&= ~I2CR_DMAEN;
>> +    imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
>> +
>>       return result;
>>   }
>>
>> @@ -427,44 +581,101 @@ static irqreturn_t i2c_imx_isr(int irq, void 
>> *dev_id)
>>
>>   static int i2c_imx_write(struct imx_i2c_struct *i2c_imx, struct 
>> i2c_msg *msgs)
>>   {
>> -    int i, result;
>> +    int i, result, timeout = IMX_I2C_DMA_TIMEOUT;
>> +    unsigned int temp = 0;
>> +    struct imx_i2c_dma *dma = i2c_imx->dma;
>> +    struct device *dev =&i2c_imx->adapter.dev;
>>
>> -    dev_dbg(&i2c_imx->adapter.dev, "<%s>  write slave address: 
>> addr=0x%x\n",
>> +    dev_dbg(dev, "<%s>  write slave address: addr=0x%x\n",
>>           __func__, msgs->addr<<  1);
>> +    if (dma&&  msgs->len>= IMX_I2C_DMA_THRESHOLD) {
>> +        reinit_completion(&i2c_imx->dma->cmd_complete);
>> +        dma->chan_using = dma->chan_tx;
>> +        dma->dma_transfer_dir = DMA_MEM_TO_DEV;
>> +        dma->dma_data_dir = DMA_TO_DEVICE;
>> +        dma->dma_len = msgs->len - 1;
>> +        result = i2c_imx_dma_xfer(i2c_imx, msgs);
>> +        if (result)
>> +            return result;
>>
>> -    /* write slave address */
>> -    imx_i2c_write_reg(msgs->addr<<  1, i2c_imx, IMX_I2C_I2DR);
>> -    result = i2c_imx_trx_complete(i2c_imx);
>> -    if (result)
>> -        return result;
>> -    result = i2c_imx_acked(i2c_imx);
>> -    if (result)
>> -        return result;
>> -    dev_dbg(&i2c_imx->adapter.dev, "<%s>  write data\n", __func__);
>> +        temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
>> +        temp |= I2CR_DMAEN;
>> +        imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
>>
>> -    /* write data */
>> -    for (i = 0; i<  msgs->len; i++) {
>> -        dev_dbg(&i2c_imx->adapter.dev,
>> -            "<%s>  write byte: B%d=0x%X\n",
>> -            __func__, i, msgs->buf[i]);
>> -        imx_i2c_write_reg(msgs->buf[i], i2c_imx, IMX_I2C_I2DR);
>> +        /* write slave address */
>> +        imx_i2c_write_reg(msgs->addr<<  1, i2c_imx, IMX_I2C_I2DR);
>> +        result = wait_for_completion_interruptible_timeout(
>> + &i2c_imx->dma->cmd_complete,
>> +                    msecs_to_jiffies(IMX_I2C_DMA_TIMEOUT));
>> +        if (result<= 0) {
>> +            dmaengine_terminate_all(dma->chan_using);
>> +            if (result)
>> +                return result;
>> +            else
>> +                return -ETIMEDOUT;
>> +        }
>> +
>> +        /* waiting for Transfer complete. */
>> +        while (timeout--) {
>> +            temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2SR);
>> +            if (temp&  I2SR_ICF)
>> +                break;
>> +            udelay(10);
>> +        }
>> +
>> +        if (!timeout)
>> +            return -ETIMEDOUT;
>> +        temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
>> +        temp&= ~I2CR_DMAEN;
>> +        imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
>> +
>> +        /* write the last byte */
>> +        imx_i2c_write_reg(msgs->buf[msgs->len-1],
>> +                    i2c_imx, IMX_I2C_I2DR);
>>           result = i2c_imx_trx_complete(i2c_imx);
>>           if (result)
>>               return result;
>> +
>> +        result = i2c_imx_acked(i2c_imx);
>> +        if (result)
>> +            return result;
>> +    } else {
>> +        /* write slave address */
>> +        imx_i2c_write_reg(msgs->addr<<  1, i2c_imx, IMX_I2C_I2DR);
>> +        result = i2c_imx_trx_complete(i2c_imx);
>> +        if (result)
>> +            return result;
>> +
>>           result = i2c_imx_acked(i2c_imx);
>>           if (result)
>>               return result;
>> +
>> +        dev_dbg(dev, "<%s>  write data\n", __func__);
>> +
>> +        /* write data */
>> +        for (i = 0; i<  msgs->len; i++) {
>> +            dev_dbg(dev, "<%s>  write byte: B%d=0x%X\n",
>> +                __func__, i, msgs->buf[i]);
>> +            imx_i2c_write_reg(msgs->buf[i], i2c_imx, IMX_I2C_I2DR);
>> +            result = i2c_imx_trx_complete(i2c_imx);
>> +            if (result)
>> +                return result;
>> +            result = i2c_imx_acked(i2c_imx);
>> +            if (result)
>> +                return result;
>> +        }
>>       }
>>       return 0;
>>   }
>>
>>   static int i2c_imx_read(struct imx_i2c_struct *i2c_imx, struct 
>> i2c_msg *msgs)
>>   {
>> -    int i, result;
>> +    int i, result, timeout = IMX_I2C_DMA_TIMEOUT;
>>       unsigned int temp;
>> +    struct imx_i2c_dma *dma = i2c_imx->dma;
>> +    struct device *dev =&i2c_imx->adapter.dev;
>>
>> -    dev_dbg(&i2c_imx->adapter.dev,
>> -        "<%s>  write slave address: addr=0x%x\n",
>> +    dev_dbg(dev, "<%s>  write slave address: addr=0x%x\n",
>>           __func__, (msgs->addr<<  1) | 0x01);
>>
>>       /* write slave address */
>> @@ -476,7 +687,7 @@ static int i2c_imx_read(struct imx_i2c_struct 
>> *i2c_imx, struct i2c_msg *msgs)
>>       if (result)
>>           return result;
>>
>> -    dev_dbg(&i2c_imx->adapter.dev, "<%s>  setup bus\n", __func__);
>> +    dev_dbg(dev, "<%s>  setup bus\n", __func__);
>>
>>       /* setup bus to read data */
>>       temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
>> @@ -486,35 +697,81 @@ static int i2c_imx_read(struct imx_i2c_struct 
>> *i2c_imx, struct i2c_msg *msgs)
>>       imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
>>       imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR); /* dummy read */
>>
>> -    dev_dbg(&i2c_imx->adapter.dev, "<%s>  read data\n", __func__);
>> +    dev_dbg(dev, "<%s>  read data\n", __func__);
>>
>> -    /* read data */
>> -    for (i = 0; i<  msgs->len; i++) {
>> -        result = i2c_imx_trx_complete(i2c_imx);
>> +    if (dma&&  msgs->len>= IMX_I2C_DMA_THRESHOLD) {
>> +        temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
>> +        temp |= I2CR_DMAEN;
>> +        imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
>> +
>> +        reinit_completion(&i2c_imx->dma->cmd_complete);
>> +        dma->chan_using = dma->chan_rx;
>> +        dma->dma_transfer_dir = DMA_DEV_TO_MEM;
>> +        dma->dma_data_dir = DMA_FROM_DEVICE;
>> +        dma->dma_len = msgs->len - 2;
>> +        result = i2c_imx_dma_xfer(i2c_imx, msgs);
>>           if (result)
>>               return result;
>> -        if (i == (msgs->len - 1)) {
>> -            /* It must generate STOP before read I2DR to prevent
>> -               controller from generating another clock cycle */
>> -            dev_dbg(&i2c_imx->adapter.dev,
>> -                "<%s>  clear MSTA\n", __func__);
>> -            temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
>> -            temp&= ~(I2CR_MSTA | I2CR_MTX);
>> -            imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
>> -            i2c_imx_bus_busy(i2c_imx, 0);
>> -            i2c_imx->stopped = 1;
>> -        } else if (i == (msgs->len - 2)) {
>> -            dev_dbg(&i2c_imx->adapter.dev,
>> -                "<%s>  set TXAK\n", __func__);
>> -            temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
>> -            temp |= I2CR_TXAK;
>> -            imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
>> +
>> +        result = wait_for_completion_interruptible_timeout(
>> + &i2c_imx->dma->cmd_complete,
>> +                    msecs_to_jiffies(IMX_I2C_DMA_TIMEOUT));
>> +        if (result<= 0) {
>> +            dmaengine_terminate_all(dma->chan_using);
>> +            if (result)
>> +                return result;
>> +            else
>> +                return -ETIMEDOUT;
>>           }
>> -        msgs->buf[i] = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR);
>> -        dev_dbg(&i2c_imx->adapter.dev,
>> -            "<%s>  read byte: B%d=0x%X\n",
>> -            __func__, i, msgs->buf[i]);
>> +
>> +        /* waiting for Transfer complete. */
>> +        while (timeout--) {
>> +            temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2SR);
>> +            if (temp&  I2SR_ICF)
>> +                break;
>> +            udelay(10);
>> +        }
>> +
>> +        if(!timeout)
>> +            return -ETIMEDOUT;
>> +        temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
>> +        temp&= ~I2CR_DMAEN;
>> +        imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
>> +    } else {
>> +        /* read data */
>> +        for (i = 0; i<  msgs->len - 2; i++) {
>> +            result = i2c_imx_trx_complete(i2c_imx);
>> +            if (result)
>> +                return result;
>> +            msgs->buf[i] = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR);
>> +            dev_dbg(dev, "<%s>  read byte: B%d=0x%X\n",
>> +                __func__, i, msgs->buf[i]);
>> +        }
>> +        result = i2c_imx_trx_complete(i2c_imx);
>>       }
>> +
>> +    /* read n-1 byte data */
>> +    temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
>> +    temp |= I2CR_TXAK;
>> +    imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
>> +
>> +    msgs->buf[msgs->len-2] = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR);
>> +    /* read n byte data */
>> +    result = i2c_imx_trx_complete(i2c_imx);
>> +    if (result)
>> +        return result;
>> +
>> +    /*
>> +     * It must generate STOP before read I2DR to prevent
>> +     * controller from generating another clock cycle
>> +     */
>> +    temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
>> +    temp&= ~(I2CR_MSTA | I2CR_MTX);
>> +    imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
>> +    i2c_imx_bus_busy(i2c_imx, 0);
>> +    i2c_imx->stopped = 1;
>> +    msgs->buf[msgs->len-1] = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR);
>> +
>>       return 0;
>>   }
>>
>> @@ -601,6 +858,7 @@ static int i2c_imx_probe(struct platform_device 
>> *pdev)
>>       void __iomem *base;
>>       int irq, ret;
>>       u32 bitrate;
>> +    u32 phy_addr;
>>
>>       dev_dbg(&pdev->dev, "<%s>\n", __func__);
>>
>> @@ -611,6 +869,7 @@ static int i2c_imx_probe(struct platform_device 
>> *pdev)
>>       }
>>
>>       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +    phy_addr = res->start;
>>       base = devm_ioremap_resource(&pdev->dev, res);
>>       if (IS_ERR(base))
>>           return PTR_ERR(base);
>> @@ -696,6 +955,10 @@ static int i2c_imx_probe(struct platform_device 
>> *pdev)
>>           i2c_imx->adapter.name);
>>       dev_info(&i2c_imx->adapter.dev, "IMX I2C adapter registered\n");
>>
>> +    /* Init DMA config if support*/
>> +    if (i2c_imx_dma_request(i2c_imx, (dma_addr_t)phy_addr))
>> +        dev_warn(&pdev->dev, "can't request DMA\n");
>> +
>>       return 0;   /* Return OK */
>>   }
>>
>> @@ -707,6 +970,9 @@ static int i2c_imx_remove(struct platform_device 
>> *pdev)
>>       dev_dbg(&i2c_imx->adapter.dev, "adapter removed\n");
>>       i2c_del_adapter(&i2c_imx->adapter);
>>
>> +    if (i2c_imx->dma)
>> +        i2c_imx_dma_free(i2c_imx);
>> +
>>       /* setup chip registers to defaults */
>>       imx_i2c_write_reg(0, i2c_imx, IMX_I2C_IADR);
>>       imx_i2c_write_reg(0, i2c_imx, IMX_I2C_IFDR);
>
> -- 
> To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html