> > > >> 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. OK, my wrong assumption. Thought dma_release_channel will NULL chan after its freed Will update and send V10