| Message ID | 20241101101410.1449891-2-peng.fan@oss.nxp.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] dmaengine: fsl-edma: cleanup chan after dma_async_device_unregister | expand |
On Fri, Nov 01, 2024 at 06:14:10PM +0800, Peng Fan (OSS) wrote: > From: Peng Fan <peng.fan@nxp.com> > > To i.MX9, there is no valid fsl_edma->txirq/errirq, so add a check in > fsl_edma_irq_exit to avoid issues. > > Fixes: 44eb827264de ("dmaengine: fsl-edma: request per-channel IRQ only when channel is allocated") > Signed-off-by: Peng Fan <peng.fan@nxp.com> > --- > drivers/dma/fsl-edma-main.c | 12 +++++++++--- > 1 file changed, 9 insertions(+), 3 deletions(-) > > diff --git a/drivers/dma/fsl-edma-main.c b/drivers/dma/fsl-edma-main.c > index 01bd5cb24a49..89c54eeb4925 100644 > --- a/drivers/dma/fsl-edma-main.c > +++ b/drivers/dma/fsl-edma-main.c > @@ -303,6 +303,7 @@ fsl_edma2_irq_init(struct platform_device *pdev, > > /* The last IRQ is for eDMA err */ > if (i == count - 1) { > + fsl_edma->errirq = irq; > ret = devm_request_irq(&pdev->dev, irq, > fsl_edma_err_handler, > 0, "eDMA2-ERR", fsl_edma); > @@ -322,10 +323,13 @@ static void fsl_edma_irq_exit( > struct platform_device *pdev, struct fsl_edma_engine *fsl_edma) > { > if (fsl_edma->txirq == fsl_edma->errirq) { > - devm_free_irq(&pdev->dev, fsl_edma->txirq, fsl_edma); > + if (fsl_edma->txirq >= 0) > + devm_free_irq(&pdev->dev, fsl_edma->txirq, fsl_edma); > } else { > - devm_free_irq(&pdev->dev, fsl_edma->txirq, fsl_edma); > - devm_free_irq(&pdev->dev, fsl_edma->errirq, fsl_edma); > + if (fsl_edma->txirq >= 0) > + devm_free_irq(&pdev->dev, fsl_edma->txirq, fsl_edma); > + if (fsl_edma->errirq >= 0) > + devm_free_irq(&pdev->dev, fsl_edma->errirq, fsl_edma); > } > } > > @@ -485,6 +489,8 @@ static int fsl_edma_probe(struct platform_device *pdev) > if (!fsl_edma) > return -ENOMEM; > > + fsl_edma->errirq = -EINVAL; > + fsl_edma->txirq = -EINVAL; why not use 0 as invalidate? Frank > fsl_edma->drvdata = drvdata; > fsl_edma->n_chans = chans; > mutex_init(&fsl_edma->fsl_edma_mutex); > -- > 2.37.1 >
> Subject: Re: [PATCH 2/2] dmaengine: fsl-edma: free irq correctly in > remove pathy > > On Fri, Nov 01, 2024 at 06:14:10PM +0800, Peng Fan (OSS) wrote: > > From: Peng Fan <peng.fan@nxp.com> > > > > To i.MX9, there is no valid fsl_edma->txirq/errirq, so add a check in > > fsl_edma_irq_exit to avoid issues. > > > > Fixes: 44eb827264de ("dmaengine: fsl-edma: request per-channel > IRQ > > only when channel is allocated") > > Signed-off-by: Peng Fan <peng.fan@nxp.com> > > --- > > drivers/dma/fsl-edma-main.c | 12 +++++++++--- > > 1 file changed, 9 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/dma/fsl-edma-main.c b/drivers/dma/fsl-edma- > main.c > > index 01bd5cb24a49..89c54eeb4925 100644 > > --- a/drivers/dma/fsl-edma-main.c > > +++ b/drivers/dma/fsl-edma-main.c > > @@ -303,6 +303,7 @@ fsl_edma2_irq_init(struct platform_device > *pdev, > > > > /* The last IRQ is for eDMA err */ > > if (i == count - 1) { > > + fsl_edma->errirq = irq; > > ret = devm_request_irq(&pdev->dev, irq, > > > fsl_edma_err_handler, > > 0, "eDMA2-ERR", > fsl_edma); > > @@ -322,10 +323,13 @@ static void fsl_edma_irq_exit( > > struct platform_device *pdev, struct fsl_edma_engine > *fsl_edma) { > > if (fsl_edma->txirq == fsl_edma->errirq) { > > - devm_free_irq(&pdev->dev, fsl_edma->txirq, > fsl_edma); > > + if (fsl_edma->txirq >= 0) > > + devm_free_irq(&pdev->dev, fsl_edma->txirq, > fsl_edma); > > } else { > > - devm_free_irq(&pdev->dev, fsl_edma->txirq, > fsl_edma); > > - devm_free_irq(&pdev->dev, fsl_edma->errirq, > fsl_edma); > > + if (fsl_edma->txirq >= 0) > > + devm_free_irq(&pdev->dev, fsl_edma->txirq, > fsl_edma); > > + if (fsl_edma->errirq >= 0) > > + devm_free_irq(&pdev->dev, fsl_edma->errirq, > fsl_edma); > > } > > } > > > > @@ -485,6 +489,8 @@ static int fsl_edma_probe(struct > platform_device *pdev) > > if (!fsl_edma) > > return -ENOMEM; > > > > + fsl_edma->errirq = -EINVAL; > > + fsl_edma->txirq = -EINVAL; > > why not use 0 as invalidate? I think 0 might be a valid value. So use -EINVAL here. Thanks, Peng. > > Frank > > > fsl_edma->drvdata = drvdata; > > fsl_edma->n_chans = chans; > > mutex_init(&fsl_edma->fsl_edma_mutex); > > -- > > 2.37.1 > >
diff --git a/drivers/dma/fsl-edma-main.c b/drivers/dma/fsl-edma-main.c index 01bd5cb24a49..89c54eeb4925 100644 --- a/drivers/dma/fsl-edma-main.c +++ b/drivers/dma/fsl-edma-main.c @@ -303,6 +303,7 @@ fsl_edma2_irq_init(struct platform_device *pdev, /* The last IRQ is for eDMA err */ if (i == count - 1) { + fsl_edma->errirq = irq; ret = devm_request_irq(&pdev->dev, irq, fsl_edma_err_handler, 0, "eDMA2-ERR", fsl_edma); @@ -322,10 +323,13 @@ static void fsl_edma_irq_exit( struct platform_device *pdev, struct fsl_edma_engine *fsl_edma) { if (fsl_edma->txirq == fsl_edma->errirq) { - devm_free_irq(&pdev->dev, fsl_edma->txirq, fsl_edma); + if (fsl_edma->txirq >= 0) + devm_free_irq(&pdev->dev, fsl_edma->txirq, fsl_edma); } else { - devm_free_irq(&pdev->dev, fsl_edma->txirq, fsl_edma); - devm_free_irq(&pdev->dev, fsl_edma->errirq, fsl_edma); + if (fsl_edma->txirq >= 0) + devm_free_irq(&pdev->dev, fsl_edma->txirq, fsl_edma); + if (fsl_edma->errirq >= 0) + devm_free_irq(&pdev->dev, fsl_edma->errirq, fsl_edma); } } @@ -485,6 +489,8 @@ static int fsl_edma_probe(struct platform_device *pdev) if (!fsl_edma) return -ENOMEM; + fsl_edma->errirq = -EINVAL; + fsl_edma->txirq = -EINVAL; fsl_edma->drvdata = drvdata; fsl_edma->n_chans = chans; mutex_init(&fsl_edma->fsl_edma_mutex);