* [PATCH 0/2] OMAP cpufreq fixes @ 2012-08-08 10:54 Rajendra Nayak 2012-08-08 10:54 ` [PATCH 1/2] cpufreq: OMAP: Handle missing frequency table on SMP systems Rajendra Nayak 2012-08-08 10:54 ` [PATCH 2/2] ARM: OMAP4: Register the OPP table only for 4430 device Rajendra Nayak 0 siblings, 2 replies; 9+ messages in thread From: Rajendra Nayak @ 2012-08-08 10:54 UTC (permalink / raw) To: linux-arm-kernel Hi Kevin, While testing cpufreq on 3.6-rc1, I noticed strange issues on OMAP4, like a lockup when booting with 'performace' governer as default, and crashes when using 'usespace' (which booted up) and switching to higher OPPs. It took me a while to realise I was using a 4460 device. The OPPs registered though seemed to be for 4430 and hence were causing issues on the 4460 device. This series basically avoids registering the 4430 OPPs on other varients like 4460 and 4470 (and hence causing random issues) and also fixes a subsequent bug uncovered by doing this in OMAP cpufreq driver. regards, Rajendra Rajendra Nayak (2): cpufreq: OMAP: Handle missing frequency table on SMP systems ARM: OMAP4: Register the OPP table only for 4430 device arch/arm/mach-omap2/opp4xxx_data.c | 2 +- drivers/cpufreq/omap-cpufreq.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) -- 1.7.9.5 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] cpufreq: OMAP: Handle missing frequency table on SMP systems 2012-08-08 10:54 [PATCH 0/2] OMAP cpufreq fixes Rajendra Nayak @ 2012-08-08 10:54 ` Rajendra Nayak 2012-08-08 11:30 ` Shilimkar, Santosh 2012-08-08 10:54 ` [PATCH 2/2] ARM: OMAP4: Register the OPP table only for 4430 device Rajendra Nayak 1 sibling, 1 reply; 9+ messages in thread From: Rajendra Nayak @ 2012-08-08 10:54 UTC (permalink / raw) To: linux-arm-kernel On OMAP4, if the first CPU fails to get a valid frequency table (this could happen if the platform does not register any OPP table), the subsequent CPU instances end up dealing with a NULL freq_table and crash. Add a check for a NULL freq_table to help error the rest of the CPU instances out. Signed-off-by: Rajendra Nayak <rnayak@ti.com> Cc: <linux-pm@vger.kernel.org> --- drivers/cpufreq/omap-cpufreq.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/cpufreq/omap-cpufreq.c b/drivers/cpufreq/omap-cpufreq.c index 17fa04d..0ee824c 100644 --- a/drivers/cpufreq/omap-cpufreq.c +++ b/drivers/cpufreq/omap-cpufreq.c @@ -221,7 +221,7 @@ static int __cpuinit omap_cpu_init(struct cpufreq_policy *policy) if (atomic_inc_return(&freq_table_users) == 1) result = opp_init_cpufreq_table(mpu_dev, &freq_table); - if (result) { + if (result || !freq_table) { dev_err(mpu_dev, "%s: cpu%d: failed creating freq table[%d]\n", __func__, policy->cpu, result); goto fail_ck; -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 1/2] cpufreq: OMAP: Handle missing frequency table on SMP systems 2012-08-08 10:54 ` [PATCH 1/2] cpufreq: OMAP: Handle missing frequency table on SMP systems Rajendra Nayak @ 2012-08-08 11:30 ` Shilimkar, Santosh 2012-08-08 17:28 ` Kevin Hilman 0 siblings, 1 reply; 9+ messages in thread From: Shilimkar, Santosh @ 2012-08-08 11:30 UTC (permalink / raw) To: linux-arm-kernel On Wed, Aug 8, 2012 at 4:24 PM, Rajendra Nayak <rnayak@ti.com> wrote: > > On OMAP4, if the first CPU fails to get a valid frequency table (this > could happen if the platform does not register any OPP table), the > subsequent CPU instances end up dealing with a NULL freq_table and > crash. Add a check for a NULL freq_table to help error the rest > of the CPU instances out. > > Signed-off-by: Rajendra Nayak <rnayak@ti.com> > Cc: <linux-pm@vger.kernel.org> > --- > drivers/cpufreq/omap-cpufreq.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/cpufreq/omap-cpufreq.c > b/drivers/cpufreq/omap-cpufreq.c > index 17fa04d..0ee824c 100644 > --- a/drivers/cpufreq/omap-cpufreq.c > +++ b/drivers/cpufreq/omap-cpufreq.c > @@ -221,7 +221,7 @@ static int __cpuinit omap_cpu_init(struct > cpufreq_policy *policy) > if (atomic_inc_return(&freq_table_users) == 1) > result = opp_init_cpufreq_table(mpu_dev, &freq_table); > > - if (result) { > + if (result || !freq_table) { > dev_err(mpu_dev, "%s: cpu%d: failed creating freq > table[%d]\n", > __func__, policy->cpu, result); > goto fail_ck; The freq_table use count seems to be buggy in that case. Something like below should fix the issue. Feel free to update your patch with below if you agree. diff --git a/drivers/cpufreq/omap-cpufreq.c b/drivers/cpufreq/omap-cpufreq.c index 17fa04d..fd97c3d 100644 --- a/drivers/cpufreq/omap-cpufreq.c +++ b/drivers/cpufreq/omap-cpufreq.c @@ -218,7 +218,7 @@ static int __cpuinit omap_cpu_init(struct cpufreq_policy *po policy->cur = policy->min = policy->max = omap_getspeed(policy->cpu); - if (atomic_inc_return(&freq_table_users) == 1) + if (freq_table) result = opp_init_cpufreq_table(mpu_dev, &freq_table); if (result) { @@ -227,6 +227,7 @@ static int __cpuinit omap_cpu_init(struct cpufreq_policy *po goto fail_ck; } + atomic_inc_return(&freq_table_users); result = cpufreq_frequency_table_cpuinfo(policy, freq_table); if (result) goto fail_table; ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 1/2] cpufreq: OMAP: Handle missing frequency table on SMP systems 2012-08-08 11:30 ` Shilimkar, Santosh @ 2012-08-08 17:28 ` Kevin Hilman 2012-08-09 5:27 ` Shilimkar, Santosh 0 siblings, 1 reply; 9+ messages in thread From: Kevin Hilman @ 2012-08-08 17:28 UTC (permalink / raw) To: linux-arm-kernel "Shilimkar, Santosh" <santosh.shilimkar@ti.com> writes: > On Wed, Aug 8, 2012 at 4:24 PM, Rajendra Nayak <rnayak@ti.com> wrote: >> >> On OMAP4, if the first CPU fails to get a valid frequency table (this >> could happen if the platform does not register any OPP table), the >> subsequent CPU instances end up dealing with a NULL freq_table and >> crash. Add a check for a NULL freq_table to help error the rest >> of the CPU instances out. >> >> Signed-off-by: Rajendra Nayak <rnayak@ti.com> >> Cc: <linux-pm@vger.kernel.org> >> --- >> drivers/cpufreq/omap-cpufreq.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/cpufreq/omap-cpufreq.c >> b/drivers/cpufreq/omap-cpufreq.c >> index 17fa04d..0ee824c 100644 >> --- a/drivers/cpufreq/omap-cpufreq.c >> +++ b/drivers/cpufreq/omap-cpufreq.c >> @@ -221,7 +221,7 @@ static int __cpuinit omap_cpu_init(struct >> cpufreq_policy *policy) >> if (atomic_inc_return(&freq_table_users) == 1) >> result = opp_init_cpufreq_table(mpu_dev, &freq_table); >> >> - if (result) { >> + if (result || !freq_table) { >> dev_err(mpu_dev, "%s: cpu%d: failed creating freq >> table[%d]\n", >> __func__, policy->cpu, result); >> goto fail_ck; > > The freq_table use count seems to be buggy in that case. > Something like below should fix the issue. > Feel free to update your patch with below if you agree. > > diff --git a/drivers/cpufreq/omap-cpufreq.c b/drivers/cpufreq/omap-cpufreq.c > index 17fa04d..fd97c3d 100644 > --- a/drivers/cpufreq/omap-cpufreq.c > +++ b/drivers/cpufreq/omap-cpufreq.c > @@ -218,7 +218,7 @@ static int __cpuinit omap_cpu_init(struct cpufreq_policy *po > > policy->cur = policy->min = policy->max = omap_getspeed(policy->cpu); > > - if (atomic_inc_return(&freq_table_users) == 1) > + if (freq_table) I think you meant 'if (!freq_table)' ? Kevin > result = opp_init_cpufreq_table(mpu_dev, &freq_table); > > if (result) { > @@ -227,6 +227,7 @@ static int __cpuinit omap_cpu_init(struct cpufreq_policy *po > goto fail_ck; > } > > + atomic_inc_return(&freq_table_users); > result = cpufreq_frequency_table_cpuinfo(policy, freq_table); > if (result) > goto fail_table; > -- > To unsubscribe from this list: send the line "unsubscribe linux-pm" in > the body of a message to majordomo at vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] cpufreq: OMAP: Handle missing frequency table on SMP systems 2012-08-08 17:28 ` Kevin Hilman @ 2012-08-09 5:27 ` Shilimkar, Santosh 0 siblings, 0 replies; 9+ messages in thread From: Shilimkar, Santosh @ 2012-08-09 5:27 UTC (permalink / raw) To: linux-arm-kernel On Wed, Aug 8, 2012 at 10:58 PM, Kevin Hilman <khilman@ti.com> wrote: > > "Shilimkar, Santosh" <santosh.shilimkar@ti.com> writes: > > > On Wed, Aug 8, 2012 at 4:24 PM, Rajendra Nayak <rnayak@ti.com> wrote: > >> > >> On OMAP4, if the first CPU fails to get a valid frequency table (this > >> could happen if the platform does not register any OPP table), the > >> subsequent CPU instances end up dealing with a NULL freq_table and > >> crash. Add a check for a NULL freq_table to help error the rest > >> of the CPU instances out. > >> > >> Signed-off-by: Rajendra Nayak <rnayak@ti.com> > >> Cc: <linux-pm@vger.kernel.org> > >> --- > >> drivers/cpufreq/omap-cpufreq.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/drivers/cpufreq/omap-cpufreq.c > >> b/drivers/cpufreq/omap-cpufreq.c > >> index 17fa04d..0ee824c 100644 > >> --- a/drivers/cpufreq/omap-cpufreq.c > >> +++ b/drivers/cpufreq/omap-cpufreq.c > >> @@ -221,7 +221,7 @@ static int __cpuinit omap_cpu_init(struct > >> cpufreq_policy *policy) > >> if (atomic_inc_return(&freq_table_users) == 1) > >> result = opp_init_cpufreq_table(mpu_dev, &freq_table); > >> > >> - if (result) { > >> + if (result || !freq_table) { > >> dev_err(mpu_dev, "%s: cpu%d: failed creating freq > >> table[%d]\n", > >> __func__, policy->cpu, result); > >> goto fail_ck; > > > > The freq_table use count seems to be buggy in that case. > > Something like below should fix the issue. > > Feel free to update your patch with below if you agree. > > > > diff --git a/drivers/cpufreq/omap-cpufreq.c > > b/drivers/cpufreq/omap-cpufreq.c > > index 17fa04d..fd97c3d 100644 > > --- a/drivers/cpufreq/omap-cpufreq.c > > +++ b/drivers/cpufreq/omap-cpufreq.c > > @@ -218,7 +218,7 @@ static int __cpuinit omap_cpu_init(struct > > cpufreq_policy *po > > > > policy->cur = policy->min = policy->max = > > omap_getspeed(policy->cpu); > > > > - if (atomic_inc_return(&freq_table_users) == 1) > > + if (freq_table) > > I think you meant 'if (!freq_table)' ? > Right. regards Santosh ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/2] ARM: OMAP4: Register the OPP table only for 4430 device 2012-08-08 10:54 [PATCH 0/2] OMAP cpufreq fixes Rajendra Nayak 2012-08-08 10:54 ` [PATCH 1/2] cpufreq: OMAP: Handle missing frequency table on SMP systems Rajendra Nayak @ 2012-08-08 10:54 ` Rajendra Nayak 2012-08-08 17:18 ` Kevin Hilman 1 sibling, 1 reply; 9+ messages in thread From: Rajendra Nayak @ 2012-08-08 10:54 UTC (permalink / raw) To: linux-arm-kernel The 4430 OPP table was being registered for all other OMAP4 variants too, like 4460 and 4470 causing issues with cpufreq driver enabled. 4460 and 4470 devices have different OPPs as compared to 4430, and they should be populated seperately. As long as that happens, let the OPP table registeration happen only on 4430 device. Signed-off-by: Rajendra Nayak <rnayak@ti.com> --- arch/arm/mach-omap2/opp4xxx_data.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm/mach-omap2/opp4xxx_data.c b/arch/arm/mach-omap2/opp4xxx_data.c index 2293ba2..c95415d 100644 --- a/arch/arm/mach-omap2/opp4xxx_data.c +++ b/arch/arm/mach-omap2/opp4xxx_data.c @@ -94,7 +94,7 @@ int __init omap4_opp_init(void) { int r = -ENODEV; - if (!cpu_is_omap44xx()) + if (!cpu_is_omap443x()) return r; r = omap_init_opp_table(omap44xx_opp_def_list, -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/2] ARM: OMAP4: Register the OPP table only for 4430 device 2012-08-08 10:54 ` [PATCH 2/2] ARM: OMAP4: Register the OPP table only for 4430 device Rajendra Nayak @ 2012-08-08 17:18 ` Kevin Hilman 2012-08-09 7:23 ` Rajendra Nayak 0 siblings, 1 reply; 9+ messages in thread From: Kevin Hilman @ 2012-08-08 17:18 UTC (permalink / raw) To: linux-arm-kernel Rajendra Nayak <rnayak@ti.com> writes: > The 4430 OPP table was being registered for all other OMAP4 variants > too, like 4460 and 4470 causing issues with cpufreq driver > enabled. 4460 and 4470 devices have different OPPs as compared to > 4430, and they should be populated seperately. As long as that > happens, let the OPP table registeration happen only on 4430 device. > > Signed-off-by: Rajendra Nayak <rnayak@ti.com> Thanks, adding to PM fixes for v3.6-rc (branch: for_3.6/fixes/pm) Kevin > --- > arch/arm/mach-omap2/opp4xxx_data.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/arm/mach-omap2/opp4xxx_data.c b/arch/arm/mach-omap2/opp4xxx_data.c > index 2293ba2..c95415d 100644 > --- a/arch/arm/mach-omap2/opp4xxx_data.c > +++ b/arch/arm/mach-omap2/opp4xxx_data.c > @@ -94,7 +94,7 @@ int __init omap4_opp_init(void) > { > int r = -ENODEV; > > - if (!cpu_is_omap44xx()) > + if (!cpu_is_omap443x()) > return r; > > r = omap_init_opp_table(omap44xx_opp_def_list, ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/2] ARM: OMAP4: Register the OPP table only for 4430 device 2012-08-08 17:18 ` Kevin Hilman @ 2012-08-09 7:23 ` Rajendra Nayak 2012-08-09 15:01 ` Kevin Hilman 0 siblings, 1 reply; 9+ messages in thread From: Rajendra Nayak @ 2012-08-09 7:23 UTC (permalink / raw) To: linux-arm-kernel Hi Kevin, On Wednesday 08 August 2012 10:48 PM, Kevin Hilman wrote: >> The 4430 OPP table was being registered for all other OMAP4 variants >> > too, like 4460 and 4470 causing issues with cpufreq driver >> > enabled. 4460 and 4470 devices have different OPPs as compared to >> > 4430, and they should be populated seperately. As long as that >> > happens, let the OPP table registeration happen only on 4430 device. >> > >> > Signed-off-by: Rajendra Nayak<rnayak@ti.com> > Thanks, adding to PM fixes for v3.6-rc (branch: for_3.6/fixes/pm) I just posted a v2 of this series addressing the issues raised by Santosh on Patch 1/2. The problem with taking just $subject fix (or taking it before Patch 1/2) is that you would hit the issue fixed by the Patch 1/2 and cause a crash on non-4430 based OMAP4 devices with cpufreq enabled. regards, Rajendra ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/2] ARM: OMAP4: Register the OPP table only for 4430 device 2012-08-09 7:23 ` Rajendra Nayak @ 2012-08-09 15:01 ` Kevin Hilman 0 siblings, 0 replies; 9+ messages in thread From: Kevin Hilman @ 2012-08-09 15:01 UTC (permalink / raw) To: linux-arm-kernel Rajendra Nayak <rnayak@ti.com> writes: > Hi Kevin, > > On Wednesday 08 August 2012 10:48 PM, Kevin Hilman wrote: >>> The 4430 OPP table was being registered for all other OMAP4 variants >>> > too, like 4460 and 4470 causing issues with cpufreq driver >>> > enabled. 4460 and 4470 devices have different OPPs as compared to >>> > 4430, and they should be populated seperately. As long as that >>> > happens, let the OPP table registeration happen only on 4430 device. >>> > >>> > Signed-off-by: Rajendra Nayak<rnayak@ti.com> >> Thanks, adding to PM fixes for v3.6-rc (branch: for_3.6/fixes/pm) > > I just posted a v2 of this series addressing the issues raised by > Santosh on Patch 1/2. The problem with taking just $subject fix (or > taking it before Patch 1/2) is that you would hit the issue fixed > by the Patch 1/2 and cause a crash on non-4430 based OMAP4 devices with > cpufreq enabled. OK, thanks for the warning. I'll be sure to keep them together. Kevin ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2012-08-09 15:01 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-08-08 10:54 [PATCH 0/2] OMAP cpufreq fixes Rajendra Nayak 2012-08-08 10:54 ` [PATCH 1/2] cpufreq: OMAP: Handle missing frequency table on SMP systems Rajendra Nayak 2012-08-08 11:30 ` Shilimkar, Santosh 2012-08-08 17:28 ` Kevin Hilman 2012-08-09 5:27 ` Shilimkar, Santosh 2012-08-08 10:54 ` [PATCH 2/2] ARM: OMAP4: Register the OPP table only for 4430 device Rajendra Nayak 2012-08-08 17:18 ` Kevin Hilman 2012-08-09 7:23 ` Rajendra Nayak 2012-08-09 15:01 ` Kevin Hilman
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).