On Fri, Jul 08, 2016 at 11:54:17AM +0800, Zhangfei Gao wrote:
> On 8 July 2016 at 11:51, Vinod Koul <vinod.koul@xxxxxxxxx> wrote:
> > On Fri, Jul 08, 2016 at 10:40:42AM +0800, Zhangfei Gao wrote:
> >> Dear Vinod
> >>
> >> On 5 July 2016 at 22:54, Vinod Koul <vinod.koul@xxxxxxxxx> wrote:
> >> > dmaengine device should explicitly call devm_free_irq() when using
> >> > devm_reqister_irq().
> >> >
> >> > The irq is still ON when devices remove is executed and irq should be
> >> > quiesced before remove is completed.
> >> >
> >> > Signed-off-by: Vinod Koul <vinod.koul@xxxxxxxxx>
> >> > Cc: Zhangfei Gao <zhangfei.gao@xxxxxxxxxx>
> >> > Cc: Daniel Mack <zonque@xxxxxxxxx>
> >> > ---
> >> > drivers/dma/mmp_pdma.c | 17 +++++++++++++++++
> >> > 1 file changed, 17 insertions(+)
> >> >
> >> > diff --git a/drivers/dma/mmp_pdma.c b/drivers/dma/mmp_pdma.c
> >> > index 56f1fd68b620..975bc4bf525b 100644
> >> > --- a/drivers/dma/mmp_pdma.c
> >> > +++ b/drivers/dma/mmp_pdma.c
> >> > @@ -931,6 +931,23 @@ static void dma_do_tasklet(unsigned long data)
> >> > static int mmp_pdma_remove(struct platform_device *op)
> >> > {
> >> > struct mmp_pdma_device *pdev = platform_get_drvdata(op);
> >> > + struct mmp_pdma_phy *phy;
> >> > + int i, irq = 0, irq_num = 0;
> >> > +
> >> > +
> >> > + irq = platform_get_irq(op, 0);
> >> > + devm_free_irq(&op->dev, irq, pdev);
> >> > +
> >> > + for (i = 0; i < pdev->dma_channels; i++) {
> >> > + if (platform_get_irq(op, i) > 0)
> >> > + irq_num++;
> >> > + }
> >> > +
> >> > + for (i = 0; i < pdev->dma_channels; i++) {
> >> > + phy = &pdev->phy[i];
> >> > + irq = (irq_num != pdev->dma_channels) ? 0 : platform_get_irq(op, i);
> >> > + devm_free_irq(&op->dev, irq, phy);
> >> > + }
> >> >
> >> Is it possible irq=0 is freed twice?
> >
> > Yes I see the point, we should do:
> >
> > for (i = 0; i < pdev->dma_channels; i++) {
> > if (platform_get_irq(op, i) > 0)
> > irq_num++;
> > }
> >
> > if (irq_num != pdev->dma_channels) {
> > irq = platform_get_irq(op, 0);
> > devm_free_irq(&op->dev, irq, pdev);
> > }
> > else {
> > for (i = 0; i < pdev->dma_channels; i++) {
> > phy = &pdev->phy[i];
> > irq = (irq_num != pdev->dma_channels) ? 0 : platform_get_irq(op, i);
>
> here irq = platform_get_irq(op, i);
> should be OK.
sounds good to me
comitting this now:
-- >8 --
diff --git a/drivers/dma/mmp_pdma.c b/drivers/dma/mmp_pdma.c
index 56f1fd68b620..f4b25fb0d040 100644
--- a/drivers/dma/mmp_pdma.c
+++ b/drivers/dma/mmp_pdma.c
@@ -931,6 +931,25 @@ static void dma_do_tasklet(unsigned long data)
static int mmp_pdma_remove(struct platform_device *op)
{
struct mmp_pdma_device *pdev = platform_get_drvdata(op);
+ struct mmp_pdma_phy *phy;
+ int i, irq = 0, irq_num = 0;
+
+
+ for (i = 0; i < pdev->dma_channels; i++) {
+ if (platform_get_irq(op, i) > 0)
+ irq_num++;
+ }
+
+ if (irq_num != pdev->dma_channels) {
+ irq = platform_get_irq(op, 0);
+ devm_free_irq(&op->dev, irq, pdev);
+ } else {
+ for (i = 0; i < pdev->dma_channels; i++) {
+ phy = &pdev->phy[i];
+ irq = platform_get_irq(op, i);
+ devm_free_irq(&op->dev, irq, phy);
+ }
+ }
--
~Vinod
--
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
