public inbox for linux-media@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/6][media] s5p-*: Use devm_clk_get APIs
@ 2012-11-23 11:50 Sachin Kamat
  2012-11-23 11:50 ` [PATCH 1/6] [media] s5p-fimc: Use devm_clk_get in mipi-csis.c Sachin Kamat
                   ` (5 more replies)
  0 siblings, 6 replies; 11+ messages in thread
From: Sachin Kamat @ 2012-11-23 11:50 UTC (permalink / raw)
  To: linux-media; +Cc: s.nawrocki, sachin.kamat, patches

This series is based on samsung/for_v3.8 branch of
git://linuxtv.org/snawrocki/media.git tree.

MFC and FIMC have been tested on Origen board.
Others build tested.

Sachin Kamat (6):
  [media] s5p-fimc: Use devm_clk_get in mipi-csis.c
  [media] s5p-fimc: Use devm_clk_get in fimc-core.c
  [media] s5p-fimc: Use devm_clk_get in fimc-lite.c
  [media] s5p-g2d: Use devm_clk_get APIs.
  [media] s5p-jpeg: Use devm_clk_get APIs.
  [media] s5p-mfc: Use devm_clk_get APIs

 drivers/media/platform/s5p-fimc/fimc-core.c |   10 ++--------
 drivers/media/platform/s5p-fimc/fimc-lite.c |    8 +-------
 drivers/media/platform/s5p-fimc/mipi-csis.c |    6 +-----
 drivers/media/platform/s5p-g2d/g2d.c        |   14 ++++----------
 drivers/media/platform/s5p-jpeg/jpeg-core.c |    4 +---
 drivers/media/platform/s5p-mfc/s5p_mfc_pm.c |   14 ++++----------
 6 files changed, 13 insertions(+), 43 deletions(-)

-- 
1.7.4.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/6] [media] s5p-fimc: Use devm_clk_get in mipi-csis.c
  2012-11-23 11:50 [PATCH 0/6][media] s5p-*: Use devm_clk_get APIs Sachin Kamat
@ 2012-11-23 11:50 ` Sachin Kamat
  2012-11-25 15:32   ` Sylwester Nawrocki
  2012-11-23 11:50 ` [PATCH 2/6] [media] s5p-fimc: Use devm_clk_get in fimc-core.c Sachin Kamat
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 11+ messages in thread
From: Sachin Kamat @ 2012-11-23 11:50 UTC (permalink / raw)
  To: linux-media; +Cc: s.nawrocki, sachin.kamat, patches

devm_clk_get is device managed and makes error handling and cleanup
a bit simpler.

Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
---
 drivers/media/platform/s5p-fimc/mipi-csis.c |    6 +-----
 1 files changed, 1 insertions(+), 5 deletions(-)

diff --git a/drivers/media/platform/s5p-fimc/mipi-csis.c b/drivers/media/platform/s5p-fimc/mipi-csis.c
index 4c961b1..d624bfa 100644
--- a/drivers/media/platform/s5p-fimc/mipi-csis.c
+++ b/drivers/media/platform/s5p-fimc/mipi-csis.c
@@ -341,8 +341,6 @@ static void s5pcsis_clk_put(struct csis_state *state)
 		if (IS_ERR_OR_NULL(state->clock[i]))
 			continue;
 		clk_unprepare(state->clock[i]);
-		clk_put(state->clock[i]);
-		state->clock[i] = NULL;
 	}
 }
 
@@ -352,13 +350,11 @@ static int s5pcsis_clk_get(struct csis_state *state)
 	int i, ret;
 
 	for (i = 0; i < NUM_CSIS_CLOCKS; i++) {
-		state->clock[i] = clk_get(dev, csi_clock_name[i]);
+		state->clock[i] = devm_clk_get(dev, csi_clock_name[i]);
 		if (IS_ERR(state->clock[i]))
 			goto err;
 		ret = clk_prepare(state->clock[i]);
 		if (ret < 0) {
-			clk_put(state->clock[i]);
-			state->clock[i] = NULL;
 			goto err;
 		}
 	}
-- 
1.7.4.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/6] [media] s5p-fimc: Use devm_clk_get in fimc-core.c
  2012-11-23 11:50 [PATCH 0/6][media] s5p-*: Use devm_clk_get APIs Sachin Kamat
  2012-11-23 11:50 ` [PATCH 1/6] [media] s5p-fimc: Use devm_clk_get in mipi-csis.c Sachin Kamat
