| Message ID | 20241111072602.1179457-2-peng.fan@oss.nxp.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [V2,1/2] dmaengine: fsl-edma: cleanup chan after dma_async_device_unregister | expand |
On Mon, Nov 11, 2024 at 03:26:01PM +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. Nik: can you add descript about what's issues? > > Fixes: 44eb827264de ("dmaengine: fsl-edma: request per-channel IRQ only when channel is allocated") > Signed-off-by: Peng Fan <peng.fan@nxp.com> > --- > > V2: > None > > 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; > fsl_edma->drvdata = drvdata; > fsl_edma->n_chans = chans; > mutex_init(&fsl_edma->fsl_edma_mutex); > -- > 2.37.1 >
> Subject: Re: [PATCH V2 2/2] dmaengine: fsl-edma: free irq correctly in > remove pathy > > On Mon, Nov 11, 2024 at 03:26:01PM +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. > > Nik: can you add descript about what's issues? It should not free an irq that was not requested. So I add a check here. You wanna to me to add the kernel dump or log in commit log? Thanks, Peng. > > > > > Fixes: 44eb827264de ("dmaengine: fsl-edma: request per-channel > IRQ > > only when channel is allocated") > > Signed-off-by: Peng Fan <peng.fan@nxp.com> > > --- > > > > V2: > > None > > > > 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; > > fsl_edma->drvdata = drvdata; > > fsl_edma->n_chans = chans; > > mutex_init(&fsl_edma->fsl_edma_mutex); > > -- > > 2.37.1 > >
On Tue, Nov 12, 2024 at 01:52:41AM +0000, Peng Fan wrote: > > Subject: Re: [PATCH V2 2/2] dmaengine: fsl-edma: free irq correctly in > > remove pathy > > > > On Mon, Nov 11, 2024 at 03:26:01PM +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. > > > > Nik: can you add descript about what's issues? > > It should not free an irq that was not requested. So I add a check here. > You wanna to me to add the kernel dump or log in commit log? Yes, at least need descript the issue. such as wrong free the no requested irq... > > Thanks, > Peng. > > > > > > > > > Fixes: 44eb827264de ("dmaengine: fsl-edma: request per-channel > > IRQ > > > only when channel is allocated") > > > Signed-off-by: Peng Fan <peng.fan@nxp.com> > > > --- > > > > > > V2: > > > None > > > > > > 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; > > > 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);