Skip to contentSkip to conversation

dmaengine: ppc4xx: convert irq_of_parse_and_map to platform_get_irq

PATCH Superseded Rosen Penev 3 participants 3 messages Sep 10 – Sep 11 dmaengine

v1 of 4 Superseded Latest is v4 →

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 dmaengine: ppc4xx: convert irq_of_parse_and_map to platform_get_irq

Replace irq_of_parse_and_map() with platform_get_irq(), which is the preferred way to obtain IRQ resources from platform devices. This eliminates the corresponding irq_dispose_mapping() calls since the framework manages the mapping.

While here, fix a latent bug in the err_req2 error path: the error IRQ was not freed when a subsequent step (I2O setup) failed.

The struct device_node *np declaration is moved to the scope where it is still needed (I2O register lookup).

Assisted-by opencode:big-pickle
---
 drivers/dma/ppc4xx/adma.c | 28 +++++++++++-----------------
 1 file changed, 11 insertions(+), 17 deletions(-)
drivers/dma/ppc4xx/adma.c +11 -17
index 279a431ccae3..42fc1791662b 100644
@@ -3865,28 +3865,25 @@ static int ppc440spe_adma_setup_irqs(struct ppc440spe_adma_device *adev,
 				     int *initcode)
 {
 	struct platform_device *ofdev;
-	struct device_node *np;
 	int ret;
 
 	ofdev = container_of(adev->dev, struct platform_device, dev);
-	np = ofdev->dev.of_node;
 	if (adev->id != PPC440SPE_XOR_ID) {
-		adev->err_irq = irq_of_parse_and_map(np, 1);
-		if (!adev->err_irq) {
+		adev->err_irq = platform_get_irq(ofdev, 1);
+		if (adev->err_irq < 0) {
 			dev_warn(adev->dev, "no err irq resource?\n");
 			*initcode = PPC_ADMA_INIT_IRQ2;
-			adev->err_irq = -ENXIO;
 		} else
 			atomic_inc(&ppc440spe_adma_err_irq_ref);
 	} else {
 		adev->err_irq = -ENXIO;
 	}
 
-	adev->irq = irq_of_parse_and_map(np, 0);
-	if (!adev->irq) {
+	adev->irq = platform_get_irq(ofdev, 0);
+	if (adev->irq < 0) {
 		dev_err(adev->dev, "no irq resource\n");
 		*initcode = PPC_ADMA_INIT_IRQ1;
-		ret = -ENXIO;
+		ret = adev->irq;
 		goto err_irq_map;
 	}
 	dev_dbg(adev->dev, "irq %d, err irq %d\n",
@@ -3927,6 +3924,7 @@ static int ppc440spe_adma_setup_irqs(struct ppc440spe_adma_device *adev,
 			    XOR_IE_ICIE_BIT | XOR_IE_RPTIE_BIT,
 			    &adev->xor_reg->ier);
 	} else {
+		struct device_node *np;
 		u32 mask, enable;
 
 		np = of_find_compatible_node(NULL, NULL, "ibm,i2o-440spe");
@@ -3956,14 +3954,13 @@ static int ppc440spe_adma_setup_irqs(struct ppc440spe_adma_device *adev,
 	return 0;
 
 err_req2:
+	if (adev->err_irq > 0)
+		free_irq(adev->err_irq, chan);
 	free_irq(adev->irq, chan);
 err_req1:
-	irq_dispose_mapping(adev->irq);
 err_irq_map:
-	if (adev->err_irq > 0) {
-		if (atomic_dec_and_test(&ppc440spe_adma_err_irq_ref))
-			irq_dispose_mapping(adev->err_irq);
-	}
+	if (adev->err_irq > 0)
+		atomic_dec(&ppc440spe_adma_err_irq_ref);
 	return ret;
 }
 
@@ -3987,13 +3984,10 @@ static void ppc440spe_adma_release_irqs(struct ppc440spe_adma_device *adev,
 		iowrite32(mask, &adev->i2o_reg->iopim);
 	}
 	free_irq(adev->irq, chan);
-	irq_dispose_mapping(adev->irq);
 	if (adev->err_irq > 0) {
 		free_irq(adev->err_irq, chan);
-		if (atomic_dec_and_test(&ppc440spe_adma_err_irq_ref)) {
-			irq_dispose_mapping(adev->err_irq);
+		if (atomic_dec_and_test(&ppc440spe_adma_err_irq_ref))
 			iounmap(adev->i2o_reg);
-		}
 	}
 }
 
Signature · 1 line
2.55.0
#1 lore raw
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] Invalid free_irq() call on error path when request_irq() fails. --

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Invalid free_irq() call on error path when request_irq() fails. --

commit b94f2fec69c0e655f6872194e27d4dbff577af31
Author: Rosen Penev <[email protected]>

dmaengine: ppc4xx: convert irq_of_parse_and_map to platform_get_irq

This commit replaces irq_of_parse_and_map() with platform_get_irq() for obtaining IRQ resources. It also eliminates the corresponding irq_dispose_mapping() calls and adjusts the error handling paths.

Quoted 4 lines
drivers/dma/ppc4xx/adma.c +0 -0
index 279a431ccae38..42fc1791662ba 100644

[ ... ]

drivers/dma/ppc4xx/adma.c static int ppc440spe_adma_setup_irqs(struct ppc440spe_adma_device *adev, · line 3958

Quoted 6 lines
drivers/dma/ppc4xx/adma.c +2 -0
@@ -3956,14 +3954,13 @@ static int ppc440spe_adma_setup_irqs(struct ppc440spe_adma_device *adev,
 	return 0;
 
 err_req2:
+	if (adev->err_irq > 0)
+		free_irq(adev->err_irq, chan);

[Severity: Medium]
Will this added free_irq() trigger a kernel warning ("Trying to free already-free IRQ") if we reach this path because the error IRQ request itself failed?

In ppc440spe_adma_setup_irqs(), if request_irq() fails for adev->err_irq, the code jumps directly to err_req2:

	ret = request_irq(adev->err_irq, ppc440spe_adma_err_handler, ...);
	if (ret) {
		dev_err(adev->dev, "can't request irq %d\n", adev->err_irq);
		*initcode = PPC_ADMA_INIT_IRQ2;
		ret = -EIO;
		goto err_req2;
	}

Since the IRQ was not successfully requested when this failure path is taken, unconditionally calling free_irq() here will attempt to free an unallocated IRQ.

Should we track whether the IRQ was successfully requested before freeing it, or perhaps introduce a separate error label to avoid this?

Quoted 12 lines
drivers/dma/ppc4xx/adma.c +2 -5
 	free_irq(adev->irq, chan);
 err_req1:
-	irq_dispose_mapping(adev->irq);
 err_irq_map:
-	if (adev->err_irq > 0) {
-		if (atomic_dec_and_test(&ppc440spe_adma_err_irq_ref))
-			irq_dispose_mapping(adev->err_irq);
-	}
+	if (adev->err_irq > 0)
+		atomic_dec(&ppc440spe_adma_err_irq_ref);
 	return ret;
 }
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
#2 lore raw
In reply to Rosen Penev

On Thu, Sep 10, 2026 at 02:27:20PM -0700, Rosen Penev wrote:

subject: function need ().

Quoted 6 lines

Replace irq_of_parse_and_map() with platform_get_irq(), which is the preferred way to obtain IRQ resources from platform devices. This eliminates the corresponding irq_dispose_mapping() calls

since the
framework manages the mapping.

Needn't this

Quoted 2 lines

While here, fix a latent bug in the err_req2 error path: the error IRQ

remove "While here", just Fix ...

Quoted 10 lines

was not freed when a subsequent step (I2O setup) failed.

The struct device_node *np declaration is moved to the scope where it is still needed (I2O register lookup).

Assisted-by opencode:big-pickle
---
 drivers/dma/ppc4xx/adma.c | 28 +++++++++++-----------------
 1 file changed, 11 insertions(+), 17 deletions(-)

please collect all ppc4xx fix into a patch serial.

Quoted 12 lines
drivers/dma/ppc4xx/adma.c +0 -1
index 279a431ccae3..42fc1791662b 100644
@@ -3865,28 +3865,25 @@ static int ppc440spe_adma_setup_irqs(struct ppc440spe_adma_device *adev,
 				     int *initcode)
 {
 	struct platform_device *ofdev;
-	struct device_node *np;
 	int ret;
 

drivers/dma/ppc4xx/adma.c static int ppc440spe_adma_setup_irqs(struct ppc440spe_adma_device *adev, · line 3878

Quoted 10 lines
drivers/dma/ppc4xx/adma.c +2 -4
 	ofdev = container_of(adev->dev, struct platform_device, dev);
-	np = ofdev->dev.of_node;
 	if (adev->id != PPC440SPE_XOR_ID) {
-		adev->err_irq = irq_of_parse_and_map(np, 1);
-		if (!adev->err_irq) {
+		adev->err_irq = platform_get_irq(ofdev, 1);
+		if (adev->err_irq < 0) {
 			dev_warn(adev->dev, "no err irq resource?\n");
 			*initcode = PPC_ADMA_INIT_IRQ2;
-			adev->err_irq = -ENXIO;

need descript in comment message to propagate irq number instead return -ENXIO.

Frank

Quoted 62 lines
drivers/dma/ppc4xx/adma.c +9 -13
 		} else
 			atomic_inc(&ppc440spe_adma_err_irq_ref);
 	} else {
 		adev->err_irq = -ENXIO;
 	}
 
-	adev->irq = irq_of_parse_and_map(np, 0);
-	if (!adev->irq) {
+	adev->irq = platform_get_irq(ofdev, 0);
+	if (adev->irq < 0) {
 		dev_err(adev->dev, "no irq resource\n");
 		*initcode = PPC_ADMA_INIT_IRQ1;
-		ret = -ENXIO;
+		ret = adev->irq;
 		goto err_irq_map;
 	}
 	dev_dbg(adev->dev, "irq %d, err irq %d\n",
@@ -3927,6 +3924,7 @@ static int ppc440spe_adma_setup_irqs(struct ppc440spe_adma_device *adev,
 			    XOR_IE_ICIE_BIT | XOR_IE_RPTIE_BIT,
 			    &adev->xor_reg->ier);
 	} else {
+		struct device_node *np;
 		u32 mask, enable;
 
 		np = of_find_compatible_node(NULL, NULL, "ibm,i2o-440spe");
@@ -3956,14 +3954,13 @@ static int ppc440spe_adma_setup_irqs(struct ppc440spe_adma_device *adev,
 	return 0;
 
 err_req2:
+	if (adev->err_irq > 0)
+		free_irq(adev->err_irq, chan);
 	free_irq(adev->irq, chan);
 err_req1:
-	irq_dispose_mapping(adev->irq);
 err_irq_map:
-	if (adev->err_irq > 0) {
-		if (atomic_dec_and_test(&ppc440spe_adma_err_irq_ref))
-			irq_dispose_mapping(adev->err_irq);
-	}
+	if (adev->err_irq > 0)
+		atomic_dec(&ppc440spe_adma_err_irq_ref);
 	return ret;
 }
 
@@ -3987,13 +3984,10 @@ static void ppc440spe_adma_release_irqs(struct ppc440spe_adma_device *adev,
 		iowrite32(mask, &adev->i2o_reg->iopim);
 	}
 	free_irq(adev->irq, chan);
-	irq_dispose_mapping(adev->irq);
 	if (adev->err_irq > 0) {
 		free_irq(adev->err_irq, chan);
-		if (atomic_dec_and_test(&ppc440spe_adma_err_irq_ref)) {
-			irq_dispose_mapping(adev->err_irq);
+		if (atomic_dec_and_test(&ppc440spe_adma_err_irq_ref))
 			iounmap(adev->i2o_reg);
-		}
 	}
 }
 
--

2.55.0