IXP4xx fixes

PATCH New Krzysztof Halasa 4 participants 12 messages Mar 22, 2014 – Mar 26, 2014

Subsystems (4)

Arm (32-bit) Arm sub-architectures GPIO Arm/Intel IXP4XX Arm architecture

lore

Share & tools

lore

Krzysztof Halasa

author

2014-03-22 23:44 UTC #0 lore raw

Hello,

I'm attaching 3 IXP4xx fixes:

1. Fix Goramo Multilink GPIO conversion (regression in v3.13) This platform is not included by default and it seems it escaped testing when the GPIO was converted.

I've noticed a patch fixing the same problem from Arnd Bergmann, however the other IXP4xx platforms now use the GPIO API and I think this single platform shouldn't be an exception.

2. Fix DMA masks (regression in v3.7)
This patch makes IXP4xx conform to the DMA API. Newly created devices will have 32-bit DMA and coherent DMA masks (effectively 28-bit for PCI devices). dma_set_coherent_mask() will now really set the mask, not only test for its validity as it used to.

A subsequent cosmetic patch for specific devices (e.g. built-in Ethernet and sync serial) setting DMA and coherent DMA masks may be needed, however it won't have any effect on the regression. Simply speaking, it's the core IXP4xx code that is broken (since the beginning I think), and it's where the fix must go.

3. Fix DMA-bounce code to allow sync from_device and to_device with bidirectional mappings.

This is an issue on (probably) E100 Ethernet only. A known problem but few people ever cared. Fixed on X86 long ago.

BTW this passes cleanly through DMA API debug. And it obviously works correctly.

This stuff is tested on Goramo Multilink and on Gateworks Cambria with both on-chip and PCI DMA-able devices.

Signature · 1 line

Krzysztof Halasa

Krzysztof Halasa

author

2014-03-23 00:34 UTC #1 lore raw

PATCH 1/3 IXP4xx: Fix Goramo Multilink GPIO conversion.

arch/arm/mach-ixp4xx/goramo_mlr.c: In function 'set_scl':
arch/arm/mach-ixp4xx/goramo_mlr.c:82: error: implicit declaration of function 'gpio_line_set'

arch/arm/mach-ixp4xx/goramo_mlr.c: In function 'output_control':
arch/arm/mach-ixp4xx/goramo_mlr.c:111: error: implicit declaration of function 'gpio_line_config' arch/arm/mach-ixp4xx/goramo_mlr.c:111: error: 'IXP4XX_GPIO_OUT' undeclared

arch/arm/mach-ixp4xx/goramo_mlr.c: In function 'hss_dcd_irq':
arch/arm/mach-ixp4xx/goramo_mlr.c:155: error: implicit declaration of function 'gpio_line_get'

arch/arm/mach-ixp4xx/goramo_mlr.c: In function 'gmlr_init':
arch/arm/mach-ixp4xx/goramo_mlr.c:416: error: 'IXP4XX_GPIO_OUT' undeclared arch/arm/mach-ixp4xx/goramo_mlr.c:421: error: 'IXP4XX_GPIO_IN' undeclared

Signed-off-by Krzysztof Ha?asa <[email protected]>

arch/arm/mach-ixp4xx/goramo_mlr.c +26 ~~-17~~

index e54ff49..80bd9d6 100644
@@ -4,6 +4,7 @@
  */
 
 #include <linux/delay.h>
+#include <linux/gpio.h>
 #include <linux/hdlc.h>
 #include <linux/i2c-gpio.h>
 #include <linux/io.h>

@@ -79,19 +80,19 @@ static u8 control_value;
 
 static void set_scl(u8 value)
 {
-	gpio_line_set(GPIO_SCL, !!value);
+	gpio_set_value(GPIO_SCL, !!value);
 	udelay(3);
 }
 
 static void set_sda(u8 value)
 {
-	gpio_line_set(GPIO_SDA, !!value);
+	gpio_set_value(GPIO_SDA, !!value);
 	udelay(3);
 }
 
 static void set_str(u8 value)
 {
-	gpio_line_set(GPIO_STR, !!value);
+	gpio_set_value(GPIO_STR, !!value);
 	udelay(3);
 }