@ 2012-11-23 11:50 ` Sachin Kamat
  2012-11-23 11:50 ` [PATCH 3/6] [media] s5p-fimc: Use devm_clk_get in fimc-lite.c Sachin Kamat
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Sachin Kamat @ 2012-11-23 11:50 UTC (permalink / raw)
  To: linux-media; +Cc: s.nawrocki, sachin.kamat, patches

devm_clk_get is device managed and makes error handling and cleanup
a bit simpler.

Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
---
 drivers/media/platform/s5p-fimc/fimc-core.c |   10 ++--------
 1 files changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/media/platform/s5p-fimc/fimc-core.c b/drivers/media/platform/s5p-fimc/fimc-core.c
index 8d0d2b9..0c45127 100644
--- a/drivers/media/platform/s5p-fimc/fimc-core.c
+++ b/drivers/media/platform/s5p-fimc/fimc-core.c
@@ -814,8 +814,6 @@ static void fimc_clk_put(struct fimc_dev *fimc)
 		if (IS_ERR_OR_NULL(fimc->clock[i]))
 			continue;
 		clk_unprepare(fimc->clock[i]);
-		clk_put(fimc->clock[i]);
-		fimc->clock[i] = NULL;
 	}
 }
 
@@ -824,19 +822,15 @@ static int fimc_clk_get(struct fimc_dev *fimc)
 	int i, ret;
 
 	for (i = 0; i < MAX_FIMC_CLOCKS; i++) {
-		fimc->clock[i] = clk_get(&fimc->pdev->dev, fimc_clocks[i]);
+		fimc->clock[i] = devm_clk_get(&fimc->pdev->dev, fimc_clocks[i]);
 		if (IS_ERR(fimc->clock[i]))
 			goto err;
 		ret = clk_prepare(fimc->clock[i]);
-		if (ret < 0) {
-			clk_put(fimc->clock[i]);
-			fimc->clock[i] = NULL;
+		if (ret < 0)
 			goto err;
-		}
 	}
 	return 0;
 err:
-	fimc_clk_put(fimc);
 	dev_err(&fimc->pdev->dev, "failed to get clock: %s\n",
 		fimc_clocks[i]);
 	return -ENXIO;
-- 
1.7.4.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 3/6] [media] s5p-fimc: Use devm_clk_get in fimc-lite.c
  2012-11-23 11:50 [PATCH 0/6][media] s5p-*: Use devm_clk_get APIs Sachin Kamat
  2012-11-23 11:50 ` [PATCH 1/6] [media] s5p-fimc: Use devm_clk_get in mipi-csis.c Sachin Kamat
  2012-11-23 11:50 ` [PATCH 2/6] [media] s5p-fimc: Use devm_clk_get in fimc-core.c Sachin Kamat
@ 2012-11-23 11:50 ` Sachin Kamat
  2012-11-23 11:50 ` [PATCH 4/6] [media] s5p-g2d: Use devm_clk_get APIs Sachin Kamat
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Sachin Kamat @ 2012-11-23 11:50 UTC (permalink / raw)
  To: linux-media; +Cc: s.nawrocki, sachin.kamat, patches

devm_clk_get is device managed and makes error handling and cleanup
a bit simpler.

Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
---
 drivers/media/platform/s5p-fimc/fimc-lite.c |    8 +-------
 1 files changed, 1 insertions(+), 7 deletions(-)

diff --git a/drivers/media/platform/s5p-fimc/fimc-lite.c b/drivers/media/platform/s5p-fimc/fimc-lite.c
index 70bcf39..02d0ff9 100644
--- a/drivers/media/platform/s5p-fimc/fimc-lite.c
+++ b/drivers/media/platform/s5p-fimc/fimc-lite.c
@@ -1382,23 +1382,17 @@ static void fimc_lite_clk_put(struct fimc_lite *fimc)
 		return;
 
 	clk_unprepare(fimc->clock);
