Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: arnd@arndb.de (Arnd Bergmann)
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 22:23:45 +0200	[thread overview]
Message-ID: <5895821.PvurJ8TWz2@wuerfel> (raw)
In-Reply-To: <5356C9D1.2060001@ti.com>

On Tuesday 22 April 2014 15:58:09 Santosh Shilimkar wrote:
> On Tuesday 22 April 2014 03:53 PM, Arnd Bergmann wrote:
> > On Tuesday 22 April 2014, Santosh Shilimkar wrote:
> >> On Tuesday 22 April 2014 10:08 AM, Arnd Bergmann wrote:
> >>> It's not the nicest API ever, but that's what it is and has been, mostly
> >>> for compatibility with x86, where the 'mov' instruction performing the
> >>> store to MMIO registers implies that all writes to DMA memory are
> >>> visible to the device.
> >>>
> >> This is not about writel() and  writel_relaxed(). The driver don't
> >> need that barrier. For example if the actual start of the DMA
> >> happens bit later, that doesn't matter for the driver.
> >>
> >> DMA APIs already do barriers today for non-coherent case. We
> >> are not talking anything new here. Sorry but I don't see the
> >> connection here.
> > 
> > I don't think they do, nor should they. Can you tell me where
> > you see a barrier in dma_sync_single_for_cpu() or 
> > arm_dma_sync_single_for_device()? For all I can tell, they
> > only deal with L1 and L2 cache maintainance in arm_dma_ops.
> > 
> The cache APIs used by dma_ops do have the necessary barriers
> at end of the of the cache operations. Thats what I meant. So for
> end user(Device driver), its transparent.

Ok, I see it now for the noncoherent operations, and I see
the same thing on PowerPC and MIPS, which also have both coherent
and noncoherent versions of their dma_map_ops.

However, I also see that neither of those does a wmb() for the
coherent version. I don't see why ARM should be different from
the others here, so if there is a reason to do a barrier there,
we should change all architectures. I still don't see a reason
why the barrier is needed though.

Can you be more specific in your driver example why you think
the barrier in the writel() is not sufficient?

	Arnd

  reply	other threads:[~2014-04-22 20:23 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
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 [this message]
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=5895821.PvurJ8TWz2@wuerfel \
    --to=arnd@arndb.de \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox