Kyungmin Park wrote:
>
> On 11/3/11, Kyungmin Park <kyungmin.park@xxxxxxxxxxx> wrote:
> > On 11/3/11, Joonyoung Shim <jy0922.shim@xxxxxxxxxxx> wrote:
> >> 11/03/2011 04:46 PM, Kukjin Kim 쓴 글:
> >>> Joonyoung Shim wrote:
> >>>> 11/03/2011 10:59 AM, Kukjin Kim 쓴 글:
> >>>>> Joonyoung Shim wrote:
> >>>>>> PWM timers use pclk("timers" clk) as parent clk. If this pclk is the
> >>>>>> disabled state when PWM driver is probed, then it causes wrong read
> >>>>>> and
> >>>>>> write operation about registers of PWM.
> >>>>>>
> >>>>>> Signed-off-by: Joonyoung Shim<jy0922.shim@xxxxxxxxxxx>
> >>>>>> Signed-off-by: Kyungmin Park<kyungmin.park@xxxxxxxxxxx>
> >>>>>> ---
> >>>>>> arch/arm/plat-samsung/pwm.c | 7 +++++++
> >>>>>> 1 files changed, 7 insertions(+), 0 deletions(-)
> >>>>>>
> >>>>>> diff --git a/arch/arm/plat-samsung/pwm.c
> >>>>>> b/arch/arm/plat-samsung/pwm.c
> >>>>>> index f37457c..dc1185d 100644
> >>>>>> --- a/arch/arm/plat-samsung/pwm.c
> >>>>>> +++ b/arch/arm/plat-samsung/pwm.c
> >>>>>> @@ -299,6 +299,9 @@ static int s3c_pwm_probe(struct platform_device
> >>>>>> *pdev)
> >>>>>> goto err_clk_tin;
> >>>>>> }
> >>>>>>
> >>>>>> + clk_enable(pwm->clk);
> >>>>>> + clk_enable(pwm->clk_div);
> >>>>>> +
> >>>>>> local_irq_save(flags);
> >>>>>>
> >>>>>> tcon = __raw_readl(S3C2410_TCON);
> >>>>>> @@ -326,6 +329,8 @@ static int s3c_pwm_probe(struct platform_device
> >>>>>> *pdev)
> >>>>>> return 0;
> >>>>>>
> >>>>>> err_clk_tdiv:
> >>>>>> + clk_disable(pwm->clk_div);
> >>>>>> + clk_disable(pwm->clk);
> >>>>>> clk_put(pwm->clk_div);
> >>>>>>
> >>>>>> err_clk_tin:
> >>>>>> @@ -340,6 +345,8 @@ static int __devexit s3c_pwm_remove(struct
> >>>>>> platform_device *pdev)
> >>>>>> {
> >>>>>> struct pwm_device *pwm = platform_get_drvdata(pdev);
> >>>>>>
> >>>>>> + clk_disable(pwm->clk_div);
> >>>>>> + clk_disable(pwm->clk);
> >>>>>> clk_put(pwm->clk_div);
> >>>>>> clk_put(pwm->clk);
> >>>>>> kfree(pwm);
> >>>>>> --
> >>>>>> 1.7.5.4
> >>>>> Well, I wonder when this is needed. I think it should be enabled
> >>>>> during
> >>>>> kernel booting...
> >>>> The exynos4 machine using just timer turns on "timer" clock in the
> >>>> past,
> >>>> but "timer" clock is disable state when boot since MCT is used. MCT
> >>>> doesn't control "timer" clock.
> >>>>
> >>>> I think pwm driver should control(enable/disable) using clocks
> >>>> regardless of their parents clock.
> >>>>
> >>> I mean, why that is disabled when kernel boot...
> >>>
> >>
> >> Please see arch/arm/mach-exynos4/clock.c
> >> There is "timers' clock in the clk init_clocks_off[].
> One more, Don't assume some clocks are enabled at bootloader. user can
> use any bootloader and any configuration.
>
Kyungmin,
NO! I didn't. Just I thought your bootloader disables the clock and why it is needed.
Anyway, Joonyoung,
How about following? And it seems enough...
--
diff --git a/arch/arm/plat-samsung/pwm-clock.c b/arch/arm/plat-samsung/pwm-clock.c
index a35ff3b..37a159b 100644
--- a/arch/arm/plat-samsung/pwm-clock.c
+++ b/arch/arm/plat-samsung/pwm-clock.c
@@ -449,6 +449,8 @@ __init void s3c_pwmclk_init(void)
return;
}
+ clk_enable(clk_timers);
+
for (clk = 0; clk < ARRAY_SIZE(clk_timer_scaler); clk++)
clk_timer_scaler[clk].parent = clk_timers;
--
Or needs separate clk_enable like following?
diff --git a/arch/arm/plat-samsung/pwm.c b/arch/arm/plat-samsung/pwm.c
index f37457c..7a77e36 100644
--- a/arch/arm/plat-samsung/pwm.c
+++ b/arch/arm/plat-samsung/pwm.c
@@ -292,6 +292,8 @@ static int s3c_pwm_probe(struct platform_device *pdev)
goto err_alloc;
}
+ clk_enable(pwm->clk);
+
pwm->clk_div = clk_get(dev, "pwm-tdiv");
if (IS_ERR(pwm->clk_div)) {
dev_err(dev, "failed to get pwm tdiv clk\n");
@@ -299,6 +301,8 @@ static int s3c_pwm_probe(struct platform_device *pdev)
goto err_clk_tin;
}
+ clk_enable(pwm->clk_div);
+
local_irq_save(flags);
tcon = __raw_readl(S3C2410_TCON);
@@ -326,9 +330,11 @@ static int s3c_pwm_probe(struct platform_device *pdev)
return 0;
err_clk_tdiv:
+ clk_disable(pwm->clk_div);
clk_put(pwm->clk_div);
err_clk_tin:
+ clk_disable(pwm->clk);
clk_put(pwm->clk);
err_alloc:
--
BTW, I think this is not really needed for this merge window because the pwm.c is used for only s3c24xx and as you know, it doesn't matter because of arch/arm/plat-samsung/time.c. And if required, this can be fixed during -rc.
Thanks.
Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@xxxxxxxxxxx>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
