Hi Wolfram, Thanks for your feedback. On 2016-05-12 23:31:27 +0200, Wolfram Sang wrote: > Hi Niklas, > > thanks for the submission. I was finally able to test this change. > > On Wed, May 04, 2016 at 02:38:23PM +0200, Niklas Söderlund wrote: > > Make it possible to transfer i2c message buffers via DMA. > > Start/Stop/Sending_Slave_Address and some data is still handled using > > the old state machine, it is sending the bulk of the data that is done > > via DMA. > > > > The first byte of a transmission and the last two bytes of reception are > > sent/received using PIO. This is needed for the HW to have access to the > > first byte before DMA transmit and to be able to set the STOP condition > > for DMA reception. > > > > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@xxxxxxxxxxxx> > > Tested-by: Wolfram Sang <wsa+renesas@xxxxxxxxxxxxxxxxxxxx> > > I did regression tests on my Salvator-X trying to trigger previously > known issues. Nothing bad happened. This could be expected since START > and STOP is done in PIO mode, but one never knows :) Also did verify > that DMA is triggered for bigger transfers. > > Did you have time to re-measure the threshold? Also, did you try booting I did not re-measure the threshold, I'm not sure how to do that in a good correct way. I reasoned that I modeled my implementation on the sh_mobile-driver and there are roughly the same amount code in the DMA code path so I used the same threshold. > without DMA and on Gen2? We had a bit of hazzle with !DMA with the > sh_mobile-driver. Boot test and basic i2cdetect will suffice. I tested in on Koelsch, but I don't have the schematics for the board so I could not hookup to an external i2c bus and look at it. But i2cdetect is working. > > Patch looks good, only minor nits: > > > + /* Do not use DMA for messages shorter then 8 bytes */ > > + if (msg->len < 8) > > + return; > > + > > + if (IS_ERR(chan)) > > + return; > > Make this one if (a || b)? Fill fix. > > > @@ -516,6 +738,7 @@ static int rcar_i2c_master_xfer(struct i2c_adapter *adap, > > out: > > pm_runtime_put(dev); > > > > + > > Unrelated change ;) Fill fix. > > Thanks, > > Wolfram > -- Regards, Niklas Söderlund
