From: Sylwester Nawrocki <sylvester.nawrocki@gmail.com>
To: Sachin Kamat <sachin.kamat@linaro.org>
Cc: linux-media@vger.kernel.org, s.nawrocki@samsung.com, patches@linaro.org
Subject: Re: [PATCH 5/8] [media] exynos-gsc: Use clk_prepare_enable and clk_disable_unprepare
Date: Sat, 20 Oct 2012 11:54:01 +0200 [thread overview]
Message-ID: <508274B9.5000401@gmail.com> (raw)
In-Reply-To: <1350472311-9748-5-git-send-email-sachin.kamat@linaro.org>
On 10/17/2012 01:11 PM, Sachin Kamat wrote:
> Replace clk_enable/clk_disable with clk_prepare_enable/clk_disable_unprepare
> as required by the common clock framework.
>
> Signed-off-by: Sachin Kamat<sachin.kamat@linaro.org>
> ---
> drivers/media/platform/exynos-gsc/gsc-core.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/platform/exynos-gsc/gsc-core.c b/drivers/media/platform/exynos-gsc/gsc-core.c
> index bfec9e6..d11668b 100644
> --- a/drivers/media/platform/exynos-gsc/gsc-core.c
> +++ b/drivers/media/platform/exynos-gsc/gsc-core.c
> @@ -1009,7 +1009,7 @@ static int gsc_clk_get(struct gsc_dev *gsc)
> if (IS_ERR(gsc->clock))
> goto err_print;
>
> - ret = clk_prepare(gsc->clock);
> + ret = clk_prepare_enable(gsc->clock);
This is not right, gsc->clock is being enabled somewhere else.
And as you can see this driver has already taken care of preparing/
unpreparing the clock.
> if (ret< 0) {
> clk_put(gsc->clock);
> gsc->clock = NULL;
> @@ -1186,7 +1186,7 @@ static int gsc_runtime_suspend(struct device *dev)
>
> ret = gsc_m2m_suspend(gsc);
> if (!ret)
> - clk_disable(gsc->clock);
> + clk_disable_unprepare(gsc->clock);
No, that will result in unbalanced clk_prepare/unprepare. Please
don't do that.
>
> pr_debug("gsc%d: state: 0x%lx", gsc->id, gsc->state);
> return ret;
This driver should already inter-work fine with the common clock
framework, just look how it handles the clock:
# git grep -n -7 clk_ -- exynos-gsc
exynos-gsc/gsc-core.c:993:static void gsc_clk_put(struct gsc_dev *gsc)
exynos-gsc/gsc-core.c-994-{
exynos-gsc/gsc-core.c-995- if (IS_ERR_OR_NULL(gsc->clock))
exynos-gsc/gsc-core.c-996- return;
exynos-gsc/gsc-core.c-997-
exynos-gsc/gsc-core.c:998: clk_unprepare(gsc->clock);
exynos-gsc/gsc-core.c:999: clk_put(gsc->clock);
exynos-gsc/gsc-core.c-1000- gsc->clock = NULL;
exynos-gsc/gsc-core.c-1001-}
exynos-gsc/gsc-core.c-1002-
exynos-gsc/gsc-core.c:1003:static int gsc_clk_get(struct gsc_dev *gsc)
exynos-gsc/gsc-core.c-1004-{
exynos-gsc/gsc-core.c-1005- int ret;
exynos-gsc/gsc-core.c-1006-
exynos-gsc/gsc-core.c:1007: dev_dbg(&gsc->pdev->dev, "gsc_clk_get Called\n");
exynos-gsc/gsc-core.c-1008-
exynos-gsc/gsc-core.c:1009: gsc->clock = clk_get(&gsc->pdev->dev, GSC_CLOCK_GATE_NAME);
exynos-gsc/gsc-core.c-1010- if (IS_ERR(gsc->clock))
exynos-gsc/gsc-core.c-1011- goto err_print;
exynos-gsc/gsc-core.c-1012-
exynos-gsc/gsc-core.c:1013: ret = clk_prepare(gsc->clock);
exynos-gsc/gsc-core.c-1014- if (ret < 0) {
exynos-gsc/gsc-core.c:1015: clk_put(gsc->clock);
exynos-gsc/gsc-core.c-1016- gsc->clock = NULL;
exynos-gsc/gsc-core.c-1017- goto err;
exynos-gsc/gsc-core.c-1018- }
exynos-gsc/gsc-core.c-1019-
exynos-gsc/gsc-core.c-1020- return 0;
exynos-gsc/gsc-core.c-1021-
exynos-gsc/gsc-core.c-1022-err:
exynos-gsc/gsc-core.c-1023- dev_err(&gsc->pdev->dev, "clock prepare failed for clock: %s\n",
exynos-gsc/gsc-core.c-1024- GSC_CLOCK_GATE_NAME);
exynos-gsc/gsc-core.c:1025: gsc_clk_put(gsc);
exynos-gsc/gsc-core.c-1026-err_print:
exynos-gsc/gsc-core.c-1027- dev_err(&gsc->pdev->dev, "failed to get clock~~~: %s\n",
exynos-gsc/gsc-core.c-1028- GSC_CLOCK_GATE_NAME);
exynos-gsc/gsc-core.c-1029- return -ENXIO;
exynos-gsc/gsc-core.c-1030-}
--
exynos-gsc/gsc-core.c-1105-
exynos-gsc/gsc-core.c-1106- res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
exynos-gsc/gsc-core.c-1107- if (!res) {
exynos-gsc/gsc-core.c-1108- dev_err(dev, "failed to get IRQ resource\n");
exynos-gsc/gsc-core.c-1109- return -ENXIO;
exynos-gsc/gsc-core.c-1110- }
exynos-gsc/gsc-core.c-1111-
exynos-gsc/gsc-core.c:1112: ret = gsc_clk_get(gsc);
exynos-gsc/gsc-core.c-1113- if (ret)
exynos-gsc/gsc-core.c-1114- return ret;
--
exynos-gsc/gsc-core.c-1142- pm_runtime_put(dev);
exynos-gsc/gsc-core.c-1143- return 0;
exynos-gsc/gsc-core.c-1144-err_pm:
exynos-gsc/gsc-core.c-1145- pm_runtime_put(dev);
exynos-gsc/gsc-core.c-1146-err_m2m:
exynos-gsc/gsc-core.c-1147- gsc_unregister_m2m_device(gsc);
exynos-gsc/gsc-core.c-1148-err_clk:
exynos-gsc/gsc-core.c:1149: gsc_clk_put(gsc);
exynos-gsc/gsc-core.c-1150- return ret;
exynos-gsc/gsc-core.c-1151-}
--
exynos-gsc/gsc-core.c-1166-static int gsc_runtime_resume(struct device *dev)
exynos-gsc/gsc-core.c-1167-{
exynos-gsc/gsc-core.c-1168- struct gsc_dev *gsc = dev_get_drvdata(dev);
exynos-gsc/gsc-core.c-1169- int ret = 0;
exynos-gsc/gsc-core.c-1170-
exynos-gsc/gsc-core.c-1171- pr_debug("gsc%d: state: 0x%lx", gsc->id, gsc->state);
exynos-gsc/gsc-core.c-1172-
exynos-gsc/gsc-core.c:1173: ret = clk_enable(gsc->clock);
exynos-gsc/gsc-core.c-1174- if (ret)
exynos-gsc/gsc-core.c-1175- return ret;
exynos-gsc/gsc-core.c-1176-
exynos-gsc/gsc-core.c-1177- gsc_hw_set_sw_reset(gsc);
exynos-gsc/gsc-core.c-1178- gsc_wait_reset(gsc);
exynos-gsc/gsc-core.c-1179-
exynos-gsc/gsc-core.c-1180- return gsc_m2m_resume(gsc);
--
exynos-gsc/gsc-core.c-1183-static int gsc_runtime_suspend(struct device *dev)
exynos-gsc/gsc-core.c-1184-{
exynos-gsc/gsc-core.c-1185- struct gsc_dev *gsc = dev_get_drvdata(dev);
exynos-gsc/gsc-core.c-1186- int ret = 0;
exynos-gsc/gsc-core.c-1187-
exynos-gsc/gsc-core.c-1188- ret = gsc_m2m_suspend(gsc);
exynos-gsc/gsc-core.c-1189- if (!ret)
exynos-gsc/gsc-core.c:1190: clk_disable(gsc->clock);
exynos-gsc/gsc-core.c-1191-
exynos-gsc/gsc-core.c-1192- pr_debug("gsc%d: state: 0x%lx", gsc->id, gsc->state);
exynos-gsc/gsc-core.c-1193- return ret;
exynos-gsc/gsc-core.c-1194-}
--
Regards,
Sylwester
next prev parent reply other threads:[~2012-10-20 9:54 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-10-17 11:11 [PATCH 1/8] [media] s5p-fimc: Use clk_prepare_enable and clk_disable_unprepare Sachin Kamat
2012-10-17 11:11 ` [PATCH 2/8] [media] s5p-g2d: " Sachin Kamat
2012-10-20 9:32 ` Sylwester Nawrocki
2012-10-17 11:11 ` [PATCH 3/8] [media] s5p-mfc: " Sachin Kamat
2012-10-20 9:36 ` Sylwester Nawrocki
2012-10-20 9:36 ` Sylwester Nawrocki
2012-10-17 11:11 ` [PATCH 4/8] [media] s5p-tv: " Sachin Kamat
2012-10-20 9:39 ` Sylwester Nawrocki
2012-10-17 11:11 ` [PATCH 5/8] [media] exynos-gsc: " Sachin Kamat
2012-10-20 9:54 ` Sylwester Nawrocki [this message]
2012-10-17 11:11 ` [PATCH 6/8] [media] exynos-gsc: Fix compilation warning Sachin Kamat
2012-10-20 9:55 ` Sylwester Nawrocki
2012-10-17 11:11 ` [PATCH 7/8] [media] s5p-mfc: Make 'clk_ref' static in s5p_mfc_pm.c Sachin Kamat
2012-10-20 9:56 ` Sylwester Nawrocki
2012-10-17 11:11 ` [PATCH 8/8] [media] s5p-fimc: Make 'fimc_pipeline_s_stream' function static Sachin Kamat
2012-10-20 9:57 ` Sylwester Nawrocki
2012-10-17 14:33 ` [PATCH 1/8] [media] s5p-fimc: Use clk_prepare_enable and clk_disable_unprepare Sylwester Nawrocki
2012-10-17 15:35 ` Sachin Kamat
2012-10-17 15:57 ` Sylwester Nawrocki
2012-10-18 14:44 ` Sachin Kamat
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=508274B9.5000401@gmail.com \
--to=sylvester.nawrocki@gmail.com \
--cc=linux-media@vger.kernel.org \
--cc=patches@linaro.org \
--cc=s.nawrocki@samsung.com \
--cc=sachin.kamat@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).