Re: [PATCH 4/4] rtc: s3c: enable/disable clocks for alarm

2015-08-11 Thread Joonyoung Shim 
On 08/12/2015 09:28 AM, Krzysztof Kozlowski wrote:
> On 11.08.2015 20:28, Joonyoung Shim wrote:
>> The clock enable/disable codes for alarm have removed from
> 
> What do you mean in this paragraph? The clock code was removing something?
> 
>> 'commit 24e1455493da ("drivers/rtc/rtc-s3c.c: delete duplicate clock
> 
> Remove the 'apostrophe.
> 
>> control")' and the clocks keep disabling even if alarm is set, so alarm
>> interrupt can't happen.
> ...and the clocks are disabled even...
> 
> 
>>
>> The s3c_rtc_setaie function can be called several times with that
>> enabled argument has same value,
> ...several times with 'enabled' argument having same value
> 
>> so it needs to check whether clocks is
>> enabled or not.
> s/is/are/
> 
>>
>> Signed-off-by: Joonyoung Shim 
> 
> Please add Cc-stable and fixes tag. To backport the patch probably
> you'll have to remove the dependency on previous patches.

Thanks for the point, i didn't think it.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 4/4] rtc: s3c: enable/disable clocks for alarm

2015-08-11 Thread Krzysztof Kozlowski 
On 11.08.2015 20:28, Joonyoung Shim wrote:
> The clock enable/disable codes for alarm have removed from

What do you mean in this paragraph? The clock code was removing something?

> 'commit 24e1455493da ("drivers/rtc/rtc-s3c.c: delete duplicate clock

Remove the 'apostrophe.

> control")' and the clocks keep disabling even if alarm is set, so alarm
> interrupt can't happen.
...and the clocks are disabled even...


> 
> The s3c_rtc_setaie function can be called several times with that
> enabled argument has same value,
...several times with 'enabled' argument having same value

> so it needs to check whether clocks is
> enabled or not.
s/is/are/

> 
> Signed-off-by: Joonyoung Shim 

Please add Cc-stable and fixes tag. To backport the patch probably
you'll have to remove the dependency on previous patches.

> ---
>  drivers/rtc/rtc-s3c.c | 24 ++--
>  1 file changed, 18 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/rtc/rtc-s3c.c b/drivers/rtc/rtc-s3c.c
> index abe2a6d..fce078c 100644
> --- a/drivers/rtc/rtc-s3c.c
> +++ b/drivers/rtc/rtc-s3c.c
> @@ -39,6 +39,7 @@ struct s3c_rtc {
>   void __iomem *base;
>   struct clk *rtc_clk;
>   struct clk *rtc_src_clk;
> + bool clk_enabled;
>  
>   struct s3c_rtc_data *data;
>  
> @@ -71,9 +72,12 @@ static void s3c_rtc_enable_clk(struct s3c_rtc *info)
>   unsigned long irq_flags;
>  
>   spin_lock_irqsave(>alarm_clk_lock, irq_flags);
> - clk_enable(info->rtc_clk);
> - if (info->data->needs_src_clk)
> - clk_enable(info->rtc_src_clk);
> + if (!info->clk_enabled) {
> + clk_enable(info->rtc_clk);
> + if (info->data->needs_src_clk)
> + clk_enable(info->rtc_src_clk);
> + info->clk_enabled = true;
> + }
>   spin_unlock_irqrestore(>alarm_clk_lock, irq_flags);
>  }
>  
> @@ -82,9 +86,12 @@ static void s3c_rtc_disable_clk(struct s3c_rtc *info)
>   unsigned long irq_flags;
>  
>   spin_lock_irqsave(>alarm_clk_lock, irq_flags);
> - if (info->data->needs_src_clk)
> - clk_disable(info->rtc_src_clk);
> - clk_disable(info->rtc_clk);
> + if (info->clk_enabled) {
> + if (info->data->needs_src_clk)
> + clk_disable(info->rtc_src_clk);
> + clk_disable(info->rtc_clk);
> + info->clk_enabled = false;
> + }
>   spin_unlock_irqrestore(>alarm_clk_lock, irq_flags);
>  }
>  
> @@ -128,6 +135,11 @@ static int s3c_rtc_setaie(struct device *dev, unsigned 
> int enabled)
>  
>   s3c_rtc_disable_clk(info);
>  
> + if (enabled)
> + s3c_rtc_enable_clk(info);
> + else
> + s3c_rtc_disable_clk(info);
> +
>   return 0;
>  }

During probe the clk_enabled is false, so the clock won't be disabled at
the end of probe with s3c_rtc_disable_clk(). Maybe previous patch
interferes here so it would work after applying all 4 patches but not
when backporting. The patch in current form looks non-backportable.

Best regards,
KRzysztof
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 4/4] rtc: s3c: enable/disable clocks for alarm

2015-08-11 Thread Joonyoung Shim 
The clock enable/disable codes for alarm have removed from
'commit 24e1455493da ("drivers/rtc/rtc-s3c.c: delete duplicate clock
control")' and the clocks keep disabling even if alarm is set, so alarm
interrupt can't happen.

