| Message ID | 20250120135059.302273-1-mordan@ispras.ru (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | dma: jz4780: fix call balance of jzdma->clk handling routines | expand |
Hi Vitalii, Le lundi 20 janvier 2025 à 16:50 +0300, Vitalii Mordan a écrit : > If the clock jzdma->clk was not enabled in jz4780_dma_probe(), it > should > not be disabled in any path. > > Conversely, if it was enabled in jz4780_dma_probe(), it must be > disabled > in all error paths to ensure proper cleanup. > > Use the devm_clk_get_enabled() helper function to ensure proper call > balance for jzdma->clk. > > Found by Linux Verification Center (linuxtesting.org) with Klever. > > Fixes: d894fc6046fe ("dmaengine: jz4780: add driver for the Ingenic > JZ4780 DMA controller") > Signed-off-by: Vitalii Mordan <mordan@ispras.ru> Acked-by: Paul Cercueil <paul@crapouillou.net> Cheers, -Paul > --- > drivers/dma/dma-jz4780.c | 14 ++++---------- > 1 file changed, 4 insertions(+), 10 deletions(-) > > diff --git a/drivers/dma/dma-jz4780.c b/drivers/dma/dma-jz4780.c > index 100057603fd4..ff9c387fd8c1 100644 > --- a/drivers/dma/dma-jz4780.c > +++ b/drivers/dma/dma-jz4780.c > @@ -896,15 +896,13 @@ static int jz4780_dma_probe(struct > platform_device *pdev) > return -EINVAL; > } > > - jzdma->clk = devm_clk_get(dev, NULL); > + jzdma->clk = devm_clk_get_enabled(dev, NULL); > if (IS_ERR(jzdma->clk)) { > - dev_err(dev, "failed to get clock\n"); > + dev_err(dev, "failed to get and enable clock\n"); > ret = PTR_ERR(jzdma->clk); > return ret; > } > > - clk_prepare_enable(jzdma->clk); > - > /* Property is optional, if it doesn't exist the value will > remain 0. */ > of_property_read_u32_index(dev->of_node, "ingenic,reserved- > channels", > 0, &jzdma->chan_reserved); > @@ -972,7 +970,7 @@ static int jz4780_dma_probe(struct > platform_device *pdev) > > ret = platform_get_irq(pdev, 0); > if (ret < 0) > - goto err_disable_clk; > + return ret; > > jzdma->irq = ret; > > @@ -980,7 +978,7 @@ static int jz4780_dma_probe(struct > platform_device *pdev) > jzdma); > if (ret) { > dev_err(dev, "failed to request IRQ %u!\n", jzdma- > >irq); > - goto err_disable_clk; > + return ret; > } > > ret = dmaenginem_async_device_register(dd); > @@ -1002,9 +1000,6 @@ static int jz4780_dma_probe(struct > platform_device *pdev) > > err_free_irq: > free_irq(jzdma->irq, jzdma); > - > -err_disable_clk: > - clk_disable_unprepare(jzdma->clk); > return ret; > } > > @@ -1015,7 +1010,6 @@ static void jz4780_dma_remove(struct > platform_device *pdev) > > of_dma_controller_free(pdev->dev.of_node); > > - clk_disable_unprepare(jzdma->clk); > free_irq(jzdma->irq, jzdma); > > for (i = 0; i < jzdma->soc_data->nb_channels; i++)
diff --git a/drivers/dma/dma-jz4780.c b/drivers/dma/dma-jz4780.c index 100057603fd4..ff9c387fd8c1 100644 --- a/drivers/dma/dma-jz4780.c +++ b/drivers/dma/dma-jz4780.c @@ -896,15 +896,13 @@ static int jz4780_dma_probe(struct platform_device *pdev) return -EINVAL; } - jzdma->clk = devm_clk_get(dev, NULL); + jzdma->clk = devm_clk_get_enabled(dev, NULL); if (IS_ERR(jzdma->clk)) { - dev_err(dev, "failed to get clock\n"); + dev_err(dev, "failed to get and enable clock\n"); ret = PTR_ERR(jzdma->clk); return ret; } - clk_prepare_enable(jzdma->clk); - /* Property is optional, if it doesn't exist the value will remain 0. */ of_property_read_u32_index(dev->of_node, "ingenic,reserved-channels", 0, &jzdma->chan_reserved); @@ -972,7 +970,7 @@ static int jz4780_dma_probe(struct platform_device *pdev) ret = platform_get_irq(pdev, 0); if (ret < 0) - goto err_disable_clk; + return ret; jzdma->irq = ret; @@ -980,7 +978,7 @@ static int jz4780_dma_probe(struct platform_device *pdev) jzdma); if (ret) { dev_err(dev, "failed to request IRQ %u!\n", jzdma->irq); - goto err_disable_clk; + return ret; } ret = dmaenginem_async_device_register(dd); @@ -1002,9 +1000,6 @@ static int jz4780_dma_probe(struct platform_device *pdev) err_free_irq: free_irq(jzdma->irq, jzdma); - -err_disable_clk: - clk_disable_unprepare(jzdma->clk); return ret; } @@ -1015,7 +1010,6 @@ static void jz4780_dma_remove(struct platform_device *pdev) of_dma_controller_free(pdev->dev.of_node); - clk_disable_unprepare(jzdma->clk); free_irq(jzdma->irq, jzdma); for (i = 0; i < jzdma->soc_data->nb_channels; i++)
If the clock jzdma->clk was not enabled in jz4780_dma_probe(), it should not be disabled in any path. Conversely, if it was enabled in jz4780_dma_probe(), it must be disabled in all error paths to ensure proper cleanup. Use the devm_clk_get_enabled() helper function to ensure proper call balance for jzdma->clk. Found by Linux Verification Center (linuxtesting.org) with Klever. Fixes: d894fc6046fe ("dmaengine: jz4780: add driver for the Ingenic JZ4780 DMA controller") Signed-off-by: Vitalii Mordan <mordan@ispras.ru> --- drivers/dma/dma-jz4780.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-)