Re: [PATCH] cpufreq: ARM big LITTLE: Add generic cpufreq driver and its DT glue

2013-03-07 Thread Viresh Kumar
On 7 March 2013 19:49, Russell King - ARM Linux wrote: > So how is this different from any other clock which may also return zero > from its clk_get_rate() ? > > If that's the condition you want to check for, call clk_get_rate() after > a successful clk_get*() and check for the condition. Don't

Re: [PATCH] cpufreq: ARM big LITTLE: Add generic cpufreq driver and its DT glue

2013-03-07 Thread Russell King - ARM Linux
On Thu, Mar 07, 2013 at 10:03:28AM +0800, Viresh Kumar wrote: > On 7 March 2013 08:51, Russell King - ARM Linux > wrote: > > On Thu, Mar 07, 2013 at 08:32:37AM +0800, Viresh Kumar wrote: > >> On 5 March 2013 18:52, Russell King - ARM Linux > >> wrote: > >> > On Tue, Mar 05, 2013 at 12:52:41PM

Re: [PATCH] cpufreq: ARM big LITTLE: Add generic cpufreq driver and its DT glue

2013-03-07 Thread Russell King - ARM Linux
On Thu, Mar 07, 2013 at 10:03:28AM +0800, Viresh Kumar wrote: On 7 March 2013 08:51, Russell King - ARM Linux li...@arm.linux.org.uk wrote: On Thu, Mar 07, 2013 at 08:32:37AM +0800, Viresh Kumar wrote: On 5 March 2013 18:52, Russell King - ARM Linux li...@arm.linux.org.uk wrote: On

Re: [PATCH] cpufreq: ARM big LITTLE: Add generic cpufreq driver and its DT glue

2013-03-07 Thread Viresh Kumar
On 7 March 2013 19:49, Russell King - ARM Linux li...@arm.linux.org.uk wrote: So how is this different from any other clock which may also return zero from its clk_get_rate() ? If that's the condition you want to check for, call clk_get_rate() after a successful clk_get*() and check for the

Re: [PATCH] cpufreq: ARM big LITTLE: Add generic cpufreq driver and its DT glue

2013-03-06 Thread Viresh Kumar
On 7 March 2013 09:50, Olof Johansson wrote: > On Wed, Mar 6, 2013 at 5:46 PM, Olof Johansson wrote: > >> This binding makes no sense to me. It needs to be substantially better >> documented, not just a couple of sentences that people that understand >> bit.LITTLE thoroughly can make sense of.

Re: [PATCH] cpufreq: ARM big LITTLE: Add generic cpufreq driver and its DT glue

2013-03-06 Thread Viresh Kumar
On 7 March 2013 08:51, Russell King - ARM Linux wrote: > On Thu, Mar 07, 2013 at 08:32:37AM +0800, Viresh Kumar wrote: >> On 5 March 2013 18:52, Russell King - ARM Linux >> wrote: >> > On Tue, Mar 05, 2013 at 12:52:41PM +0800, Viresh Kumar wrote: >> >> + clk[cluster] = clk_get(NULL, name);

Re: [PATCH] cpufreq: ARM big LITTLE: Add generic cpufreq driver and its DT glue

2013-03-06 Thread Olof Johansson
On Wed, Mar 6, 2013 at 5:46 PM, Olof Johansson wrote: > This binding makes no sense to me. It needs to be substantially better > documented, not just a couple of sentences that people that understand > bit.LITTLE thoroughly can make sense of. > > It also duplicates the cpu binding. I suspect

Re: [PATCH] cpufreq: ARM big LITTLE: Add generic cpufreq driver and its DT glue

2013-03-06 Thread Olof Johansson
On Tue, Mar 05, 2013 at 12:52:41PM +0800, Viresh Kumar wrote: > +++ b/Documentation/devicetree/bindings/cpufreq/arm_big_little_dt.txt > @@ -0,0 +1,29 @@ > +Generic ARM big LITTLE cpufreq driver's DT glue > +--- > + > +It is DT specific glue layer for

Re: [PATCH] cpufreq: ARM big LITTLE: Add generic cpufreq driver and its DT glue

