[PATCH 0/2] OMAP cpufreq fixes

[PATCH 0/2] OMAP cpufreq fixes

[Rajendra Nayak]

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

[PATCH 1/2] cpufreq: OMAP: Handle missing frequency table on SMP systems

[Rajendra Nayak]

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

[PATCH 2/2] ARM: OMAP4: Register the OPP table only for 4430 device

[Rajendra Nayak]

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

[PATCH 1/2] cpufreq: OMAP: Handle missing frequency table on SMP systems

[Shilimkar, Santosh]

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;

[PATCH 2/2] ARM: OMAP4: Register the OPP table only for 4430 device

[Kevin Hilman]

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,

[PATCH 1/2] cpufreq: OMAP: Handle missing frequency table on SMP systems

[Kevin Hilman]

"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

[PATCH 1/2] cpufreq: OMAP: Handle missing frequency table on SMP systems

[Shilimkar, Santosh]

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

[PATCH 2/2] ARM: OMAP4: Register the OPP table only for 4430 device

[Rajendra Nayak]

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

[PATCH 2/2] ARM: OMAP4: Register the OPP table only for 4430 device

[Kevin Hilman]

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