diff mbox

[RESEND,v2,1/2] ARM: ixp4xx: Make dma_set_coherent_mask common, correct implementation

Message ID 20140317144235.546d2e19@marrow.netinsight.se (mailing list archive)
State New, archived
Headers show

Commit Message

Simon Kagstrom March 17, 2014, 1:42 p.m. UTC
Non-PCI devices can use the entire 32-bit range, PCI dittos are
limited to the first 64MiB.

Also actually setup coherent_dma_mask.

The patch has been verified on a board with 128MiB memory, one
ipx4xx_eth device and a e100 PCI device.

Signed-off-by: Simon Kagstrom <simon.kagstrom@netinsight.net>
---
ChangeLog:

* v2: Move EXPORT_SYMBOL together with dma_set_coherent_mask (Anrd Bergmann)

 arch/arm/mach-ixp4xx/common-pci.c |    9 ---------
 arch/arm/mach-ixp4xx/common.c     |   12 ++++++++++++
 2 files changed, 12 insertions(+), 9 deletions(-)

Comments

Arnd Bergmann March 18, 2014, 2:58 p.m. UTC | #1
On Monday 17 March 2014, Simon Kågström wrote:
> Non-PCI devices can use the entire 32-bit range, PCI dittos are
> limited to the first 64MiB.
> 
> Also actually setup coherent_dma_mask.
> 
> The patch has been verified on a board with 128MiB memory, one
> ipx4xx_eth device and a e100 PCI device.
> 
> Signed-off-by: Simon Kagstrom <simon.kagstrom@netinsight.net>
> ---

I've applied this patch to the next/fixes-non-critical branch of arm-soc.
Please let me know if you need to have it backported to older kernels
as well.

Patch 2 should go through the netdev tree.

	Arnd
Simon Kagstrom March 20, 2014, 8:03 a.m. UTC | #2
On Tue, 18 Mar 2014 15:58:36 +0100
Arnd Bergmann <arnd@arndb.de> wrote:

> On Monday 17 March 2014, Simon Kågström wrote:
> > Non-PCI devices can use the entire 32-bit range, PCI dittos are
> > limited to the first 64MiB.
> > 
> > Also actually setup coherent_dma_mask.
> > 
> > The patch has been verified on a board with 128MiB memory, one
> > ipx4xx_eth device and a e100 PCI device.
> 
> I've applied this patch to the next/fixes-non-critical branch of arm-soc.
> Please let me know if you need to have it backported to older kernels
> as well.

Thanks. However, Krzysztof thinks that this implementation is
incorrect. It certainly fixes my ixp4xx issues, but might not be good
for everyone.

// Simon
Krzysztof Hałasa March 20, 2014, 9:34 a.m. UTC | #3
Simon Kågström <simon.kagstrom@netinsight.net> writes:

> Thanks. However, Krzysztof thinks that this implementation is
> incorrect. It certainly fixes my ixp4xx issues, but might not be good
> for everyone.

It because you don't have PCI devices setting coherent mask to 32-bits
(and that's what normal PCI devices do).

BTW PCI (and other) devices requesting 32-bit coherent allocs don't care
if they're given 26-bit memory. 26 bits fit perfectly in 32 bits. It's
the streaming mapping which is (may be) constrained by hardware.
Arnd Bergmann March 20, 2014, 11:02 a.m. UTC | #4
On Thursday 20 March 2014, Krzysztof Ha?asa wrote:
> Simon Kågström <simon.kagstrom@netinsight.net> writes:
> 
> > Thanks. However, Krzysztof thinks that this implementation is
> > incorrect. It certainly fixes my ixp4xx issues, but might not be good
> > for everyone.
> 
> It because you don't have PCI devices setting coherent mask to 32-bits
> (and that's what normal PCI devices do).
> 
> BTW PCI (and other) devices requesting 32-bit coherent allocs don't care
> if they're given 26-bit memory. 26 bits fit perfectly in 32 bits. It's
> the streaming mapping which is (may be) constrained by hardware.

Should I revert the patch then until we have a better fix?

	Arnd
Simon Kagstrom March 20, 2014, 11:05 a.m. UTC | #5
On Thu, 20 Mar 2014 12:02:36 +0100
Arnd Bergmann <arnd@arndb.de> wrote:

> On Thursday 20 March 2014, Krzysztof Ha?asa wrote:
> > Simon Kågström <simon.kagstrom@netinsight.net> writes:
> > 
> > > Thanks. However, Krzysztof thinks that this implementation is
> > > incorrect. It certainly fixes my ixp4xx issues, but might not be good
> > > for everyone.
> > 
> > It because you don't have PCI devices setting coherent mask to 32-bits
> > (and that's what normal PCI devices do).
> > 
> > BTW PCI (and other) devices requesting 32-bit coherent allocs don't care
> > if they're given 26-bit memory. 26 bits fit perfectly in 32 bits. It's
> > the streaming mapping which is (may be) constrained by hardware.
> 
> Should I revert the patch then until we have a better fix?

Yes, I think that would be good. Thanks to all for looking into it!

// Simon
diff mbox

Patch

diff --git a/arch/arm/mach-ixp4xx/common-pci.c b/arch/arm/mach-ixp4xx/common-pci.c
index 200970d..055d816 100644
--- a/arch/arm/mach-ixp4xx/common-pci.c
+++ b/arch/arm/mach-ixp4xx/common-pci.c
@@ -481,14 +481,5 @@  int ixp4xx_setup(int nr, struct pci_sys_data *sys)
 	return 1;
 }
 
-int dma_set_coherent_mask(struct device *dev, u64 mask)
-{
-	if (mask >= SZ_64M - 1)
-		return 0;
-
-	return -EIO;
-}
-
 EXPORT_SYMBOL(ixp4xx_pci_read);
 EXPORT_SYMBOL(ixp4xx_pci_write);
-EXPORT_SYMBOL(dma_set_coherent_mask);
diff --git a/arch/arm/mach-ixp4xx/common.c b/arch/arm/mach-ixp4xx/common.c
index 6d68aed..df82a2b 100644
--- a/arch/arm/mach-ixp4xx/common.c
+++ b/arch/arm/mach-ixp4xx/common.c
@@ -31,6 +31,7 @@ 
 #include <linux/gpio.h>
 #include <linux/cpu.h>
 #include <linux/sched_clock.h>
+#include <linux/pci.h>
 
 #include <mach/udc.h>
 #include <mach/hardware.h>
@@ -578,6 +579,17 @@  void ixp4xx_restart(enum reboot_mode mode, const char *cmd)
 	}
 }
 
+int dma_set_coherent_mask(struct device *dev, u64 mask)
+{
+	if (dev_is_pci(dev) && mask >= SZ_64M)
+		return -EIO;
+
+	dev->coherent_dma_mask = mask;
+
+	return 0;
+}
+EXPORT_SYMBOL(dma_set_coherent_mask);
+
 #ifdef CONFIG_IXP4XX_INDIRECT_PCI
 /*
  * In the case of using indirect PCI, we simply return the actual PCI