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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



11/03/2011 06:08 PM, Kukjin Kim 쓴 글:
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;

No, there is no the reason "timers" clock turns on always.

--
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:
--

I don't care about this.


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.

It's the problem to assume that the related clocks turned on by other
and the pwm is used for exynos4 also.

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


[Index of Archives]     [Linux SoC Development]     [Linux Rockchip Development]     [Linux USB Development]     [Video for Linux]     [Linux Audio Users]     [Linux SCSI]     [Yosemite News]

  Powered by Linux