linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: arnd@arndb.de (Arnd Bergmann)
To: linux-arm-kernel@lists.infradead.org
Subject: DMABOUNCE in pci-rcar
Date: Mon, 24 Feb 2014 12:00:21 +0100	[thread overview]
Message-ID: <201402241200.21944.arnd@arndb.de> (raw)

Hi Magnus,

I noticed during randconfig testing that you enabled DMABOUNCE for the
pci-rcar-gen2 driver as posted in this patch https://lkml.org/lkml/2014/2/5/30

I didn't see the original post unfortunately, but I fear we have to
revert it and come up with a better solution, as your approach seems
wrong on a number of levels:

* We really don't want any new users of DMABOUNCE on ARM but instead leave
  it to the PXA/IXP/SA1100 based platforms using it today.

* If your SoCs have an IOMMU, you should really use it, as that would
  give you much better performance than bounce buffers anyway

* If the hardware is so screwed up that you have to use bounce buffers,
  use the SWIOTLB code that is reasonably modern.

* The window base and size in the driver should not be hardcoded, as
  this is likely not a device specific property but rather an artifact of
  how it's connected. Use the standard "dma-ranges" property.

* You should not need your own dma_map_ops in the driver. What you
  do there isn't really specific to your host but should live in some
  place where it can be shared with other drivers.

* The implementation of the dma_map_ops is wrong: at the very least,
  you cannot use the dma_mask of the pci host when translating
  calls from the device, since each pci device can have a different
  dma mask that is set by the driver if it needs larger or smaller
  than 32-bit DMA space.

* The block bounce code can't be enabled if CONFIG_BLOCK is disabled,
  this is how I noticed your changes.

* The block layer assumes that it will bounce any highmem pages,
  but that isn't necessarily what you want here: if you have 2GB
  of lowmem space (CONFIG_VMSPLIT_2G) or more, you will also have
  lowmem pages that need bouncing.

* (completely unrelated, I noticed you set up a bogus I/O space
  window. Don't do that, you will get get a panic if a device happens
  to use it. Not providing an resource should work fine though)

On the bright side, your other changes in the same series all look
good. Thanks especially for sorting out the probe() function, I was
already wondering if we could change that the way you did.

	Arnd

             reply	other threads:[~2014-02-24 11:00 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-02-24 11:00 Arnd Bergmann [this message]
2014-02-24 23:49 ` DMABOUNCE in pci-rcar Magnus Damm
2014-02-25  0:17   ` Russell King - ARM Linux
2014-02-25  2:00     ` Magnus Damm
2014-02-25 15:44       ` Arnd Bergmann
2014-02-25 12:15   ` Arnd Bergmann
2014-02-26 19:48 ` Bjorn Helgaas
2014-02-26 19:57   ` Arnd Bergmann
2014-02-26 21:02     ` Bjorn Helgaas
2014-02-26 21:52       ` Arnd Bergmann
2014-03-20 15:04     ` Ben Dooks
2014-03-20 15:26       ` Russell King - ARM Linux
2014-03-20 15:36         ` Ben Dooks
2014-03-20 16:09       ` Arnd Bergmann
2014-03-20 16:12         ` Ben Dooks
2014-03-20 16:41           ` Arnd Bergmann
2014-03-20 17:25             ` Ben Dooks
2014-03-20 17:31               ` Jason Gunthorpe
2014-03-20 17:32                 ` Ben Dooks
2014-03-20 18:29                   ` Arnd Bergmann
2014-03-20 18:39                     ` Russell King - ARM Linux
2014-03-20 19:04                       ` Geert Uytterhoeven
2014-03-20 22:11                       ` Arnd Bergmann
2014-03-20 22:50                         ` Russell King - ARM Linux

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=201402241200.21944.arnd@arndb.de \
    --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;
as well as URLs for NNTP newsgroup(s).