[PATCH] cpufreq: tegra: don't error target() when suspended

[PATCH] cpufreq: tegra: don't error target() when suspended

[Stephen Warren]

From: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

d4019f0a92ab "cpufreq: move freq change notifications to cpufreq core"
added code to the cpufreq core to print an error if a cpufreq driver's
.target() function returned an error. This exposed the fact that Tegra's
cpufreq driver returns an error when it is ignoring requests due to the
system being suspended.

Modify Tegra's .target() function not to return an error in this case;
this prevents the error prints. The argument is that since the suspend
hook can't and doesn't inform the cpufreq core when its requests will
be ignored, there's no way for the cpufreq core to squelch them, so it's
not an error for the requests to keep coming. This change make the Tegra
driver consistent with how the Exynos handles the same situation. Note
that s5pv210-cpufreq.c probably suffers from this same issue though.

Signed-off-by: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
This is a fix for 3.13.

Commit d4019f0a92ab also failed to update the Tegra pm_notifier hook to
emit cpufreq notifications. That hook calls the target() implementation
directly, which used to emit the notifications. However, now that the
notifications are made outside of target(), they no longer occur when
target() is called directly. I'm not sure if this is an issue or not?
---
 drivers/cpufreq/tegra-cpufreq.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/cpufreq/tegra-cpufreq.c b/drivers/cpufreq/tegra-cpufreq.c
index f42df7ec03c5..b7309c37033d 100644
--- a/drivers/cpufreq/tegra-cpufreq.c
+++ b/drivers/cpufreq/tegra-cpufreq.c
@@ -142,10 +142,8 @@ static int tegra_target(struct cpufreq_policy *policy, unsigned int index)
 
 	mutex_lock(&tegra_cpu_lock);
 
-	if (is_suspended) {
-		ret = -EBUSY;
+	if (is_suspended)
 		goto out;
-	}
 
 	freq = freq_table[index].frequency;
 
-- 
1.8.1.5

Re: [PATCH] cpufreq: tegra: don't error target() when suspended

[Rafael J. Wysocki]

On Wednesday, November 20, 2013 02:06:22 PM Stephen Warren wrote:
> From: Stephen Warren <swarren@nvidia.com>
> 
> d4019f0a92ab "cpufreq: move freq change notifications to cpufreq core"
> added code to the cpufreq core to print an error if a cpufreq driver's
> .target() function returned an error. This exposed the fact that Tegra's
> cpufreq driver returns an error when it is ignoring requests due to the
> system being suspended.
> 
> Modify Tegra's .target() function not to return an error in this case;
> this prevents the error prints. The argument is that since the suspend
> hook can't and doesn't inform the cpufreq core when its requests will
> be ignored, there's no way for the cpufreq core to squelch them, so it's
> not an error for the requests to keep coming. This change make the Tegra
> driver consistent with how the Exynos handles the same situation. Note
> that s5pv210-cpufreq.c probably suffers from this same issue though.
> 
> Signed-off-by: Stephen Warren <swarren@nvidia.com>

Queued up for the next PM pull request, thanks!

> ---
> This is a fix for 3.13.
> 
> Commit d4019f0a92ab also failed to update the Tegra pm_notifier hook to
> emit cpufreq notifications. That hook calls the target() implementation
> directly, which used to emit the notifications. However, now that the
> notifications are made outside of target(), they no longer occur when
> target() is called directly. I'm not sure if this is an issue or not?
> ---
>  drivers/cpufreq/tegra-cpufreq.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/cpufreq/tegra-cpufreq.c b/drivers/cpufreq/tegra-cpufreq.c
> index f42df7ec03c5..b7309c37033d 100644
> --- a/drivers/cpufreq/tegra-cpufreq.c
> +++ b/drivers/cpufreq/tegra-cpufreq.c
> @@ -142,10 +142,8 @@ static int tegra_target(struct cpufreq_policy *policy, unsigned int index)
>  
>  	mutex_lock(&tegra_cpu_lock);
>  
> -	if (is_suspended) {
> -		ret = -EBUSY;
> +	if (is_suspended)
>  		goto out;
> -	}
>  
>  	freq = freq_table[index].frequency;
>  
> 
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH] cpufreq: tegra: don't error target() when suspended

