Hi,

mostly looking good...

> +#define IMX_I2C_DMA_THRESHOLD        16

Have you guessed or measured this value? A comment about this value
would be nice.

>  
> +struct imx_i2c_dma {
> +     struct dma_chan         *chan_tx;
> +     struct dma_chan         *chan_rx;
> +     struct dma_chan         *chan_using;
> +     struct completion       cmd_complete;
> +     dma_addr_t              dma_buf;
> +     unsigned int            dma_len;
> +     unsigned int            dma_transfer_dir;
> +     unsigned int            dma_data_dir;

Please use proper types as there are things like 'enum
dma_data_direction' and the likes...

> +     dmaengine_submit(txdesc);

This can fail, too. Use dma_submit_error() to check.

> +     result = wait_for_completion_interruptible_timeout(
> +                             &i2c_imx->dma->cmd_complete,
> +                             msecs_to_jiffies(IMX_I2C_DMA_TIMEOUT));

Have you tested signals extensively? I can't really recommend the
_intrruptible_ here. I don't see any cleaning up to get the bus to a
consistent state again. Most people drop the interruptible sooner or
later.

> +     /* Init DMA config if support*/
> +     i2c_imx_dma_request(i2c_imx, phy_addr);

Make this function void? DMA is optional anyhow.

Thanks,

   Wolfram

Attachment: signature.asc
Description: Digital signature

Reply via email to