-	clk_put(fimc->clock);
-	fimc->clock = NULL;
 }
 
 static int fimc_lite_clk_get(struct fimc_lite *fimc)
 {
 	int ret;
 
-	fimc->clock = clk_get(&fimc->pdev->dev, FLITE_CLK_NAME);
+	fimc->clock = devm_clk_get(&fimc->pdev->dev, FLITE_CLK_NAME);
 	if (IS_ERR(fimc->clock))
 		return PTR_ERR(fimc->clock);
 
 	ret = clk_prepare(fimc->clock);
-	if (ret < 0) {
-		clk_put(fimc->clock);
-		fimc->clock = NULL;
-	}
 	return ret;
 }
 
-- 
1.7.4.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 4/6] [media] s5p-g2d: Use devm_clk_get APIs.
  2012-11-23 11:50 [PATCH 0/6][media] s5p-*: Use devm_clk_get APIs Sachin Kamat
                   ` (2 preceding siblings ...)
  2012-11-23 11:50 ` [PATCH 3/6] [media] s5p-fimc: Use devm_clk_get in fimc-lite.c Sachin Kamat
@ 2012-11-23 11:50 ` Sachin Kamat
  2012-11-28 10:13   ` Kamil Debski
  2012-11-23 11:50 ` [PATCH 5/6] [media] s5p-jpeg: " Sachin Kamat
  2012-11-23 11:50 ` [PATCH 6/6] [media] s5p-mfc: " Sachin Kamat
  5 siblings, 1 reply; 11+ messages in thread
From: Sachin Kamat @ 2012-11-23 11:50 UTC (permalink / raw)
  To: linux-media; +Cc: s.nawrocki, sachin.kamat, patches, Kamil Debski

devm_clk_get() is device managed function and makes error handling
and exit code a bit simpler.

Cc: Kamil Debski <k.debski@samsung.com>
Signed-off-by: Sachin Kamat <sachin.kamat@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


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 5/6] [media] s5p-jpeg: Use devm_clk_get APIs.
  2012-11-23 11:50 [PATCH 0/6][media] s5p-*: Use devm_clk_get APIs Sachin Kamat
                   ` (3 preceding siblings ...)
  2012-11-23 11:50 ` [PATCH 4/6] [media] s5p-g2d: Use devm_clk_get APIs Sachin Kamat
@ 2012-11-23 11:50 ` Sachin Kamat
  2012-11-23 11:50 ` [PATCH 6/6] [media] s5p-mfc: " Sachin Kamat
  5 siblings, 0 replies; 11+ messages in thread
From: Sachin Kamat @ 2012-11-23 11:50 UTC (permalink / raw)
  To: linux-media; +Cc: s.nawrocki, sachin.kamat, patches, Andrzej Pietrasiewicz

devm_clk_get() is device managed function and makes error handling
and exit code a bit simpler.

Cc: Andrzej Pietrasiewicz <andrzej.p@samsung.com>
Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
---
 drivers/media/platform/s5p-jpeg/jpeg-core.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c b/drivers/media/platform/s5p-jpeg/jpeg-core.c
index 17983c4..fc63d27 100644
--- a/drivers/media/platform/s5p-jpeg/jpeg-core.c
+++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c
@@ -1346,7 +1346,7 @@ static int s5p_jpeg_probe(struct platform_device *pdev)
 	}
 
 	/* clocks */
-	jpeg->clk = clk_get(&pdev->dev, "jpeg");
+	jpeg->clk = devm_clk_get(&pdev->dev, "jpeg");
 	if (IS_ERR(jpeg->clk)) {
 		dev_err(&pdev->dev, "cannot get clock\n");
 		ret = PTR_ERR(jpeg->clk);
@@ -1461,7 +1461,6 @@ device_register_rollback:
 
 clk_get_rollback:
 	clk_disable_unprepare(jpeg->clk);
-	clk_put(jpeg->clk);
 
 	return ret;
 }
@@ -1481,7 +1480,6 @@ static int s5p_jpeg_remove(struct platform_device *pdev)
 	v4l2_device_unregister(&jpeg->v4l2_dev);
 
 	clk_disable_unprepare(jpeg->clk);
