[PATCH][pm-wip/cpufreq] OMAP2+: CPUfreq: Remove superfluous check in target() for online CPU's.

[PATCH][pm-wip/cpufreq] OMAP2+: CPUfreq: Remove superfluous check in target() for online CPU's.

[Santosh Shilimkar]

Current OMAP2PLUS CPUfreq tagret() functions returns when all
the CPU's are not online. This breaks CPUfreq when secondary CPUs
are offlined on SMP system.

The intention of that check was just avoid CPU frequency change
during the window when CPU becomes online but it's cpufreq_init is
not done yet. Otherwise it can lead to notifiers being sent on
a CPU which is not yet registered to the governor.

But this race conditions is already managed by the CPUfreq
core driver by updating the available cpumask accordingly.

OMAP CPUFReq driver make use same cpumask for the notifiers
so the above problem doesn't exist. In my initial implementation
of the OMAP4 CPUFreq driver, I was using 'for_each_online_cpu()'
for notifiers which lead me to add that check. Later I fixed
the notifies but didn't realise that the check has become
redundant then.

Fix it by removing the superfluous check in target().

Thanks for Nishant Menon <nm@ti.com> for reporting issue
with hot-plug and Kevin Hilman <khilman@ti.com> for his
comment on excessive check in target().

Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
Reported-by: Nishanth Menon <nm@ti.com>
Tested-by: Vishwanath BS <vishwanath.bs@ti.com>
Cc: Kevin Hilman <khilman@ti.com>
---
This change is an outcome of discussion on another patch to fix
the hot-plug issue. For details refer below thread:
http://www.mail-archive.com/linux-omap@vger.kernel.org/msg50348.html

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

diff --git a/arch/arm/mach-omap2/omap2plus-cpufreq.c b/arch/arm/mach-omap2/omap2plus-cpufreq.c
index 33a91ec..3bf1704 100644
--- a/arch/arm/mach-omap2/omap2plus-cpufreq.c
+++ b/arch/arm/mach-omap2/omap2plus-cpufreq.c
@@ -81,10 +81,6 @@ static int omap_target(struct cpufreq_policy *policy,
 	int i, ret = 0;
 	struct cpufreq_freqs freqs;
 
-	/* Changes not allowed until all CPUs are online */
-	if (is_smp() && (num_online_cpus() < NR_CPUS))
-		return ret;
-
 	/* Ensure desired rate is within allowed range.  Some govenors
 	 * (ondemand) will just pass target_freq=0 to get the minimum. */
 	if (target_freq < policy->min)
-- 
1.6.0.4

Re: [PATCH][pm-wip/cpufreq] OMAP2+: CPUfreq: Remove superfluous check in target() for online CPU's.

[Menon, Nishanth]

On Fri, Jun 3, 2011 at 07:16, Santosh Shilimkar
<santosh.shilimkar@ti.com> wrote:
> Current OMAP2PLUS CPUfreq tagret() functions returns when all
> the CPU's are not online. This breaks CPUfreq when secondary CPUs
> are offlined on SMP system.
>
> The intention of that check was just avoid CPU frequency change
> during the window when CPU becomes online but it's cpufreq_init is
> not done yet. Otherwise it can lead to notifiers being sent on
> a CPU which is not yet registered to the governor.
>
> But this race conditions is already managed by the CPUfreq
> core driver by updating the available cpumask accordingly.
>
> OMAP CPUFReq driver make use same cpumask for the notifiers
> so the above problem doesn't exist. In my initial implementation
> of the OMAP4 CPUFreq driver, I was using 'for_each_online_cpu()'
> for notifiers which lead me to add that check. Later I fixed
> the notifies but didn't realise that the check has become
> redundant then.
>
> Fix it by removing the superfluous check in target().
>
> Thanks for Nishant Menon <nm@ti.com> for reporting issue
> with hot-plug and Kevin Hilman <khilman@ti.com> for his
> comment on excessive check in target().
>
> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
> Reported-by: Nishanth Menon <nm@ti.com>
> Tested-by: Vishwanath BS <vishwanath.bs@ti.com>
> Cc: Kevin Hilman <khilman@ti.com>

Tested-by: Nishanth Menon <nm@ti.com>
Regards,
Nishanth Menon


> ---
> This change is an outcome of discussion on another patch to fix
> the hot-plug issue. For details refer below thread:
> http://www.mail-archive.com/linux-omap@vger.kernel.org/msg50348.html
>
>  arch/arm/mach-omap2/omap2plus-cpufreq.c |    4 ----
>  1 files changed, 0 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/omap2plus-cpufreq.c b/arch/arm/mach-omap2/omap2plus-cpufreq.c
> index 33a91ec..3bf1704 100644
> --- a/arch/arm/mach-omap2/omap2plus-cpufreq.c
> +++ b/arch/arm/mach-omap2/omap2plus-cpufreq.c
> @@ -81,10 +81,6 @@ static int omap_target(struct cpufreq_policy *policy,
>        int i, ret = 0;
>        struct cpufreq_freqs freqs;
>
> -       /* Changes not allowed until all CPUs are online */
> -       if (is_smp() && (num_online_cpus() < NR_CPUS))
> -               return ret;
> -
>        /* Ensure desired rate is within allowed range.  Some govenors
>         * (ondemand) will just pass target_freq=0 to get the minimum. */
>        if (target_freq < policy->min)
> --
> 1.6.0.4
>
> --
> 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
>
--
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: [PATCH][pm-wip/cpufreq] OMAP2+: CPUfreq: Remove superfluous check in target() for online CPU's.

[Kevin Hilman]

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

> On Fri, Jun 3, 2011 at 07:16, Santosh Shilimkar
> <santosh.shilimkar@ti.com> wrote:
>> Current OMAP2PLUS CPUfreq tagret() functions returns when all
>> the CPU's are not online. This breaks CPUfreq when secondary CPUs
>> are offlined on SMP system.
>>
>> The intention of that check was just avoid CPU frequency change
>> during the window when CPU becomes online but it's cpufreq_init is
>> not done yet. Otherwise it can lead to notifiers being sent on
>> a CPU which is not yet registered to the governor.
>>
>> But this race conditions is already managed by the CPUfreq
>> core driver by updating the available cpumask accordingly.
>>
>> OMAP CPUFReq driver make use same cpumask for the notifiers
>> so the above problem doesn't exist. In my initial implementation
>> of the OMAP4 CPUFreq driver, I was using 'for_each_online_cpu()'
>> for notifiers which lead me to add that check. Later I fixed
>> the notifies but didn't realise that the check has become
>> redundant then.
>>
>> Fix it by removing the superfluous check in target().
>>
>> Thanks for Nishant Menon <nm@ti.com> for reporting issue
>> with hot-plug and Kevin Hilman <khilman@ti.com> for his
>> comment on excessive check in target().
>>
>> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
>> Reported-by: Nishanth Menon <nm@ti.com>
>> Tested-by: Vishwanath BS <vishwanath.bs@ti.com>
>> Cc: Kevin Hilman <khilman@ti.com>
>
> Tested-by: Nishanth Menon <nm@ti.com>

Thanks, applied to pm-wip/cpufreq

Kevin