Re: [PATCH V5 3/5] i2c: tegra: Add DMA Support

[Dmitry Osipenko]

Ð Wed, 30 Jan 2019 04:22:10 +0000
Sowjanya Komatineni <skomatineni@xxxxxxxxxx> ÐÐÑÐÑ:

> [Correction]
> 
> > > Could you please tell whether you missed my comments to V3 [0] or
> > > chose to ignore them? If the former, then I'd want to get answers
> > > to those questions and comments. I'll stop here for now.
> > > 
> > > [0] https://patchwork.ozlabs.org/patch/1031379/  
> >
> > Somehow missed those from multiple comments. Will go thru and
> > respond back.  
> 
> V6 includes feedback changes. Want to clarify on few feedback points
> 
> - ALIGN is used for 4 byte boundary to use with DMA but extra bytes
> doesnât get transferred over I2C as I2C controller transfer bytes
> based on size specified in the packet header. DMA length and memory
> address need to be 4 byte boundary.

Okay, I see now. Thanks.

> - RX channel releasing when TX init fails?
>   For I2C both TX and RX doesnât happen in same transaction and no
> dependency. So if RX channel request & buffer allocation succeeds but
> TX channel request fails, then RX DMA can still be used for Msg reads
> and transmits can happen on PIO mode

That's not what my point is about. In a case of TX initing failure, the
RX channel and DMA buffer are not getting released in the probe
function. Please be more careful about managing allocated resources.

> - dma_burst < 8 negatively affects transfer efficiency? Performance
> stats for DMA Vs PIO mode. Tested with 256 bytes of transfer and DMA
> Vs PIO mode transfer rate is almost same. But the main reason for
> adding DMA mode is to address couple of cases mentioned earlier and
> not mainly from the transfer performance perspective. 

Could you please add a clarifying comment to the code, saying that the
whole purpose of the DMA transfer is solely to avoid delaying of the
transactions?

Thanks for the replies! I'll take a look at V6 later today.