[pm-wip/cpufreq][PATCH 0/3] OMAP2+: cpufreq: fixes

[pm-wip/cpufreq][PATCH 0/3] OMAP2+: cpufreq: fixes

[Nishanth Menon]

Hi,
Here are a set of fixes on cpufreq based.
Basic tests with ondemand, userspace governors on:
	SDP3630
	SDP4430
Applies on:
git://git.kernel.org/pub/scm/linux/kernel/git/khilman/linux-omap-pm.git
cpufreq  8651d1b OMAP2+: cpufreq: fix freq_table leak

Colin Cross (1):
  OMAP2+: cpufreq: notify even with bad boot frequency

Nishanth Menon (2):
  OMAP2+: cpufreq: minor flow beautification
  OMAP2+: cpufreq: do lateinit

 arch/arm/mach-omap2/omap2plus-cpufreq.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

Regards,
Nishanth Menon

[pm-wip/cpufreq][PATCH 1/3] OMAP2+: cpufreq: minor flow beautification

[Nishanth Menon]

Probably should be squashed to:
"OMAP2+: cpufreq: fix freq_table leak"

Looks like I've been lazy and added up a useless else case!

Signed-off-by: Nishanth Menon <nm@ti.com>
---
 arch/arm/mach-omap2/omap2plus-cpufreq.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-omap2/omap2plus-cpufreq.c b/arch/arm/mach-omap2/omap2plus-cpufreq.c
index 1406577..a962a31 100644
--- a/arch/arm/mach-omap2/omap2plus-cpufreq.c
+++ b/arch/arm/mach-omap2/omap2plus-cpufreq.c
@@ -186,11 +186,11 @@ static int __cpuinit omap_cpu_init(struct cpufreq_policy *policy)
 	}
 
 	result = cpufreq_frequency_table_cpuinfo(policy, freq_table);
-	if (!result)
-		cpufreq_frequency_table_get_attr(freq_table, policy->cpu);
-	else
+	if (result)
 		goto fail_table;
 
+	cpufreq_frequency_table_get_attr(freq_table, policy->cpu);
+
 	policy->min = policy->cpuinfo.min_freq;
 	policy->max = policy->cpuinfo.max_freq;
 	policy->cur = omap_getspeed(policy->cpu);
-- 
1.7.1

Re: [pm-wip/cpufreq][PATCH 1/3] OMAP2+: cpufreq: minor flow beautification

[Kevin Hilman]

Nishanth Menon <nm@ti.com> writes:

> Probably should be squashed to:
> "OMAP2+: cpufreq: fix freq_table leak"
>
> Looks like I've been lazy and added up a useless else case!
>
> Signed-off-by: Nishanth Menon <nm@ti.com>

Thanks, squashed.

Kevin

[pm-wip/cpufreq][PATCH 2/3] OMAP2+: cpufreq: notify even with bad boot frequency

[Nishanth Menon]

From: Colin Cross <ccross@google.com>

Sometimes, bootloaders starts up with a frequency which is not
in the OPP table. At cpu_init, policy->cur contains the frequency
we pick at boot.  It is possible that system might have fixed
it's boot frequency later on as part of power initialization.
After this condition, the first call to omap_target results in the
following:

omap_getspeed(actual device frequency) != policy->cur(frequency that
cpufreq thinks that the system is at), and it is possible that
freqs.old == freqs.new (because the governor requested a scale down).

We exit without triggering the notifiers in the current code, which
does'nt let code which depends on cpufreq_notify_transition to have
accurate information as to what the system frequency is.

Instead, we do a normal transition if policy->cur is wrong, then,
freqs.old will be the actual cpu frequency, freqs.new will be the
actual new cpu frequency and all required notifiers have the accurate
information.

Acked-by: Nishanth Menon <nm@ti.com>
Signed-off-by: Colin Cross <ccross@google.com>
---
 arch/arm/mach-omap2/omap2plus-cpufreq.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/omap2plus-cpufreq.c b/arch/arm/mach-omap2/omap2plus-cpufreq.c