-	clk_put(jpeg->clk);
 
 	return 0;
 }
-- 
1.7.4.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 6/6] [media] s5p-mfc: Use devm_clk_get APIs
  2012-11-23 11:50 [PATCH 0/6][media] s5p-*: Use devm_clk_get APIs Sachin Kamat
                   ` (4 preceding siblings ...)
  2012-11-23 11:50 ` [PATCH 5/6] [media] s5p-jpeg: " Sachin Kamat
@ 2012-11-23 11:50 ` Sachin Kamat
  2012-11-28 10:13   ` Kamil Debski
  5 siblings, 1 reply; 11+ messages in thread
From: Sachin Kamat @ 2012-11-23 11:50 UTC (permalink / raw)
  To: linux-media; +Cc: s.nawrocki, sachin.kamat, patches, Kamil Debski

devm_clk_get() is device managed function and makes error handling
and exit code a bit simpler.

Cc: Kamil Debski <k.debski@samsung.com>
Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
---
 drivers/media/platform/s5p-mfc/s5p_mfc_pm.c |   14 ++++----------
 1 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_pm.c b/drivers/media/platform/s5p-mfc/s5p_mfc_pm.c
index 2895333..4864d93 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_pm.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_pm.c
@@ -37,7 +37,7 @@ int s5p_mfc_init_pm(struct s5p_mfc_dev *dev)
 
 	pm = &dev->pm;
 	p_dev = dev;
