Re: [PATCH V2 3/4] i2c: tegra: Add DMA Support

[Thierry Reding <thierry.reding@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2594 bytes --]

On Fri, Jan 25, 2019 at 09:11:54PM +0000, Sowjanya Komatineni wrote:
> 
> 
> > >>> +	if (i2c_dev->has_dma) {
> > >>> +		ret = tegra_i2c_init_dma_param(i2c_dev, true);
> > >>> +		if (ret == -EPROBE_DEFER)
> > >>> +			goto disable_div_clk;
> > >>> +		ret = tegra_i2c_init_dma_param(i2c_dev, false);
> > >>> +		if (ret == -EPROBE_DEFER)
> > >>> +			goto disable_div_clk;
> > >>
> > >> So tegra_i2c_init_dma_param() could fail, printing a error
> > >> message, and probe will succeed? If allocation fails during the
> > >> driver's probe, then just fail the probe. Please give the
> > >> rationale.
> > > 
> > > If APB DMA probe doesn’t happen prior to tegra i2c, DMA is not
> > > available by the time tegra_init_dma_param tries to request slave
> > > channel and in those cases dma_request_slave_channel_reason will
> > > return EPROBE_DEFER for tegra I2C probe to retry
> > > 
> > > In case if DMA is available but DMA buffer allocation fails, then
> > > tegra_i2c_init_dma_param returns ENOMEM and probe also fails
> > > returning same ENOMEM
> >
> > Is that what you're going to change in the next version? Your
> > current variant of the code doesn't fail the probe on ENOMEM and
> > there is duplicated attempt to invoke tegra_i2c_init_dma_param()
> > during the transfer.
> 
> Sorry correction to my previous reply.  If DMA buffer allocation
> fails, tegra_i2c_init_dma_param returns ENOMEM but probe will succeed
> as i2c transaction need to happen during  boot for some platform
> device programming for successful boot and they use PIO mode as xfer
> bytes is less and deferring i2c probe for ENOMEM causes boot to fail
> so during probe EPROBE_DEFER is only taken care.
> 
> Re-attempt of tegra_i2c_init_dma_param in xfer happens only if no
> successful DMA channel allocation happens prior to that ( during probe
> in case of ENOMEM).
> DMA mode is mainly for large transfer, and i2c xfer returning failure
> due to failing DMA buffer allocation causes boot to hang as platform
> device programming need to happen which doesn’t need to use DMA.
> Will fix this and will send updated patch to reattempt DMA request and
> buffer allocation during DMA mode transfer and will return fail for
> DMA mode I2C transfer...

I'm wondering if we shouldn't gracefully to PIO if we fail to allocate
DMA buffers. Even if it is unlikely that large transfers (> than the
threshold that activates the DMA paths) will be needed by critical I2C
clients, there's really no reason why we can't do PIO if DMA doesn't
work for some reason.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]