[PATCH v2 1/3] dmaengine: shdma: modify the DMAC Address Error

[PATCH v2 1/3] dmaengine: shdma: modify the DMAC Address Error

[Shimoda, Yoshihiro]

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.

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 about v2:
  - use a resource name for the registration.
  - rid of the "CPU_SH4 || ARCH_SHMOBILE" ifdefs.

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

diff --git a/drivers/dma/shdma.c b/drivers/dma/shdma.c
index 81809c2..4fff02b 100644
--- a/drivers/dma/shdma.c
+++ b/drivers/dma/shdma.c
@@ -1151,10 +1151,10 @@ static int __init sh_dmae_probe(struct platform_device *pdev)
 	struct sh_dmae_pdata *pdata = pdev->dev.platform_data;
 	unsigned long irqflags = IRQF_DISABLED,
 		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;
-
-	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;
+	errirq_res = platform_get_resource_byname(pdev, IORESOURCE_IRQ,
+						  "error_irq");
+	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,7 +1307,7 @@ 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
@@ -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);
-- 
1.7.1

Re: [PATCH v2 1/3] dmaengine: shdma: modify the DMAC Address Error registration

[Paul Mundt]

On Tue, Jan 10, 2012 at 02:20:16PM +0900, Shimoda, Yoshihiro wrote:
> +		if (chanirq_res = errirq_res ||
> +		    (errirq_res->flags & IORESOURCE_BITS) =
> +		    IORESOURCE_IRQ_SHAREABLE)
> +			irqflags = IRQF_SHARED;
> +
While not your fault, it seems that there are a few of these cases in
which the irqflags are clobbered rather than added to. Is there some
particular reason why we aren't doing |= IRQF_SHARED on top of the
default IRQF_DISABLED case?

Re: [PATCH v2 1/3] dmaengine: shdma: modify the DMAC Address Error

[Shimoda, Yoshihiro]

2012/01/10 14:33, Paul Mundt wrote:
> On Tue, Jan 10, 2012 at 02:20:16PM +0900, Shimoda, Yoshihiro wrote:
>> +		if (chanirq_res = errirq_res ||
>> +		    (errirq_res->flags & IORESOURCE_BITS) =
>> +		    IORESOURCE_IRQ_SHAREABLE)
>> +			irqflags = IRQF_SHARED;
>> +
> While not your fault, it seems that there are a few of these cases in
> which the irqflags are clobbered rather than added to. Is there some
> particular reason why we aren't doing |= IRQF_SHARED on top of the
> default IRQF_DISABLED case?
> 

I guess that old kernel (before the commit of 6932bf37 "genirq: Remove IRQF_DISABLED from
core code") output some warning in request_irq() when irqflags is set to
"IRQF_DISABLED | IRQF_SHARED".

By the way, according to the "feature-removal-schedule.txt", the IRQF_DISABLED will remove.
So, we can remove the IRQF_DISABLED in the shdma driver, I think.

Best regards,
Yoshihiro Shimoda

Re: [PATCH v2 1/3] dmaengine: shdma: modify the DMAC Address Error registration

[Paul Mundt]

On Tue, Jan 10, 2012 at 02:56:33PM +0900, Shimoda, Yoshihiro wrote:
> 2012/01/10 14:33, Paul Mundt wrote:
> > On Tue, Jan 10, 2012 at 02:20:16PM +0900, Shimoda, Yoshihiro wrote:
> >> +		if (chanirq_res = errirq_res ||
> >> +		    (errirq_res->flags & IORESOURCE_BITS) =
> >> +		    IORESOURCE_IRQ_SHAREABLE)
> >> +			irqflags = IRQF_SHARED;
> >> +
> > While not your fault, it seems that there are a few of these cases in
> > which the irqflags are clobbered rather than added to. Is there some
> > particular reason why we aren't doing |= IRQF_SHARED on top of the
> > default IRQF_DISABLED case?
> > 
> 
> I guess that old kernel (before the commit of 6932bf37 "genirq: Remove IRQF_DISABLED from
> core code") output some warning in request_irq() when irqflags is set to
> "IRQF_DISABLED | IRQF_SHARED".
> 
> By the way, according to the "feature-removal-schedule.txt", the IRQF_DISABLED will remove.
> So, we can remove the IRQF_DISABLED in the shdma driver, I think.
> 
Yes, it's deprecated now, so there's no need to keep it around. The
non-shared case can just be 0.

Re: [PATCH v2 1/3] dmaengine: shdma: modify the DMAC Address Error

[Shimoda, Yoshihiro]

2012/01/10 15:01, Paul Mundt wrote:
> On Tue, Jan 10, 2012 at 02:56:33PM +0900, Shimoda, Yoshihiro wrote:
>> 2012/01/10 14:33, Paul Mundt wrote:
>>> On Tue, Jan 10, 2012 at 02:20:16PM +0900, Shimoda, Yoshihiro wrote:
>>>> +		if (chanirq_res = errirq_res ||
>>>> +		    (errirq_res->flags & IORESOURCE_BITS) =
>>>> +		    IORESOURCE_IRQ_SHAREABLE)
>>>> +			irqflags = IRQF_SHARED;
>>>> +
>>> While not your fault, it seems that there are a few of these cases in
>>> which the irqflags are clobbered rather than added to. Is there some
>>> particular reason why we aren't doing |= IRQF_SHARED on top of the
>>> default IRQF_DISABLED case?
>>>
>>
>> I guess that old kernel (before the commit of 6932bf37 "genirq: Remove IRQF_DISABLED from
>> core code") output some warning in request_irq() when irqflags is set to
>> "IRQF_DISABLED | IRQF_SHARED".
>>
>> By the way, according to the "feature-removal-schedule.txt", the IRQF_DISABLED will remove.
>> So, we can remove the IRQF_DISABLED in the shdma driver, I think.
>>
> Yes, it's deprecated now, so there's no need to keep it around. The
> non-shared case can just be 0.
> 
OK, I will modify the patch and submit it again.

Best regards,
Yoshihiro Shimoda