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@xxxxxxxxxx> --- 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 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html