dmaengine: shdma: modify the DMAC Address Error

The USB-DMAC/SUDMAC don't have the interrupt of DMAC Address Error. So, only when the resource has a name and it is "error_irq", the driver calls request_irq() for DMAC Address Error.

This patch is also useful for the generic DMAC which doesn't have DMAC Address Error. So, we can get rid of the "CPU_SH4 || ARCH_SHMOBILE" ifdefs.
This patch also changes the IRQF_DISABLED to 0.

Signed-off-by Yoshihiro Shimoda <[email protected]>

---
about v3:
- changes the IRQF_DISABLED to 0.

 drivers/dma/shdma.c |   78 ++++++++++++++++++++++++++-------------------------
 1 files changed, 40 insertions(+), 38 deletions(-)

The USB-DMAC/SUDMAC don't have the interrupt of DMAC Address Error. So, only when the resource has a name and it is "error_irq", the driver calls request_irq() for DMAC Address Error.

This patch is also useful for the generic DMAC which doesn't have DMAC Address Error. So, we can get rid of the "CPU_SH4 || ARCH_SHMOBILE" ifdefs.
This patch also changes the IRQF_DISABLED to 0.

Signed-off-by Yoshihiro Shimoda <[email protected]>

---
about v3:
- changes the IRQF_DISABLED to 0.

 drivers/dma/shdma.c |   78 ++++++++++++++++++++++++++-------------------------
 1 files changed, 40 insertions(+), 38 deletions(-)

drivers/dma/shdma.c +5 -18

