[PATCHv2] dmaengine: bestcomm: use platform machinery to get irq

[PATCHv2] dmaengine: bestcomm: use platform machinery to get irq

[Rosen Penev]

platform_get_irq() avoids having to explicitly map and dispose of an irq
by having the platform_device machinery take care of it.

This requires replacing the device_node with a platform_device in the
bcom_eng struct so that platform_get_irq() can use it.

Conveniently, this also allows getting rid of of_node_get and
of_node_put as the platform_device machinery also handles that.

Replace the open-coded resource lookup, request_mem_region, ioremap and
the manual iounmap/release_mem_region cleanup in probe/remove with the
managed devm_platform_get_and_ioremap_resource() helper. This removes the
now-unused error-unmap/release paths and simplifies the driver.

Adjust the goto labels in bcom_task_alloc to match what is being freed.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 v2: resend.
 drivers/dma/bestcomm/bestcomm.c            | 47 ++++++++--------------
 include/linux/fsl/bestcomm/bestcomm_priv.h |  2 +-
 2 files changed, 18 insertions(+), 31 deletions(-)

diff --git a/drivers/dma/bestcomm/bestcomm.c b/drivers/dma/bestcomm/bestcomm.c
index 241a91936fe8..1006030ff167 100644
--- a/drivers/dma/bestcomm/bestcomm.c
+++ b/drivers/dma/bestcomm/bestcomm.c
@@ -47,6 +47,7 @@ bcom_task_alloc(int bd_count, int bd_size, int priv_size)
 {
 	int i, tasknum = -1;
 	struct bcom_task *tsk;
+	int irq;
 
 	/* Don't try to do anything if bestcomm init failed */
 	if (!bcom_eng)
@@ -67,30 +68,32 @@ bcom_task_alloc(int bd_count, int bd_size, int priv_size)
 	if (tasknum < 0)
 		return NULL;
 
+	irq = platform_get_irq(bcom_eng->pdev, tasknum);
+	if (irq < 0)
+		goto err1;
+
 	/* Allocate our structure */
 	tsk = kzalloc(sizeof(struct bcom_task) + priv_size, GFP_KERNEL);
 	if (!tsk)
-		goto error;
+		goto err1;
 
 	tsk->tasknum = tasknum;
 	if (priv_size)
 		tsk->priv = (void*)tsk + sizeof(struct bcom_task);
 
 	/* Get IRQ of that task */
-	tsk->irq = irq_of_parse_and_map(bcom_eng->ofnode, tsk->tasknum);
-	if (!tsk->irq)
-		goto error;
+	tsk->irq = irq;
 
 	/* Init the BDs, if needed */
 	if (bd_count) {
 		tsk->cookie = kmalloc_array(bd_count, sizeof(void *),
 					    GFP_KERNEL);
 		if (!tsk->cookie)
-			goto error;
+			goto err2;
 
 		tsk->bd = bcom_sram_alloc(bd_count * bd_size, 4, &tsk->bd_pa);
 		if (!tsk->bd)
-			goto error;
+			goto err3;
 		memset_io(tsk->bd, 0x00, bd_count * bd_size);
 
 		tsk->num_bd = bd_count;
@@ -99,17 +102,12 @@ bcom_task_alloc(int bd_count, int bd_size, int priv_size)
 
 	return tsk;
 
-error:
-	if (tsk) {
-		if (tsk->irq)
-			irq_dispose_mapping(tsk->irq);
-		bcom_sram_free(tsk->bd);
-		kfree(tsk->cookie);
-		kfree(tsk);
-	}
-
+err3:
+	kfree(tsk->cookie);
+err2:
+	kfree(tsk);
+err1:
 	bcom_eng->tdt[tasknum].stop = 0;
-
 	return NULL;
 }
 EXPORT_SYMBOL_GPL(bcom_task_alloc);
@@ -125,7 +123,6 @@ bcom_task_free(struct bcom_task *tsk)
 	bcom_eng->tdt[tsk->tasknum].stop  = 0;
 
 	/* Free everything */
-	irq_dispose_mapping(tsk->irq);
 	bcom_sram_free(tsk->bd);
 	kfree(tsk->cookie);
 	kfree(tsk);
@@ -377,16 +374,12 @@ static int mpc52xx_bcom_probe(struct platform_device *op)
 	if (IS_ERR(regs))
 		return PTR_ERR(regs);
 
-	/* Get the bestcomm node */
-	of_node_get(op->dev.of_node);
-
 	/* Prepare SRAM */
 	ofn_sram = of_find_matching_node(NULL, mpc52xx_sram_ids);
 	if (!ofn_sram) {
 		printk(KERN_ERR DRIVER_NAME ": "
 			"No SRAM found in device tree\n");
-		rv = -ENODEV;
-		goto error_ofput;
+		return -ENODEV;
 	}
 	rv = bcom_sram_init(ofn_sram, DRIVER_NAME);
 	of_node_put(ofn_sram);
@@ -394,7 +387,7 @@ static int mpc52xx_bcom_probe(struct platform_device *op)
 	if (rv) {
 		printk(KERN_ERR DRIVER_NAME ": "
 			"Error in SRAM init\n");
-		goto error_ofput;
+		return rv;
 	}
 
 	/* Get a clean struct */
@@ -405,8 +398,7 @@ static int mpc52xx_bcom_probe(struct platform_device *op)
 	}
 
 	/* Save the node */