2013-03-06 Thread Viresh Kumar
On 7 March 2013 08:49, Harvey Harrison wrote: > On Wed, Mar 6, 2013 at 4:32 PM, Viresh Kumar wrote: > >> clk[cluster] = clk_get_sys(name, NULL); >> - if (!IS_ERR(clk[cluster])) { >> + if (!IS_ERR_OR_NULL(clk[cluster])) { >> pr_debug("%s: clk: %p & freq table:

Re: [PATCH] cpufreq: ARM big LITTLE: Add generic cpufreq driver and its DT glue

2013-03-06 Thread Russell King - ARM Linux
On Thu, Mar 07, 2013 at 08:32:37AM +0800, Viresh Kumar wrote: > On 5 March 2013 18:52, Russell King - ARM Linux > wrote: > > On Tue, Mar 05, 2013 at 12:52:41PM +0800, Viresh Kumar wrote: > >> + clk[cluster] = clk_get(NULL, name); > >> + if (!IS_ERR_OR_NULL(clk[cluster])) { > > > > NAK.

Re: [PATCH] cpufreq: ARM big LITTLE: Add generic cpufreq driver and its DT glue

2013-03-06 Thread Harvey Harrison
On Wed, Mar 6, 2013 at 4:32 PM, Viresh Kumar wrote: > clk[cluster] = clk_get_sys(name, NULL); > - if (!IS_ERR(clk[cluster])) { > + if (!IS_ERR_OR_NULL(clk[cluster])) { > pr_debug("%s: clk: %p & freq table: %p, cluster: %d\n", >

Re: [PATCH] cpufreq: ARM big LITTLE: Add generic cpufreq driver and its DT glue

2013-03-06 Thread Viresh Kumar
On 5 March 2013 18:52, Russell King - ARM Linux wrote: > On Tue, Mar 05, 2013 at 12:52:41PM +0800, Viresh Kumar wrote: >> + clk[cluster] = clk_get(NULL, name); >> + if (!IS_ERR_OR_NULL(clk[cluster])) { > > NAK. Two reasons. > > 1. IS_ERR_OR_NULL. You know about this, it's been on the

Re: [PATCH] cpufreq: ARM big LITTLE: Add generic cpufreq driver and its DT glue

2013-03-06 Thread Viresh Kumar
On 7 March 2013 01:25, Russell King - ARM Linux wrote: > On Wed, Mar 06, 2013 at 12:38:36AM +0800, Viresh Kumar wrote: >> So how does below fix look to you? > > Much better, but I think you want a better device name than just "clusterN". I will try to find some other name, maybe cpu-cluster (as

Re: [PATCH] cpufreq: ARM big LITTLE: Add generic cpufreq driver and its DT glue

2013-03-06 Thread Russell King - ARM Linux
On Wed, Mar 06, 2013 at 12:38:36AM +0800, Viresh Kumar wrote: > So how does below fix look to you? Much better, but I think you want a better device name than just "clusterN". Also, please get rid of that "default n" - n is the default default default default default default there's no need

Re: [PATCH] cpufreq: ARM big LITTLE: Add generic cpufreq driver and its DT glue

2013-03-06 Thread Russell King - ARM Linux
On Wed, Mar 06, 2013 at 12:38:36AM +0800, Viresh Kumar wrote: So how does below fix look to you? Much better, but I think you want a better device name than just clusterN. Also, please get rid of that default n - n is the default default default default default default there's no need to

Re: [PATCH] cpufreq: ARM big LITTLE: Add generic cpufreq driver and its DT glue

2013-03-06 Thread Viresh Kumar
On 7 March 2013 01:25, Russell King - ARM Linux li...@arm.linux.org.uk wrote: On Wed, Mar 06, 2013 at 12:38:36AM +0800, Viresh Kumar wrote: So how does below fix look to you? Much better, but I think you want a better device name than just clusterN. I will try to find some other name, maybe

Re: [PATCH] cpufreq: ARM big LITTLE: Add generic cpufreq driver and its DT glue

2013-03-06 Thread Viresh Kumar
On 5 March 2013 18:52, Russell King - ARM Linux li...@arm.linux.org.uk wrote: On Tue, Mar 05, 2013 at 12:52:41PM +0800, Viresh Kumar wrote: + clk[cluster] = clk_get(NULL, name); + if (!IS_ERR_OR_NULL(clk[cluster])) { NAK. Two reasons. 1. IS_ERR_OR_NULL. You know about this, it's

Re: [PATCH] cpufreq: ARM big LITTLE: Add generic cpufreq driver and its DT glue

2013-03-06 Thread Harvey Harrison
On Wed, Mar 6, 2013 at 4:32 PM, Viresh Kumar viresh.ku...@linaro.org wrote: clk[cluster] = clk_get_sys(name, NULL); - if (!IS_ERR(clk[cluster])) { + if (!IS_ERR_OR_NULL(clk[cluster])) { pr_debug(%s: clk: %p freq table: %p, cluster: %d\n,

Re: [PATCH] cpufreq: ARM big LITTLE: Add generic cpufreq driver and its DT glue

2013-03-06 Thread Russell King - ARM Linux
On Thu, Mar 07, 2013 at 08:32:37AM +0800, Viresh Kumar wrote: On 5 March 2013 18:52, Russell King - ARM Linux li...@arm.linux.org.uk wrote: On Tue, Mar 05, 2013 at 12:52:41PM +0800, Viresh Kumar wrote: + clk[cluster] = clk_get(NULL, name); + if (!IS_ERR_OR_NULL(clk[cluster])) {

Re: [PATCH] cpufreq: ARM big LITTLE: Add generic cpufreq driver and its DT glue

2013-03-06 Thread Viresh Kumar
On 7 March 2013 08:49, Harvey Harrison harvey.harri...@gmail.com wrote: On Wed, Mar 6, 2013 at 4:32 PM, Viresh Kumar viresh.ku...@linaro.org wrote: clk[cluster] = clk_get_sys(name, NULL); - if (!IS_ERR(clk[cluster])) { + if (!IS_ERR_OR_NULL(clk[cluster])) {

Re: [PATCH] cpufreq: ARM big LITTLE: Add generic cpufreq driver and its DT glue

2013-03-06 Thread Olof Johansson
On Tue, Mar 05, 2013 at 12:52:41PM +0800, Viresh Kumar wrote: +++ b/Documentation/devicetree/bindings/cpufreq/arm_big_little_dt.txt @@ -0,0 +1,29 @@ +Generic ARM big LITTLE cpufreq driver's DT glue +--- + +It is DT specific glue layer for generic

Re: [PATCH] cpufreq: ARM big LITTLE: Add generic cpufreq driver and its DT glue

2013-03-06 Thread Olof Johansson
On Wed, Mar 6, 2013 at 5:46 PM, Olof Johansson o...@lixom.net wrote: This binding makes no sense to me. It needs to be substantially better documented, not just a couple of sentences that people that understand bit.LITTLE thoroughly can make sense of. It also duplicates the cpu binding. I

Re: [PATCH] cpufreq: ARM big LITTLE: Add generic cpufreq driver and its DT glue

2013-03-06 Thread Viresh Kumar
On 7 March 2013 08:51, Russell King - ARM Linux li...@arm.linux.org.uk wrote: On Thu, Mar 07, 2013 at 08:32:37AM +0800, Viresh Kumar wrote: On 5 March 2013 18:52, Russell King - ARM Linux li...@arm.linux.org.uk wrote: On Tue, Mar 05, 2013 at 12:52:41PM +0800, Viresh Kumar wrote: +

Re: [PATCH] cpufreq: ARM big LITTLE: Add generic cpufreq driver and its DT glue

2013-03-06 Thread Viresh Kumar
On 7 March 2013 09:50, Olof Johansson o...@lixom.net wrote: On Wed, Mar 6, 2013 at 5:46 PM, Olof Johansson o...@lixom.net wrote: This binding makes no sense to me. It needs to be substantially better documented, not just a couple of sentences that people that understand bit.LITTLE thoroughly

Re: [PATCH] cpufreq: ARM big LITTLE: Add generic cpufreq driver and its DT glue

2013-03-05 Thread Viresh Kumar
On 5 March 2013 18:52, Russell King - ARM Linux wrote: > On Tue, Mar 05, 2013 at 12:52:41PM +0800, Viresh Kumar wrote: >> +static void put_cluster_clk_and_freq_table(u32 cluster) >> +{ >> + if (!atomic_dec_return(_usage[cluster])) { >> + clk_put(clk[cluster]); >> +

Re: [PATCH] cpufreq: ARM big LITTLE: Add generic cpufreq driver and its DT glue

2013-03-05 Thread Russell King - ARM Linux
On Tue, Mar 05, 2013 at 12:52:41PM +0800, Viresh Kumar wrote: > +static void put_cluster_clk_and_freq_table(u32 cluster) > +{ > + if (!atomic_dec_return(_usage[cluster])) { > + clk_put(clk[cluster]); > + clk[cluster] = NULL; What's the point in setting the clk to NULL?

Re: [PATCH] cpufreq: ARM big LITTLE: Add generic cpufreq driver and its DT glue

2013-03-05 Thread Russell King - ARM Linux
On Tue, Mar 05, 2013 at 12:52:41PM +0800, Viresh Kumar wrote: +static void put_cluster_clk_and_freq_table(u32 cluster) +{ + if (!atomic_dec_return(cluster_usage[cluster])) { + clk_put(clk[cluster]); + clk[cluster] = NULL; What's the point in setting the clk to

Re: [PATCH] cpufreq: ARM big LITTLE: Add generic cpufreq driver and its DT glue

2013-03-05 Thread Viresh Kumar
On 5 March 2013 18:52, Russell King - ARM Linux li...@arm.linux.org.uk wrote: On Tue, Mar 05, 2013 at 12:52:41PM +0800, Viresh Kumar wrote: +static void put_cluster_clk_and_freq_table(u32 cluster) +{ + if (!atomic_dec_return(cluster_usage[cluster])) { + clk_put(clk[cluster]);

[PATCH] cpufreq: ARM big LITTLE: Add generic cpufreq driver and its DT glue

2013-03-04 Thread Viresh Kumar
big LITTLE is ARM's new Architecture focussing power/performance needs of modern world. More information about big LITTLE can be found here: http://www.arm.com/products/processors/technologies/biglittleprocessing.php http://lwn.net/Articles/481055/ In order to keep cpufreq support for all big

[PATCH] cpufreq: ARM big LITTLE: Add generic cpufreq driver and its DT glue

2013-03-04 Thread Viresh Kumar
big LITTLE is ARM's new Architecture focussing power/performance needs of modern world. More information about big LITTLE can be found here: http://www.arm.com/products/processors/technologies/biglittleprocessing.php http://lwn.net/Articles/481055/ In order to keep cpufreq support for all big