index 81809c2..55149ed 100644
@@ -1149,12 +1149,12 @@ static void sh_dmae_chan_remove(struct sh_dmae_device *shdev)
 static int __init sh_dmae_probe(struct platform_device *pdev)
 {
 	struct sh_dmae_pdata *pdata = pdev->dev.platform_data;
-	unsigned long irqflags = IRQF_DISABLED,
+	unsigned long irqflags = 0,
 		chan_flag[SH_DMAC_MAX_CHANNELS] = {};
-	int errirq, chan_irq[SH_DMAC_MAX_CHANNELS];
+	int errirq = 0, chan_irq[SH_DMAC_MAX_CHANNELS];
 	int err, i, irq_cnt = 0, irqres = 0, irq_cap = 0;
 	struct sh_dmae_device *shdev;
-	struct resource *chan, *dmars, *errirq_res, *chanirq_res;
+	struct resource *chan, *dmars, *errirq_res, *irq_res, *chanirq_res;
 
 	/* get platform data */
 	if (!pdata || !pdata->channel_num)

@@ -1179,8 +1179,8 @@ static int __init sh_dmae_probe(struct platform_device *pdev)
 	 *    specify IORESOURCE_IRQ_SHAREABLE in their resources, they will be
 	 *    requested with the IRQF_SHARED flag
 	 */
-	errirq_res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
-	if (!chan || !errirq_res)
+	irq_res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+	if (!chan || !irq_res)
 		return -ENODEV;
 
 	if (!request_mem_region(chan->start, resource_size(chan), pdev->name)) {

@@ -1258,33 +1258,35 @@ static int __init sh_dmae_probe(struct platform_device *pdev)
 	/* Default transfer size of 32 bytes requires 32-byte alignment */
 	shdev->common.copy_align = LOG2_DEFAULT_XFER_SIZE;
 
-#if defined(CONFIG_CPU_SH4) || defined(CONFIG_ARCH_SHMOBILE)
-	chanirq_res = platform_get_resource(pdev, IORESOURCE_IRQ, 1);
-
-	if (!chanirq_res)
-		chanirq_res = errirq_res;
-	else
-		irqres++;
-
-	if (chanirq_res = errirq_res ||
-	    (errirq_res->flags & IORESOURCE_BITS) = IORESOURCE_IRQ_SHAREABLE)
-		irqflags = IRQF_SHARED;
-
-	errirq = errirq_res->start;

drivers/dma/shdma.c +33 -12

+	if (errirq_res) {
+		chanirq_res = platform_get_resource(pdev, IORESOURCE_IRQ, 1);
+
+		if (!chanirq_res)
+			chanirq_res = errirq_res;
+		else
+			irqres++;
+
+		if (chanirq_res = errirq_res ||
+		    (errirq_res->flags & IORESOURCE_BITS) =
+		    IORESOURCE_IRQ_SHAREABLE)
+			irqflags = IRQF_SHARED;
+
+		errirq = errirq_res->start;
+
+		err = request_irq(errirq, sh_dmae_err, irqflags,
+				  "DMAC Address Error", shdev);
+		if (err) {
+			dev_err(&pdev->dev,
+				"DMA failed requesting irq #%d, error %d\n",
+				errirq, err);
+			goto eirq_err;
+		}
+	} else {
+		chanirq_res = irq_res;
 	}
 
-#else
-	chanirq_res = errirq_res;
-#endif /* CONFIG_CPU_SH4 || CONFIG_ARCH_SHMOBILE */
-
 	if (chanirq_res->start = chanirq_res->end &&
 	    !platform_get_resource(pdev, IORESOURCE_IRQ, 1)) {
 		/* Special case - all multiplexed */

@@ -1305,11 +1307,11 @@ static int __init sh_dmae_probe(struct platform_device *pdev)
 					break;
 				}
 
-				if ((errirq_res->flags & IORESOURCE_BITS) =
+				if ((irq_res->flags & IORESOURCE_BITS) =
 				    IORESOURCE_IRQ_SHAREABLE)
 					chan_flag[irq_cnt] = IRQF_SHARED;
 				else
-					chan_flag[irq_cnt] = IRQF_DISABLED;
+					chan_flag[irq_cnt] = 0;
 				dev_dbg(&pdev->dev,
 					"Found IRQ %d for channel %d\n",
 					i, irq_cnt);

@@ -1345,10 +1347,9 @@ static int __init sh_dmae_probe(struct platform_device *pdev)
 chan_probe_err:
 	sh_dmae_chan_remove(shdev);
 
-#if defined(CONFIG_CPU_SH4) || defined(CONFIG_ARCH_SHMOBILE)
-	free_irq(errirq, shdev);
+	if (errirq_res)
+		free_irq(errirq, shdev);
 eirq_err:
-#endif
 rst_err:
 	spin_lock_irq(&sh_dmae_lock);
 	list_del_rcu(&shdev->node);

@@ -1379,12 +1380,13 @@ static int __exit sh_dmae_remove(struct platform_device *pdev)
 {
 	struct sh_dmae_device *shdev = platform_get_drvdata(pdev);
 	struct resource *res;
-	int errirq = platform_get_irq(pdev, 0);
+	struct resource *errirq_res = platform_get_resource_byname(pdev,
+						IORESOURCE_IRQ, "error_irq");
 
 	dma_async_device_unregister(&shdev->common);
 
-	if (errirq > 0)
-		free_irq(errirq, shdev);
+	if (errirq_res)
+		free_irq(errirq_res->start, shdev);
 
 	spin_lock_irq(&sh_dmae_lock);
 	list_del_rcu(&shdev->node);

Signature · 1 line

1.7.1

Hello Shimoda-san

Thanks for your work on this! Nice to see patches improving!

drivers/dma/shdma.c static int __init sh_dmae_probe(struct platform_device *pdev) · line 1262

Quoted 2 lines · Yoshihiro

drivers/dma/shdma.c +2 -0

+	errirq_res = platform_get_resource_byname(pdev, IORESOURCE_IRQ,
+						  "error_irq");

Wouldn't this break all platforms, providing error IRQ, because they don't yet have any IRQ resources with that name? Also, I think, it would be good to define macros for DMAC IRQ names, similar to SH_MOBILE_SDHI_IRQ_* macros in include/linux/mmc/sh_mobile_sdhi.h. To avoid breakages you, probably, want to

1. add IRQ resource name definition macros to the header
2. add IRQ resource names to platforms. This shouldn't break anything, you still can request those IRQs without name.
3. convert the driver. Here I would try to think of a scheme, where we wouldn't have to mix named and unnamed IRQ resources. So, maybe use all of them as named. So, how about

3.1. Request both "error irq" and "channel irq" IRQ resources 3.2. If none is supplied - error out
3.3. If an error IRQ is provided - request it
3.4. If no separate channel IRQs are provided - request error IRQ additional times, according to the channel count
3.5. If separate channel IRQs are provided - request them just like now

Would this make sense?

Thanks
Guennadi