: 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

// 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.

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

index 200970d..b02c764 100644
@@ -481,14 +481,6 @@ 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..b221c78 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,16 @@ 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;
+}
+
 #ifdef CONFIG_IXP4XX_INDIRECT_PCI
 /*
  * In the case of using indirect PCI, we simply return the actual PCI

1.7.9.6

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

If you have drivers missing this call, that's part of the problem:

| For correct operation, you must interrogate the kernel in your device
| probe routine to see if the DMA controller on the machine can properly
| support the DMA addressing limitation your device has.  It is good
| style to do this even if your device holds the default setting,
| because this shows that you did think about these issues wrt. your
| device.
| 
| The query is performed via a call to dma_set_mask_and_coherent():
| 
|         int dma_set_mask_and_coherent(struct device *dev, u64 mask);
| 
| which will query the mask for both streaming and coherent APIs together.
| If you have some special requirements, then the following two separate
| queries can be used instead:
| 
|         The query for streaming mappings is performed via a call to
|         dma_set_mask():
| 
|                 int dma_set_mask(struct device *dev, u64 mask);
| 
|         The query for consistent allocations is performed via a call
|         to dma_set_coherent_mask():
| 
|                 int dma_set_coherent_mask(struct device *dev, u64 mask);
| 
| Here, dev is a pointer to the device struct of your device, and mask
| is a bit mask describing which bits of an address your device
| supports.  It returns zero if your card can perform DMA properly on
| the machine given the address mask you provided.  In general, the
| device struct of your device is embedded in the bus specific device
| struct of your device.  For example, a pointer to the device struct of
| your PCI device is pdev->dev (pdev is a pointer to the PCI device
| struct of your device).
| 
| If it returns non-zero, your device cannot perform DMA properly on
| this platform, and attempting to do so will result in undefined
| behavior.  You must either use a different mask, or not use DMA.
| 
| ...

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

Well, we already know it can. Actually, the DMA controller is a part of the CPU + RAM controller chip :-)

But I guess with this new wording it's something the drivers can use.

Krzysztof Halasa

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

With that wording, I would say these two patches should be applicable:
The first corrects dma_set_coherent_mask() to actually setup the coherent_dma_mask (and make it applicable outside of the PCI domain) and the second sets it up in the driver, as per

Without these patches (or Krzysztofs patch), the ixp4xx platform is broken for many common configurations.

// Simon

What I quoted is not "new wording" apart from the addition of the interface which sets both masks. This has been documented as a requirement since before there was a dma_* API (the pci_dma_* API predated that, and also had this requirement.)

It's something that has long been ignored on ARM because "oh we can do it in the platform code" - not anymore, not with DT. This short-cut has finally bitten, and we now must conform.

So, in the interests of everything working as it should, it's something that needs to be fixed irrespective of anything else: all our drivers which perform DMA should make the appropriate DMA mask calls.

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