index a962a31..2177381 100644
--- a/arch/arm/mach-omap2/omap2plus-cpufreq.c
+++ b/arch/arm/mach-omap2/omap2plus-cpufreq.c
@@ -97,7 +97,7 @@ static int omap_target(struct cpufreq_policy *policy,
 	freqs.old = omap_getspeed(policy->cpu);
 	freqs.cpu = policy->cpu;
 
-	if (freqs.old == freqs.new)
+	if (freqs.old == freqs.new && policy->cur == freqs.new)
 		return ret;
 
 	if (!is_smp()) {
-- 
1.7.1

[pm-wip/cpufreq][PATCH 3/3] OMAP2+: cpufreq: do lateinit

[Nishanth Menon]

Since we do module_init, cpufreq initializes before power late_init
where many of the required data structures are registered. Move
cpufreq init to late_initcall instead. Further CONFIG_CPU_FREQ
on which the build depends is bool and does'nt support modules yet.

Signed-off-by: Nishanth Menon <nm@ti.com>
---
 arch/arm/mach-omap2/omap2plus-cpufreq.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/omap2plus-cpufreq.c b/arch/arm/mach-omap2/omap2plus-cpufreq.c
index 2177381..07c2ab9 100644
--- a/arch/arm/mach-omap2/omap2plus-cpufreq.c
+++ b/arch/arm/mach-omap2/omap2plus-cpufreq.c
@@ -273,5 +273,5 @@ static void __exit omap_cpufreq_exit(void)
 
 MODULE_DESCRIPTION("cpufreq driver for OMAP2PLUS SOCs");
 MODULE_LICENSE("GPL");
-module_init(omap_cpufreq_init);
+late_initcall(omap_cpufreq_init);
 module_exit(omap_cpufreq_exit);
-- 
1.7.1

Re: [pm-wip/cpufreq][PATCH 3/3] OMAP2+: cpufreq: do lateinit

[Santosh Shilimkar]

On 6/7/2011 7:35 AM, Nishanth Menon wrote:
> Since we do module_init, cpufreq initializes before power late_init
> where many of the required data structures are registered. Move
> cpufreq init to late_initcall instead. Further CONFIG_CPU_FREQ
> on which the build depends is bool and does'nt support modules yet.
>
You might want to fix sequence instead of this change
considering we want to make OMAP CPUFReq as a loadable module.


> Signed-off-by: Nishanth Menon<nm@ti.com>
> ---
>   arch/arm/mach-omap2/omap2plus-cpufreq.c |    2 +-
>   1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/omap2plus-cpufreq.c b/arch/arm/mach-omap2/omap2plus-cpufreq.c
> index 2177381..07c2ab9 100644
> --- a/arch/arm/mach-omap2/omap2plus-cpufreq.c
> +++ b/arch/arm/mach-omap2/omap2plus-cpufreq.c
> @@ -273,5 +273,5 @@ static void __exit omap_cpufreq_exit(void)
>
>   MODULE_DESCRIPTION("cpufreq driver for OMAP2PLUS SOCs");
>   MODULE_LICENSE("GPL");
> -module_init(omap_cpufreq_init);
> +late_initcall(omap_cpufreq_init);
>   module_exit(omap_cpufreq_exit);

Re: [pm-wip/cpufreq][PATCH 3/3] OMAP2+: cpufreq: do lateinit

[Menon, Nishanth]

On Tue, Jun 7, 2011 at 03:15, Santosh Shilimkar
<santosh.shilimkar@ti.com> wrote:
> On 6/7/2011 7:35 AM, Nishanth Menon wrote:
>>
>> Since we do module_init, cpufreq initializes before power late_init
>> where many of the required data structures are registered. Move
>> cpufreq init to late_initcall instead. Further CONFIG_CPU_FREQ
>> on which the build depends is bool and does'nt support modules yet.
>>
> You might want to fix sequence instead of this change
> considering we want to make OMAP CPUFReq as a loadable module.

Unless I add a is_omap_pm_ready() in omap_target() - it is not really
safe. but smartreflex.c has a similar issue as well - I am open to
suggestions on how we should fix this in a clean manner. Current
omap2-cpufreq.c does not do dvfs - so it has dependency only on clocks
- but the moment it depends on anything PM code does,we'd be dead as,
for instance, dvfs requires a lot of those pieces to fall in place
before we can execute omap_target.

Regards,
Nishanth Menon

>
>
>> Signed-off-by: Nishanth Menon<nm@ti.com>
>> ---
>>  arch/arm/mach-omap2/omap2plus-cpufreq.c |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/omap2plus-cpufreq.c
>> b/arch/arm/mach-omap2/omap2plus-cpufreq.c
>> index 2177381..07c2ab9 100644
>> --- a/arch/arm/mach-omap2/omap2plus-cpufreq.c
>> +++ b/arch/arm/mach-omap2/omap2plus-cpufreq.c
>> @@ -273,5 +273,5 @@ static void __exit omap_cpufreq_exit(void)
>>
>>  MODULE_DESCRIPTION("cpufreq driver for OMAP2PLUS SOCs");
>>  MODULE_LICENSE("GPL");
>> -module_init(omap_cpufreq_init);
>> +late_initcall(omap_cpufreq_init);
>>  module_exit(omap_cpufreq_exit);
>
>
--
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

Re: [pm-wip/cpufreq][PATCH 3/3] OMAP2+: cpufreq: do lateinit

[Kevin Hilman]

Nishanth Menon <nm@ti.com> writes:

> Since we do module_init, cpufreq initializes before power late_init
> where many of the required data structures are registered. 

What exactly are the dependencies here?  The only thing I see is the
dependency on omap2_get_mpuss_device(), and those devices should be
created as a postcore_initcall.

If there are other dependencies, they're probably created a side effect
of your earlier patches where you moved stuff from the ->init hook
(which happens much later) into the initcall function.  Maybe that needs
a rethink?

> Move cpufreq init to late_initcall instead. Further CONFIG_CPU_FREQ on
> which the build depends is bool and does'nt support modules yet.
>
> Signed-off-by: Nishanth Menon <nm@ti.com>

If this works, it's only because of the link order defined by the
Makefile ordering since both are the same level of initcall.

When I move this driver to drivers/cpufreq, then the link order is less
obvious.

Kevin

> ---
>  arch/arm/mach-omap2/omap2plus-cpufreq.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/omap2plus-cpufreq.c b/arch/arm/mach-omap2/omap2plus-cpufreq.c
> index 2177381..07c2ab9 100644
> --- a/arch/arm/mach-omap2/omap2plus-cpufreq.c
> +++ b/arch/arm/mach-omap2/omap2plus-cpufreq.c
> @@ -273,5 +273,5 @@ static void __exit omap_cpufreq_exit(void)
>  
>  MODULE_DESCRIPTION("cpufreq driver for OMAP2PLUS SOCs");
>  MODULE_LICENSE("GPL");
> -module_init(omap_cpufreq_init);
> +late_initcall(omap_cpufreq_init);
>  module_exit(omap_cpufreq_exit);

Re: [pm-wip/cpufreq][PATCH 3/3] OMAP2+: cpufreq: do lateinit

[Menon, Nishanth]

On Tue, Jun 7, 2011 at 16:49, Kevin Hilman <khilman@ti.com> wrote:
> Nishanth Menon <nm@ti.com> writes:
>
>> Since we do module_init, cpufreq initializes before power late_init
>> where many of the required data structures are registered.
>
> What exactly are the dependencies here?  The only thing I see is the
> dependency on omap2_get_mpuss_device(), and those devices should be
> created as a postcore_initcall.
>
> If there are other dependencies, they're probably created a side effect
> of your earlier patches where you moved stuff from the ->init hook
> (which happens much later) into the initcall function.  Maybe that needs
> a rethink?
>
>> Move cpufreq init to late_initcall instead. Further CONFIG_CPU_FREQ on
>> which the build depends is bool and does'nt support modules yet.
>>
>> Signed-off-by: Nishanth Menon <nm@ti.com>
>
> If this works, it's only because of the link order defined by the
> Makefile ordering since both are the same level of initcall.
>
> When I move this driver to drivers/cpufreq, then the link order is less
> obvious.

the issue is as follows:
currently we dont do voltage transitions. when we do that
eventually(and my current code has an forked implementation of dvfs,
the following steps happen):
late_initcall(omap2_common_pm_late_init);
does pmic inits, omap_voltage_late_init, init_voltages and SR dev initialization

without these, there is no way to transition MPU to proper voltage,
frequency combination. The requirement will have to be that
omap2-cpufreq.c allows for cpufreq transitions only after voltage and
clk layers are ready for transitions - if we ever want to do dvfs -
which we will eventually need to.

Regards,
Nishanth Menon


>
> Kevin
>
>> ---
>>  arch/arm/mach-omap2/omap2plus-cpufreq.c |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/omap2plus-cpufreq.c b/arch/arm/mach-omap2/omap2plus-cpufreq.c
>> index 2177381..07c2ab9 100644
>> --- a/arch/arm/mach-omap2/omap2plus-cpufreq.c
>> +++ b/arch/arm/mach-omap2/omap2plus-cpufreq.c
>> @@ -273,5 +273,5 @@ static void __exit omap_cpufreq_exit(void)
>>
>>  MODULE_DESCRIPTION("cpufreq driver for OMAP2PLUS SOCs");
>>  MODULE_LICENSE("GPL");
>> -module_init(omap_cpufreq_init);
>> +late_initcall(omap_cpufreq_init);
>>  module_exit(omap_cpufreq_exit);
>
--
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

Re: [pm-wip/cpufreq][PATCH 3/3] OMAP2+: cpufreq: do lateinit

[Kevin Hilman]

"Menon, Nishanth" <nm@ti.com> writes:

> On Tue, Jun 7, 2011 at 16:49, Kevin Hilman <khilman@ti.com> wrote:
>> Nishanth Menon <nm@ti.com> writes:
>>
>>> Since we do module_init, cpufreq initializes before power late_init
>>> where many of the required data structures are registered.
>>
>> What exactly are the dependencies here?  The only thing I see is the
>> dependency on omap2_get_mpuss_device(), and those devices should be
>> created as a postcore_initcall.
>>
>> If there are other dependencies, they're probably created a side effect
>> of your earlier patches where you moved stuff from the ->init hook
>> (which happens much later) into the initcall function.  Maybe that needs
>> a rethink?
>>
>>> Move cpufreq init to late_initcall instead. Further CONFIG_CPU_FREQ on
>>> which the build depends is bool and does'nt support modules yet.
>>>
>>> Signed-off-by: Nishanth Menon <nm@ti.com>
>>
>> If this works, it's only because of the link order defined by the
>> Makefile ordering since both are the same level of initcall.
>>
>> When I move this driver to drivers/cpufreq, then the link order is less
>> obvious.
>
> the issue is as follows:
> currently we dont do voltage transitions. when we do that
> eventually(and my current code has an forked implementation of dvfs,
> the following steps happen):
> late_initcall(omap2_common_pm_late_init);
> does pmic inits, omap_voltage_late_init, init_voltages and SR dev initialization
>
> without these, there is no way to transition MPU to proper voltage,
> frequency combination. The requirement will have to be that
> omap2-cpufreq.c allows for cpufreq transitions only after voltage and
> clk layers are ready for transitions - if we ever want to do dvfs -
> which we will eventually need to.

Yes, I understand.

But $SUBJECT patch is fixing this as an _init_ time ordering problem,
What you're describing is a runtime requirement that doesn't exist until
a DVFS transition is done.  IOW, the requirement is that the voltage
etc. layers have to be init'd before the first transition.

So, rather than fix this with initcall ordering (which will have to be
redone as things git moved and converted to modules), just create a type
of late init function in this driver, which gets called on the first
transition.

Kevin

--
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

Re: [pm-wip/cpufreq][PATCH 3/3] OMAP2+: cpufreq: do lateinit

[Menon, Nishanth]

On Wed, Jun 8, 2011 at 13:51, Kevin Hilman <khilman@ti.com> wrote:
[..]
>> the issue is as follows:
>> currently we dont do voltage transitions. when we do that
>> eventually(and my current code has an forked implementation of dvfs,
>> the following steps happen):
>> late_initcall(omap2_common_pm_late_init);
>> does pmic inits, omap_voltage_late_init, init_voltages and SR dev initialization
>>
>> without these, there is no way to transition MPU to proper voltage,
>> frequency combination. The requirement will have to be that
>> omap2-cpufreq.c allows for cpufreq transitions only after voltage and
>> clk layers are ready for transitions - if we ever want to do dvfs -
>> which we will eventually need to.
>
> Yes, I understand.
>
> But $SUBJECT patch is fixing this as an _init_ time ordering problem,
> What you're describing is a runtime requirement that doesn't exist until
> a DVFS transition is done.  IOW, the requirement is that the voltage
> etc. layers have to be init'd before the first transition.
>
> So, rather than fix this with initcall ordering (which will have to be
> redone as things git moved and converted to modules), just create a type
> of late init function in this driver, which gets called on the first
> transition.

The tricky part ofcourse is for the registration - if we do the
registration, omap_cpu_init will get called once the registration
happens - no reason to stop it, which in turn triggers omap_target the
moment the governors are ready to do their thing. Is the following
what you are talking about? I am not completely sure how this helps..

diff --git a/arch/arm/mach-omap2/omap2plus-cpufreq.c
b/arch/arm/mach-omap2/omap2plus-cpufreq.c
index 77efcb0..8586df8 100644
--- a/arch/arm/mach-omap2/omap2plus-cpufreq.c
+++ b/arch/arm/mach-omap2/omap2plus-cpufreq.c
@@ -253,6 +253,11 @@ static struct cpufreq_driver omap_driver = {
        .attr           = omap_cpufreq_attr,
 };

+static int __init omap_cpufreq_lateinit(void)
+{
+       return cpufreq_register_driver(&omap_driver);
+}
+
 static int __init omap_cpufreq_init(void)
 {
        if (cpu_is_omap24xx()) {
@@ -277,8 +282,7 @@ static int __init omap_cpufreq_init(void)
                pr_warning("%s: unable to get the mpu device\n", __func__);
                return -EINVAL;
        }
-
-       return cpufreq_register_driver(&omap_driver);
+       return 0;
 }

 static void __exit omap_cpufreq_exit(void)
@@ -288,5 +292,6 @@ static void __exit omap_cpufreq_exit(void)

 MODULE_DESCRIPTION("cpufreq driver for OMAP2PLUS SOCs");
 MODULE_LICENSE("GPL");
+late_initcall(omap_cpufreq_lateinit);
 module_init(omap_cpufreq_init);
 module_exit(omap_cpufreq_exit);


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

Re: [pm-wip/cpufreq][PATCH 3/3] OMAP2+: cpufreq: do lateinit

[Kevin Hilman]

"Menon, Nishanth" <nm@ti.com> writes:

> On Wed, Jun 8, 2011 at 13:51, Kevin Hilman <khilman@ti.com> wrote:
> [..]
>>> the issue is as follows:
>>> currently we dont do voltage transitions. when we do that
>>> eventually(and my current code has an forked implementation of dvfs,
>>> the following steps happen):
>>> late_initcall(omap2_common_pm_late_init);
>>> does pmic inits, omap_voltage_late_init, init_voltages and SR dev initialization
>>>
>>> without these, there is no way to transition MPU to proper voltage,
>>> frequency combination. The requirement will have to be that
>>> omap2-cpufreq.c allows for cpufreq transitions only after voltage and
>>> clk layers are ready for transitions - if we ever want to do dvfs -
>>> which we will eventually need to.
>>
>> Yes, I understand.
>>
>> But $SUBJECT patch is fixing this as an _init_ time ordering problem,
>> What you're describing is a runtime requirement that doesn't exist until
>> a DVFS transition is done.  IOW, the requirement is that the voltage
>> etc. layers have to be init'd before the first transition.
>>
>> So, rather than fix this with initcall ordering (which will have to be
>> redone as things git moved and converted to modules), just create a type
>> of late init function in this driver, which gets called on the first
>> transition.
>
> The tricky part ofcourse is for the registration - if we do the
> registration, omap_cpu_init will get called once the registration
> happens - no reason to stop it, which in turn triggers omap_target the
> moment the governors are ready to do their thing. 

Yes.

> Is the following what you are talking about? I am not completely sure
> how this helps..

No.  I was thinking of doing registration as it is today, but have a
late hook that is called in the driver's->target() callback that checks
if the other frameworks are ready.  

But now I see now that ->target() is called right after ->init(), so I
don't think this helps.  Sheesh.  The PM init sequence/dependencies
right now are a complete mess.

Probably a better solution to this, would be to actually create a
platform driver out of our CPUfreq driver.  Then, PM core late init
would just register the platform device when it's ready.  This would
also work if/when the CPUfreq driver is a module.

Kevin


> diff --git a/arch/arm/mach-omap2/omap2plus-cpufreq.c
> b/arch/arm/mach-omap2/omap2plus-cpufreq.c
> index 77efcb0..8586df8 100644
> --- a/arch/arm/mach-omap2/omap2plus-cpufreq.c
> +++ b/arch/arm/mach-omap2/omap2plus-cpufreq.c
> @@ -253,6 +253,11 @@ static struct cpufreq_driver omap_driver = {
>         .attr           = omap_cpufreq_attr,
>  };
>
> +static int __init omap_cpufreq_lateinit(void)
> +{
> +       return cpufreq_register_driver(&omap_driver);
> +}
> +
>  static int __init omap_cpufreq_init(void)
>  {
>         if (cpu_is_omap24xx()) {
> @@ -277,8 +282,7 @@ static int __init omap_cpufreq_init(void)
>                 pr_warning("%s: unable to get the mpu device\n", __func__);
>                 return -EINVAL;
>         }
> -
> -       return cpufreq_register_driver(&omap_driver);
> +       return 0;
>  }
>
>  static void __exit omap_cpufreq_exit(void)
> @@ -288,5 +292,6 @@ static void __exit omap_cpufreq_exit(void)
>
>  MODULE_DESCRIPTION("cpufreq driver for OMAP2PLUS SOCs");
>  MODULE_LICENSE("GPL");
> +late_initcall(omap_cpufreq_lateinit);
>  module_init(omap_cpufreq_init);
>  module_exit(omap_cpufreq_exit);
>
>
> 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