The s3c_rtc_setaie function can be called several times with that
enabled argument has same value, so it needs to check whether clocks is
enabled or not.

Signed-off-by: Joonyoung Shim 
---
 drivers/rtc/rtc-s3c.c | 24 ++--
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/drivers/rtc/rtc-s3c.c b/drivers/rtc/rtc-s3c.c
index abe2a6d..fce078c 100644
--- a/drivers/rtc/rtc-s3c.c
+++ b/drivers/rtc/rtc-s3c.c
@@ -39,6 +39,7 @@ struct s3c_rtc {
void __iomem *base;
struct clk *rtc_clk;
struct clk *rtc_src_clk;
+   bool clk_enabled;
 
struct s3c_rtc_data *data;
 
@@ -71,9 +72,12 @@ static void s3c_rtc_enable_clk(struct s3c_rtc *info)
unsigned long irq_flags;
 
spin_lock_irqsave(>alarm_clk_lock, irq_flags);
-   clk_enable(info->rtc_clk);
-   if (info->data->needs_src_clk)
-   clk_enable(info->rtc_src_clk);
+   if (!info->clk_enabled) {
+   clk_enable(info->rtc_clk);
+   if (info->data->needs_src_clk)
+   clk_enable(info->rtc_src_clk);
+   info->clk_enabled = true;
+   }
spin_unlock_irqrestore(>alarm_clk_lock, irq_flags);
 }
 
@@ -82,9 +86,12 @@ static void s3c_rtc_disable_clk(struct s3c_rtc *info)
unsigned long irq_flags;
 
spin_lock_irqsave(>alarm_clk_lock, irq_flags);
-   if (info->data->needs_src_clk)
-   clk_disable(info->rtc_src_clk);
-   clk_disable(info->rtc_clk);
+   if (info->clk_enabled) {
+   if (info->data->needs_src_clk)
+   clk_disable(info->rtc_src_clk);
+   clk_disable(info->rtc_clk);
+   info->clk_enabled = false;
+   }
spin_unlock_irqrestore(>alarm_clk_lock, irq_flags);
 }
 
@@ -128,6 +135,11 @@ static int s3c_rtc_setaie(struct device *dev, unsigned int 
enabled)
 
s3c_rtc_disable_clk(info);
 
+   if (enabled)
+   s3c_rtc_enable_clk(info);
+   else
+   s3c_rtc_disable_clk(info);
+
return 0;
 }
 
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 4/4] rtc: s3c: enable/disable clocks for alarm

2015-08-11 Thread Joonyoung Shim 
The clock enable/disable codes for alarm have removed from
'commit 24e1455493da (drivers/rtc/rtc-s3c.c: delete duplicate clock
control)' and the clocks keep disabling even if alarm is set, so alarm
interrupt can't happen.

The s3c_rtc_setaie function can be called several times with that
enabled argument has same value, so it needs to check whether clocks is
enabled or not.

Signed-off-by: Joonyoung Shim jy0922.s...@samsung.com
---
 drivers/rtc/rtc-s3c.c | 24 ++--
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/drivers/rtc/rtc-s3c.c b/drivers/rtc/rtc-s3c.c
index abe2a6d..fce078c 100644
--- a/drivers/rtc/rtc-s3c.c
+++ b/drivers/rtc/rtc-s3c.c
@@ -39,6 +39,7 @@ struct s3c_rtc {
void __iomem *base;
struct clk *rtc_clk;
struct clk *rtc_src_clk;
+   bool clk_enabled;
 
struct s3c_rtc_data *data;
 
@@ -71,9 +72,12 @@ static void s3c_rtc_enable_clk(struct s3c_rtc *info)
unsigned long irq_flags;
 
spin_lock_irqsave(info-alarm_clk_lock, irq_flags);
-   clk_enable(info-rtc_clk);
-   if (info-data-needs_src_clk)
-   clk_enable(info-rtc_src_clk);
+   if (!info-clk_enabled) {
+   clk_enable(info-rtc_clk);
+   if (info-data-needs_src_clk)
+   clk_enable(info-rtc_src_clk);
+   info-clk_enabled = true;
+   }
spin_unlock_irqrestore(info-alarm_clk_lock, irq_flags);
 }
 
@@ -82,9 +86,12 @@ static void s3c_rtc_disable_clk(struct s3c_rtc *info)
unsigned long irq_flags;
 
spin_lock_irqsave(info-alarm_clk_lock, irq_flags);
-   if (info-data-needs_src_clk)
-   clk_disable(info-rtc_src_clk);
-   clk_disable(info-rtc_clk);
+   if (info-clk_enabled) {
+   if (info-data-needs_src_clk)
+   clk_disable(info-rtc_src_clk);
+   clk_disable(info-rtc_clk);
+   info-clk_enabled = false;
+   }
spin_unlock_irqrestore(info-alarm_clk_lock, irq_flags);
 }
 
@@ -128,6 +135,11 @@ static int s3c_rtc_setaie(struct device *dev, unsigned int 
enabled)
 
