* [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* 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
* [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* 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
* [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 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