diff mbox

[18/18,V3] ARM: OMAP3: cpuidle - set global variables static

Message ID 878vh9ko3j.fsf@ti.com
State Accepted
Commit 34fd57bffe60807f5639fc8bbe7a46ce97e45f50
Headers show

Commit Message

Kevin Hilman May 3, 2012, 8:26 p.m. UTC 
Daniel Lezcano <daniel.lezcano@linaro.org> writes:

> and check the powerdomain lookup is successful.
>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> Reviewed-by: Jean Pihet <j-pihet@ti.com>
> ---
>  arch/arm/mach-omap2/cpuidle34xx.c |    5 ++++-
>  1 files changed, 4 insertions(+), 1 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-omap2/cpuidle34xx.c
> index 0d28596..116a0d8 100644
> --- a/arch/arm/mach-omap2/cpuidle34xx.c
> +++ b/arch/arm/mach-omap2/cpuidle34xx.c
> @@ -73,7 +73,7 @@ static struct omap3_idle_statedata omap3_idle_data[] = {
>  	},
>  };
>  
> -struct powerdomain *mpu_pd, *core_pd, *per_pd, *cam_pd;
> +static struct powerdomain *mpu_pd, *core_pd, *per_pd, *cam_pd;
>  
>  static int _cpuidle_allow_idle(struct powerdomain *pwrdm,
>  				struct clockdomain *clkdm)
> @@ -252,6 +252,9 @@ static int omap3_enter_idle_bm(struct cpuidle_device *dev,
>  	 *        its own code.
>  	 */
>  
> +	if (!mpu_pd || !core_pd || !per_pd || !cam_pd)
> +		return -ENODEV;
> +

Why check here?  cam_pd is actually used just above this code.

Also, this is in the fast path.  This should just be checked once at
init time instead of every idle path.

For those reasons, I've dropped this part of the patch (updated patch
below.)

If desired, you can send an additional patch that adds this error
checking at init time and then refuses to register the driver if any of
the pwrdms don't exist.

Kevin


From 7870c78d0343ab39050bec01d88cdb19245fdaa9 Mon Sep 17 00:00:00 2001
From: Daniel Lezcano <daniel.lezcano@linaro.org>
Date: Tue, 24 Apr 2012 16:05:39 +0200
Subject: [PATCH] ARM: OMAP3: cpuidle - set global variables static

struct powerdomain varialbes are all file local, make them static.

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Reviewed-by: Jean Pihet <j-pihet@ti.com>
Reviewed-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
Tested-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
Tested-by: Kevin Hilman <khilman@ti.com>
[khilman@ti.com: update changelog, drop error check in fast path]
Signed-off-by: Kevin Hilman <khilman@ti.com>
---
 arch/arm/mach-omap2/cpuidle34xx.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Daniel Lezcano May 3, 2012, 11:02 p.m. UTC | #1 
On 05/03/2012 10:26 PM, Kevin Hilman wrote:
> Daniel Lezcano<daniel.lezcano@linaro.org>  writes:
>
>> and check the powerdomain lookup is successful.
>>
>> Signed-off-by: Daniel Lezcano<daniel.lezcano@linaro.org>
>> Reviewed-by: Jean Pihet<j-pihet@ti.com>
>> ---
>>   arch/arm/mach-omap2/cpuidle34xx.c |    5 ++++-
>>   1 files changed, 4 insertions(+), 1 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-omap2/cpuidle34xx.c
>> index 0d28596..116a0d8 100644
>> --- a/arch/arm/mach-omap2/cpuidle34xx.c
>> +++ b/arch/arm/mach-omap2/cpuidle34xx.c
>> @@ -73,7 +73,7 @@ static struct omap3_idle_statedata omap3_idle_data[] = {
>>   	},
>>   };
>>
>> -struct powerdomain *mpu_pd, *core_pd, *per_pd, *cam_pd;
>> +static struct powerdomain *mpu_pd, *core_pd, *per_pd, *cam_pd;
>>
>>   static int _cpuidle_allow_idle(struct powerdomain *pwrdm,
>>   				struct clockdomain *clkdm)
>> @@ -252,6 +252,9 @@ static int omap3_enter_idle_bm(struct cpuidle_device *dev,
>>   	 *        its own code.
>>   	 */
>>
>> +	if (!mpu_pd || !core_pd || !per_pd || !cam_pd)
>> +		return -ENODEV;
>> +
>
> Why check here?  cam_pd is actually used just above this code.
>
> Also, this is in the fast path.  This should just be checked once at
> init time instead of every idle path.
>
> For those reasons, I've dropped this part of the patch (updated patch
> below.)

Hmm, weird this part should have been in the init function, I likely 
missed a big fuzz I think when porting the patchset to the latest kernel 
version. Thanks for fixing it.

> If desired, you can send an additional patch that adds this error
> checking at init time and then refuses to register the driver if any of
> the pwrdms don't exist.

Yes, I will do it.

   -- Daniel
diff mbox

Patch

diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-omap2/cpuidle34xx.c
index 0d28596..9429c65 100644
--- a/arch/arm/mach-omap2/cpuidle34xx.c
+++ b/arch/arm/mach-omap2/cpuidle34xx.c
@@ -73,7 +73,7 @@  static struct omap3_idle_statedata omap3_idle_data[] = {
 	},
 };
 
-struct powerdomain *mpu_pd, *core_pd, *per_pd, *cam_pd;
+static struct powerdomain *mpu_pd, *core_pd, *per_pd, *cam_pd;
 
 static int _cpuidle_allow_idle(struct powerdomain *pwrdm,
 				struct clockdomain *clkdm)