-	pm->clock_gate = clk_get(&dev->plat_dev->dev, MFC_GATE_CLK_NAME);
+	pm->clock_gate = devm_clk_get(&dev->plat_dev->dev, MFC_GATE_CLK_NAME);
 	if (IS_ERR(pm->clock_gate)) {
 		mfc_err("Failed to get clock-gating control\n");
 		ret = PTR_ERR(pm->clock_gate);
@@ -47,10 +47,10 @@ int s5p_mfc_init_pm(struct s5p_mfc_dev *dev)
 	ret = clk_prepare(pm->clock_gate);
 	if (ret) {
 		mfc_err("Failed to preapre clock-gating control\n");
-		goto err_p_ip_clk;
+		goto err_g_ip_clk;
 	}
 
-	pm->clock = clk_get(&dev->plat_dev->dev, dev->variant->mclk_name);
+	pm->clock = devm_clk_get(&dev->plat_dev->dev, dev->variant->mclk_name);
 	if (IS_ERR(pm->clock)) {
 		mfc_err("Failed to get MFC clock\n");
 		ret = PTR_ERR(pm->clock);
@@ -60,7 +60,7 @@ int s5p_mfc_init_pm(struct s5p_mfc_dev *dev)
 	ret = clk_prepare(pm->clock);
 	if (ret) {
 		mfc_err("Failed to prepare MFC clock\n");
-		goto err_p_ip_clk_2;
+		goto err_g_ip_clk_2;
 	}
 
 	atomic_set(&pm->power, 0);
@@ -72,12 +72,8 @@ int s5p_mfc_init_pm(struct s5p_mfc_dev *dev)
 	atomic_set(&clk_ref, 0);
 #endif
 	return 0;
-err_p_ip_clk_2:
-	clk_put(pm->clock);
 err_g_ip_clk_2:
 	clk_unprepare(pm->clock_gate);
-err_p_ip_clk:
-	clk_put(pm->clock_gate);
 err_g_ip_clk:
 	return ret;
 }
@@ -85,9 +81,7 @@ err_g_ip_clk:
 void s5p_mfc_final_pm(struct s5p_mfc_dev *dev)
 {
 	clk_unprepare(pm->clock_gate);
-	clk_put(pm->clock_gate);
 	clk_unprepare(pm->clock);
-	clk_put(pm->clock);
 #ifdef CONFIG_PM_RUNTIME
 	pm_runtime_disable(pm->device);
 #endif
-- 
1.7.4.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/6] [media] s5p-fimc: Use devm_clk_get in mipi-csis.c
  2012-11-23 11:50 ` [PATCH 1/6] [media] s5p-fimc: Use devm_clk_get in mipi-csis.c Sachin Kamat
@ 2012-11-25 15:32   ` Sylwester Nawrocki
  2012-11-26  4:54     ` Sachin Kamat
  0 siblings, 1 reply; 11+ messages in thread
From: Sylwester Nawrocki @ 2012-11-25 15:32 UTC (permalink / raw)
  To: Sachin Kamat; +Cc: linux-media, s.nawrocki, patches

Hi Sachin,

On 11/23/2012 12:50 PM, Sachin Kamat wrote:
> devm_clk_get is device managed and makes error handling and cleanup
> a bit simpler.

Can we postpone this once devm_clk_prepare() is available ?

> Signed-off-by: Sachin Kamat<sachin.kamat@linaro.org>
> ---
>   drivers/media/platform/s5p-fimc/mipi-csis.c |    6 +-----
>   1 files changed, 1 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/media/platform/s5p-fimc/mipi-csis.c b/drivers/media/platform/s5p-fimc/mipi-csis.c
> index 4c961b1..d624bfa 100644
> --- a/drivers/media/platform/s5p-fimc/mipi-csis.c
> +++ b/drivers/media/platform/s5p-fimc/mipi-csis.c
> @@ -341,8 +341,6 @@ static void s5pcsis_clk_put(struct csis_state *state)
>   		if (IS_ERR_OR_NULL(state->clock[i]))
>   			continue;
>   		clk_unprepare(state->clock[i]);
> -		clk_put(state->clock[i]);
> -		state->clock[i] = NULL;

This line shouldn't be removed, it protects from releasing already
released clock resource. In fact state->clock[i] = ERR_PTR(-EINVAL);
would be more correct, but that's a different story.

>   	}
>   }
>
> @@ -352,13 +350,11 @@ static int s5pcsis_clk_get(struct csis_state *state)
>   	int i, ret;
>
>   	for (i = 0; i<  NUM_CSIS_CLOCKS; i++) {
> -		state->clock[i] = clk_get(dev, csi_clock_name[i]);
> +		state->clock[i] = devm_clk_get(dev, csi_clock_name[i]);
>   		if (IS_ERR(state->clock[i]))
>   			goto err;
>   		ret = clk_prepare(state->clock[i]);
>   		if (ret<  0) {
> -			clk_put(state->clock[i]);
> -			state->clock[i] = NULL;

And same here, now we have a pointer to valid, unprepared clock in
state->clock[i]. When s5pcsis_clk_put() gets called afterwards it will
invoke unbalanced clk_unprepare() in this clock.

I would prefer to hold on with that sort of changes in s5p-fimc driver,
until after devm_clk_prepare() is available.

>   			goto err;
>   		}
>   	}

Regards,
Sylwester

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/6] [media] s5p-fimc: Use devm_clk_get in mipi-csis.c
  2012-11-25 15:32   ` Sylwester Nawrocki
@ 2012-11-26  4:54     ` Sachin Kamat
  0 siblings, 0 replies; 11+ messages in thread
From: Sachin Kamat @ 2012-11-26  4:54 UTC (permalink / raw)
  To: Sylwester Nawrocki; +Cc: linux-media, s.nawrocki, patches

Hi Sylwester,

On 25 November 2012 21:02, Sylwester Nawrocki
<sylvester.nawrocki@gmail.com> wrote:
> Hi Sachin,
>
>
> On 11/23/2012 12:50 PM, Sachin Kamat wrote:
>>
>> devm_clk_get is device managed and makes error handling and cleanup
>> a bit simpler.
>
>
> Can we postpone this once devm_clk_prepare() is available ?
Ok. No problem. I will hold on till then.

>
>
>> Signed-off-by: Sachin Kamat<sachin.kamat@linaro.org>
>> ---
>>   drivers/media/platform/s5p-fimc/mipi-csis.c |    6 +-----
>>   1 files changed, 1 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/media/platform/s5p-fimc/mipi-csis.c
>> b/drivers/media/platform/s5p-fimc/mipi-csis.c
>> index 4c961b1..d624bfa 100644
>> --- a/drivers/media/platform/s5p-fimc/mipi-csis.c
>> +++ b/drivers/media/platform/s5p-fimc/mipi-csis.c
>> @@ -341,8 +341,6 @@ static void s5pcsis_clk_put(struct csis_state *state)
>>                 if (IS_ERR_OR_NULL(state->clock[i]))
>>                         continue;
>>                 clk_unprepare(state->clock[i]);
>> -               clk_put(state->clock[i]);
>> -               state->clock[i] = NULL;
>
>
> This line shouldn't be removed, it protects from releasing already
> released clock resource.

Wouldn't 'state->clock[i] = NULL' cause a problem when put gets called
upon exit from this function by devres f/w as its argument would be
NULL?
In that case devm_clk_put would be better here?

> In fact state->clock[i] = ERR_PTR(-EINVAL);
> would be more correct, but that's a different story.
>
>
>>         }
>>   }
>>
>> @@ -352,13 +350,11 @@ static int s5pcsis_clk_get(struct csis_state *state)
>>         int i, ret;
>>
>>         for (i = 0; i<  NUM_CSIS_CLOCKS; i++) {
>> -               state->clock[i] = clk_get(dev, csi_clock_name[i]);
>> +               state->clock[i] = devm_clk_get(dev, csi_clock_name[i]);
>>                 if (IS_ERR(state->clock[i]))
>>                         goto err;
>>                 ret = clk_prepare(state->clock[i]);
>>                 if (ret<  0) {
>> -                       clk_put(state->clock[i]);
>> -                       state->clock[i] = NULL;
>
>
> And same here, now we have a pointer to valid, unprepared clock in
> state->clock[i]. When s5pcsis_clk_put() gets called afterwards it will
> invoke unbalanced clk_unprepare() in this clock.
>
> I would prefer to hold on with that sort of changes in s5p-fimc driver,
> until after devm_clk_prepare() is available.

Ok.

>
>>                         goto err;
>>                 }
>>         }
>
>
> Regards,
> Sylwester



-- 
With warm regards,
Sachin

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH 4/6] [media] s5p-g2d: Use devm_clk_get APIs.
  2012-11-23 11:50 ` [PATCH 4/6] [media] s5p-g2d: Use devm_clk_get APIs Sachin Kamat
@ 2012-11-28 10:13   ` Kamil Debski
  0 siblings, 0 replies; 11+ messages in thread
From: Kamil Debski @ 2012-11-28 10:13 UTC (permalink / raw)
  To: 'Sachin Kamat', linux-media; +Cc: Sylwester Nawrocki, patches

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-owner@vger.kernel.org [mailto:linux-media-
> owner@vger.kernel.org] On Behalf Of Sachin Kamat
> Sent: Friday, November 23, 2012 12:51 PM
> To: linux-media@vger.kernel.org
> Cc: s.nawrocki@samsung.com; sachin.kamat@linaro.org;
> patches@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.debski@samsung.com>
> Signed-off-by: Sachin Kamat <sachin.kamat@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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH 6/6] [media] s5p-mfc: Use devm_clk_get APIs
  2012-11-23 11:50 ` [PATCH 6/6] [media] s5p-mfc: " Sachin Kamat
@ 2012-11-28 10:13   ` Kamil Debski
  0 siblings, 0 replies; 11+ messages in thread
From: Kamil Debski @ 2012-11-28 10:13 UTC (permalink / raw)
  To: 'Sachin Kamat', linux-media; +Cc: Sylwester Nawrocki, patches

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-owner@vger.kernel.org [mailto:linux-media-
> owner@vger.kernel.org] On Behalf Of Sachin Kamat
> Sent: Friday, November 23, 2012 12:51 PM
> To: linux-media@vger.kernel.org
> Cc: s.nawrocki@samsung.com; sachin.kamat@linaro.org;
> patches@linaro.org; Kamil Debski
> Subject: [PATCH 6/6] [media] s5p-mfc: 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.debski@samsung.com>
> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
> ---
>  drivers/media/platform/s5p-mfc/s5p_mfc_pm.c |   14 ++++----------
>  1 files changed, 4 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_pm.c
> b/drivers/media/platform/s5p-mfc/s5p_mfc_pm.c
> index 2895333..4864d93 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_pm.c
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_pm.c
> @@ -37,7 +37,7 @@ int s5p_mfc_init_pm(struct s5p_mfc_dev *dev)
> 
>  	pm = &dev->pm;
>  	p_dev = dev;
> -	pm->clock_gate = clk_get(&dev->plat_dev->dev, MFC_GATE_CLK_NAME);
> +	pm->clock_gate = devm_clk_get(&dev->plat_dev->dev,
> MFC_GATE_CLK_NAME);
>  	if (IS_ERR(pm->clock_gate)) {
>  		mfc_err("Failed to get clock-gating control\n");
>  		ret = PTR_ERR(pm->clock_gate);
> @@ -47,10 +47,10 @@ int s5p_mfc_init_pm(struct s5p_mfc_dev *dev)
>  	ret = clk_prepare(pm->clock_gate);
>  	if (ret) {
>  		mfc_err("Failed to preapre clock-gating control\n");
> -		goto err_p_ip_clk;
> +		goto err_g_ip_clk;
>  	}
> 
> -	pm->clock = clk_get(&dev->plat_dev->dev, dev->variant-
> >mclk_name);
> +	pm->clock = devm_clk_get(&dev->plat_dev->dev,
> +dev->variant->mclk_name);
>  	if (IS_ERR(pm->clock)) {
>  		mfc_err("Failed to get MFC clock\n");
>  		ret = PTR_ERR(pm->clock);
> @@ -60,7 +60,7 @@ int s5p_mfc_init_pm(struct s5p_mfc_dev *dev)
>  	ret = clk_prepare(pm->clock);
>  	if (ret) {
>  		mfc_err("Failed to prepare MFC clock\n");
> -		goto err_p_ip_clk_2;
> +		goto err_g_ip_clk_2;
>  	}
> 
>  	atomic_set(&pm->power, 0);
> @@ -72,12 +72,8 @@ int s5p_mfc_init_pm(struct s5p_mfc_dev *dev)
>  	atomic_set(&clk_ref, 0);
>  #endif
>  	return 0;
> -err_p_ip_clk_2:
> -	clk_put(pm->clock);
>  err_g_ip_clk_2:
>  	clk_unprepare(pm->clock_gate);
> -err_p_ip_clk:
> -	clk_put(pm->clock_gate);
>  err_g_ip_clk:
>  	return ret;
>  }
> @@ -85,9 +81,7 @@ err_g_ip_clk:
>  void s5p_mfc_final_pm(struct s5p_mfc_dev *dev)  {
>  	clk_unprepare(pm->clock_gate);
> -	clk_put(pm->clock_gate);
>  	clk_unprepare(pm->clock);
> -	clk_put(pm->clock);
>  #ifdef CONFIG_PM_RUNTIME
>  	pm_runtime_disable(pm->device);
>  #endif
> --
> 1.7.4.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media"
> in the body of a message to majordomo@vger.kernel.org More majordomo
> info at  http://vger.kernel.org/majordomo-info.html



^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2012-11-28 10:13 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-11-23 11:50 [PATCH 0/6][media] s5p-*: Use devm_clk_get APIs Sachin Kamat
2012-11-23 11:50 ` [PATCH 1/6] [media] s5p-fimc: Use devm_clk_get in mipi-csis.c Sachin Kamat
2012-11-25 15:32   ` Sylwester Nawrocki
2012-11-26  4:54     ` Sachin Kamat
2012-11-23 11:50 ` [PATCH 2/6] [media] s5p-fimc: Use devm_clk_get in fimc-core.c Sachin Kamat
2012-11-23 11:50 ` [PATCH 3/6] [media] s5p-fimc: Use devm_clk_get in fimc-lite.c Sachin Kamat
2012-11-23 11:50 ` [PATCH 4/6] [media] s5p-g2d: Use devm_clk_get APIs Sachin Kamat
2012-11-28 10:13   ` Kamil Debski
2012-11-23 11:50 ` [PATCH 5/6] [media] s5p-jpeg: " Sachin Kamat
2012-11-23 11:50 ` [PATCH 6/6] [media] s5p-mfc: " Sachin Kamat
2012-11-28 10:13   ` Kamil Debski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox