> >>> time_left = wait_for_completion_timeout(&i2c_dev->msg_complete,
> >>> TEGRA_I2C_TIMEOUT);
> >>> tegra_i2c_mask_irq(i2c_dev, int_mask);
> >>>
> >>> if (time_left == 0) {
> >>> dev_err(i2c_dev->dev, "i2c transfer timed out\n");
> >>> + if (dma) {
> >>> + dmaengine_terminate_all(chan);
> >>> + complete(&i2c_dev->dma_complete);
> >>> + }
> >>
> >> DMA transfer has been completed at this point, hence this hunk isn't
> >> needed. Please remove it.
> >
> > DMA complete alone doesn’t guarantee the transfer. Packets/All packets xfer
> > interrupt from I2C confirms complete transaction along with dma complete
> > check.
> > So still need to check for msg_complete timeout.
>
> You're waiting for DMA completion and then for the I2C message completion.
>
> Hence your code is structured like this:
>
> 1. Issue DMA transfer
> 2. Wait for DMA completion
> 3. Wait for message completion
>
> Why do you need to abort DMA in 3 if it was already completed in 2?
Ok, thought you are referring to msg complete timeout check in dma mode. Yes no
need for terminating DMA when msg timeout. Will fix it.
> >>> @@ -740,6 +925,32 @@ static int tegra_i2c_xfer_msg(struct tegra_i2c_dev
> >>> *i2c_dev,
> >>> u32 int_mask;
> >>> unsigned long time_left;
> >>> unsigned long flags;
> >>> + size_t xfer_size;
> >>> + u32 *buffer = 0;
> >>> + int ret = 0;
> >>> + bool dma = false;
> >>> +
> >>> + if (msg->flags & I2C_M_RD)
> >>> + xfer_size = msg->len;
> >>> + else
> >>> + xfer_size = msg->len + I2C_PACKET_HEADER_SIZE;
> >>> +
> >>> + xfer_size = ALIGN(xfer_size, BYTES_PER_FIFO_WORD);
> >>> + dma = (xfer_size > I2C_PIO_MODE_MAX_LEN);
> >>> + if (dma) {
> >>> + if ((msg->flags & I2C_M_RD) && !i2c_dev->rx_dma_chan)
> >>> + ret = tegra_i2c_init_dma_param(i2c_dev, true);
> >>> + else if (!i2c_dev->tx_dma_chan)
> >>> + ret = tegra_i2c_init_dma_param(i2c_dev, false);
> >>
> >> In the comment to V3 I mentioned that it's not a good idea to request
> >> channels dynamically because suspend-resume order is based on devices
> >> registration order, in this case APB DMA must be probed before I2C. Please
> >> move channels allocation into the probe.
> >>
> >> This also raises the question about the need to register I2C driver from
> >> the subsys-init level because APB driver is getting registered from the
> >> module-init level and hence I2C probing will be deferred until APB DMA
> >> driver is registered. It looks to me that the subsys-init is a relict of
> >> the past and it should be fine to move I2C driver registration into the
> >> module-init level, of course it's not strictly necessary and could be done
> >> later on if desired.
> >>
> >>> + if (ret < 0) {
> >>> + dev_dbg(i2c_dev->dev, "Switching to PIO mode\n");
> >>> + dma = false;
> >>> + ret = 0;
> >>> + }
> >>> + }
> >>> +
> >>> + i2c_dev->is_curr_dma_xfer = dma;
> >>
> >>
> > Since your previous feedback suggest "let's postpone channels requesting
> > and dma_buf allocation until they are really needed", I thought it make
> > sense to not request channels and allocate till DMA is needed.
> > So moved from probe to xfer_msg function. By the time it gets to xfer msg
> > function, devices registration should be done already along with apb dma
> > probe.
> >
> >
>
> Yes, I made that comment, but then corrected myself. Seems you missed the
> correction: https://lkml.org/lkml/2019/1/26/217
>
> If you're having troubles with the corporate email, maybe you could try to
> switch to something else like gmail.
>
> I've tried to apply this series locally, but again it fails to apply. What's
> the kernel base you're using? You should make your patches on top linux-next
> (preferably) or mainline.
Yeah somehow missed that. Will move it back to probe..
I am using 5.0-rc1