s3c_rtc_disable_clk(info);
 
+   if (enabled)
+   s3c_rtc_enable_clk(info);
+   else
+   s3c_rtc_disable_clk(info);
+
return 0;
 }
 
-- 
1.9.1

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 4/4] rtc: s3c: enable/disable clocks for alarm

2015-08-11 Thread Krzysztof Kozlowski 
On 11.08.2015 20:28, Joonyoung Shim wrote:
 The clock enable/disable codes for alarm have removed from

What do you mean in this paragraph? The clock code was removing something?

 'commit 24e1455493da (drivers/rtc/rtc-s3c.c: delete duplicate clock

Remove the 'apostrophe.

 control)' and the clocks keep disabling even if alarm is set, so alarm
 interrupt can't happen.
...and the clocks are disabled even...


 
 The s3c_rtc_setaie function can be called several times with that
 enabled argument has same value,
...several times with 'enabled' argument having same value

 so it needs to check whether clocks is
 enabled or not.
s/is/are/

 
 Signed-off-by: Joonyoung Shim jy0922.s...@samsung.com

Please add Cc-stable and fixes tag. To backport the patch probably
you'll have to remove the dependency on previous patches.

 ---
  drivers/rtc/rtc-s3c.c | 24 ++--
  1 file changed, 18 insertions(+), 6 deletions(-)
 
 diff --git a/drivers/rtc/rtc-s3c.c b/drivers/rtc/rtc-s3c.c
 index abe2a6d..fce078c 100644
 --- a/drivers/rtc/rtc-s3c.c
 +++ b/drivers/rtc/rtc-s3c.c
 @@ -39,6 +39,7 @@ struct s3c_rtc {
   void __iomem *base;
   struct clk *rtc_clk;
   struct clk *rtc_src_clk;
 + bool clk_enabled;
  
   struct s3c_rtc_data *data;
  
 @@ -71,9 +72,12 @@ static void s3c_rtc_enable_clk(struct s3c_rtc *info)
   unsigned long irq_flags;
  
   spin_lock_irqsave(info-alarm_clk_lock, irq_flags);
 - clk_enable(info-rtc_clk);
 - if (info-data-needs_src_clk)
 - clk_enable(info-rtc_src_clk);
 + if (!info-clk_enabled) {
 + clk_enable(info-rtc_clk);
 + if (info-data-needs_src_clk)
 + clk_enable(info-rtc_src_clk);
 + info-clk_enabled = true;
 + }
   spin_unlock_irqrestore(info-alarm_clk_lock, irq_flags);
  }
  
 @@ -82,9 +86,12 @@ static void s3c_rtc_disable_clk(struct s3c_rtc *info)
   unsigned long irq_flags;
  
   spin_lock_irqsave(info-alarm_clk_lock, irq_flags);
 - if (info-data-needs_src_clk)
 - clk_disable(info-rtc_src_clk);
 - clk_disable(info-rtc_clk);
 + if (info-clk_enabled) {
 + if (info-data-needs_src_clk)
 + clk_disable(info-rtc_src_clk);
 + clk_disable(info-rtc_clk);
 + info-clk_enabled = false;
 + }
   spin_unlock_irqrestore(info-alarm_clk_lock, irq_flags);
  }
  
 @@ -128,6 +135,11 @@ static int s3c_rtc_setaie(struct device *dev, unsigned 
 int enabled)
  
   s3c_rtc_disable_clk(info);
  
 + if (enabled)
 + s3c_rtc_enable_clk(info);
 + else
 + s3c_rtc_disable_clk(info);
 +
   return 0;
  }

During probe the clk_enabled is false, so the clock won't be disabled at
the end of probe with s3c_rtc_disable_clk(). Maybe previous patch
interferes here so it would work after applying all 4 patches but not
when backporting. The patch in current form looks non-backportable.

Best regards,
KRzysztof
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 4/4] rtc: s3c: enable/disable clocks for alarm

2015-08-11 Thread Joonyoung Shim 
On 08/12/2015 09:28 AM, Krzysztof Kozlowski wrote:
 On 11.08.2015 20:28, Joonyoung Shim wrote:
 The clock enable/disable codes for alarm have removed from
 
 What do you mean in this paragraph? The clock code was removing something?
 
 'commit 24e1455493da (drivers/rtc/rtc-s3c.c: delete duplicate clock
 
 Remove the 'apostrophe.
 
 control)' and the clocks keep disabling even if alarm is set, so alarm
 interrupt can't happen.
 ...and the clocks are disabled even...
 
 

 The s3c_rtc_setaie function can be called several times with that
 enabled argument has same value,
 ...several times with 'enabled' argument having same value
 
 so it needs to check whether clocks is
 enabled or not.
 s/is/are/
 

 Signed-off-by: Joonyoung Shim jy0922.s...@samsung.com
 
 Please add Cc-stable and fixes tag. To backport the patch probably
 you'll have to remove the dependency on previous patches.

Thanks for the point, i didn't think it.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/