linux-kernel - RE: [PATCH V2 3/4] i2c: tegra: Add DMA Support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <BYAPR12MB33989F7CC9FCB45AE0BFC561C29B0@BYAPR12MB3398.namprd12.prod.outlook.com>
Date:   Fri, 25 Jan 2019 23:29:43 +0000
From:   Sowjanya Komatineni <skomatineni@...dia.com>
To:     Thierry Reding <thierry.reding@...il.com>
CC:     Dmitry Osipenko <digetx@...il.com>,
        Jonathan Hunter <jonathanh@...dia.com>,
        Mantravadi Karthik <mkarthik@...dia.com>,
        "Shardar Mohammed" <smohammed@...dia.com>,
        Timo Alho <talho@...dia.com>,
        "linux-tegra@...r.kernel.org" <linux-tegra@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-i2c@...r.kernel.org" <linux-i2c@...r.kernel.org>
Subject: RE: [PATCH V2 3/4] i2c: tegra: Add DMA Support



> > > >>> +	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

Yes we can fall back to PIO mode incase of DMA buffer allocation failure. Will add this in next revised patch

Sowjanya