All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kevin Hilman <khilman@ti.com>
To: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: santosh.shilimkar@ti.com, jean.pihet@newoldbits.com,
	tony@atomide.com, linux-omap@vger.kernel.org,
	linaro-dev@lists.linaro.org,
	linux-arm-kernel@lists.infradead.org, patches@linaro.org
Subject: Re: [PATCH 18/18][V3] ARM: OMAP3: cpuidle - set global variables static
Date: Thu, 03 May 2012 13:26:40 -0700	[thread overview]
Message-ID: <878vh9ko3j.fsf@ti.com> (raw)
In-Reply-To: <1335276339-11135-19-git-send-email-daniel.lezcano@linaro.org> (Daniel Lezcano's message of "Tue, 24 Apr 2012 16:05:39 +0200")

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

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


WARNING: multiple messages have this Message-ID (diff)
From: khilman@ti.com (Kevin Hilman)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 18/18][V3] ARM: OMAP3: cpuidle - set global variables static
Date: Thu, 03 May 2012 13:26:40 -0700	[thread overview]
Message-ID: <878vh9ko3j.fsf@ti.com> (raw)
In-Reply-To: <1335276339-11135-19-git-send-email-daniel.lezcano@linaro.org> (Daniel Lezcano's message of "Tue, 24 Apr 2012 16:05:39 +0200")

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 at 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(-)

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

  reply	other threads:[~2012-05-03 20:26 UTC|newest]

Thread overview: 65+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-04-24 14:05 [PATCH 00/18][V3] ARM: OMAP3/4 : cpuidle34xx and cpuidle44xx cleanups Daniel Lezcano
2012-04-24 14:05 ` Daniel Lezcano
2012-04-24 14:05 ` [PATCH 01/18][V3] ARM: OMAP4: cpuidle - Remove unused valid field Daniel Lezcano
2012-04-24 14:05   ` Daniel Lezcano
2012-04-24 14:05 ` [PATCH 02/18][V3] ARM: OMAP4: cpuidle - Declare the states with the driver declaration Daniel Lezcano
2012-04-24 14:05   ` Daniel Lezcano
2012-04-24 14:05 ` [PATCH 03/18][V3] ARM: OMAP4: cpuidle - Remove the cpuidle_params_table table Daniel Lezcano
2012-04-24 14:05   ` Daniel Lezcano
2012-04-24 14:05 ` [PATCH 04/18][V3] ARM: OMAP4: cpuidle - fix static omap4_idle_data declaration Daniel Lezcano
2012-04-24 14:05   ` Daniel Lezcano
2012-04-24 14:05 ` [PATCH 05/18][V3] ARM: OMAP4: cpuidle - Initialize omap4_idle_data at compile time Daniel Lezcano
2012-04-24 14:05   ` Daniel Lezcano
2012-04-24 14:05 ` [PATCH 06/18][V3] ARM: OMAP4: cpuidle - use the omap4_idle_data variable directly Daniel Lezcano
2012-04-24 14:05   ` Daniel Lezcano
2012-04-24 14:05 ` [PATCH 07/18][V3] ARM: OMAP4: cpuidle - remove omap4_idle_data initialization at boot time Daniel Lezcano
2012-04-24 14:05   ` Daniel Lezcano
2012-04-24 14:05 ` [PATCH 08/18][V3] ARM: OMAP3: cpuidle - remove rx51 cpuidle parameters table Daniel Lezcano
2012-04-24 14:05   ` Daniel Lezcano
2012-04-24 14:05 ` [PATCH 09/18][V3] ARM: OMAP3: define cpuidle statically Daniel Lezcano
2012-04-24 14:05   ` Daniel Lezcano
2012-04-24 14:05 ` [PATCH 10/18][V3] ARM: OMAP3: cpuidle - remove errata check in the init function Daniel Lezcano
2012-04-24 14:05   ` Daniel Lezcano
2012-04-24 14:05 ` [PATCH 11/18][V3] ARM: OMAP3: cpuidle - remove the 'valid' field Daniel Lezcano
2012-04-24 14:05   ` Daniel Lezcano
2012-05-03 20:19   ` Kevin Hilman
2012-05-03 20:19     ` Kevin Hilman
2012-05-03 20:29     ` Daniel Lezcano
2012-05-03 20:29       ` Daniel Lezcano
2012-04-24 14:05 ` [PATCH 12/18][V3] ARM: OMAP3: cpuidle - remove cpuidle_params_table Daniel Lezcano
2012-04-24 14:05   ` Daniel Lezcano
2012-04-24 14:05 ` [PATCH 13/18][V3] ARM: OMAP3: define statically the omap3_idle_data Daniel Lezcano
2012-04-24 14:05   ` Daniel Lezcano
2012-04-24 14:05 ` [PATCH 14/18][V3] ARM: OMAP3: cpuidle - use omap3_idle_data directly Daniel Lezcano
2012-04-24 14:05   ` Daniel Lezcano
     [not found] ` <1335276339-11135-1-git-send-email-daniel.lezcano-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2012-04-24 14:05   ` [PATCH 15/18][V3] ARM: OMAP3: cpuidle - simplify next_valid_state Daniel Lezcano
2012-04-24 14:05     ` Daniel Lezcano
2012-04-24 14:05   ` [PATCH 16/18][V3] ARM: OMAP3: set omap3_idle_data as static Daniel Lezcano
2012-04-24 14:05     ` Daniel Lezcano
2012-04-24 14:05   ` [PATCH 17/18][V3] ARM: OMAP3/4: consolidate cpuidle Makefile Daniel Lezcano
2012-04-24 14:05     ` Daniel Lezcano
2012-05-03 20:34     ` Kevin Hilman
2012-05-03 20:34       ` Kevin Hilman
2012-05-03 23:03       ` Daniel Lezcano
2012-05-03 23:03         ` Daniel Lezcano
2012-04-24 14:05 ` [PATCH 18/18][V3] ARM: OMAP3: cpuidle - set global variables static Daniel Lezcano
2012-04-24 14:05   ` Daniel Lezcano
2012-05-03 20:26   ` Kevin Hilman [this message]
2012-05-03 20:26     ` Kevin Hilman
2012-05-03 23:02     ` Daniel Lezcano
2012-05-03 23:02       ` Daniel Lezcano
2012-04-25 22:00 ` [PATCH 00/18][V3] ARM: OMAP3/4 : cpuidle34xx and cpuidle44xx cleanups Daniel Lezcano
2012-04-25 22:00   ` Daniel Lezcano
2012-04-30 22:58   ` Kevin Hilman
2012-04-30 22:58     ` Kevin Hilman
2012-05-01  8:16     ` Daniel Lezcano
2012-05-01  8:16       ` Daniel Lezcano
     [not found]       ` <4F9F9BC1.9020908-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2012-05-01  9:25         ` Amit Kucheria
2012-05-01  9:25           ` Amit Kucheria
2012-05-01  9:55         ` Shilimkar, Santosh
2012-05-01 10:20           ` Daniel Lezcano
2012-05-01 10:20             ` Daniel Lezcano
2012-05-02  6:20             ` Shilimkar, Santosh
2012-05-02  6:20               ` Shilimkar, Santosh
2012-05-03 20:39 ` Kevin Hilman
2012-05-03 20:39   ` Kevin Hilman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=878vh9ko3j.fsf@ti.com \
    --to=khilman@ti.com \
    --cc=daniel.lezcano@linaro.org \
    --cc=jean.pihet@newoldbits.com \
    --cc=linaro-dev@lists.linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=patches@linaro.org \
    --cc=santosh.shilimkar@ti.com \
    --cc=tony@atomide.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.