linux-kernel - Re: [PATCH v4 3/3] i2c: tegra: Remove dma_sync

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: <t6lt7sazxaqf2qis55kjg65epcakoyj2cow5bbcjzyxj7ztywm@4r2oukvdhdu6>
Date: Tue, 10 Jun 2025 10:03:49 +0200
From: Thierry Reding <thierry.reding@...il.com>
To: Akhil R <akhilrajeev@...dia.com>
Cc: andi.shyti@...nel.org, robh@...nel.org, krzk+dt@...nel.org, 
	conor+dt@...nel.org, jonathanh@...dia.com, ldewangan@...dia.com, digetx@...il.com, 
	p.zabel@...gutronix.de, linux-i2c@...r.kernel.org, devicetree@...r.kernel.org, 
	linux-tegra@...r.kernel.org, linux-kernel@...r.kernel.org, 
	Robin Murphy <robin.murphy@....com>
Subject: Re: [PATCH v4 3/3] i2c: tegra: Remove dma_sync_*() calls

On Tue, Jun 03, 2025 at 09:00:22PM +0530, Akhil R wrote:
> Calling dma_sync_*() on a buffer from dma_alloc_coherent() is pointless.
> The driver should not be doing its own bounce-buffering if the buffer is
> allocated through dma_alloc_coherent()
> 
> Suggested-by: Robin Murphy <robin.murphy@....com>
> Signed-off-by: Akhil R <akhilrajeev@...dia.com>
> ---
> v3->v4: No change
> v2->v3: No change
> v1->v2: No change
> 
>  drivers/i2c/busses/i2c-tegra.c | 20 +-------------------
>  1 file changed, 1 insertion(+), 19 deletions(-)

I've had a patch like this in my local tree for a while and never got
around to send it out. It turns out that this is actually not just
unnecessary but can also cause issues. We were seeing really strange
crashes in the QSPI driver that seem related to this. I don't know
exactly what causes it, but removing the dma_sync_*() calls (for memory
that is also dma_alloc_coherent()-allocated) fixes things.

For QSPI I'm thinking it would probably be better to move to streaming
DMA mappings because the buffers in DMA mode are sufficiently large (at
least 2 cache lines) that we might see some benefit from the caching. I
don't know if this would apply for I2C as well? I vaguely recall that
certain transfers can be quite large and I wonder if cached mappings
would be advantageous here as well. Do you have good data on what the
usage patterns are?

That said, maybe the extra amount of work isn't worth it because both
I2C and QSPI are fairly slow busses, so the impact of caches is probably
minimal in comparison.

Anyway:

Reviewed-by: Thierry Reding <treding@...dia.com>

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)