Re: [PM-WIP_CPUFREQ][PATCH 0/6 V3] Cleanups for cpufreq

["Menon, Nishanth" <nm@ti.com>]

On Fri, May 27, 2011 at 18:27, Kevin Hilman <khilman@ti.com> wrote:
> All of this came up because this series is going through contortions to
> make two CPUfreq registrations only using one freq_table, protect
> against concurrent access from different CPUs etc.,  which led me to
> wonder why we need two registrations.
I believe we have two cpu_inits and exits per cpu -> the
/sys/devices/system/cpu/cpu1/online and
/sys/devices/system/cpu/cpu0/online
are not soft links. this is where the issue starts

Try this diff on the pm-wip/cpufreq branch:
diff --git a/arch/arm/mach-omap2/omap2plus-cpufreq.c
b/arch/arm/mach-omap2/omap2plus-cpufreq.c
index 33a91ec..f3e82ce 100644
--- a/arch/arm/mach-omap2/omap2plus-cpufreq.c
+++ b/arch/arm/mach-omap2/omap2plus-cpufreq.c
@@ -199,11 +199,15 @@ static int __cpuinit omap_cpu_init(struct
cpufreq_policy *policy)
        /* FIXME: what's the actual transition time? */
        policy->cpuinfo.transition_latency = 300 * 1000;

+       dev_err(mpu_dev, "%s: XXX: cpu%d freq_table pointer=%p\n", __func__,
+                       policy->cpu, freq_table);
        return 0;
 }

 static int omap_cpu_exit(struct cpufreq_policy *policy)
 {
+       dev_err(mpu_dev, "%s: XXX: cpu%d freq_table pointer=%p\n", __func__,
+                       policy->cpu, freq_table);
        clk_exit_cpufreq_table(&freq_table);
        clk_put(mpu_clk);
        return 0;

[    2.045623] platform mpu.0: omap_cpu_init: XXX: cpu0 freq_table
pointer=eeff7e20
[    2.055664] platform mpu.0: omap_cpu_init: XXX: cpu1 freq_table
pointer=eeff7de0 <- Freq table eeff7e20 got overwritten by the new
alloc
[    2.063537] platform mpu.0: omap_cpu_exit: XXX: cpu1 freq_table
pointer=eeff7de0 <- if I had a kfree/ opp_freqtable_free in exit, we'd
have had a dangling pointer.

in this series:
patch 1: OMAP2+: cpufreq: dont support !freq_table -> This is a proper
fix to cleanup the code which seems to think that silicon with no
freq_table is to be supported - this is a hang over from OMAP1 time
and should be removed.
patch 2: OMAP2+: cpufreq: use OPP library -> Since we are using OPP
table and from your recommendation of a previous patch incarnation, I
have moved the cpufreq usage depdendent on OPP.
Patch 3: OMAP2+: cpufreq: put clk if cpu_init failed -> this is a
valid fix as well
Patch 4: OMAP2+: cpufreq: fix freq_table leak -> This is what I have
explained above -> Since online variables are not really a softlink, I
dont think we should be confused as to what the fix should look like!

table creation and registration is done as part of cpu_init - this is
probably the most appropriate place for it. but we should consider the
potential of cpu onlining and offlining dynamically in the system as
well. hence the need for patch 4 where I have used freq_table users.

Regards,
Nishanth Menon
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html