: ixp4xx: Fix 3.7 regression for IXP4xx ethernet driver

Hi!

These two patches fixes the regression introduced in 3.7 by commit 1a4901177574083c35fafc24c4d151c2a7c7647c,

ixp4xx_eth: avoid calling dma_pool_create() with NULL dev

The patch above is not incorrect itself, but causes the driver to fail with

  [   33.055473] net eth1: coherent DMA mask is unset
  [   33.055523] net eth1: coherent allocation too big (requested 0x1000 mask 0x0)

The two patches fix this by generalizing and correcting the ixp4xx dma_set_coherent_mask(), which earlier only dealt with the PCI case. The second patch simply sets up the mask for the ixp4xx ethernet device.

The patches have been verified on a board with 256MiB memory, one ipx4xx_eth device and a e100 PCI device, running Linus' tip.

This issue has been discussed before:

https://lkml.org/lkml/2014/1/2/46
http://lists.infradead.org/pipermail/linux-arm-kernel/2013-February/148811.html

This is a resend of the patch series. There was some discussion about it here:

http://www.spinics.net/lists/arm-kernel/msg313082.html

and if I understand Russels comments (as per the documentation), drivers should setup the coherent DMA mask even if it happens to be the platform/CPU default.

ChangeLog:

v2: Move EXPORT_SYMBOL together with dma_set_coherent_mask

// Simon

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.

---
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(-)

index 200970d..055d816 100644
@@ -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);

index 6d68aed..df82a2b 100644
@@ -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

1.7.9.6

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

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

// Simon

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.

Krzysztof Halasa

Research Institute for Automation and Measurements PIAP
Al. Jerozolimskie 202, 02-486 Warsaw, Poland

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

Arnd

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

// Simon

This driver has been broken since commit 1a4901177574083c35fafc24c4d151c2a7c7647c

ixp4xx_eth: avoid calling dma_pool_create() with NULL dev

in 3.7 and would fail with

[   33.055473] net eth1: coherent DMA mask is unset
[   33.055523] net eth1: coherent allocation too big (requested 0x1000 mask 0x0)

Fix by setting up the DMA mask to the entire 32-bit range.

---
 drivers/net/ethernet/xscale/ixp4xx_eth.c |    4 ++++
 1 file changed, 4 insertions(+)

index 25283f1..e540e51 100644
@@ -1426,6 +1426,10 @@ static int eth_init_one(struct platform_device *pdev)
 	port->netdev = dev;
 	port->id = pdev->id;
 
+	err = dma_set_coherent_mask(&dev->dev, DMA_BIT_MASK(32));
+	if (err < 0)
+		goto err_free;
+
 	switch (port->id) {
 	case IXP4XX_ETH_NPEA:
 		port->regs = (struct eth_regs __iomem *)IXP4XX_EthA_BASE_VIRT;

1.7.9.6

The other issue with this patch set is (from DMA-API-HOWTO.txt):

The coherent coherent mask will always be able to set the same or a smaller mask as the streaming mask.

and code in places assumes that's true.

FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.