All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC v2 1/1] clk: Add notifier support in clk_prepare/clk_unprepare
@ 2013-03-14  9:31 ` Bill Huang
  0 siblings, 0 replies; 10+ messages in thread
From: Bill Huang @ 2013-03-14  9:31 UTC (permalink / raw)
  To: mturquette, patches, linux-kernel, linaro-dev, linux-arm-kernel
  Cc: Bill Huang

Add the below two notifier events so drivers which are interested in
knowing the clock status can act accordingly. This is extremely useful
in some of the DVFS (Dynamic Voltage Frequency Scaling) design.

CLK_PREPARED
CLK_UNPREPARED

Signed-off-by: Bill Huang <bilhuang@nvidia.com>
---
 drivers/clk/clk.c   |    3 +++
 include/linux/clk.h |    2 ++
 2 files changed, 5 insertions(+)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index ed87b24..3292cec 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -550,6 +550,7 @@ void clk_unprepare(struct clk *clk)
 {
 	mutex_lock(&prepare_lock);
 	__clk_unprepare(clk);
+	__clk_notify(clk, CLK_UNPREPARED, clk->rate, clk->rate);
 	mutex_unlock(&prepare_lock);
 }
 EXPORT_SYMBOL_GPL(clk_unprepare);
@@ -598,6 +599,8 @@ int clk_prepare(struct clk *clk)
 
 	mutex_lock(&prepare_lock);
 	ret = __clk_prepare(clk);
+	if (!ret)
+		__clk_notify(clk, CLK_PREPARED, clk->rate, clk->rate);
 	mutex_unlock(&prepare_lock);
 
 	return ret;
diff --git a/include/linux/clk.h b/include/linux/clk.h
index b3ac22d..16c1d92 100644
--- a/include/linux/clk.h
+++ b/include/linux/clk.h
@@ -43,6 +43,8 @@ struct clk;
 #define PRE_RATE_CHANGE			BIT(0)
 #define POST_RATE_CHANGE		BIT(1)
 #define ABORT_RATE_CHANGE		BIT(2)
+#define CLK_PREPARED			BIT(3)
+#define CLK_UNPREPARED			BIT(4)
 
 /**
  * struct clk_notifier - associate a clk with a notifier
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [RFC v2 1/1] clk: Add notifier support in clk_prepare/clk_unprepare
@ 2013-03-14  9:31 ` Bill Huang
  0 siblings, 0 replies; 10+ messages in thread
From: Bill Huang @ 2013-03-14  9:31 UTC (permalink / raw)
  To: linux-arm-kernel

Add the below two notifier events so drivers which are interested in
knowing the clock status can act accordingly. This is extremely useful
in some of the DVFS (Dynamic Voltage Frequency Scaling) design.

CLK_PREPARED
CLK_UNPREPARED

Signed-off-by: Bill Huang <bilhuang@nvidia.com>
---
 drivers/clk/clk.c   |    3 +++
 include/linux/clk.h |    2 ++
 2 files changed, 5 insertions(+)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index ed87b24..3292cec 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -550,6 +550,7 @@ void clk_unprepare(struct clk *clk)
 {
 	mutex_lock(&prepare_lock);
 	__clk_unprepare(clk);
+	__clk_notify(clk, CLK_UNPREPARED, clk->rate, clk->rate);
 	mutex_unlock(&prepare_lock);
 }
 EXPORT_SYMBOL_GPL(clk_unprepare);
@@ -598,6 +599,8 @@ int clk_prepare(struct clk *clk)
 
 	mutex_lock(&prepare_lock);
 	ret = __clk_prepare(clk);
+	if (!ret)
+		__clk_notify(clk, CLK_PREPARED, clk->rate, clk->rate);
 	mutex_unlock(&prepare_lock);
 
 	return ret;
diff --git a/include/linux/clk.h b/include/linux/clk.h
index b3ac22d..16c1d92 100644
--- a/include/linux/clk.h
+++ b/include/linux/clk.h
@@ -43,6 +43,8 @@ struct clk;
 #define PRE_RATE_CHANGE			BIT(0)
 #define POST_RATE_CHANGE		BIT(1)
 #define ABORT_RATE_CHANGE		BIT(2)
