On 5 March 2013 18:52, Russell King - ARM Linux linux@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]);clk[cluster] = NULL;What's the point in setting the clk to NULL?
I couldn't find one and the same is true for freq_table[] too.
+static int get_cluster_clk_and_freq_table(u32 cluster) +{
char name[9] = "cluster";int count;if (atomic_inc_return(&cluster_usage[cluster]) != 1)return 0;freq_table[cluster] = arm_bL_ops->get_freq_tbl(cluster, &count);if (!freq_table[cluster])goto atomic_dec;name[7] = cluster + '0';clk[cluster] = clk_get(NULL, name);if (!IS_ERR_OR_NULL(clk[cluster])) {NAK. Two reasons.
- IS_ERR_OR_NULL. You know about this, it's been on the list several times.
AAHHHHHH .. How can i mess up with this concept.. I am really feeling bad now.
- Maybe clk_get_sys() rather than clk_get() and use a sensible device name ("cpu-cluster.%u" maybe) rather than a connection name with a NULL device?
That's a good comment (rather than pointing at some stupid mistake), I will probably keep the same name for the device as well.
So how does below fix look to you?
----------x-----------------x----------------- diff --git a/drivers/cpufreq/arm_big_little.c b/drivers/cpufreq/arm_big_little.c index 0d6de0e..2486b9a 100644 --- a/drivers/cpufreq/arm_big_little.c +++ b/drivers/cpufreq/arm_big_little.c @@ -140,9 +140,7 @@ static void put_cluster_clk_and_freq_table(u32 cluster) { if (!atomic_dec_return(&cluster_usage[cluster])) { clk_put(clk[cluster]); - clk[cluster] = NULL; arm_bL_ops->put_freq_tbl(cluster); - freq_table[cluster] = NULL; pr_debug("%s: cluster: %d\n", __func__, cluster); } } @@ -160,8 +158,8 @@ static int get_cluster_clk_and_freq_table(u32 cluster) goto atomic_dec;
name[7] = cluster + '0'; - clk[cluster] = clk_get(NULL, name); - if (!IS_ERR_OR_NULL(clk[cluster])) { + clk[cluster] = clk_get_sys(name, NULL); + if (!IS_ERR(clk[cluster])) { pr_debug("%s: clk: %p & freq table: %p, cluster: %d\n", __func__, clk[cluster], freq_table[cluster], cluster);