Hi Sachin,

The comments from Sylwester also apply to the patches you have sent to MFC
and G2D.

I don't see the benefit of switching to devm_clk_get(), it does some
allocations and hence uses more resources. Leaving that aside there is no
support for devm_clk_prepare and I would rather wait until it is included in
the kernel.

Best wishes,
-- 
Kamil Debski
Linux Platform Group
Samsung Poland R&D Center


> -----Original Message-----
> From: linux-media-ow...@vger.kernel.org [mailto:linux-media-
> ow...@vger.kernel.org] On Behalf Of Sachin Kamat
> Sent: Friday, November 23, 2012 12:51 PM
> To: linux-media@vger.kernel.org
> Cc: s.nawro...@samsung.com; sachin.ka...@linaro.org;
> patc...@linaro.org; Kamil Debski
> Subject: [PATCH 4/6] [media] s5p-g2d: Use devm_clk_get APIs.
> 
> devm_clk_get() is device managed function and makes error handling
> and exit code a bit simpler.
> 
> Cc: Kamil Debski <k.deb...@samsung.com>
> Signed-off-by: Sachin Kamat <sachin.ka...@linaro.org>
> ---
>  drivers/media/platform/s5p-g2d/g2d.c |   14 ++++----------
>  1 files changed, 4 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/media/platform/s5p-g2d/g2d.c
> b/drivers/media/platform/s5p-g2d/g2d.c
> index 1bfbc32..77819d3 100644
> --- a/drivers/media/platform/s5p-g2d/g2d.c
> +++ b/drivers/media/platform/s5p-g2d/g2d.c
> @@ -714,7 +714,7 @@ static int g2d_probe(struct platform_device *pdev)
>                       return -ENOENT;
>       }
> 
> -     dev->clk = clk_get(&pdev->dev, "sclk_fimg2d");
> +     dev->clk = devm_clk_get(&pdev->dev, "sclk_fimg2d");
>       if (IS_ERR_OR_NULL(dev->clk)) {
>               dev_err(&pdev->dev, "failed to get g2d clock\n");
>               return -ENXIO;
> @@ -726,7 +726,7 @@ static int g2d_probe(struct platform_device *pdev)
>               goto put_clk;
>       }
> 
> -     dev->gate = clk_get(&pdev->dev, "fimg2d");
> +     dev->gate = devm_clk_get(&pdev->dev, "fimg2d");
>       if (IS_ERR_OR_NULL(dev->gate)) {
>               dev_err(&pdev->dev, "failed to get g2d clock gate\n");
>               ret = -ENXIO;
> @@ -736,7 +736,7 @@ static int g2d_probe(struct platform_device *pdev)
>       ret = clk_prepare(dev->gate);
>       if (ret) {
>               dev_err(&pdev->dev, "failed to prepare g2d clock gate\n");
> -             goto put_clk_gate;
> +             goto unprep_clk;
>       }
> 
>       res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> @@ -752,7 +752,7 @@ static int g2d_probe(struct platform_device *pdev)
>                                               0, pdev->name, dev);
>       if (ret) {
>               dev_err(&pdev->dev, "failed to install IRQ\n");
> -             goto put_clk_gate;
> +             goto unprep_clk;
>       }
> 
>       dev->alloc_ctx = vb2_dma_contig_init_ctx(&pdev->dev);
> @@ -804,13 +804,9 @@ alloc_ctx_cleanup:
>       vb2_dma_contig_cleanup_ctx(dev->alloc_ctx);
>  unprep_clk_gate:
>       clk_unprepare(dev->gate);
> -put_clk_gate:
> -     clk_put(dev->gate);
>  unprep_clk:
>       clk_unprepare(dev->clk);
>  put_clk:
> -     clk_put(dev->clk);
> -
>       return ret;
>  }
> 
> @@ -824,9 +820,7 @@ static int g2d_remove(struct platform_device *pdev)
>       v4l2_device_unregister(&dev->v4l2_dev);
>       vb2_dma_contig_cleanup_ctx(dev->alloc_ctx);
>       clk_unprepare(dev->gate);
> -     clk_put(dev->gate);
>       clk_unprepare(dev->clk);
> -     clk_put(dev->clk);
>       return 0;
>  }
> 
> --
> 1.7.4.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media"
> in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to