-	bcom_eng->ofnode = op->dev.of_node;
-
+	bcom_eng->pdev = op;
 	bcom_eng->regs = regs;
 	bcom_eng->regs_base = res_bcom->start;
 
@@ -426,8 +418,6 @@ static int mpc52xx_bcom_probe(struct platform_device *op)
 	kfree(bcom_eng);
 	bcom_eng = NULL;
 	bcom_sram_cleanup();
-error_ofput:
-	of_node_put(op->dev.of_node);
 
 	printk(KERN_ERR "DMA: MPC52xx BestComm init failed !\n");
 
@@ -443,9 +433,6 @@ static void mpc52xx_bcom_remove(struct platform_device *op)
 	/* Cleanup SRAM */
 	bcom_sram_cleanup();
 
-	/* Release the node */
-	of_node_put(bcom_eng->ofnode);
-
 	/* Release memory */
 	kfree(bcom_eng);
 	bcom_eng = NULL;
diff --git a/include/linux/fsl/bestcomm/bestcomm_priv.h b/include/linux/fsl/bestcomm/bestcomm_priv.h
index 3b52f3ffbdf8..cddde7c74cb5 100644
--- a/include/linux/fsl/bestcomm/bestcomm_priv.h
+++ b/include/linux/fsl/bestcomm/bestcomm_priv.h
@@ -66,7 +66,7 @@ struct bcom_tdt {
  * This holds all info needed globaly to handle the engine
  */
 struct bcom_engine {
-	struct device_node		*ofnode;
+	struct platform_device		*pdev;
 	struct mpc52xx_sdma __iomem     *regs;
 	phys_addr_t                      regs_base;
 
-- 
2.55.0

Re: [PATCHv2] dmaengine: bestcomm: use platform machinery to get irq

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] misc: commit message and code mismatch regarding devm_platform_get_and_ioremap_resource
--

commit e9aa0bc87e702e8cdbc8918ca965897d4edffc03
Author: Rosen Penev <rosenp@gmail.com>

dmaengine: bestcomm: use platform machinery to get irq

This commit updates the bestcomm driver to use platform_get_irq() for
interrupt mapping and replaces of_node references with platform_device. It
also removes the manual device tree node reference counting.

> Replace the open-coded resource lookup, request_mem_region, ioremap and
> the manual iounmap/release_mem_region cleanup in probe/remove with the
> managed devm_platform_get_and_ioremap_resource() helper. This removes the
> now-unused error-unmap/release paths and simplifies the driver.

[Severity: Medium]
Is this paragraph in the commit message accurate for this patch?

Looking at mpc52xx_bcom_probe() in drivers/dma/bestcomm/bestcomm.c around
line 374, the devm_platform_get_and_ioremap_resource() call already exists
prior to this patch (having been introduced in a prior commit,
0a8dda0a15d392).

Since this patch does not actually contain the changes described in this
paragraph, could the commit message be updated to only reflect the changes
introduced here?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260916224156.276511-1-rosenp@gmail.com?part=1

Re: [PATCHv2] dmaengine: bestcomm: use platform machinery to get irq

[Frank Li]

On Wed, Sep 16, 2026 at 03:41:56PM -0700, Rosen Penev wrote:
> platform_get_irq() avoids having to explicitly map and dispose of an irq
> by having the platform_device machinery take care of it.
>
> This requires replacing the device_node with a platform_device in the
> bcom_eng struct so that platform_get_irq() can use it.
>
> Conveniently, this also allows getting rid of of_node_get and
> of_node_put as the platform_device machinery also handles that.
>
> Replace the open-coded resource lookup, request_mem_region, ioremap and
> the manual iounmap/release_mem_region cleanup in probe/remove with the
> managed devm_platform_get_and_ioremap_resource() helper. This removes the

update commit message, no devm_platform_get_and_ioremap_resource() in patch.

> now-unused error-unmap/release paths and simplifies the driver.
>
> Adjust the goto labels in bcom_task_alloc to match what is being freed.
>
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
>  v2: resend.

If resend, don't pump version.

Frank
>  drivers/dma/bestcomm/bestcomm.c            | 47 ++++++++--------------
>  include/linux/fsl/bestcomm/bestcomm_priv.h |  2 +-
>  2 files changed, 18 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/dma/bestcomm/bestcomm.c b/drivers/dma/bestcomm/bestcomm.c
> index 241a91936fe8..1006030ff167 100644
> --- a/drivers/dma/bestcomm/bestcomm.c
> +++ b/drivers/dma/bestcomm/bestcomm.c
> @@ -47,6 +47,7 @@ bcom_task_alloc(int bd_count, int bd_size, int priv_size)
>  {
>  	int i, tasknum = -1;
>  	struct bcom_task *tsk;
> +	int irq;
>
>  	/* Don't try to do anything if bestcomm init failed */
>  	if (!bcom_eng)
> @@ -67,30 +68,32 @@ bcom_task_alloc(int bd_count, int bd_size, int priv_size)
>  	if (tasknum < 0)
>  		return NULL;
>
> +	irq = platform_get_irq(bcom_eng->pdev, tasknum);
> +	if (irq < 0)
> +		goto err1;
> +
>  	/* Allocate our structure */
>  	tsk = kzalloc(sizeof(struct bcom_task) + priv_size, GFP_KERNEL);
>  	if (!tsk)
> -		goto error;
> +		goto err1;
>
>  	tsk->tasknum = tasknum;
>  	if (priv_size)
>  		tsk->priv = (void*)tsk + sizeof(struct bcom_task);
>
>  	/* Get IRQ of that task */
> -	tsk->irq = irq_of_parse_and_map(bcom_eng->ofnode, tsk->tasknum);
> -	if (!tsk->irq)
> -		goto error;
> +	tsk->irq = irq;
>
>  	/* Init the BDs, if needed */
>  	if (bd_count) {
>  		tsk->cookie = kmalloc_array(bd_count, sizeof(void *),
>  					    GFP_KERNEL);
>  		if (!tsk->cookie)
> -			goto error;
> +			goto err2;
>
>  		tsk->bd = bcom_sram_alloc(bd_count * bd_size, 4, &tsk->bd_pa);
>  		if (!tsk->bd)
> -			goto error;
> +			goto err3;
>  		memset_io(tsk->bd, 0x00, bd_count * bd_size);
>
>  		tsk->num_bd = bd_count;
> @@ -99,17 +102,12 @@ bcom_task_alloc(int bd_count, int bd_size, int priv_size)
>
>  	return tsk;
>
> -error:
> -	if (tsk) {
> -		if (tsk->irq)
> -			irq_dispose_mapping(tsk->irq);
> -		bcom_sram_free(tsk->bd);
> -		kfree(tsk->cookie);
> -		kfree(tsk);
> -	}
> -
> +err3:
> +	kfree(tsk->cookie);
> +err2:
> +	kfree(tsk);
> +err1:
>  	bcom_eng->tdt[tasknum].stop = 0;
> -

avoid unnecesary change.

>  	return NULL;
>  }
>  EXPORT_SYMBOL_GPL(bcom_task_alloc);
> @@ -125,7 +123,6 @@ bcom_task_free(struct bcom_task *tsk)
>  	bcom_eng->tdt[tsk->tasknum].stop  = 0;
>
>  	/* Free everything */
> -	irq_dispose_mapping(tsk->irq);
>  	bcom_sram_free(tsk->bd);
>  	kfree(tsk->cookie);
>  	kfree(tsk);
> @@ -377,16 +374,12 @@ static int mpc52xx_bcom_probe(struct platform_device *op)
>  	if (IS_ERR(regs))
>  		return PTR_ERR(regs);
>
> -	/* Get the bestcomm node */
> -	of_node_get(op->dev.of_node);
> -
>  	/* Prepare SRAM */
>  	ofn_sram = of_find_matching_node(NULL, mpc52xx_sram_ids);
>  	if (!ofn_sram) {
>  		printk(KERN_ERR DRIVER_NAME ": "
>  			"No SRAM found in device tree\n");
> -		rv = -ENODEV;
> -		goto error_ofput;
> +		return -ENODEV;
>  	}
>  	rv = bcom_sram_init(ofn_sram, DRIVER_NAME);
>  	of_node_put(ofn_sram);
> @@ -394,7 +387,7 @@ static int mpc52xx_bcom_probe(struct platform_device *op)
>  	if (rv) {
>  		printk(KERN_ERR DRIVER_NAME ": "
>  			"Error in SRAM init\n");
> -		goto error_ofput;
> +		return rv;
>  	}
>
>  	/* Get a clean struct */
> @@ -405,8 +398,7 @@ static int mpc52xx_bcom_probe(struct platform_device *op)
>  	}
>
>  	/* Save the node */
> -	bcom_eng->ofnode = op->dev.of_node;
> -
> +	bcom_eng->pdev = op;
>  	bcom_eng->regs = regs;
>  	bcom_eng->regs_base = res_bcom->start;
>
> @@ -426,8 +418,6 @@ static int mpc52xx_bcom_probe(struct platform_device *op)
>  	kfree(bcom_eng);
>  	bcom_eng = NULL;
>  	bcom_sram_cleanup();
> -error_ofput:
> -	of_node_put(op->dev.of_node);
>
>  	printk(KERN_ERR "DMA: MPC52xx BestComm init failed !\n");
>
> @@ -443,9 +433,6 @@ static void mpc52xx_bcom_remove(struct platform_device *op)
>  	/* Cleanup SRAM */
>  	bcom_sram_cleanup();
>
> -	/* Release the node */
> -	of_node_put(bcom_eng->ofnode);
> -
>  	/* Release memory */
>  	kfree(bcom_eng);
>  	bcom_eng = NULL;
> diff --git a/include/linux/fsl/bestcomm/bestcomm_priv.h b/include/linux/fsl/bestcomm/bestcomm_priv.h
> index 3b52f3ffbdf8..cddde7c74cb5 100644
> --- a/include/linux/fsl/bestcomm/bestcomm_priv.h
> +++ b/include/linux/fsl/bestcomm/bestcomm_priv.h
> @@ -66,7 +66,7 @@ struct bcom_tdt {
>   * This holds all info needed globaly to handle the engine
>   */
>  struct bcom_engine {
> -	struct device_node		*ofnode;
> +	struct platform_device		*pdev;
>  	struct mpc52xx_sdma __iomem     *regs;
>  	phys_addr_t                      regs_base;
>
> --
> 2.55.0
>

Re: [PATCHv2] dmaengine: bestcomm: use platform machinery to get irq

[Rosen Penev]

On Wed, Sep 16, 2026 at 8:25 PM Frank Li <Frank.li@oss.nxp.com> wrote:
>
> On Wed, Sep 16, 2026 at 03:41:56PM -0700, Rosen Penev wrote:
> > platform_get_irq() avoids having to explicitly map and dispose of an irq
> > by having the platform_device machinery take care of it.
> >
> > This requires replacing the device_node with a platform_device in the
> > bcom_eng struct so that platform_get_irq() can use it.
> >
> > Conveniently, this also allows getting rid of of_node_get and
> > of_node_put as the platform_device machinery also handles that.
> >
> > Replace the open-coded resource lookup, request_mem_region, ioremap and
> > the manual iounmap/release_mem_region cleanup in probe/remove with the
> > managed devm_platform_get_and_ioremap_resource() helper. This removes the
>
> update commit message, no devm_platform_get_and_ioremap_resource() in patch.
>
> > now-unused error-unmap/release paths and simplifies the driver.
> >
> > Adjust the goto labels in bcom_task_alloc to match what is being freed.
> >
> > Signed-off-by: Rosen Penev <rosenp@gmail.com>
> > ---
> >  v2: resend.
>
> If resend, don't pump version.
>
> Frank
> >  drivers/dma/bestcomm/bestcomm.c            | 47 ++++++++--------------
> >  include/linux/fsl/bestcomm/bestcomm_priv.h |  2 +-
> >  2 files changed, 18 insertions(+), 31 deletions(-)
> >
> > diff --git a/drivers/dma/bestcomm/bestcomm.c b/drivers/dma/bestcomm/bestcomm.c
> > index 241a91936fe8..1006030ff167 100644
> > --- a/drivers/dma/bestcomm/bestcomm.c
> > +++ b/drivers/dma/bestcomm/bestcomm.c
> > @@ -47,6 +47,7 @@ bcom_task_alloc(int bd_count, int bd_size, int priv_size)
> >  {
> >       int i, tasknum = -1;
> >       struct bcom_task *tsk;
> > +     int irq;
> >
> >       /* Don't try to do anything if bestcomm init failed */
> >       if (!bcom_eng)
> > @@ -67,30 +68,32 @@ bcom_task_alloc(int bd_count, int bd_size, int priv_size)
> >       if (tasknum < 0)
> >               return NULL;
> >
> > +     irq = platform_get_irq(bcom_eng->pdev, tasknum);
> > +     if (irq < 0)
> > +             goto err1;
> > +
> >       /* Allocate our structure */
> >       tsk = kzalloc(sizeof(struct bcom_task) + priv_size, GFP_KERNEL);
> >       if (!tsk)
> > -             goto error;
> > +             goto err1;
> >
> >       tsk->tasknum = tasknum;
> >       if (priv_size)
> >               tsk->priv = (void*)tsk + sizeof(struct bcom_task);
> >
> >       /* Get IRQ of that task */
> > -     tsk->irq = irq_of_parse_and_map(bcom_eng->ofnode, tsk->tasknum);
> > -     if (!tsk->irq)
> > -             goto error;
> > +     tsk->irq = irq;
> >
> >       /* Init the BDs, if needed */
> >       if (bd_count) {
> >               tsk->cookie = kmalloc_array(bd_count, sizeof(void *),
> >                                           GFP_KERNEL);
> >               if (!tsk->cookie)
> > -                     goto error;
> > +                     goto err2;
> >
> >               tsk->bd = bcom_sram_alloc(bd_count * bd_size, 4, &tsk->bd_pa);
> >               if (!tsk->bd)
> > -                     goto error;
> > +                     goto err3;
> >               memset_io(tsk->bd, 0x00, bd_count * bd_size);
> >
> >               tsk->num_bd = bd_count;
> > @@ -99,17 +102,12 @@ bcom_task_alloc(int bd_count, int bd_size, int priv_size)
> >
> >       return tsk;
> >
> > -error:
> > -     if (tsk) {
> > -             if (tsk->irq)
> > -                     irq_dispose_mapping(tsk->irq);
> > -             bcom_sram_free(tsk->bd);
> > -             kfree(tsk->cookie);
> > -             kfree(tsk);
> > -     }
> > -
> > +err3:
> > +     kfree(tsk->cookie);
> > +err2:
> > +     kfree(tsk);
> > +err1:
> >       bcom_eng->tdt[tasknum].stop = 0;
> > -
>
> avoid unnecesary change.
There's nothing unnecessary about this. irq_dispose_mapping goes away.
bcom_sram_free is a mistake here. It's not reachable after
bcom_sram_alloc. This is much clearer to look at.

Moving bcom_sram_free removal to its own commit is pointless churn.
>
> >       return NULL;
> >  }
> >  EXPORT_SYMBOL_GPL(bcom_task_alloc);
> > @@ -125,7 +123,6 @@ bcom_task_free(struct bcom_task *tsk)
> >       bcom_eng->tdt[tsk->tasknum].stop  = 0;
> >
> >       /* Free everything */
> > -     irq_dispose_mapping(tsk->irq);
> >       bcom_sram_free(tsk->bd);
> >       kfree(tsk->cookie);
> >       kfree(tsk);
> > @@ -377,16 +374,12 @@ static int mpc52xx_bcom_probe(struct platform_device *op)
> >       if (IS_ERR(regs))
> >               return PTR_ERR(regs);
> >
> > -     /* Get the bestcomm node */
> > -     of_node_get(op->dev.of_node);
> > -
> >       /* Prepare SRAM */
> >       ofn_sram = of_find_matching_node(NULL, mpc52xx_sram_ids);
> >       if (!ofn_sram) {
> >               printk(KERN_ERR DRIVER_NAME ": "
> >                       "No SRAM found in device tree\n");
> > -             rv = -ENODEV;
> > -             goto error_ofput;
> > +             return -ENODEV;
> >       }
> >       rv = bcom_sram_init(ofn_sram, DRIVER_NAME);
> >       of_node_put(ofn_sram);
> > @@ -394,7 +387,7 @@ static int mpc52xx_bcom_probe(struct platform_device *op)
> >       if (rv) {
> >               printk(KERN_ERR DRIVER_NAME ": "
> >                       "Error in SRAM init\n");
> > -             goto error_ofput;
> > +             return rv;
> >       }
> >
> >       /* Get a clean struct */
> > @@ -405,8 +398,7 @@ static int mpc52xx_bcom_probe(struct platform_device *op)
> >       }
> >
> >       /* Save the node */
> > -     bcom_eng->ofnode = op->dev.of_node;
> > -
> > +     bcom_eng->pdev = op;
> >       bcom_eng->regs = regs;
> >       bcom_eng->regs_base = res_bcom->start;
> >
> > @@ -426,8 +418,6 @@ static int mpc52xx_bcom_probe(struct platform_device *op)
> >       kfree(bcom_eng);
> >       bcom_eng = NULL;
> >       bcom_sram_cleanup();
> > -error_ofput:
> > -     of_node_put(op->dev.of_node);
> >
> >       printk(KERN_ERR "DMA: MPC52xx BestComm init failed !\n");
> >
> > @@ -443,9 +433,6 @@ static void mpc52xx_bcom_remove(struct platform_device *op)
> >       /* Cleanup SRAM */
> >       bcom_sram_cleanup();
> >
> > -     /* Release the node */
> > -     of_node_put(bcom_eng->ofnode);
> > -
> >       /* Release memory */
> >       kfree(bcom_eng);
> >       bcom_eng = NULL;
> > diff --git a/include/linux/fsl/bestcomm/bestcomm_priv.h b/include/linux/fsl/bestcomm/bestcomm_priv.h
> > index 3b52f3ffbdf8..cddde7c74cb5 100644
> > --- a/include/linux/fsl/bestcomm/bestcomm_priv.h
> > +++ b/include/linux/fsl/bestcomm/bestcomm_priv.h
> > @@ -66,7 +66,7 @@ struct bcom_tdt {
> >   * This holds all info needed globaly to handle the engine
> >   */
> >  struct bcom_engine {
> > -     struct device_node              *ofnode;
> > +     struct platform_device          *pdev;
> >       struct mpc52xx_sdma __iomem     *regs;
> >       phys_addr_t                      regs_base;
> >
> > --
> > 2.55.0
> >

Re: [PATCHv2] dmaengine: bestcomm: use platform machinery to get irq

[Christophe Leroy (CS GROUP)]

Le 18/09/2026 à 02:16, Rosen Penev a écrit :
> On Wed, Sep 16, 2026 at 8:25 PM Frank Li <Frank.li@oss.nxp.com> wrote:
>>
>> On Wed, Sep 16, 2026 at 03:41:56PM -0700, Rosen Penev wrote:
>>> platform_get_irq() avoids having to explicitly map and dispose of an irq
>>> by having the platform_device machinery take care of it.
>>>
>>> This requires replacing the device_node with a platform_device in the
>>> bcom_eng struct so that platform_get_irq() can use it.
>>>
>>> Conveniently, this also allows getting rid of of_node_get and
>>> of_node_put as the platform_device machinery also handles that.
>>>
>>> Replace the open-coded resource lookup, request_mem_region, ioremap and
>>> the manual iounmap/release_mem_region cleanup in probe/remove with the
>>> managed devm_platform_get_and_ioremap_resource() helper. This removes the
>>
>> update commit message, no devm_platform_get_and_ioremap_resource() in patch.
>>
>>> now-unused error-unmap/release paths and simplifies the driver.
>>>
>>> Adjust the goto labels in bcom_task_alloc to match what is being freed.
>>>
>>> Signed-off-by: Rosen Penev <rosenp@gmail.com>
>>> ---
>>>   v2: resend.
>>
>> If resend, don't pump version.
>>
>> Frank
>>>   drivers/dma/bestcomm/bestcomm.c            | 47 ++++++++--------------
>>>   include/linux/fsl/bestcomm/bestcomm_priv.h |  2 +-
>>>   2 files changed, 18 insertions(+), 31 deletions(-)
>>>
>>> diff --git a/drivers/dma/bestcomm/bestcomm.c b/drivers/dma/bestcomm/bestcomm.c
>>> index 241a91936fe8..1006030ff167 100644
>>> --- a/drivers/dma/bestcomm/bestcomm.c
>>> +++ b/drivers/dma/bestcomm/bestcomm.c
>>> @@ -47,6 +47,7 @@ bcom_task_alloc(int bd_count, int bd_size, int priv_size)
>>>   {
>>>        int i, tasknum = -1;
>>>        struct bcom_task *tsk;
>>> +     int irq;
>>>
>>>        /* Don't try to do anything if bestcomm init failed */
>>>        if (!bcom_eng)
>>> @@ -67,30 +68,32 @@ bcom_task_alloc(int bd_count, int bd_size, int priv_size)
>>>        if (tasknum < 0)
>>>                return NULL;
>>>
>>> +     irq = platform_get_irq(bcom_eng->pdev, tasknum);
>>> +     if (irq < 0)
>>> +             goto err1;
>>> +
>>>        /* Allocate our structure */
>>>        tsk = kzalloc(sizeof(struct bcom_task) + priv_size, GFP_KERNEL);
>>>        if (!tsk)
>>> -             goto error;
>>> +             goto err1;
>>>
>>>        tsk->tasknum = tasknum;
>>>        if (priv_size)
>>>                tsk->priv = (void*)tsk + sizeof(struct bcom_task);
>>>
>>>        /* Get IRQ of that task */
>>> -     tsk->irq = irq_of_parse_and_map(bcom_eng->ofnode, tsk->tasknum);
>>> -     if (!tsk->irq)
>>> -             goto error;
>>> +     tsk->irq = irq;
>>>
>>>        /* Init the BDs, if needed */
>>>        if (bd_count) {
>>>                tsk->cookie = kmalloc_array(bd_count, sizeof(void *),
>>>                                            GFP_KERNEL);
>>>                if (!tsk->cookie)
>>> -                     goto error;
>>> +                     goto err2;
>>>
>>>                tsk->bd = bcom_sram_alloc(bd_count * bd_size, 4, &tsk->bd_pa);
>>>                if (!tsk->bd)
>>> -                     goto error;
>>> +                     goto err3;
>>>                memset_io(tsk->bd, 0x00, bd_count * bd_size);
>>>
>>>                tsk->num_bd = bd_count;
>>> @@ -99,17 +102,12 @@ bcom_task_alloc(int bd_count, int bd_size, int priv_size)
>>>
>>>        return tsk;
>>>
>>> -error:
>>> -     if (tsk) {
>>> -             if (tsk->irq)
>>> -                     irq_dispose_mapping(tsk->irq);
>>> -             bcom_sram_free(tsk->bd);
>>> -             kfree(tsk->cookie);
>>> -             kfree(tsk);
>>> -     }
>>> -
>>> +err3:
>>> +     kfree(tsk->cookie);
>>> +err2:
>>> +     kfree(tsk);
>>> +err1:
>>>        bcom_eng->tdt[tasknum].stop = 0;
>>> -
>>
>> avoid unnecesary change.
> There's nothing unnecessary about this. irq_dispose_mapping goes away.
> bcom_sram_free is a mistake here. It's not reachable after
> bcom_sram_alloc. This is much clearer to look at.

The comment is about the removal of the blank line. Nothing else. I 
confirm it is unnecessary.

> 
> Moving bcom_sram_free removal to its own commit is pointless churn.
>>
>>>        return NULL;
>>>   }
>>>   EXPORT_SYMBOL_GPL(bcom_task_alloc);
>>> @@ -125,7 +123,6 @@ bcom_task_free(struct bcom_task *tsk)
>>>        bcom_eng->tdt[tsk->tasknum].stop  = 0;
>>>
>>>        /* Free everything */
>>> -     irq_dispose_mapping(tsk->irq);
>>>        bcom_sram_free(tsk->bd);
>>>        kfree(tsk->cookie);
>>>        kfree(tsk);
>>> @@ -377,16 +374,12 @@ static int mpc52xx_bcom_probe(struct platform_device *op)
>>>        if (IS_ERR(regs))
>>>                return PTR_ERR(regs);
>>>
>>> -     /* Get the bestcomm node */
>>> -     of_node_get(op->dev.of_node);
>>> -
>>>        /* Prepare SRAM */
>>>        ofn_sram = of_find_matching_node(NULL, mpc52xx_sram_ids);
>>>        if (!ofn_sram) {
>>>                printk(KERN_ERR DRIVER_NAME ": "
>>>                        "No SRAM found in device tree\n");
>>> -             rv = -ENODEV;
>>> -             goto error_ofput;
>>> +             return -ENODEV;
>>>        }
>>>        rv = bcom_sram_init(ofn_sram, DRIVER_NAME);
>>>        of_node_put(ofn_sram);
>>> @@ -394,7 +387,7 @@ static int mpc52xx_bcom_probe(struct platform_device *op)
>>>        if (rv) {
>>>                printk(KERN_ERR DRIVER_NAME ": "
>>>                        "Error in SRAM init\n");
>>> -             goto error_ofput;
>>> +             return rv;
>>>        }
>>>
>>>        /* Get a clean struct */
>>> @@ -405,8 +398,7 @@ static int mpc52xx_bcom_probe(struct platform_device *op)
>>>        }
>>>
>>>        /* Save the node */
>>> -     bcom_eng->ofnode = op->dev.of_node;
>>> -
>>> +     bcom_eng->pdev = op;
>>>        bcom_eng->regs = regs;
>>>        bcom_eng->regs_base = res_bcom->start;
>>>
>>> @@ -426,8 +418,6 @@ static int mpc52xx_bcom_probe(struct platform_device *op)
>>>        kfree(bcom_eng);
>>>        bcom_eng = NULL;
>>>        bcom_sram_cleanup();
>>> -error_ofput:
>>> -     of_node_put(op->dev.of_node);
>>>
>>>        printk(KERN_ERR "DMA: MPC52xx BestComm init failed !\n");
>>>
>>> @@ -443,9 +433,6 @@ static void mpc52xx_bcom_remove(struct platform_device *op)
>>>        /* Cleanup SRAM */
>>>        bcom_sram_cleanup();
>>>
>>> -     /* Release the node */
>>> -     of_node_put(bcom_eng->ofnode);
>>> -
>>>        /* Release memory */
>>>        kfree(bcom_eng);
>>>        bcom_eng = NULL;
>>> diff --git a/include/linux/fsl/bestcomm/bestcomm_priv.h b/include/linux/fsl/bestcomm/bestcomm_priv.h
>>> index 3b52f3ffbdf8..cddde7c74cb5 100644
>>> --- a/include/linux/fsl/bestcomm/bestcomm_priv.h
>>> +++ b/include/linux/fsl/bestcomm/bestcomm_priv.h
>>> @@ -66,7 +66,7 @@ struct bcom_tdt {
>>>    * This holds all info needed globaly to handle the engine
>>>    */
>>>   struct bcom_engine {
>>> -     struct device_node              *ofnode;
>>> +     struct platform_device          *pdev;
>>>        struct mpc52xx_sdma __iomem     *regs;
>>>        phys_addr_t                      regs_base;
>>>
>>> --
>>> 2.55.0
>>>