+#define CLK_PREPARED			BIT(3)
+#define CLK_UNPREPARED			BIT(4)
 
 /**
  * struct clk_notifier - associate a clk with a notifier
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [RFC v2 1/1] clk: Add notifier support in clk_prepare/clk_unprepare
  2013-03-14  9:31 ` Bill Huang
@ 2013-03-14  9:51   ` Bill Huang
  -1 siblings, 0 replies; 10+ messages in thread
From: Bill Huang @ 2013-03-14  9:51 UTC (permalink / raw)
  To: mturquette; +Cc: patches, linux-kernel, linaro-dev, linux-arm-kernel

On Thu, 2013-03-14 at 17:31 +0800, Bill Huang wrote:
> Add the below two notifier events so drivers which are interested in
> knowing the clock status can act accordingly. This is extremely useful
> in some of the DVFS (Dynamic Voltage Frequency Scaling) design.
> 
> CLK_PREPARED
> CLK_UNPREPARED
> 
> Signed-off-by: Bill Huang <bilhuang@nvidia.com>
> ---
>  drivers/clk/clk.c   |    3 +++
>  include/linux/clk.h |    2 ++
>  2 files changed, 5 insertions(+)
> 
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index ed87b24..3292cec 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -550,6 +550,7 @@ void clk_unprepare(struct clk *clk)
>  {
>  	mutex_lock(&prepare_lock);
>  	__clk_unprepare(clk);
> +	__clk_notify(clk, CLK_UNPREPARED, clk->rate, clk->rate);
>  	mutex_unlock(&prepare_lock);
>  }
>  EXPORT_SYMBOL_GPL(clk_unprepare);
> @@ -598,6 +599,8 @@ int clk_prepare(struct clk *clk)
>  
>  	mutex_lock(&prepare_lock);
>  	ret = __clk_prepare(clk);
> +	if (!ret)
> +		__clk_notify(clk, CLK_PREPARED, clk->rate, clk->rate);
>  	mutex_unlock(&prepare_lock);
>  
>  	return ret;
> diff --git a/include/linux/clk.h b/include/linux/clk.h
> index b3ac22d..16c1d92 100644
> --- a/include/linux/clk.h
> +++ b/include/linux/clk.h
> @@ -43,6 +43,8 @@ struct clk;
>  #define PRE_RATE_CHANGE			BIT(0)
>  #define POST_RATE_CHANGE		BIT(1)
>  #define ABORT_RATE_CHANGE		BIT(2)
> +#define CLK_PREPARED			BIT(3)
> +#define CLK_UNPREPARED			BIT(4)
>  
>  /**
>   * struct clk_notifier - associate a clk with a notifier
Sorry, I'll send v3 since this bread build.



^ permalink raw reply	[flat|nested] 10+ messages in thread

* [RFC v2 1/1] clk: Add notifier support in clk_prepare/clk_unprepare
@ 2013-03-14  9:51   ` Bill Huang
  0 siblings, 0 replies; 10+ messages in thread
From: Bill Huang @ 2013-03-14  9:51 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, 2013-03-14 at 17:31 +0800, Bill Huang wrote:
> Add the below two notifier events so drivers which are interested in
> knowing the clock status can act accordingly. This is extremely useful
> in some of the DVFS (Dynamic Voltage Frequency Scaling) design.
> 
> CLK_PREPARED
> CLK_UNPREPARED
> 
> Signed-off-by: Bill Huang <bilhuang@nvidia.com>
> ---
>  drivers/clk/clk.c   |    3 +++
>  include/linux/clk.h |    2 ++
>  2 files changed, 5 insertions(+)
> 
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index ed87b24..3292cec 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -550,6 +550,7 @@ void clk_unprepare(struct clk *clk)
>  {
>  	mutex_lock(&prepare_lock);
>  	__clk_unprepare(clk);
> +	__clk_notify(clk, CLK_UNPREPARED, clk->rate, clk->rate);
>  	mutex_unlock(&prepare_lock);
>  }
>  EXPORT_SYMBOL_GPL(clk_unprepare);
> @@ -598,6 +599,8 @@ int clk_prepare(struct clk *clk)
>  
>  	mutex_lock(&prepare_lock);
>  	ret = __clk_prepare(clk);
> +	if (!ret)
> +		__clk_notify(clk, CLK_PREPARED, clk->rate, clk->rate);
>  	mutex_unlock(&prepare_lock);
>  
>  	return ret;
> diff --git a/include/linux/clk.h b/include/linux/clk.h
> index b3ac22d..16c1d92 100644
> --- a/include/linux/clk.h
> +++ b/include/linux/clk.h
> @@ -43,6 +43,8 @@ struct clk;
>  #define PRE_RATE_CHANGE			BIT(0)
>  #define POST_RATE_CHANGE		BIT(1)
>  #define ABORT_RATE_CHANGE		BIT(2)
> +#define CLK_PREPARED			BIT(3)
> +#define CLK_UNPREPARED			BIT(4)
>  
>  /**
>   * struct clk_notifier - associate a clk with a notifier
Sorry, I'll send v3 since this bread build.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC v2 1/1] clk: Add notifier support in clk_prepare/clk_unprepare
  2013-03-14  9:31 ` Bill Huang
@ 2013-03-15 17:45   ` Russell King - ARM Linux
  -1 siblings, 0 replies; 10+ messages in thread
From: Russell King - ARM Linux @ 2013-03-15 17:45 UTC (permalink / raw)
  To: Bill Huang
  Cc: mturquette, patches, linux-kernel, linaro-dev, linux-arm-kernel

On Thu, Mar 14, 2013 at 02:31:04AM -0700, Bill Huang wrote:
> Add the below two notifier events so drivers which are interested in
> knowing the clock status can act accordingly. This is extremely useful
> in some of the DVFS (Dynamic Voltage Frequency Scaling) design.
> 
> CLK_PREPARED
> CLK_UNPREPARED
> 
> Signed-off-by: Bill Huang <bilhuang@nvidia.com>
> ---
>  drivers/clk/clk.c   |    3 +++
>  include/linux/clk.h |    2 ++
>  2 files changed, 5 insertions(+)
> 
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index ed87b24..3292cec 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -550,6 +550,7 @@ void clk_unprepare(struct clk *clk)
>  {
>  	mutex_lock(&prepare_lock);
>  	__clk_unprepare(clk);
> +	__clk_notify(clk, CLK_UNPREPARED, clk->rate, clk->rate);
>  	mutex_unlock(&prepare_lock);
>  }
>  EXPORT_SYMBOL_GPL(clk_unprepare);
> @@ -598,6 +599,8 @@ int clk_prepare(struct clk *clk)
>  
>  	mutex_lock(&prepare_lock);
>  	ret = __clk_prepare(clk);
> +	if (!ret)
> +		__clk_notify(clk, CLK_PREPARED, clk->rate, clk->rate);

So, on prepare, we notify after we've prepared the clock.  On unprepare,
we notify after the clock has been shut down.  Are you sure that's the
correct ordering?  Would it not be better to view it in a stack-like
fashion, iow:

get
	prepare
		notify-prepare
			enable
			disable
		notify-unprepare
	unprepare
put

?

> diff --git a/include/linux/clk.h b/include/linux/clk.h
> index b3ac22d..16c1d92 100644
> --- a/include/linux/clk.h
> +++ b/include/linux/clk.h
> @@ -43,6 +43,8 @@ struct clk;
>  #define PRE_RATE_CHANGE			BIT(0)
>  #define POST_RATE_CHANGE		BIT(1)
>  #define ABORT_RATE_CHANGE		BIT(2)
> +#define CLK_PREPARED			BIT(3)
> +#define CLK_UNPREPARED			BIT(4)

This implies that we're only going to have a maximum of 32 reason codes
here.  Is that enough?

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [RFC v2 1/1] clk: Add notifier support in clk_prepare/clk_unprepare
@ 2013-03-15 17:45   ` Russell King - ARM Linux
  0 siblings, 0 replies; 10+ messages in thread
From: Russell King - ARM Linux @ 2013-03-15 17:45 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Mar 14, 2013 at 02:31:04AM -0700, Bill Huang wrote:
> Add the below two notifier events so drivers which are interested in
> knowing the clock status can act accordingly. This is extremely useful
> in some of the DVFS (Dynamic Voltage Frequency Scaling) design.
> 
> CLK_PREPARED
> CLK_UNPREPARED
> 
> Signed-off-by: Bill Huang <bilhuang@nvidia.com>
> ---
>  drivers/clk/clk.c   |    3 +++
>  include/linux/clk.h |    2 ++
>  2 files changed, 5 insertions(+)
> 
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index ed87b24..3292cec 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -550,6 +550,7 @@ void clk_unprepare(struct clk *clk)
>  {
>  	mutex_lock(&prepare_lock);
>  	__clk_unprepare(clk);
> +	__clk_notify(clk, CLK_UNPREPARED, clk->rate, clk->rate);
>  	mutex_unlock(&prepare_lock);
>  }
>  EXPORT_SYMBOL_GPL(clk_unprepare);
> @@ -598,6 +599,8 @@ int clk_prepare(struct clk *clk)
>  
>  	mutex_lock(&prepare_lock);
>  	ret = __clk_prepare(clk);
> +	if (!ret)
> +		__clk_notify(clk, CLK_PREPARED, clk->rate, clk->rate);

So, on prepare, we notify after we've prepared the clock.  On unprepare,
we notify after the clock has been shut down.  Are you sure that's the
correct ordering?  Would it not be better to view it in a stack-like
fashion, iow:

get
	prepare
		notify-prepare
			enable
			disable
		notify-unprepare
	unprepare
put

?

> diff --git a/include/linux/clk.h b/include/linux/clk.h
> index b3ac22d..16c1d92 100644
> --- a/include/linux/clk.h
> +++ b/include/linux/clk.h
> @@ -43,6 +43,8 @@ struct clk;
>  #define PRE_RATE_CHANGE			BIT(0)
>  #define POST_RATE_CHANGE		BIT(1)
>  #define ABORT_RATE_CHANGE		BIT(2)
> +#define CLK_PREPARED			BIT(3)
> +#define CLK_UNPREPARED			BIT(4)

This implies that we're only going to have a maximum of 32 reason codes
here.  Is that enough?

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC v2 1/1] clk: Add notifier support in clk_prepare/clk_unprepare
  2013-03-15 17:45   ` Russell King - ARM Linux
@ 2013-03-15 19:51     ` Stephen Warren
  -1 siblings, 0 replies; 10+ messages in thread
From: Stephen Warren @ 2013-03-15 19:51 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Bill Huang, mturquette, patches, linux-kernel, linaro-dev,
	linux-arm-kernel

On 03/15/2013 11:45 AM, Russell King - ARM Linux wrote:
> On Thu, Mar 14, 2013 at 02:31:04AM -0700, Bill Huang wrote:
>> Add the below two notifier events so drivers which are interested in
>> knowing the clock status can act accordingly. This is extremely useful
>> in some of the DVFS (Dynamic Voltage Frequency Scaling) design.
>>
>> CLK_PREPARED
>> CLK_UNPREPARED
>>
>> Signed-off-by: Bill Huang <bilhuang@nvidia.com>
>> ---
>>  drivers/clk/clk.c   |    3 +++
>>  include/linux/clk.h |    2 ++
>>  2 files changed, 5 insertions(+)
>>
>> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
>> index ed87b24..3292cec 100644
>> --- a/drivers/clk/clk.c
>> +++ b/drivers/clk/clk.c
>> @@ -550,6 +550,7 @@ void clk_unprepare(struct clk *clk)
>>  {
>>  	mutex_lock(&prepare_lock);
>>  	__clk_unprepare(clk);
>> +	__clk_notify(clk, CLK_UNPREPARED, clk->rate, clk->rate);
>>  	mutex_unlock(&prepare_lock);
>>  }
>>  EXPORT_SYMBOL_GPL(clk_unprepare);
>> @@ -598,6 +599,8 @@ int clk_prepare(struct clk *clk)
>>  
>>  	mutex_lock(&prepare_lock);
>>  	ret = __clk_prepare(clk);
>> +	if (!ret)
>> +		__clk_notify(clk, CLK_PREPARED, clk->rate, clk->rate);
> 
> So, on prepare, we notify after we've prepared the clock.  On unprepare,
> we notify after the clock has been shut down.  Are you sure that's the
> correct ordering?  Would it not be better to view it in a stack-like
> fashion, iow:

> get
> 	prepare
> 		notify-prepare
> 			enable
> 			disable
> 		notify-unprepare
> 	unprepare
> put

Yes, these should be stacked/nested better for consistency.

But for DVFS, the voltage needs to be raised before the prepare body is
run, so that if clk_prepare actually enables the clock, the voltage is
already at the safe level required by that clock. Similarly, for
unprepare, you can only lower the voltage after having turned off the
clock, which is guaranteed after the unprepare body. So, I think you
want to move the notifier for prepare in the code above (and rename it
to pre/before_prepare?), rather than the notifier for unprepare.

In order to cover more cases, you might have both
{pre,post}_{un,}prepare notifiers, although I'm not sure when you'd use
the other two options.

>> diff --git a/include/linux/clk.h b/include/linux/clk.h

>> +#define CLK_PREPARED			BIT(3)
>> +#define CLK_UNPREPARED			BIT(4)


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [RFC v2 1/1] clk: Add notifier support in clk_prepare/clk_unprepare
@ 2013-03-15 19:51     ` Stephen Warren
  0 siblings, 0 replies; 10+ messages in thread
From: Stephen Warren @ 2013-03-15 19:51 UTC (permalink / raw)
  To: linux-arm-kernel

On 03/15/2013 11:45 AM, Russell King - ARM Linux wrote:
> On Thu, Mar 14, 2013 at 02:31:04AM -0700, Bill Huang wrote:
>> Add the below two notifier events so drivers which are interested in
>> knowing the clock status can act accordingly. This is extremely useful
>> in some of the DVFS (Dynamic Voltage Frequency Scaling) design.
>>
>> CLK_PREPARED
>> CLK_UNPREPARED
>>
>> Signed-off-by: Bill Huang <bilhuang@nvidia.com>
>> ---
>>  drivers/clk/clk.c   |    3 +++
>>  include/linux/clk.h |    2 ++
>>  2 files changed, 5 insertions(+)
>>
>> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
>> index ed87b24..3292cec 100644
>> --- a/drivers/clk/clk.c
>> +++ b/drivers/clk/clk.c
>> @@ -550,6 +550,7 @@ void clk_unprepare(struct clk *clk)
>>  {
>>  	mutex_lock(&prepare_lock);
>>  	__clk_unprepare(clk);
>> +	__clk_notify(clk, CLK_UNPREPARED, clk->rate, clk->rate);
>>  	mutex_unlock(&prepare_lock);
>>  }
>>  EXPORT_SYMBOL_GPL(clk_unprepare);
>> @@ -598,6 +599,8 @@ int clk_prepare(struct clk *clk)
>>  
>>  	mutex_lock(&prepare_lock);
>>  	ret = __clk_prepare(clk);
>> +	if (!ret)
>> +		__clk_notify(clk, CLK_PREPARED, clk->rate, clk->rate);
> 
> So, on prepare, we notify after we've prepared the clock.  On unprepare,
> we notify after the clock has been shut down.  Are you sure that's the
> correct ordering?  Would it not be better to view it in a stack-like
> fashion, iow:

> get
> 	prepare
> 		notify-prepare
> 			enable
> 			disable
> 		notify-unprepare
> 	unprepare
> put

Yes, these should be stacked/nested better for consistency.

But for DVFS, the voltage needs to be raised before the prepare body is
run, so that if clk_prepare actually enables the clock, the voltage is
already at the safe level required by that clock. Similarly, for
unprepare, you can only lower the voltage after having turned off the
clock, which is guaranteed after the unprepare body. So, I think you
want to move the notifier for prepare in the code above (and rename it
to pre/before_prepare?), rather than the notifier for unprepare.

In order to cover more cases, you might have both
{pre,post}_{un,}prepare notifiers, although I'm not sure when you'd use
the other two options.

>> diff --git a/include/linux/clk.h b/include/linux/clk.h

>> +#define CLK_PREPARED			BIT(3)
>> +#define CLK_UNPREPARED			BIT(4)

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC v2 1/1] clk: Add notifier support in clk_prepare/clk_unprepare
  2013-03-15 19:51     ` Stephen Warren
@ 2013-03-16  1:58       ` Bill Huang
  -1 siblings, 0 replies; 10+ messages in thread
From: Bill Huang @ 2013-03-16  1:58 UTC (permalink / raw)
  To: Stephen Warren
  Cc: Russell King - ARM Linux, mturquette, patches, linux-kernel,
	linaro-dev, linux-arm-kernel

On Sat, 2013-03-16 at 03:51 +0800, Stephen Warren wrote:
> On 03/15/2013 11:45 AM, Russell King - ARM Linux wrote:
> > On Thu, Mar 14, 2013 at 02:31:04AM -0700, Bill Huang wrote:
> >> Add the below two notifier events so drivers which are interested in
> >> knowing the clock status can act accordingly. This is extremely useful
> >> in some of the DVFS (Dynamic Voltage Frequency Scaling) design.
> >>
> >> CLK_PREPARED
> >> CLK_UNPREPARED
> >>
> >> Signed-off-by: Bill Huang <bilhuang@nvidia.com>
> >> ---
> >>  drivers/clk/clk.c   |    3 +++
> >>  include/linux/clk.h |    2 ++
> >>  2 files changed, 5 insertions(+)
> >>
> >> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> >> index ed87b24..3292cec 100644
> >> --- a/drivers/clk/clk.c
> >> +++ b/drivers/clk/clk.c
> >> @@ -550,6 +550,7 @@ void clk_unprepare(struct clk *clk)
> >>  {
> >>  	mutex_lock(&prepare_lock);
> >>  	__clk_unprepare(clk);
> >> +	__clk_notify(clk, CLK_UNPREPARED, clk->rate, clk->rate);
> >>  	mutex_unlock(&prepare_lock);
> >>  }
> >>  EXPORT_SYMBOL_GPL(clk_unprepare);
> >> @@ -598,6 +599,8 @@ int clk_prepare(struct clk *clk)
> >>  
> >>  	mutex_lock(&prepare_lock);
> >>  	ret = __clk_prepare(clk);
> >> +	if (!ret)
> >> +		__clk_notify(clk, CLK_PREPARED, clk->rate, clk->rate);
> > 
> > So, on prepare, we notify after we've prepared the clock.  On unprepare,
> > we notify after the clock has been shut down.  Are you sure that's the
> > correct ordering?  Would it not be better to view it in a stack-like
> > fashion, iow:
> 
> > get
> > 	prepare
> > 		notify-prepare
> > 			enable
> > 			disable
> > 		notify-unprepare
> > 	unprepare
> > put
> 
> Yes, these should be stacked/nested better for consistency.
> 
> But for DVFS, the voltage needs to be raised before the prepare body is
> run, so that if clk_prepare actually enables the clock, the voltage is
> already at the safe level required by that clock. Similarly, for
> unprepare, you can only lower the voltage after having turned off the
> clock, which is guaranteed after the unprepare body. So, I think you
> want to move the notifier for prepare in the code above (and rename it
> to pre/before_prepare?), rather than the notifier for unprepare.

Oh yes I should raised notify before prepare body is run.
> 
> In order to cover more cases, you might have both
> {pre,post}_{un,}prepare notifiers, although I'm not sure when you'd use
> the other two options.

Right, maybe {pre,post}_{un,}prepare will be useful.
> 
> >> diff --git a/include/linux/clk.h b/include/linux/clk.h
> 
> >> +#define CLK_PREPARED			BIT(3)
> >> +#define CLK_UNPREPARED			BIT(4)
> 



^ permalink raw reply	[flat|nested] 10+ messages in thread

* [RFC v2 1/1] clk: Add notifier support in clk_prepare/clk_unprepare
@ 2013-03-16  1:58       ` Bill Huang
  0 siblings, 0 replies; 10+ messages in thread
From: Bill Huang @ 2013-03-16  1:58 UTC (permalink / raw)
  To: linux-arm-kernel

On Sat, 2013-03-16 at 03:51 +0800, Stephen Warren wrote:
> On 03/15/2013 11:45 AM, Russell King - ARM Linux wrote:
> > On Thu, Mar 14, 2013 at 02:31:04AM -0700, Bill Huang wrote:
> >> Add the below two notifier events so drivers which are interested in
> >> knowing the clock status can act accordingly. This is extremely useful
> >> in some of the DVFS (Dynamic Voltage Frequency Scaling) design.
> >>
> >> CLK_PREPARED
> >> CLK_UNPREPARED
> >>
> >> Signed-off-by: Bill Huang <bilhuang@nvidia.com>
> >> ---
> >>  drivers/clk/clk.c   |    3 +++
> >>  include/linux/clk.h |    2 ++
> >>  2 files changed, 5 insertions(+)
> >>
> >> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> >> index ed87b24..3292cec 100644
> >> --- a/drivers/clk/clk.c
> >> +++ b/drivers/clk/clk.c
> >> @@ -550,6 +550,7 @@ void clk_unprepare(struct clk *clk)
> >>  {
> >>  	mutex_lock(&prepare_lock);
> >>  	__clk_unprepare(clk);
> >> +	__clk_notify(clk, CLK_UNPREPARED, clk->rate, clk->rate);
> >>  	mutex_unlock(&prepare_lock);
> >>  }
> >>  EXPORT_SYMBOL_GPL(clk_unprepare);
> >> @@ -598,6 +599,8 @@ int clk_prepare(struct clk *clk)
> >>  
> >>  	mutex_lock(&prepare_lock);
> >>  	ret = __clk_prepare(clk);
> >> +	if (!ret)
> >> +		__clk_notify(clk, CLK_PREPARED, clk->rate, clk->rate);
> > 
> > So, on prepare, we notify after we've prepared the clock.  On unprepare,
> > we notify after the clock has been shut down.  Are you sure that's the
> > correct ordering?  Would it not be better to view it in a stack-like
> > fashion, iow:
> 
> > get
> > 	prepare
> > 		notify-prepare
> > 			enable
> > 			disable
> > 		notify-unprepare
> > 	unprepare
> > put
> 
> Yes, these should be stacked/nested better for consistency.
> 
> But for DVFS, the voltage needs to be raised before the prepare body is
> run, so that if clk_prepare actually enables the clock, the voltage is
> already at the safe level required by that clock. Similarly, for
> unprepare, you can only lower the voltage after having turned off the
> clock, which is guaranteed after the unprepare body. So, I think you
> want to move the notifier for prepare in the code above (and rename it
> to pre/before_prepare?), rather than the notifier for unprepare.

Oh yes I should raised notify before prepare body is run.
> 
> In order to cover more cases, you might have both
> {pre,post}_{un,}prepare notifiers, although I'm not sure when you'd use
> the other two options.

Right, maybe {pre,post}_{un,}prepare will be useful.
> 
> >> diff --git a/include/linux/clk.h b/include/linux/clk.h
> 
> >> +#define CLK_PREPARED			BIT(3)
> >> +#define CLK_UNPREPARED			BIT(4)
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2013-03-16  1:58 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-03-14  9:31 [RFC v2 1/1] clk: Add notifier support in clk_prepare/clk_unprepare Bill Huang
2013-03-14  9:31 ` Bill Huang
2013-03-14  9:51 ` Bill Huang
2013-03-14  9:51   ` Bill Huang
2013-03-15 17:45 ` Russell King - ARM Linux
2013-03-15 17:45   ` Russell King - ARM Linux
2013-03-15 19:51   ` Stephen Warren
2013-03-15 19:51     ` Stephen Warren
2013-03-16  1:58     ` Bill Huang
2013-03-16  1:58       ` Bill Huang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.