[Viresh Kumar]

On 21 November 2013 02:36, Stephen Warren <swarren@wwwdotorg.org> wrote:
> From: Stephen Warren <swarren@nvidia.com>
>
> d4019f0a92ab "cpufreq: move freq change notifications to cpufreq core"
> added code to the cpufreq core to print an error if a cpufreq driver's
> .target() function returned an error. This exposed the fact that Tegra's
> cpufreq driver returns an error when it is ignoring requests due to the
> system being suspended.
>
> Modify Tegra's .target() function not to return an error in this case;
> this prevents the error prints. The argument is that since the suspend
> hook can't and doesn't inform the cpufreq core when its requests will
> be ignored, there's no way for the cpufreq core to squelch them, so it's
> not an error for the requests to keep coming. This change make the Tegra
> driver consistent with how the Exynos handles the same situation. Note
> that s5pv210-cpufreq.c probably suffers from this same issue though.
>
> Signed-off-by: Stephen Warren <swarren@nvidia.com>

Thanks..

> ---
> This is a fix for 3.13.
>
> Commit d4019f0a92ab also failed to update the Tegra pm_notifier hook to
> emit cpufreq notifications. That hook calls the target() implementation
> directly, which used to emit the notifications. However, now that the
> notifications are made outside of target(), they no longer occur when
> target() is called directly. I'm not sure if this is an issue or not?

Hmm.. Yes and no.. Frequency after this point will not be in sync with
cpufreq core, but at the same time after resume cpufreq core will check
this out and get the correct frequency..

Re: [PATCH] cpufreq: tegra: don't error target() when suspended

[Stephen Warren]

On 11/20/2013 08:44 PM, Viresh Kumar wrote:
> On 21 November 2013 02:36, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> From: Stephen Warren <swarren@nvidia.com>
>>
>> d4019f0a92ab "cpufreq: move freq change notifications to cpufreq core"
>> added code to the cpufreq core to print an error if a cpufreq driver's
>> .target() function returned an error. This exposed the fact that Tegra's
>> cpufreq driver returns an error when it is ignoring requests due to the
>> system being suspended.
>>
>> Modify Tegra's .target() function not to return an error in this case;
>> this prevents the error prints. The argument is that since the suspend
>> hook can't and doesn't inform the cpufreq core when its requests will
>> be ignored, there's no way for the cpufreq core to squelch them, so it's
>> not an error for the requests to keep coming. This change make the Tegra
>> driver consistent with how the Exynos handles the same situation. Note
>> that s5pv210-cpufreq.c probably suffers from this same issue though.
>>
>> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> 
> Thanks..
> 
>> ---
>> This is a fix for 3.13.
>>
>> Commit d4019f0a92ab also failed to update the Tegra pm_notifier hook to
>> emit cpufreq notifications. That hook calls the target() implementation
>> directly, which used to emit the notifications. However, now that the
>> notifications are made outside of target(), they no longer occur when
>> target() is called directly. I'm not sure if this is an issue or not?
> 
> Hmm.. Yes and no.. Frequency after this point will not be in sync with
> cpufreq core, but at the same time after resume cpufreq core will check
> this out and get the correct frequency..

OK, sounds like I should ignore this issue then?

Re: [PATCH] cpufreq: tegra: don't error target() when suspended

[viresh kumar]

On Friday 22 November 2013 12:11 AM, Stephen Warren wrote:
> OK, sounds like I should ignore this issue then?

Yeah. And actually we are looking for a more generic solution instead of these
per-driver PM notifiers. So, these should be removed soon..


https://lkml.org/lkml/2013/11/15/107‎