01.02.2019 23:21, Sowjanya Komatineni пишет:
>
>> rx_chan = dma_request_slave_channel_reason(i2c_dev->dev, "rx");
>> if (IS_ERR(rx_chan))
>> return PTR_ERR(rx_chan);
>>
>>
>>> +
>>> + dma_chan = dma_request_slave_channel_reason(i2c_dev->dev, "tx");
>>> + if (IS_ERR(dma_chan)) {
>>> + err = PTR_ERR(dma_chan);
>>> + goto error;
>>
>> It's a good practice to release resources in opposite order to the allocation. Hence better to write this as:
>>
>> goto err_release_rx;
>>
>>> + }
>>> +
>>> + i2c_dev->tx_dma_chan = dma_chan;
>>> +
>>> + dma_buf = dma_alloc_coherent(i2c_dev->dev,
>>> + i2c_dev->dma_buf_size, &dma_phys,
>>> + GFP_KERNEL | __GFP_NOWARN);
>>> +
>>> + if (!dma_buf) {
>>> + dev_err(i2c_dev->dev, "failed to allocate the DMA buffer\n");
>>> + err = -ENOMEM;
>>> + goto error;
>>
>> goto err_release_tx;
>>
>>> + }
>>> +
>>> + i2c_dev->dma_buf = dma_buf;
>>> + i2c_dev->dma_phys = dma_phys;
i2c_dev->rx_dma_chan = rx_chan;
>> i2c_dev->tx_dma_chan = tx_chan;
>>
>>> + return 0;
>>> +
>>> +error:
>>> + if (i2c_dev->tx_dma_chan)
>>> + dma_release_channel(i2c_dev->tx_dma_chan);
>>> +
>>> + if (i2c_dev->rx_dma_chan)
>>> + dma_release_channel(i2c_dev->rx_dma_chan);
>>
>> error_release_tx:
>> dma_release_channel(tx_chan);
>> error_release_rx:
>> dma_release_channel(rx_chan);
>>
>>> +
>>> + return err;
>
> I am releasing resources in reverse order to allocation.
> Trying for rx allocation followed by tx allocation
> During release releasing tx and then rx.
> In case if tx allocation fails, doesn’t go thru release. If rx or buf allocation fails, releases tx first and then rx
>
Okay. Anyway it's a good-n-common practice to write it in the way I'm suggesting.
And please set rx_chan and tx_chan after dma_buf allocation as I'm suggesting because you current variant will crash kernel since if dma_buf allocation fails, both rx and tx channels will be released and you're not setting them to NULL in that case.
