Re: [PATCH v3 1/4] i2c: add helpers to ease DMA handling

From: Wolfram Sang
Date: Wed Aug 16 2017 - 16:58:33 EST


Hi Jonathan,

> I like the basic idea of this patch set a lot btw!

Thanks!

> Jonathan

Could you delete irrelevant parts of the message, please? I nearly
missed your other comment which would have been a great loss!

> I'm trying to get my head around whether this is a sufficient set of conditions
> for a dma safe buffer and I'm not sure it is.
>
> We go to a lot of effort with SPI buffers to ensure they are in their own cache
> lines. Do we not need to do the same here? The issue would be the
> classic case of embedding a buffer inside a larger structure which is on
> the stack. Need the magic of __cacheline_aligned to force it into it's
> own line for devices with dma-incoherent caches.
> DMA-API-HOWTO.txt (not read that for a while at it is a rather good
> at describing this stuff these days!)
>
> So that one is easy enough to check by checking if it is cache line aligned,
> but what do you do to know there is nothing much after it?

Thank you, really!

This patch series addressed all cases I have created, but I also
wondered if there are cases left which I missed. Now, also because you
mentioned cacheline alignments, the faint idea of "maybe it could be
fragile" became a strong "it is too fragile"! lib/dma-debug.c is >40KB
for a reason. I just rewrote this series...

> Not sure there is a way around this short of auditing i2c drivers to
> change any that do this.

... to do something like this, more precisely: an opt-in approach. I
introduce a new flag for i2c_msg, namely I2C_M_DMA_SAFE which can opt-in
DMA if we know the i2c_msg buffer is DMA safe. I finished a
proof-of-concept this evening and pushed it here:

git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git renesas/topic/i2c-core-dma-rfc-v4

I will test it on HW tomorrow and send it out, then. There are still
some bits missing, but for getting opinions, it should be good enough.

Thanks and kind regards,

Wolfram

Attachment: signature.asc
Description: PGP signature