All of lore.kernel.org
 help / color / mirror / Atom feed
From: santosh.shilimkar@ti.com (Santosh Shilimkar)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] ARM: mm: dma: Update coherent streaming apis with missing memory barrier
Date: Tue, 22 Apr 2014 09:49:06 -0400	[thread overview]
Message-ID: <53567352.1080306@ti.com> (raw)
In-Reply-To: <20140422102834.GE7484@arm.com>

Hi Will,

On Tuesday 22 April 2014 06:28 AM, Will Deacon wrote:
> Hi Santosh,
> 
> On Mon, Apr 21, 2014 at 07:03:10PM +0100, Santosh Shilimkar wrote:
>> ARM coherent CPU dma map APIS are assumed to be nops on cache coherent
>> machines. While this is true, one still needs to ensure that no
>> outstanding writes are pending in CPU write buffers. To take care
>> of that, we at least need a memory barrier to commit those changes
>> to main memory.
>>
>> Patch is trying to fix those cases. Without such a patch, you will
>> end up patching device drivers to avoid the synchronisation issues.
> 
> Don't you only need these barriers if you're passing ownership of a CPU
> buffer to a device? In that case, I would expect a subsequent writel to tell
> the device about the new buffer, which includes the required __iowmb().
> That's the reason for the relaxed accessors: to avoid this barrier when it's
> not needed. Perhaps you're using the relaxed accessors where you actually
> need the stronger ordering guarantees?
> 
I kind of guessed some one will bring up above point. Infact this is how
mostly people have been living with the issue on coherent machines. On
Keystone too, we did explicit barriers in respective drivers.

I have added these barriers only on CPU to device streaming APIs because on
other direction, the memory is already upto date from CPU's perspective.

But if you look at the actual problem, its really responsibility of
DMA streaming APIs which we are trying to push on to drivers. A device
driver should be independent of whether it is running on a coherent or
a non-coherent CPU.

Lets take a example....
MMC controller driver running on a non-coherent and coherent machine.
Driver has below code sequence which is generic.
1. Prepare SG list
2. Perform CMO using DMA streaming API
3. Start DMA transfer...

Step 3 expects that step 2 has done its job and buffer is
completely in the main memory. And thats what also happens
on non-coherent machines.

Now, on coherent machines, as you mentioned, we are saying drivers
should add a barrier because Step2 is just NOP which is not correct.
The Step3 itself which is just suppose to start DMA doesn't need
any barrier as such. This is the whole rationale behind the patch.

Regards,
Santosh

 

  reply	other threads:[~2014-04-22 13:49 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-04-21 18:03 [PATCH] ARM: mm: dma: Update coherent streaming apis with missing memory barrier Santosh Shilimkar
2014-04-22 10:28 ` Will Deacon
2014-04-22 13:49   ` Santosh Shilimkar [this message]
2014-04-22 14:08     ` Arnd Bergmann
2014-04-22 14:36       ` Santosh Shilimkar
2014-04-22 19:53         ` Arnd Bergmann
2014-04-22 19:58           ` Santosh Shilimkar
2014-04-22 20:23             ` Arnd Bergmann
2014-04-22 20:30               ` Santosh Shilimkar
2014-04-23  9:02                 ` Will Deacon
2014-04-23 16:02                   ` Catalin Marinas
2014-04-23 17:17                     ` Will Deacon
2014-04-23 18:37                       ` Russell King - ARM Linux
2014-04-23 18:58                         ` Arnd Bergmann
2014-04-23 19:04                           ` Russell King - ARM Linux
2014-04-24 10:47                             ` Catalin Marinas
2014-04-24 11:15                               ` Russell King - ARM Linux
2014-04-24 11:21                                 ` Will Deacon
2014-04-24 13:38                                   ` Santosh Shilimkar
2014-04-24 14:09                                     ` Will Deacon
2014-04-24 14:44                                       ` Santosh Shilimkar
2014-04-24 19:12                                         ` Russell King - ARM Linux
2014-04-23 19:34                           ` Jason Gunthorpe
2014-04-24 10:58                           ` Will Deacon
2014-04-24 12:12                             ` Arnd Bergmann
2014-04-24 12:37                               ` Will Deacon
2014-04-24  9:54                         ` Catalin Marinas
2014-04-24 11:13                           ` Russell King - ARM Linux
2014-04-24  9:09                       ` Catalin Marinas
2014-04-24  9:16                         ` Russell King - ARM Linux
2014-04-24 10:13                           ` Catalin Marinas
2014-05-02 21:33                       ` Joel Fernandes
2014-05-06 10:01                         ` Will Deacon
2014-04-22 15:07     ` Catalin Marinas
2014-04-22 15:18       ` Santosh Shilimkar
2014-04-22 15:30         ` Catalin Marinas

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=53567352.1080306@ti.com \
    --to=santosh.shilimkar@ti.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.