On 21 March 2014 16:13, Gautham R Shenoy [off-list ref] wrote:

On Fri, Mar 21, 2014 at 02:11:32PM +0530, Viresh Kumar wrote:

quoted

quoted

+               pr_debug("PState id %d freq %d MHz\n", id, freq);
+               powernv_freqs[i].driver_data = i;

I don't think you are using this field at all and this is the field you can
use for driver_data and so you can get rid of powernv_pstate_ids[ ].

Using driver_data to record powernv_pstate_ids won't work since
powernv_pstate_ids can be negative. So a pstate_id -3 can be confused
with CPUFREQ_BOOST_FREQ thereby not displaying the frequency
corresponding to pstate id -3. So for now I think we will be retaining
powernv_pstate_ids.

As I said earlier, this field is only used by cpufreq drivers and cpufreq core
isn't supposed to touch it. CPUFREQ_BOOST_FREQ and other macros
are there for .frequency field and not this one.

quoted

quoted

+static int powernv_cpufreq_cpu_init(struct cpufreq_policy *policy)
+{
+       int base, i;
+
+#ifdef CONFIG_SMP

What will break if you don't have this ifdef here? Without that as well
below code should work?

Missed this one?

quoted

quoted

+       base = cpu_first_thread_sibling(policy->cpu);
+
+       for (i = 0; i < threads_per_core; i++)
+               cpumask_set_cpu(base + i, policy->cpus);
+#endif
+       policy->cpuinfo.transition_latency = 25000;
+
+       policy->cur = powernv_freqs[0].frequency;
+       cpufreq_frequency_table_get_attr(powernv_freqs, policy->cpu);

This doesn't exist anymore.

Didn't get this comment!

cpufreq_frequency_table_get_attr() routine doesn't exist anymore.

quoted

quoted

+static int powernv_cpufreq_cpu_exit(struct cpufreq_policy *policy)
+{
+       cpufreq_frequency_table_put_attr(policy->cpu);
+       return 0;
+}

You don't need this..

Why not ?

Because cpufreq_frequency_table_get_attr() and
cpufreq_frequency_table_put_attr() don't exist anymore :)

quoted

quoted

+module_init(powernv_cpufreq_init);
+module_exit(powernv_cpufreq_exit);

Place these right after the routines without any blank lines in

between.

Is this the new convention ?

Don't know I have been following this since long time, probably from the
time I started with Mainline stuff.. I have seen many maintainers advising this
as it make code more readable, specially if these routines are quite big..

Probably it isn't mentioned in coding guidelines but people follow it most of
the times.