linux-kernel - RE: [PATCH RESEND 1/2] i2c: tegra: Add GPCDMA 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: <SJ1PR12MB6339E13E9AFDCA1EAB9A72E9C0729@SJ1PR12MB6339.namprd12.prod.outlook.com>
Date:   Thu, 25 Aug 2022 16:41:08 +0000
From:   Akhil R <akhilrajeev@...dia.com>
To:     Dmitry Osipenko <dmitry.osipenko@...labora.com>,
        Dmitry Osipenko <digetx@...il.com>,
        Jonathan Hunter <jonathanh@...dia.com>,
        Laxman Dewangan <ldewangan@...dia.com>,
        "linux-i2c@...r.kernel.org" <linux-i2c@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-tegra@...r.kernel.org" <linux-tegra@...r.kernel.org>,
        "thierry.reding@...il.com" <thierry.reding@...il.com>,
        "wsa@...nel.org" <wsa@...nel.org>
CC:     "robh+dt@...nel.org" <robh+dt@...nel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "christian.koenig@....com" <christian.koenig@....com>,
        "sumit.semwal@...aro.org" <sumit.semwal@...aro.org>,
        "Kartik ." <kkartik@...dia.com>
Subject: RE: [PATCH RESEND 1/2] i2c: tegra: Add GPCDMA support

> On 8/23/22 15:55, Akhil R wrote:
> ...
> >>>> What I am trying for is to have a mechanism that doesn't halt the i2c
> >> transfers
> >>>> till DMA is available. Also, I do not want to drop DMA because it was
> >> unavailable
> >>>> during probe().
> >>>
> >>> Why is it unavailable? Sounds like you're not packaging kernel properly.
> > Unavailable until initramfs or systemd is started since the module has to be
> > loaded from either of it.
> >
> >>>
> >>>> This situation is sure to hit if we have I2C driver as built in and DMA driver
> as a
> >>>> module. In such cases, I2C will never be able to use the DMA.
> >>>
> >>> For Tegra I2C built-in + DMA driver module you should add the dma.ko to
> >>> initramfs and then it will work. This is a common practice for many
> >>> kernel drivers.
> >>>
> >>> It's also similar to a problem with firmware files that must be
> >>> available to drivers during boot,
> >
> > Isn't the initramfs loaded after the driver initcalls? Wasn't very much clear for
> me
> > from the code and docs. We did try adding the module in initramfs initially, but
> > couldn't find much of a difference from when it is loaded by systemd in rootfs.
> > Will explore more on this if this really helps.
> 
> It doesn't matter when initramfs is loaded. Tegra I2C should be
> re-probed once DMA driver is ready, that's the point of deferred
> probing. I'd assume that your DMA driver module isn't loading.

DMA module does get loaded. I2C probe eventually succeeds and can
use DMA then.

But my idea here is to avoid blocking of I2C transfers until DMA is available.
I am looking for a way that I2C can work in PIO mode until DMA comes up and
then switch to DMA once it is available.
I am hoping that it would help other I2C dependent drivers as well during boot.


> > Since I2C can work without DMA, wouldn't it limit the flexibility of I2C driver.
> 
> There are kernel configurations that are not worthwhile to support
> because nobody use them in practice. I think this is exactly the case
> here. The TEGRA20_APB_DMA driver dependency created troubles for a long
> time.
> 
> If DMA driver is enabled in kernel config, then you should provide the
> driver module to kernel and it will work.
> 
> If DMA driver is disabled in kernel config, then Tegra I2C driver should
> take that into account. I'm now recalling that this was the reason of
> "!IS_ENABLED(CONFIG_TEGRA20_APB_DMA)" in the code.
> 
> Since all h/w gens now provide DMA support for Tegra I2C, then should be
> better and easier to make DMA a dependency for Tegra I2C and don't
> maintain kernel build configurations that nobody cares about.

I am somehow still not very much inclined to add GPCDMA as a dependency
for I2C. Even if we discard the fact that the driver extends to support chips
that does not have GPCDMA, I would look at DMA as an additional feature only
and not a dependency.

Regards,
Akhil