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 for Android: free password hash cracker in your pocket

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

Message-ID: <4f791065-e0dd-6ed5-f152-86d7be683490@collabora.com>
Date:   Tue, 23 Aug 2022 16:32:11 +0300
From:   Dmitry Osipenko <dmitry.osipenko@...labora.com>
To:     Akhil R <akhilrajeev@...dia.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.

>>>> Another option I thought about was to request and free DMA channel for
>> each
>>>> transfer, which many serial drivers already do. But I am a bit anxious if that
>> will
>>>> increase the latency of transfer.
>>>
>>> Perhaps all you need to do is to add MODULE_SOFTDEP to Tegra I2C driver
>>> like we did it for the EMC driver [1].
>>>
>>> [1]
>>> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-
>> next.git/commit/?id=14b43c20c283de36131da0cb44f3170b9ffa7630
>>>
>>
>> Although, probably MODULE_SOFTDEP won't work for a built-in driver. In
>> that case, change Tegra I2C kconfig to depend on the DMA driver.
> 
> 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.

-- 
Best regards,
Dmitry