Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1436403 > unrolled thread

[RFC PATCH 1/2] rtc: s3c: Remove unnecessary call to disable already disabled clock

Back to article view | Back to linux.kernel

Contents

  [RFC PATCH 1/2] rtc: s3c: Remove unnecessary call to disable already  disabled clock Alim Akhtar <alim.akhtar@samsung.com> - 2016-07-04 13:10 +0200
    Re: [RFC PATCH 1/2] rtc: s3c: Remove unnecessary call to disable  already disabled clock Krzysztof Kozlowski <k.kozlowski@samsung.com> - 2016-07-05 09:20 +0200
      Re: [RFC PATCH 1/2] rtc: s3c: Remove unnecessary call to disable  already disabled clock Alim Akhtar <alim.akhtar@samsung.com> - 2016-07-05 10:50 +0200
        Re: [RFC PATCH 1/2] rtc: s3c: Remove unnecessary call to disable  already disabled clock Krzysztof Kozlowski <k.kozlowski@samsung.com> - 2016-07-05 11:00 +0200

#1436403 — [RFC PATCH 1/2] rtc: s3c: Remove unnecessary call to disable already disabled clock

At the end of s3c_rtc_probe(), s3c_rtc_disable_clk() being called with rtc
clock already disabled, which looks extra and unnecessary call.
Lets clean it up.

Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
---
 drivers/rtc/rtc-s3c.c |    2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/rtc/rtc-s3c.c b/drivers/rtc/rtc-s3c.c
index d01ad7e..b083840 100644
--- a/drivers/rtc/rtc-s3c.c
+++ b/drivers/rtc/rtc-s3c.c
@@ -577,8 +577,6 @@ static int s3c_rtc_probe(struct platform_device *pdev)
 
 	s3c_rtc_setfreq(info, 1);
 
-	s3c_rtc_disable_clk(info);
-
 	return 0;
 
  err_nortc:
-- 
1.7.10.4

[toc] | [next] | [standalone]

#1436811 — Re: [RFC PATCH 1/2] rtc: s3c: Remove unnecessary call to disable already disabled clock

On 07/04/2016 01:03 PM, Alim Akhtar wrote:
> At the end of s3c_rtc_probe(), s3c_rtc_disable_clk() being called with rtc
> clock already disabled, which looks extra and unnecessary call.
> Lets clean it up.

Does not look right. Till that place, the clocks are enabled. Then
s3c_rtc_setaie() is called which expects that clocks are disabled...
otherwise counters get mixed.

So overall this looks like wrong approach unless I am missing something?

Best regards,
Krzysztof

> 
> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
> ---
>  drivers/rtc/rtc-s3c.c |    2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/rtc/rtc-s3c.c b/drivers/rtc/rtc-s3c.c
> index d01ad7e..b083840 100644
> --- a/drivers/rtc/rtc-s3c.c
> +++ b/drivers/rtc/rtc-s3c.c
> @@ -577,8 +577,6 @@ static int s3c_rtc_probe(struct platform_device *pdev)
>  
>  	s3c_rtc_setfreq(info, 1);
>  
> -	s3c_rtc_disable_clk(info);
> -
>  	return 0;
>  
>   err_nortc:

[toc] | [prev] | [next] | [standalone]

#1436846 — Re: [RFC PATCH 1/2] rtc: s3c: Remove unnecessary call to disable already disabled clock

Hi Krzsztof,

On 07/05/2016 12:48 PM, Krzysztof Kozlowski wrote:
> On 07/04/2016 01:03 PM, Alim Akhtar wrote:
>> At the end of s3c_rtc_probe(), s3c_rtc_disable_clk() being called with rtc
>> clock already disabled, which looks extra and unnecessary call.
>> Lets clean it up.
>
> Does not look right. Till that place, the clocks are enabled. Then
> s3c_rtc_setaie() is called which expects that clocks are disabled...
> otherwise counters get mixed.
>

The clock is always disabled when it reach s3c_rtc_setfreq() in probe(),
because s3c_rtc_gettime() will always disable the clock.

As far as s3c_rtc_setaie() is concern, it enables clock while entering 
and disables it while leaving the function. And in 
s3c_rtc_{enable,disable}_clk() there is check info->clk_disabled flag 
which will make sure clock balancing.

> So overall this looks like wrong approach unless I am missing something?
>
> Best regards,
> Krzysztof
>
>>
>> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
>> ---
>>   drivers/rtc/rtc-s3c.c |    2 --
>>   1 file changed, 2 deletions(-)
>>
>> diff --git a/drivers/rtc/rtc-s3c.c b/drivers/rtc/rtc-s3c.c
>> index d01ad7e..b083840 100644
>> --- a/drivers/rtc/rtc-s3c.c
>> +++ b/drivers/rtc/rtc-s3c.c
>> @@ -577,8 +577,6 @@ static int s3c_rtc_probe(struct platform_device *pdev)
>>
>>   	s3c_rtc_setfreq(info, 1);
>>
>> -	s3c_rtc_disable_clk(info);
>> -
>>   	return 0;
>>
>>    err_nortc:
>
>

[toc] | [prev] | [next] | [standalone]

#1436850 — Re: [RFC PATCH 1/2] rtc: s3c: Remove unnecessary call to disable already disabled clock

On 07/05/2016 10:46 AM, Alim Akhtar wrote:
> Hi Krzsztof,
> 
> On 07/05/2016 12:48 PM, Krzysztof Kozlowski wrote:
>> On 07/04/2016 01:03 PM, Alim Akhtar wrote:
>>> At the end of s3c_rtc_probe(), s3c_rtc_disable_clk() being called
>>> with rtc
>>> clock already disabled, which looks extra and unnecessary call.
>>> Lets clean it up.
>>
>> Does not look right. Till that place, the clocks are enabled. Then
>> s3c_rtc_setaie() is called which expects that clocks are disabled...
>> otherwise counters get mixed.
>>
> 
> The clock is always disabled when it reach s3c_rtc_setfreq() in probe(),
> because s3c_rtc_gettime() will always disable the clock.
> 
> As far as s3c_rtc_setaie() is concern, it enables clock while entering
> and disables it while leaving the function. And in
> s3c_rtc_{enable,disable}_clk() there is check info->clk_disabled flag
> which will make sure clock balancing.

Ah, you're right. Looks correct although the information that clock is
disabled because of s3c_rtc_gettime() would be useful in the commit message.

Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>

Best regards,
Krzysztof

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web