@@ -108,8 +109,8 @@ static void output_control(void)
 {
 	int i;
 
-	gpio_line_config(GPIO_SCL, IXP4XX_GPIO_OUT);
-	gpio_line_config(GPIO_SDA, IXP4XX_GPIO_OUT);
+	gpio_direction_output(GPIO_SCL, 1);
+	gpio_direction_output(GPIO_SDA, 1);
 
 	for (i = 0; i < 8; i++) {
 		set_scl(0);

@@ -151,8 +152,8 @@ static int hss_set_clock(int port, unsigned int clock_type)
 
 static irqreturn_t hss_dcd_irq(int irq, void *pdev)
 {
-	int i, port = (irq == IXP4XX_GPIO_IRQ(GPIO_HSS1_DCD_N));
-	gpio_line_get(port ? GPIO_HSS1_DCD_N : GPIO_HSS0_DCD_N, &i);
+	int port = (irq == IXP4XX_GPIO_IRQ(GPIO_HSS1_DCD_N));
+	int i = gpio_get_value(port ? GPIO_HSS1_DCD_N : GPIO_HSS0_DCD_N);
 	set_carrier_cb_tab[port](pdev, !i);
 	return IRQ_HANDLED;
 }

@@ -168,7 +169,7 @@ static int hss_open(int port, void *pdev,
 	else
 		irq = IXP4XX_GPIO_IRQ(GPIO_HSS1_DCD_N);
 
-	gpio_line_get(port ? GPIO_HSS1_DCD_N : GPIO_HSS0_DCD_N, &i);
+	i = gpio_get_value(port ? GPIO_HSS1_DCD_N : GPIO_HSS0_DCD_N);
 	set_carrier_cb(pdev, !i);
 
 	set_carrier_cb_tab[!!port] = set_carrier_cb;

@@ -181,7 +182,7 @@ static int hss_open(int port, void *pdev,
 
 	set_control(port ? CONTROL_HSS1_DTR_N : CONTROL_HSS0_DTR_N, 0);
 	output_control();
-	gpio_line_set(port ? GPIO_HSS1_RTS_N : GPIO_HSS0_RTS_N, 0);
+	gpio_set_value(port ? GPIO_HSS1_RTS_N : GPIO_HSS0_RTS_N, 0);
 	return 0;
 }

@@ -193,7 +194,7 @@ static void hss_close(int port, void *pdev)
 
 	set_control(port ? CONTROL_HSS1_DTR_N : CONTROL_HSS0_DTR_N, 1);
 	output_control();
-	gpio_line_set(port ? GPIO_HSS1_RTS_N : GPIO_HSS0_RTS_N, 1);
+	gpio_set_value(port ? GPIO_HSS1_RTS_N : GPIO_HSS0_RTS_N, 1);
 }

@@ -413,13 +414,21 @@ static void __init gmlr_init(void)
 	if (hw_bits & CFG_HW_HAS_EEPROM)
 		device_tab[devices++] = &device_i2c; /* max index 6 */
 
-	gpio_line_config(GPIO_SCL, IXP4XX_GPIO_OUT);
-	gpio_line_config(GPIO_SDA, IXP4XX_GPIO_OUT);
-	gpio_line_config(GPIO_STR, IXP4XX_GPIO_OUT);
-	gpio_line_config(GPIO_HSS0_RTS_N, IXP4XX_GPIO_OUT);
-	gpio_line_config(GPIO_HSS1_RTS_N, IXP4XX_GPIO_OUT);
-	gpio_line_config(GPIO_HSS0_DCD_N, IXP4XX_GPIO_IN);
-	gpio_line_config(GPIO_HSS1_DCD_N, IXP4XX_GPIO_IN);
+	gpio_request(GPIO_SCL, "SCL/clock");
+	gpio_request(GPIO_SDA, "SDA/data");
+	gpio_request(GPIO_STR, "strobe");
+	gpio_request(GPIO_HSS0_RTS_N, "HSS0 RTS");
+	gpio_request(GPIO_HSS1_RTS_N, "HSS1 RTS");
+	gpio_request(GPIO_HSS0_DCD_N, "HSS0 DCD");
+	gpio_request(GPIO_HSS1_DCD_N, "HSS1 DCD");
+
+	gpio_direction_output(GPIO_SCL, 1);
+	gpio_direction_output(GPIO_SDA, 1);
+	gpio_direction_output(GPIO_STR, 0);
+	gpio_direction_output(GPIO_HSS0_RTS_N, 1);
+	gpio_direction_output(GPIO_HSS1_RTS_N, 1);
+	gpio_direction_input(GPIO_HSS0_DCD_N);
+	gpio_direction_input(GPIO_HSS1_DCD_N);
 	irq_set_irq_type(IXP4XX_GPIO_IRQ(GPIO_HSS0_DCD_N), IRQ_TYPE_EDGE_BOTH);
 	irq_set_irq_type(IXP4XX_GPIO_IRQ(GPIO_HSS1_DCD_N), IRQ_TYPE_EDGE_BOTH);

Linus Walleij

2014-03-24 15:27 UTC #2 lore raw

PATCH 1/3 IXP4xx: Fix Goramo Multilink GPIO conversion.

Quoted 7 lines · Krzysztof Halasa

arch/arm/mach-ixp4xx/goramo_mlr.c: In function 'set_scl':
arch/arm/mach-ixp4xx/goramo_mlr.c:82: error: implicit declaration of function 'gpio_line_set'
arch/arm/mach-ixp4xx/goramo_mlr.c: In function 'output_control':
arch/arm/mach-ixp4xx/goramo_mlr.c:111: error: implicit declaration of function 'gpio_line_config' arch/arm/mach-ixp4xx/goramo_mlr.c:111: error: 'IXP4XX_GPIO_OUT' undeclared

Quoted 8 lines

arch/arm/mach-ixp4xx/goramo_mlr.c: In function 'hss_dcd_irq':
arch/arm/mach-ixp4xx/goramo_mlr.c:155: error: implicit declaration of function 'gpio_line_get'
arch/arm/mach-ixp4xx/goramo_mlr.c: In function 'gmlr_init':
arch/arm/mach-ixp4xx/goramo_mlr.c:416: error: 'IXP4XX_GPIO_OUT' undeclared arch/arm/mach-ixp4xx/goramo_mlr.c:421: error: 'IXP4XX_GPIO_IN' undeclared
Signed-off-by Krzysztof Ha?asa <[email protected]>

Reviewed-by Linus Walleij <[email protected]>

Mea culpa.

Arnd: can you apply this one patch directly for ARM SoC fixes and
Cc: stable at vger.kernel.org on this one?

Alternatively can you send this for fixes to arm at kernel.org with Cc:
stable added Krzysztof?

Yours,
Linus Walleij

Krzysztof Halasa

author

2014-03-24 20:36 UTC #3 lore raw

PATCH 1/3 IXP4xx: Fix Goramo Multilink GPIO conversion.

Quoted 7 lines · Linus Walleij

Quoted 4 lines
arch/arm/mach-ixp4xx/goramo_mlr.c:416: error: 'IXP4XX_GPIO_OUT' undeclared arch/arm/mach-ixp4xx/goramo_mlr.c:421: error: 'IXP4XX_GPIO_IN' undeclared
Signed-off-by Krzysztof Ha?asa <[email protected]>
Alternatively can you send this for fixes to arm at kernel.org with Cc:
stable added Krzysztof?

Given the platform's specifics, I guess it's enough to just fix it in mainline.

Signature · 1 line

Krzysztof Halasa

Arnd Bergmann

2014-03-26 22:06 UTC #4 lore raw

PATCH 1/3 IXP4xx: Fix Goramo Multilink GPIO conversion.

In reply to Linus Walleij

Quoted 20 lines · Linus Walleij

Quoted 7 lines · Krzysztof Halasa
arch/arm/mach-ixp4xx/goramo_mlr.c: In function 'set_scl':
arch/arm/mach-ixp4xx/goramo_mlr.c:82: error: implicit declaration of function 'gpio_line_set'
arch/arm/mach-ixp4xx/goramo_mlr.c: In function 'output_control':
arch/arm/mach-ixp4xx/goramo_mlr.c:111: error: implicit declaration of function 'gpio_line_config' arch/arm/mach-ixp4xx/goramo_mlr.c:111: error: 'IXP4XX_GPIO_OUT' undeclared
Quoted 8 lines
arch/arm/mach-ixp4xx/goramo_mlr.c: In function 'hss_dcd_irq':
arch/arm/mach-ixp4xx/goramo_mlr.c:155: error: implicit declaration of function 'gpio_line_get'
arch/arm/mach-ixp4xx/goramo_mlr.c: In function 'gmlr_init':
arch/arm/mach-ixp4xx/goramo_mlr.c:416: error: 'IXP4XX_GPIO_OUT' undeclared arch/arm/mach-ixp4xx/goramo_mlr.c:421: error: 'IXP4XX_GPIO_IN' undeclared
Signed-off-by Krzysztof Ha?asa <[email protected]>
Reviewed-by Linus Walleij <[email protected]>

Quoted 7 lines

Mea culpa.
Arnd: can you apply this one patch directly for ARM SoC fixes and
Cc: stable at vger.kernel.org on this one?
Alternatively can you send this for fixes to arm at kernel.org with Cc:
stable added Krzysztof?

I reverted my earlier patch and applied this instead. Thanks a lot!

Did not add a stable tag, as suggested by Krzysztof.

Arnd

Krzysztof Halasa

author

2014-03-23 00:36 UTC #5 lore raw

PATCH 2/3 IXP4xx: Fix DMA masks.

In reply to Krzysztof Halasa

Now, devices will have 32-bit default DMA masks (0xFFFFFFFF) as per DMA API.

Fixes:

$ ifconfig eth0 up
net eth0: coherent DMA mask is unset

$ ifconfig hdlc0 up
net hdlc0: coherent DMA mask is unset

Also fixes a cosmetic off-by-one bug which caused DMA transfers ending exactly on the 64 MiB boundary to go through dmabounce unnecessarily.

Signed-off-by Krzysztof Ha?asa <[email protected]>

arch/arm/mach-ixp4xx/common-pci.c +0 ~~-39~~

index 200970d..4977296 100644
@@ -315,33 +315,6 @@ static int abort_handler(unsigned long addr, unsigned int fsr, struct pt_regs *r
 	return 0;
 }
 
-
-static int ixp4xx_needs_bounce(struct device *dev, dma_addr_t dma_addr, size_t size)
-{
-	return (dma_addr + size) >= SZ_64M;
-}
-
-/*
- * Setup DMA mask to 64MB on PCI devices. Ignore all other devices.
- */
-static int ixp4xx_pci_platform_notify(struct device *dev)
-{
-	if (dev_is_pci(dev)) {
-		*dev->dma_mask =  SZ_64M - 1;
-		dev->coherent_dma_mask = SZ_64M - 1;
-		dmabounce_register_dev(dev, 2048, 4096, ixp4xx_needs_bounce);
-	}
-	return 0;
-}
-
-static int ixp4xx_pci_platform_notify_remove(struct device *dev)
-{
-	if (dev_is_pci(dev))
-		dmabounce_unregister_dev(dev);
-
-	return 0;
-}
-
 void __init ixp4xx_pci_preinit(void)
 {
 	unsigned long cpuid = read_cpuid_id();

@@ -475,20 +448,8 @@ int ixp4xx_setup(int nr, struct pci_sys_data *sys)
 	pci_add_resource_offset(&sys->resources, &res[0], sys->io_offset);
 	pci_add_resource_offset(&sys->resources, &res[1], sys->mem_offset);
 
-	platform_notify = ixp4xx_pci_platform_notify;
-	platform_notify_remove = ixp4xx_pci_platform_notify_remove;
-
 	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);

arch/arm/mach-ixp4xx/common.c +56 -5

89 lines not on this page · raw message

Simon Kågström

2014-03-24 13:50 UTC #6 lore raw

PATCH 2/3 IXP4xx: Fix DMA masks.

Quoted 6 lines · On Sun, 23 Mar 2014 01:36:48 +0100 Krzysztof Halasa <[email protected]> wrote:

Now, devices will have 32-bit default DMA masks (0xFFFFFFFF) as per DMA API.
Fixes:
$ ifconfig eth0 up
Signed-off-by Krzysztof Ha?asa <[email protected]>

Tested-by Simon Kagstrom <[email protected]>

Quoted 19 lines

arch/arm/mach-ixp4xx/common.c +19 -0

+#ifdef CONFIG_PCI
+static int ixp4xx_needs_bounce(struct device *dev, dma_addr_t dma_addr, size_t size)
+{
+	return (dma_addr + size) > SZ_64M;
+}
+
+static int ixp4xx_platform_notify_remove(struct device *dev)
+{
+	if (dev_is_pci(dev))
+		dmabounce_unregister_dev(dev);
+
+	return 0;
+}
+#endif
+
+/*
+ * Setup DMA mask to 64MB on PCI devices and 4 GB on all other things.
+ */
+static int ixp4xx_platform_notify(struct device *dev)

arch/arm/mach-ixp4xx/common.c void ixp4xx_restart(enum reboot_mode mode, const char *cmd) · line 608

Quoted 10 lines

arch/arm/mach-ixp4xx/common.c +10 -0

+{
+	dev->dma_mask = &dev->coherent_dma_mask;
+
+#ifdef CONFIG_PCI
+	if (dev_is_pci(dev)) {
+		dev->coherent_dma_mask = DMA_BIT_MASK(28); /* 64 MB */
+		dmabounce_register_dev(dev, 2048, 4096, ixp4xx_needs_bounce);
+		return 0;
+	}
+#endif

I would simply remove the #ifdef CONFIG_PCI statements: dev_is_pci(dev) should return false at compile time if CONFIG_PCI is not set, and then I'd trust the compiler to simply remove all dead code.

// Simon

Krzysztof Halasa

author

2014-03-24 20:30 UTC #7 lore raw

PATCH 2/3 IXP4xx: Fix DMA masks.

Quoted 3 lines · Simon K?gstr?m

I would simply remove the #ifdef CONFIG_PCI statements: dev_is_pci(dev) should return false at compile time if CONFIG_PCI is not set, and then I'd trust the compiler to simply remove all dead code.

This will be the same with these #ifdefs. I can imagine someone using other compiler or e.g. -O0, and without the #ifdefs the link would fail due to unavailability of DMA bouncer.

Signature · 1 line

Krzysztof Halasa

Arnd Bergmann

2014-03-26 22:08 UTC #8 lore raw

PATCH 2/3 IXP4xx: Fix DMA masks.

In reply to Simon Kågström

Quoted 11 lines · Simon K?gstr?m

Quoted 6 lines · On Sun, 23 Mar 2014 01:36:48 +0100 Krzysztof Halasa <[email protected]> wrote:
Now, devices will have 32-bit default DMA masks (0xFFFFFFFF) as per DMA API.
Fixes:
$ ifconfig eth0 up
Signed-off-by Krzysztof Ha?asa <[email protected]>
Tested-by Simon Kagstrom <[email protected]>

Reverted the earlier patch and applied this one, with your Tested-by tag. Thanks!

Arnd

Krzysztof Halasa

author

2014-03-23 00:38 UTC #9 lore raw

PATCH 3/3 ARM: Fix DMA-bounce code to allow sync from_device and to_device with bidirectional mappings.

In reply to Krzysztof Halasa

This is equivalent of a more generic change to lib/dma-debug.c which is present since v2.6.33:

commit 42d53b4ff7d61487d18274ebdf1f70c1aef6f122
dma-debug: allow DMA_BIDIRECTIONAL mappings to be synced with DMA_FROM_DEVICE and DMA_TO_DEVICE.

There is no need to perform full BIDIR sync (copying the buffers in case of swiotlb and similar schemes) if we know that the owner (CPU or device) hasn't altered the data.

Addresses the false-positive reported at
http://bugzilla.kernel.org/show_bug.cgi?id=14169

Signed-off-by Krzysztof Ha?asa <[email protected]>

arch/arm/common/dmabounce.c +2 -2

index 1143c4d..b4c17cf 100644
@@ -375,7 +375,7 @@ static int __dmabounce_sync_for_cpu(struct device *dev, dma_addr_t addr,
 
 	off = addr - buf->safe_dma_addr;
 
-	BUG_ON(buf->direction != dir);
+	BUG_ON(buf->direction != dir && buf->direction != DMA_BIDIRECTIONAL);
 
 	dev_dbg(dev, "%s: unsafe buffer %p (dma=%#x off=%#lx) mapped to %p (dma=%#x)\n",
 		__func__, buf->ptr, virt_to_dma(dev, buf->ptr), off,

@@ -415,7 +415,7 @@ static int __dmabounce_sync_for_device(struct device *dev, dma_addr_t addr,
 
 	off = addr - buf->safe_dma_addr;
 
-	BUG_ON(buf->direction != dir);
+	BUG_ON(buf->direction != dir && buf->direction != DMA_BIDIRECTIONAL);
 
 	dev_dbg(dev, "%s: unsafe buffer %p (dma=%#x off=%#lx) mapped to %p (dma=%#x)\n",
 		__func__, buf->ptr, virt_to_dma(dev, buf->ptr), off,

Simon Kågström

2014-03-24 13:51 UTC #10 lore raw

PATCH 3/3 ARM: Fix DMA-bounce code to allow sync from_device and to_device with bidirectional mappings.

Quoted 7 lines · On Sun, 23 Mar 2014 01:38:41 +0100 Krzysztof Halasa <[email protected]> wrote:

This is equivalent of a more generic change to lib/dma-debug.c which is present since v2.6.33:
commit 42d53b4ff7d61487d18274ebdf1f70c1aef6f122
dma-debug: allow DMA_BIDIRECTIONAL mappings to be synced with DMA_FROM_DEVICE and DMA_TO_DEVICE.

Quoted 8 lines

There is no need to perform full BIDIR sync (copying the buffers in case of swiotlb and similar schemes) if we know that the owner (CPU or device) hasn't altered the data.
Addresses the false-positive reported at
http://bugzilla.kernel.org/show_bug.cgi?id=14169
Signed-off-by Krzysztof Ha?asa <[email protected]>

Tested-by Simon Kagstrom <[email protected]>

(Tested with a e100 device which previously triggered this).

// Simon

Arnd Bergmann

2014-03-26 22:10 UTC #11 lore raw

PATCH 3/3 ARM: Fix DMA-bounce code to allow sync from_device and to_device with bidirectional mappings.

In reply to Krzysztof Halasa

Quoted 7 lines · Krzysztof Halasa

This is equivalent of a more generic change to lib/dma-debug.c which is present since v2.6.33:
commit 42d53b4ff7d61487d18274ebdf1f70c1aef6f122
dma-debug: allow DMA_BIDIRECTIONAL mappings to be synced with DMA_FROM_DEVICE and DMA_TO_DEVICE.

Quoted 8 lines

There is no need to perform full BIDIR sync (copying the buffers in case of swiotlb and similar schemes) if we know that the owner (CPU or device) hasn't altered the data.
Addresses the false-positive reported at
http://bugzilla.kernel.org/show_bug.cgi?id=14169
Signed-off-by Krzysztof Ha?asa <[email protected]>

Looks good to me. Can you add it to Russell's patch tracker?

Acked-by Arnd Bergmann <[email protected]>

Arnd

Quoted 23 lines

arch/arm/common/dmabounce.c +2 -2

index 1143c4d..b4c17cf 100644
@@ -375,7 +375,7 @@ static int __dmabounce_sync_for_cpu(struct device *dev, dma_addr_t addr,
 
 	off = addr - buf->safe_dma_addr;
 
-	BUG_ON(buf->direction != dir);
+	BUG_ON(buf->direction != dir && buf->direction != DMA_BIDIRECTIONAL);
 
 	dev_dbg(dev, "%s: unsafe buffer %p (dma=%#x off=%#lx) mapped to %p (dma=%#x)\n",
 		__func__, buf->ptr, virt_to_dma(dev, buf->ptr), off,

@@ -415,7 +415,7 @@ static int __dmabounce_sync_for_device(struct device *dev, dma_addr_t addr,
 
 	off = addr - buf->safe_dma_addr;
 
-	BUG_ON(buf->direction != dir);
+	BUG_ON(buf->direction != dir && buf->direction != DMA_BIDIRECTIONAL);
 
 	dev_dbg(dev, "%s: unsafe buffer %p (dma=%#x off=%#lx) mapped to %p (dma=%#x)\n",
 		__func__, buf->ptr, virt_to_dma(dev, buf->ptr), off,