From patchwork Sun May 29 17:25:18 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Nishanth Menon X-Patchwork-Id: 827842 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by demeter2.kernel.org (8.14.4/8.14.3) with ESMTP id p4THPmJa029676 for ; Sun, 29 May 2011 17:25:49 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753507Ab1E2RZk (ORCPT ); Sun, 29 May 2011 13:25:40 -0400 Received: from na3sys009aog117.obsmtp.com ([74.125.149.242]:32981 "EHLO na3sys009aog117.obsmtp.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752841Ab1E2RZk convert rfc822-to-8bit (ORCPT ); Sun, 29 May 2011 13:25:40 -0400 Received: from mail-wy0-f176.google.com ([74.125.82.176]) (using TLSv1) by na3sys009aob117.postini.com ([74.125.148.12]) with SMTP ID DSNKTeKBkouih1+5ti8wJJpd8FYBAFWdb9jO@postini.com; Sun, 29 May 2011 10:25:39 PDT Received: by mail-wy0-f176.google.com with SMTP id 40so3181334wyb.21 for ; Sun, 29 May 2011 10:25:38 -0700 (PDT) Received: by 10.216.58.136 with SMTP id q8mr3803811wec.97.1306689938111; Sun, 29 May 2011 10:25:38 -0700 (PDT) MIME-Version: 1.0 Received: by 10.216.72.199 with HTTP; Sun, 29 May 2011 10:25:18 -0700 (PDT) In-Reply-To: <871uzjwwq7.fsf@ti.com> References: <1306366733-8439-1-git-send-email-nm@ti.com> <87ipsxcoz0.fsf@ti.com> <4DDF3169.9070503@ti.com> <4DDF4424.2000706@ti.com> <871uzjwwq7.fsf@ti.com> From: "Menon, Nishanth" Date: Sun, 29 May 2011 12:25:18 -0500 Message-ID: Subject: Re: [PM-WIP_CPUFREQ][PATCH 0/6 V3] Cleanups for cpufreq To: Kevin Hilman Cc: "Turquette, Mike" , Santosh Shilimkar , linux-omap Sender: linux-omap-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-omap@vger.kernel.org X-Greylist: IP, sender and recipient auto-whitelisted, not delayed by milter-greylist-4.2.6 (demeter2.kernel.org [140.211.167.43]); Sun, 29 May 2011 17:25:49 +0000 (UTC) On Fri, May 27, 2011 at 18:27, Kevin Hilman 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: 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 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);