[PATCH] ARM: SAMSUNG: Add clk enable/disable of pwm

[Kukjin Kim]

Kyungmin Park wrote:
> 
> On 11/3/11, Kyungmin Park <kyungmin.park at samsung.com> wrote:
> > On 11/3/11, Joonyoung Shim <jy0922.shim at samsung.com> 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 at samsung.com>
> >>>>>> Signed-off-by: Kyungmin Park<kyungmin.park at samsung.com>
> >>>>>> ---
> >>>>>>    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 at samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.