[PATCH] ARM: OMAP2+: PM: Fix the dt return condition in pm_late_init()

[PATCH] ARM: OMAP2+: PM: Fix the dt return condition in pm_late_init()

[Santosh Shilimkar]

Commit 1416408d {ARM: OMAP2+: PM: share some suspend-related functions
across OMAP2, 3, 4} moved suspend code to common place but now with
that change, for DT build on OMAP4, suspend hooks are not getting
registered which results in no suspend support.

The DT return condition is limited to PMIC and smartreflex
initialization and hence restrict it so that suspend ops gets
registered.

Cc: Paul Walmsley <paul@pwsan.com>
Cc: Kevin Hilman <khilman@deeprootsystems.com>

Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
---
 arch/arm/mach-omap2/pm.c |   20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/arch/arm/mach-omap2/pm.c b/arch/arm/mach-omap2/pm.c
index f4b3143..1ec4299 100644
--- a/arch/arm/mach-omap2/pm.c
+++ b/arch/arm/mach-omap2/pm.c
@@ -345,19 +345,19 @@ int __init omap2_common_pm_late_init(void)
 	 * a completely different mechanism.
 	 * Disable this part if a DT blob is available.
 	 */
-	if (of_have_populated_dt())
-		return 0;
+	if (!of_have_populated_dt()) {
 
-	/* Init the voltage layer */
-	omap_pmic_late_init();
-	omap_voltage_late_init();
+		/* Init the voltage layer */
+		omap_pmic_late_init();
+		omap_voltage_late_init();
 
-	/* Initialize the voltages */
-	omap3_init_voltages();
-	omap4_init_voltages();
+		/* Initialize the voltages */
+		omap3_init_voltages();
+		omap4_init_voltages();
 
-	/* Smartreflex device init */
-	omap_devinit_smartreflex();
+		/* Smartreflex device init */
+		omap_devinit_smartreflex();
+	}
 
 #ifdef CONFIG_SUSPEND
 	suspend_set_ops(&omap_pm_ops);
-- 
1.7.9.5

[PATCH] ARM: OMAP2+: PM: Fix the dt return condition in pm_late_init()

[Santosh Shilimkar]

Commit 1416408d {ARM: OMAP2+: PM: share some suspend-related functions
across OMAP2, 3, 4} moved suspend code to common place but now with
that change, for DT build on OMAP4, suspend hooks are not getting
registered which results in no suspend support.

The DT return condition is limited to PMIC and smartreflex
initialization and hence restrict it so that suspend ops gets
registered.

Cc: Paul Walmsley <paul@pwsan.com>
Cc: Kevin Hilman <khilman@deeprootsystems.com>

Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
---
 arch/arm/mach-omap2/pm.c |   20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/arch/arm/mach-omap2/pm.c b/arch/arm/mach-omap2/pm.c
index f4b3143..1ec4299 100644
--- a/arch/arm/mach-omap2/pm.c
+++ b/arch/arm/mach-omap2/pm.c
@@ -345,19 +345,19 @@ int __init omap2_common_pm_late_init(void)
 	 * a completely different mechanism.
 	 * Disable this part if a DT blob is available.
 	 */
-	if (of_have_populated_dt())
-		return 0;
+	if (!of_have_populated_dt()) {
 
-	/* Init the voltage layer */
-	omap_pmic_late_init();
-	omap_voltage_late_init();
+		/* Init the voltage layer */
+		omap_pmic_late_init();
+		omap_voltage_late_init();
 
-	/* Initialize the voltages */
-	omap3_init_voltages();
-	omap4_init_voltages();
+		/* Initialize the voltages */
+		omap3_init_voltages();
+		omap4_init_voltages();
 
-	/* Smartreflex device init */
-	omap_devinit_smartreflex();
+		/* Smartreflex device init */
+		omap_devinit_smartreflex();
+	}
 
 #ifdef CONFIG_SUSPEND
 	suspend_set_ops(&omap_pm_ops);
-- 
1.7.9.5

[PATCH] ARM: OMAP2+: Remove duplicate omap4430_init_late() declaration

[Santosh Shilimkar]

Commit bbd707ac {ARM: omap2: use machine specific hook for late init}
accidentally added two declarations for omap4430_init_late().

Remove the duplicate declaration.

Cc: Shawn Guo <shawn.guo@linaro.org>
Cc: Tony Lindgren <tony@atomide.com>

Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
---
 arch/arm/mach-omap2/common.h |    1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm/mach-omap2/common.h b/arch/arm/mach-omap2/common.h
index 948bcaa..e0f9ea7 100644
--- a/arch/arm/mach-omap2/common.h
+++ b/arch/arm/mach-omap2/common.h
@@ -108,7 +108,6 @@ void omap35xx_init_late(void);
 void omap3630_init_late(void);
 void am35xx_init_late(void);
 void ti81xx_init_late(void);
-void omap4430_init_late(void);
 int omap2_common_pm_late_init(void);
 
 #if defined(CONFIG_SOC_OMAP2420) || defined(CONFIG_SOC_OMAP2430)
-- 
1.7.9.5

[PATCH] ARM: OMAP2+: Remove duplicate omap4430_init_late() declaration

[Santosh Shilimkar]

Commit bbd707ac {ARM: omap2: use machine specific hook for late init}
accidentally added two declarations for omap4430_init_late().

Remove the duplicate declaration.

Cc: Shawn Guo <shawn.guo@linaro.org>
Cc: Tony Lindgren <tony@atomide.com>

Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
---
 arch/arm/mach-omap2/common.h |    1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm/mach-omap2/common.h b/arch/arm/mach-omap2/common.h
index 948bcaa..e0f9ea7 100644
--- a/arch/arm/mach-omap2/common.h
+++ b/arch/arm/mach-omap2/common.h
@@ -108,7 +108,6 @@ void omap35xx_init_late(void);
 void omap3630_init_late(void);
 void am35xx_init_late(void);
 void ti81xx_init_late(void);
-void omap4430_init_late(void);
 int omap2_common_pm_late_init(void);
 
 #if defined(CONFIG_SOC_OMAP2420) || defined(CONFIG_SOC_OMAP2430)
-- 
1.7.9.5

Re: [PATCH] ARM: OMAP2+: Remove duplicate omap4430_init_late() declaration

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1277 bytes --]

On Fri, Feb 08, 2013 at 08:41:45PM +0530, Santosh Shilimkar wrote:
> Commit bbd707ac {ARM: omap2: use machine specific hook for late init}
> accidentally added two declarations for omap4430_init_late().
> 
> Remove the duplicate declaration.
> 
> Cc: Shawn Guo <shawn.guo@linaro.org>
> Cc: Tony Lindgren <tony@atomide.com>
> 
> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>

nice catch

Reviewed-by: Felipe Balbi <balbi@ti.com>

> ---
>  arch/arm/mach-omap2/common.h |    1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/arch/arm/mach-omap2/common.h b/arch/arm/mach-omap2/common.h
> index 948bcaa..e0f9ea7 100644
> --- a/arch/arm/mach-omap2/common.h
> +++ b/arch/arm/mach-omap2/common.h
> @@ -108,7 +108,6 @@ void omap35xx_init_late(void);
>  void omap3630_init_late(void);
>  void am35xx_init_late(void);
>  void ti81xx_init_late(void);
> -void omap4430_init_late(void);
>  int omap2_common_pm_late_init(void);
>  
>  #if defined(CONFIG_SOC_OMAP2420) || defined(CONFIG_SOC_OMAP2430)
> -- 
> 1.7.9.5
> 
> --
> 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

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH] ARM: OMAP2+: Remove duplicate omap4430_init_late() declaration

[Felipe Balbi]

On Fri, Feb 08, 2013 at 08:41:45PM +0530, Santosh Shilimkar wrote:
> Commit bbd707ac {ARM: omap2: use machine specific hook for late init}
> accidentally added two declarations for omap4430_init_late().
> 
> Remove the duplicate declaration.
> 
> Cc: Shawn Guo <shawn.guo@linaro.org>
> Cc: Tony Lindgren <tony@atomide.com>
> 
> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>

nice catch

Reviewed-by: Felipe Balbi <balbi@ti.com>

> ---
>  arch/arm/mach-omap2/common.h |    1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/arch/arm/mach-omap2/common.h b/arch/arm/mach-omap2/common.h
> index 948bcaa..e0f9ea7 100644
> --- a/arch/arm/mach-omap2/common.h
> +++ b/arch/arm/mach-omap2/common.h
> @@ -108,7 +108,6 @@ void omap35xx_init_late(void);
>  void omap3630_init_late(void);
>  void am35xx_init_late(void);
>  void ti81xx_init_late(void);
> -void omap4430_init_late(void);
>  int omap2_common_pm_late_init(void);
>  
>  #if defined(CONFIG_SOC_OMAP2420) || defined(CONFIG_SOC_OMAP2430)
> -- 
> 1.7.9.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130208/faaeef3e/attachment-0001.sig>

Re: [PATCH] ARM: OMAP2+: Remove duplicate omap4430_init_late() declaration

[Santosh Shilimkar]

Tony,

On Friday 08 February 2013 08:41 PM, Santosh Shilimkar wrote:
> Commit bbd707ac {ARM: omap2: use machine specific hook for late init}
> accidentally added two declarations for omap4430_init_late().
>
> Remove the duplicate declaration.
>
> Cc: Shawn Guo <shawn.guo@linaro.org>
> Cc: Tony Lindgren <tony@atomide.com>
>
> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
> ---
>   arch/arm/mach-omap2/common.h |    1 -
>   1 file changed, 1 deletion(-)
>
> diff --git a/arch/arm/mach-omap2/common.h b/arch/arm/mach-omap2/common.h
> index 948bcaa..e0f9ea7 100644
> --- a/arch/arm/mach-omap2/common.h
> +++ b/arch/arm/mach-omap2/common.h
> @@ -108,7 +108,6 @@ void omap35xx_init_late(void);
>   void omap3630_init_late(void);
>   void am35xx_init_late(void);
>   void ti81xx_init_late(void);
> -void omap4430_init_late(void);
>   int omap2_common_pm_late_init(void);
>
>   #if defined(CONFIG_SOC_OMAP2420) || defined(CONFIG_SOC_OMAP2430)
>
Can you pick this up ?

Regards,
Santosh

[PATCH] ARM: OMAP2+: Remove duplicate omap4430_init_late() declaration

[Santosh Shilimkar]

Tony,

On Friday 08 February 2013 08:41 PM, Santosh Shilimkar wrote:
> Commit bbd707ac {ARM: omap2: use machine specific hook for late init}
> accidentally added two declarations for omap4430_init_late().
>
> Remove the duplicate declaration.
>
> Cc: Shawn Guo <shawn.guo@linaro.org>
> Cc: Tony Lindgren <tony@atomide.com>
>
> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
> ---
>   arch/arm/mach-omap2/common.h |    1 -
>   1 file changed, 1 deletion(-)
>
> diff --git a/arch/arm/mach-omap2/common.h b/arch/arm/mach-omap2/common.h
> index 948bcaa..e0f9ea7 100644
> --- a/arch/arm/mach-omap2/common.h
> +++ b/arch/arm/mach-omap2/common.h
> @@ -108,7 +108,6 @@ void omap35xx_init_late(void);
>   void omap3630_init_late(void);
>   void am35xx_init_late(void);
>   void ti81xx_init_late(void);
> -void omap4430_init_late(void);
>   int omap2_common_pm_late_init(void);
>
>   #if defined(CONFIG_SOC_OMAP2420) || defined(CONFIG_SOC_OMAP2430)
>
Can you pick this up ?

Regards,
Santosh

Re: [PATCH] ARM: OMAP2+: Remove duplicate omap4430_init_late() declaration

[Tony Lindgren]

* Santosh Shilimkar <santosh.shilimkar@ti.com> [130215 01:38]:
> Tony,
> 
> On Friday 08 February 2013 08:41 PM, Santosh Shilimkar wrote:
> >Commit bbd707ac {ARM: omap2: use machine specific hook for late init}
> >accidentally added two declarations for omap4430_init_late().
> >
> >Remove the duplicate declaration.
> >
> >Cc: Shawn Guo <shawn.guo@linaro.org>
> >Cc: Tony Lindgren <tony@atomide.com>
> >
> >Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
> >---
> >  arch/arm/mach-omap2/common.h |    1 -
> >  1 file changed, 1 deletion(-)
> >
> >diff --git a/arch/arm/mach-omap2/common.h b/arch/arm/mach-omap2/common.h
> >index 948bcaa..e0f9ea7 100644
> >--- a/arch/arm/mach-omap2/common.h
> >+++ b/arch/arm/mach-omap2/common.h
> >@@ -108,7 +108,6 @@ void omap35xx_init_late(void);
> >  void omap3630_init_late(void);
> >  void am35xx_init_late(void);
> >  void ti81xx_init_late(void);
> >-void omap4430_init_late(void);
> >  int omap2_common_pm_late_init(void);
> >
> >  #if defined(CONFIG_SOC_OMAP2420) || defined(CONFIG_SOC_OMAP2430)
> >
> Can you pick this up ?

Thanks applying into omap-for-v3.9-rc1/fixes.

Regards,

Tony

[PATCH] ARM: OMAP2+: Remove duplicate omap4430_init_late() declaration

[Tony Lindgren]

* Santosh Shilimkar <santosh.shilimkar@ti.com> [130215 01:38]:
> Tony,
> 
> On Friday 08 February 2013 08:41 PM, Santosh Shilimkar wrote:
> >Commit bbd707ac {ARM: omap2: use machine specific hook for late init}
> >accidentally added two declarations for omap4430_init_late().
> >
> >Remove the duplicate declaration.
> >
> >Cc: Shawn Guo <shawn.guo@linaro.org>
> >Cc: Tony Lindgren <tony@atomide.com>
> >
> >Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
> >---
> >  arch/arm/mach-omap2/common.h |    1 -
> >  1 file changed, 1 deletion(-)
> >
> >diff --git a/arch/arm/mach-omap2/common.h b/arch/arm/mach-omap2/common.h
> >index 948bcaa..e0f9ea7 100644
> >--- a/arch/arm/mach-omap2/common.h
> >+++ b/arch/arm/mach-omap2/common.h
> >@@ -108,7 +108,6 @@ void omap35xx_init_late(void);
> >  void omap3630_init_late(void);
> >  void am35xx_init_late(void);
> >  void ti81xx_init_late(void);
> >-void omap4430_init_late(void);
> >  int omap2_common_pm_late_init(void);
> >
> >  #if defined(CONFIG_SOC_OMAP2420) || defined(CONFIG_SOC_OMAP2430)
> >
> Can you pick this up ?

Thanks applying into omap-for-v3.9-rc1/fixes.

Regards,

Tony

[PATCH] ARM: OMAP4: PM: Avoid expensive cpu_suspend() path for all CPU power states except off

[Santosh Shilimkar]

Current CPU PM code code make use of common cpu_suspend() path for all the
CPU power states which is not optimal. In fact cpu_suspend() path is needed
only when we put CPU power domain to off state where the CPU context is lost.

Update the code accordingly so that the expensive cpu_suspend() path
can be avoided for the shallow CPU power states like CPU PD INA/CSWR.

Cc: Kevin Hilman <khilman@deeprootsystems.com>

Reported-by: Richard Woodruff <r-woodruff2@ti.com>
Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
---
 arch/arm/mach-omap2/omap-mpuss-lowpower.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-omap2/omap-mpuss-lowpower.c b/arch/arm/mach-omap2/omap-mpuss-lowpower.c
index aac46bf..abdd0f6 100644
--- a/arch/arm/mach-omap2/omap-mpuss-lowpower.c
+++ b/arch/arm/mach-omap2/omap-mpuss-lowpower.c
@@ -276,7 +276,10 @@ int omap4_enter_lowpower(unsigned int cpu, unsigned int power_state)
 	/*
 	 * Call low level function  with targeted low power state.
 	 */
-	cpu_suspend(save_state, omap4_finish_suspend);
+	if (save_state)
+		cpu_suspend(save_state, omap4_finish_suspend);
+	else
+		omap4_finish_suspend(save_state);
 
 	/*
 	 * Restore the CPUx power state to ON otherwise CPUx
-- 
1.7.9.5

[PATCH] ARM: OMAP4: PM: Avoid expensive cpu_suspend() path for all CPU power states except off

[Santosh Shilimkar]

Current CPU PM code code make use of common cpu_suspend() path for all the
CPU power states which is not optimal. In fact cpu_suspend() path is needed
only when we put CPU power domain to off state where the CPU context is lost.

Update the code accordingly so that the expensive cpu_suspend() path
can be avoided for the shallow CPU power states like CPU PD INA/CSWR.

Cc: Kevin Hilman <khilman@deeprootsystems.com>

Reported-by: Richard Woodruff <r-woodruff2@ti.com>
Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
---
 arch/arm/mach-omap2/omap-mpuss-lowpower.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-omap2/omap-mpuss-lowpower.c b/arch/arm/mach-omap2/omap-mpuss-lowpower.c
index aac46bf..abdd0f6 100644
--- a/arch/arm/mach-omap2/omap-mpuss-lowpower.c
+++ b/arch/arm/mach-omap2/omap-mpuss-lowpower.c
@@ -276,7 +276,10 @@ int omap4_enter_lowpower(unsigned int cpu, unsigned int power_state)
 	/*
 	 * Call low level function  with targeted low power state.
 	 */
-	cpu_suspend(save_state, omap4_finish_suspend);
+	if (save_state)
+		cpu_suspend(save_state, omap4_finish_suspend);
+	else
+		omap4_finish_suspend(save_state);
 
 	/*
 	 * Restore the CPUx power state to ON otherwise CPUx
-- 
1.7.9.5

Re: [PATCH] ARM: OMAP4: PM: Avoid expensive cpu_suspend() path for all CPU power states except off

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 697 bytes --]

On Fri, Feb 08, 2013 at 08:41:46PM +0530, Santosh Shilimkar wrote:
> Current CPU PM code code make use of common cpu_suspend() path for all the
> CPU power states which is not optimal. In fact cpu_suspend() path is needed
> only when we put CPU power domain to off state where the CPU context is lost.
> 
> Update the code accordingly so that the expensive cpu_suspend() path
> can be avoided for the shallow CPU power states like CPU PD INA/CSWR.
> 
> Cc: Kevin Hilman <khilman@deeprootsystems.com>
> 
> Reported-by: Richard Woodruff <r-woodruff2@ti.com>
> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>

some numbers showing the benefit would be cool...

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH] ARM: OMAP4: PM: Avoid expensive cpu_suspend() path for all CPU power states except off

[Felipe Balbi]

On Fri, Feb 08, 2013 at 08:41:46PM +0530, Santosh Shilimkar wrote:
> Current CPU PM code code make use of common cpu_suspend() path for all the
> CPU power states which is not optimal. In fact cpu_suspend() path is needed
> only when we put CPU power domain to off state where the CPU context is lost.
> 
> Update the code accordingly so that the expensive cpu_suspend() path
> can be avoided for the shallow CPU power states like CPU PD INA/CSWR.
> 
> Cc: Kevin Hilman <khilman@deeprootsystems.com>
> 
> Reported-by: Richard Woodruff <r-woodruff2@ti.com>
> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>

some numbers showing the benefit would be cool...

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130208/e0a56f93/attachment.sig>

Re: [PATCH] ARM: OMAP4: PM: Avoid expensive cpu_suspend() path for all CPU power states except off

[Santosh Shilimkar]

On Friday 08 February 2013 08:56 PM, Felipe Balbi wrote:
> On Fri, Feb 08, 2013 at 08:41:46PM +0530, Santosh Shilimkar wrote:
>> Current CPU PM code code make use of common cpu_suspend() path for all the
>> CPU power states which is not optimal. In fact cpu_suspend() path is needed
>> only when we put CPU power domain to off state where the CPU context is lost.
>>
>> Update the code accordingly so that the expensive cpu_suspend() path
>> can be avoided for the shallow CPU power states like CPU PD INA/CSWR.
>>
>> Cc: Kevin Hilman <khilman@deeprootsystems.com>
>>
>> Reported-by: Richard Woodruff <r-woodruff2@ti.com>
>> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
>
> some numbers showing the benefit would be cool...
>
The numbers depends on CPU clock you are running generally. The main
issue is, for the shallow power states where we were not loosing
CPU context or Caches, we were doing all the book keeping. States
like INA are of 6-8 uS and above overhead is quite significant
their.

Regards
Santosh

[PATCH] ARM: OMAP4: PM: Avoid expensive cpu_suspend() path for all CPU power states except off

[Santosh Shilimkar]

On Friday 08 February 2013 08:56 PM, Felipe Balbi wrote:
> On Fri, Feb 08, 2013 at 08:41:46PM +0530, Santosh Shilimkar wrote:
>> Current CPU PM code code make use of common cpu_suspend() path for all the
>> CPU power states which is not optimal. In fact cpu_suspend() path is needed
>> only when we put CPU power domain to off state where the CPU context is lost.
>>
>> Update the code accordingly so that the expensive cpu_suspend() path
>> can be avoided for the shallow CPU power states like CPU PD INA/CSWR.
>>
>> Cc: Kevin Hilman <khilman@deeprootsystems.com>
>>
>> Reported-by: Richard Woodruff <r-woodruff2@ti.com>
>> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
>
> some numbers showing the benefit would be cool...
>
The numbers depends on CPU clock you are running generally. The main
issue is, for the shallow power states where we were not loosing
CPU context or Caches, we were doing all the book keeping. States
like INA are of 6-8 uS and above overhead is quite significant
their.

Regards
Santosh

Re: [PATCH] ARM: OMAP4: PM: Avoid expensive cpu_suspend() path for all CPU power states except off

[Kevin Hilman]

Santosh Shilimkar <santosh.shilimkar@ti.com> writes:

> Current CPU PM code code make use of common cpu_suspend() path for all the
> CPU power states which is not optimal. In fact cpu_suspend() path is needed
> only when we put CPU power domain to off state where the CPU context is lost.
>
> Update the code accordingly so that the expensive cpu_suspend() path
> can be avoided for the shallow CPU power states like CPU PD INA/CSWR.
>
> Cc: Kevin Hilman <khilman@deeprootsystems.com>
>
> Reported-by: Richard Woodruff <r-woodruff2@ti.com>
> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>

Looks OK at first glance, but are you sure this is right for the
various ways both clusters might idle using coupled CPUidle?

Some description of the testing would be helpful here.

Thanks,

Kevin

> ---
>  arch/arm/mach-omap2/omap-mpuss-lowpower.c |    5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-omap2/omap-mpuss-lowpower.c b/arch/arm/mach-omap2/omap-mpuss-lowpower.c
> index aac46bf..abdd0f6 100644
> --- a/arch/arm/mach-omap2/omap-mpuss-lowpower.c
> +++ b/arch/arm/mach-omap2/omap-mpuss-lowpower.c
> @@ -276,7 +276,10 @@ int omap4_enter_lowpower(unsigned int cpu, unsigned int power_state)
>  	/*
>  	 * Call low level function  with targeted low power state.
>  	 */
> -	cpu_suspend(save_state, omap4_finish_suspend);
> +	if (save_state)
> +		cpu_suspend(save_state, omap4_finish_suspend);
> +	else
> +		omap4_finish_suspend(save_state);
>  
>  	/*
>  	 * Restore the CPUx power state to ON otherwise CPUx

[PATCH] ARM: OMAP4: PM: Avoid expensive cpu_suspend() path for all CPU power states except off

[Kevin Hilman]

Santosh Shilimkar <santosh.shilimkar@ti.com> writes:

> Current CPU PM code code make use of common cpu_suspend() path for all the
> CPU power states which is not optimal. In fact cpu_suspend() path is needed
> only when we put CPU power domain to off state where the CPU context is lost.
>
> Update the code accordingly so that the expensive cpu_suspend() path
> can be avoided for the shallow CPU power states like CPU PD INA/CSWR.
>
> Cc: Kevin Hilman <khilman@deeprootsystems.com>
>
> Reported-by: Richard Woodruff <r-woodruff2@ti.com>
> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>

Looks OK at first glance, but are you sure this is right for the
various ways both clusters might idle using coupled CPUidle?

Some description of the testing would be helpful here.

Thanks,

Kevin

> ---
>  arch/arm/mach-omap2/omap-mpuss-lowpower.c |    5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-omap2/omap-mpuss-lowpower.c b/arch/arm/mach-omap2/omap-mpuss-lowpower.c
> index aac46bf..abdd0f6 100644
> --- a/arch/arm/mach-omap2/omap-mpuss-lowpower.c
> +++ b/arch/arm/mach-omap2/omap-mpuss-lowpower.c
> @@ -276,7 +276,10 @@ int omap4_enter_lowpower(unsigned int cpu, unsigned int power_state)
>  	/*
>  	 * Call low level function  with targeted low power state.
>  	 */
> -	cpu_suspend(save_state, omap4_finish_suspend);
> +	if (save_state)
> +		cpu_suspend(save_state, omap4_finish_suspend);
> +	else
> +		omap4_finish_suspend(save_state);
>  
>  	/*
>  	 * Restore the CPUx power state to ON otherwise CPUx

Re: [PATCH] ARM: OMAP4: PM: Avoid expensive cpu_suspend() path for all CPU power states except off

[Santosh Shilimkar]

On Saturday 09 February 2013 02:49 AM, Kevin Hilman wrote:
> Santosh Shilimkar <santosh.shilimkar@ti.com> writes:
>
>> Current CPU PM code code make use of common cpu_suspend() path for all the
>> CPU power states which is not optimal. In fact cpu_suspend() path is needed
>> only when we put CPU power domain to off state where the CPU context is lost.
>>
>> Update the code accordingly so that the expensive cpu_suspend() path
>> can be avoided for the shallow CPU power states like CPU PD INA/CSWR.
>>
>> Cc: Kevin Hilman <khilman@deeprootsystems.com>
>>
>> Reported-by: Richard Woodruff <r-woodruff2@ti.com>
>> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
>
> Looks OK at first glance, but are you sure this is right for the
> various ways both clusters might idle using coupled CPUidle?
>
Yes it is perfectly safe from all C-states. This patch has been part of
the OMAP4/OMAP5 product port for some time. I forgot to send it upstream
some how :(

> Some description of the testing would be helpful here.
>
Sorry. Should have mentioned it in first place.
Patch is being tested on OMAP4430 (idle/suspend) and OMAP5 with
few out of tree patches.

Regards,
Santosh

[PATCH] ARM: OMAP4: PM: Avoid expensive cpu_suspend() path for all CPU power states except off

[Santosh Shilimkar]

On Saturday 09 February 2013 02:49 AM, Kevin Hilman wrote:
> Santosh Shilimkar <santosh.shilimkar@ti.com> writes:
>
>> Current CPU PM code code make use of common cpu_suspend() path for all the
>> CPU power states which is not optimal. In fact cpu_suspend() path is needed
>> only when we put CPU power domain to off state where the CPU context is lost.
>>
>> Update the code accordingly so that the expensive cpu_suspend() path
>> can be avoided for the shallow CPU power states like CPU PD INA/CSWR.
>>
>> Cc: Kevin Hilman <khilman@deeprootsystems.com>
>>
>> Reported-by: Richard Woodruff <r-woodruff2@ti.com>
>> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
>
> Looks OK at first glance, but are you sure this is right for the
> various ways both clusters might idle using coupled CPUidle?
>
Yes it is perfectly safe from all C-states. This patch has been part of
the OMAP4/OMAP5 product port for some time. I forgot to send it upstream
some how :(

> Some description of the testing would be helpful here.
>
Sorry. Should have mentioned it in first place.
Patch is being tested on OMAP4430 (idle/suspend) and OMAP5 with
few out of tree patches.

Regards,
Santosh

Re: [PATCH] ARM: OMAP4: PM: Avoid expensive cpu_suspend() path for all CPU power states except off

[Kevin Hilman]

Santosh Shilimkar <santosh.shilimkar@ti.com> writes:

> On Saturday 09 February 2013 02:49 AM, Kevin Hilman wrote:
>> Santosh Shilimkar <santosh.shilimkar@ti.com> writes:
>>
>>> Current CPU PM code code make use of common cpu_suspend() path for all the
>>> CPU power states which is not optimal. In fact cpu_suspend() path is needed
>>> only when we put CPU power domain to off state where the CPU context is lost.
>>>
>>> Update the code accordingly so that the expensive cpu_suspend() path
>>> can be avoided for the shallow CPU power states like CPU PD INA/CSWR.
>>>
>>> Cc: Kevin Hilman <khilman@deeprootsystems.com>
>>>
>>> Reported-by: Richard Woodruff <r-woodruff2@ti.com>
>>> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
>>
>> Looks OK at first glance, but are you sure this is right for the
>> various ways both clusters might idle using coupled CPUidle?
>>
> Yes it is perfectly safe from all C-states. This patch has been part of
> the OMAP4/OMAP5 product port for some time. I forgot to send it upstream
> some how :(
>
>> Some description of the testing would be helpful here.
>>
> Sorry. Should have mentioned it in first place.
> Patch is being tested on OMAP4430 (idle/suspend) and OMAP5 with
> few out of tree patches.

OK, please update changelog with a brief description of how it was
tested, and on which platforms.

Thanks,

Kevin

[PATCH] ARM: OMAP4: PM: Avoid expensive cpu_suspend() path for all CPU power states except off

[Kevin Hilman]

Santosh Shilimkar <santosh.shilimkar@ti.com> writes:

> On Saturday 09 February 2013 02:49 AM, Kevin Hilman wrote:
>> Santosh Shilimkar <santosh.shilimkar@ti.com> writes:
>>
>>> Current CPU PM code code make use of common cpu_suspend() path for all the
>>> CPU power states which is not optimal. In fact cpu_suspend() path is needed
>>> only when we put CPU power domain to off state where the CPU context is lost.
>>>
>>> Update the code accordingly so that the expensive cpu_suspend() path
>>> can be avoided for the shallow CPU power states like CPU PD INA/CSWR.
>>>
>>> Cc: Kevin Hilman <khilman@deeprootsystems.com>
>>>
>>> Reported-by: Richard Woodruff <r-woodruff2@ti.com>
>>> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
>>
>> Looks OK at first glance, but are you sure this is right for the
>> various ways both clusters might idle using coupled CPUidle?
>>
> Yes it is perfectly safe from all C-states. This patch has been part of
> the OMAP4/OMAP5 product port for some time. I forgot to send it upstream
> some how :(
>
>> Some description of the testing would be helpful here.
>>
> Sorry. Should have mentioned it in first place.
> Patch is being tested on OMAP4430 (idle/suspend) and OMAP5 with
> few out of tree patches.

OK, please update changelog with a brief description of how it was
tested, and on which platforms.

Thanks,

Kevin

Re: [PATCH] ARM: OMAP4: PM: Avoid expensive cpu_suspend() path for all CPU power states except off

[Santosh Shilimkar]

On Tuesday 12 February 2013 08:48 PM, Kevin Hilman wrote:
> Santosh Shilimkar <santosh.shilimkar@ti.com> writes:
>
>> On Saturday 09 February 2013 02:49 AM, Kevin Hilman wrote:
>>> Santosh Shilimkar <santosh.shilimkar@ti.com> writes:
>>>
>>>> Current CPU PM code code make use of common cpu_suspend() path for all the
>>>> CPU power states which is not optimal. In fact cpu_suspend() path is needed
>>>> only when we put CPU power domain to off state where the CPU context is lost.
>>>>
>>>> Update the code accordingly so that the expensive cpu_suspend() path
>>>> can be avoided for the shallow CPU power states like CPU PD INA/CSWR.
>>>>
>>>> Cc: Kevin Hilman <khilman@deeprootsystems.com>
>>>>
>>>> Reported-by: Richard Woodruff <r-woodruff2@ti.com>
>>>> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
>>>
>>> Looks OK at first glance, but are you sure this is right for the
>>> various ways both clusters might idle using coupled CPUidle?
>>>
>> Yes it is perfectly safe from all C-states. This patch has been part of
>> the OMAP4/OMAP5 product port for some time. I forgot to send it upstream
>> some how :(
>>
>>> Some description of the testing would be helpful here.
>>>
>> Sorry. Should have mentioned it in first place.
>> Patch is being tested on OMAP4430 (idle/suspend) and OMAP5 with
>> few out of tree patches.
>
> OK, please update changelog with a brief description of how it was
> tested, and on which platforms.
>
Will update the changelog and post it

Regards,
Santosh

[PATCH] ARM: OMAP4: PM: Avoid expensive cpu_suspend() path for all CPU power states except off

[Santosh Shilimkar]

On Tuesday 12 February 2013 08:48 PM, Kevin Hilman wrote:
> Santosh Shilimkar <santosh.shilimkar@ti.com> writes:
>
>> On Saturday 09 February 2013 02:49 AM, Kevin Hilman wrote:
>>> Santosh Shilimkar <santosh.shilimkar@ti.com> writes:
>>>
>>>> Current CPU PM code code make use of common cpu_suspend() path for all the
>>>> CPU power states which is not optimal. In fact cpu_suspend() path is needed
>>>> only when we put CPU power domain to off state where the CPU context is lost.
>>>>
>>>> Update the code accordingly so that the expensive cpu_suspend() path
>>>> can be avoided for the shallow CPU power states like CPU PD INA/CSWR.
>>>>
>>>> Cc: Kevin Hilman <khilman@deeprootsystems.com>
>>>>
>>>> Reported-by: Richard Woodruff <r-woodruff2@ti.com>
>>>> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
>>>
>>> Looks OK at first glance, but are you sure this is right for the
>>> various ways both clusters might idle using coupled CPUidle?
>>>
>> Yes it is perfectly safe from all C-states. This patch has been part of
>> the OMAP4/OMAP5 product port for some time. I forgot to send it upstream
>> some how :(
>>
>>> Some description of the testing would be helpful here.
>>>
>> Sorry. Should have mentioned it in first place.
>> Patch is being tested on OMAP4430 (idle/suspend) and OMAP5 with
>> few out of tree patches.
>
> OK, please update changelog with a brief description of how it was
> tested, and on which platforms.
>
Will update the changelog and post it

Regards,
Santosh

Re: [PATCH] ARM: OMAP2+: PM: Fix the dt return condition in pm_late_init()

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 2182 bytes --]

On Fri, Feb 08, 2013 at 08:41:44PM +0530, Santosh Shilimkar wrote:
> Commit 1416408d {ARM: OMAP2+: PM: share some suspend-related functions
> across OMAP2, 3, 4} moved suspend code to common place but now with
> that change, for DT build on OMAP4, suspend hooks are not getting
> registered which results in no suspend support.
> 
> The DT return condition is limited to PMIC and smartreflex
> initialization and hence restrict it so that suspend ops gets
> registered.
> 
> Cc: Paul Walmsley <paul@pwsan.com>
> Cc: Kevin Hilman <khilman@deeprootsystems.com>
> 
> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>

makes sense to me, we still need to set the global suspend_ops pointer
to something valid in DT boots:

Reviewed-by: Felipe Balbi <balbi@ti.com>

> ---
>  arch/arm/mach-omap2/pm.c |   20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/pm.c b/arch/arm/mach-omap2/pm.c
> index f4b3143..1ec4299 100644
> --- a/arch/arm/mach-omap2/pm.c
> +++ b/arch/arm/mach-omap2/pm.c
> @@ -345,19 +345,19 @@ int __init omap2_common_pm_late_init(void)
>  	 * a completely different mechanism.
>  	 * Disable this part if a DT blob is available.
>  	 */
> -	if (of_have_populated_dt())
> -		return 0;
> +	if (!of_have_populated_dt()) {
>  
> -	/* Init the voltage layer */
> -	omap_pmic_late_init();
> -	omap_voltage_late_init();
> +		/* Init the voltage layer */
> +		omap_pmic_late_init();
> +		omap_voltage_late_init();
>  
> -	/* Initialize the voltages */
> -	omap3_init_voltages();
> -	omap4_init_voltages();
> +		/* Initialize the voltages */
> +		omap3_init_voltages();
> +		omap4_init_voltages();
>  
> -	/* Smartreflex device init */
> -	omap_devinit_smartreflex();
> +		/* Smartreflex device init */
> +		omap_devinit_smartreflex();
> +	}
>  
>  #ifdef CONFIG_SUSPEND
>  	suspend_set_ops(&omap_pm_ops);
> -- 
> 1.7.9.5
> 
> --
> 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

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH] ARM: OMAP2+: PM: Fix the dt return condition in pm_late_init()

[Felipe Balbi]

On Fri, Feb 08, 2013 at 08:41:44PM +0530, Santosh Shilimkar wrote:
> Commit 1416408d {ARM: OMAP2+: PM: share some suspend-related functions
> across OMAP2, 3, 4} moved suspend code to common place but now with
> that change, for DT build on OMAP4, suspend hooks are not getting
> registered which results in no suspend support.
> 
> The DT return condition is limited to PMIC and smartreflex
> initialization and hence restrict it so that suspend ops gets
> registered.
> 
> Cc: Paul Walmsley <paul@pwsan.com>
> Cc: Kevin Hilman <khilman@deeprootsystems.com>
> 
> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>

makes sense to me, we still need to set the global suspend_ops pointer
to something valid in DT boots:

Reviewed-by: Felipe Balbi <balbi@ti.com>

> ---
>  arch/arm/mach-omap2/pm.c |   20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/pm.c b/arch/arm/mach-omap2/pm.c
> index f4b3143..1ec4299 100644
> --- a/arch/arm/mach-omap2/pm.c
> +++ b/arch/arm/mach-omap2/pm.c
> @@ -345,19 +345,19 @@ int __init omap2_common_pm_late_init(void)
>  	 * a completely different mechanism.
>  	 * Disable this part if a DT blob is available.
>  	 */
> -	if (of_have_populated_dt())
> -		return 0;
> +	if (!of_have_populated_dt()) {
>  
> -	/* Init the voltage layer */
> -	omap_pmic_late_init();
> -	omap_voltage_late_init();
> +		/* Init the voltage layer */
> +		omap_pmic_late_init();
> +		omap_voltage_late_init();
>  
> -	/* Initialize the voltages */
> -	omap3_init_voltages();
> -	omap4_init_voltages();
> +		/* Initialize the voltages */
> +		omap3_init_voltages();
> +		omap4_init_voltages();
>  
> -	/* Smartreflex device init */
> -	omap_devinit_smartreflex();
> +		/* Smartreflex device init */
> +		omap_devinit_smartreflex();
> +	}
>  
>  #ifdef CONFIG_SUSPEND
>  	suspend_set_ops(&omap_pm_ops);
> -- 
> 1.7.9.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130208/f9d23752/attachment.sig>

Re: [PATCH] ARM: OMAP2+: PM: Fix the dt return condition in pm_late_init()

[Paul Walmsley]

On Fri, 8 Feb 2013, Santosh Shilimkar wrote:

> Commit 1416408d {ARM: OMAP2+: PM: share some suspend-related functions
> across OMAP2, 3, 4} moved suspend code to common place but now with
> that change, for DT build on OMAP4, suspend hooks are not getting
> registered which results in no suspend support.
> 
> The DT return condition is limited to PMIC and smartreflex
> initialization and hence restrict it so that suspend ops gets
> registered.
> 
> Cc: Paul Walmsley <paul@pwsan.com>
> Cc: Kevin Hilman <khilman@deeprootsystems.com>
> 
> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>

Thanks, queued here with Felipe's Reviewed-by: along with the other OMAP4 
PM fixes.

- Paul

[PATCH] ARM: OMAP2+: PM: Fix the dt return condition in pm_late_init()

[Paul Walmsley]

On Fri, 8 Feb 2013, Santosh Shilimkar wrote:

> Commit 1416408d {ARM: OMAP2+: PM: share some suspend-related functions
> across OMAP2, 3, 4} moved suspend code to common place but now with
> that change, for DT build on OMAP4, suspend hooks are not getting
> registered which results in no suspend support.
> 
> The DT return condition is limited to PMIC and smartreflex
> initialization and hence restrict it so that suspend ops gets
> registered.
> 
> Cc: Paul Walmsley <paul@pwsan.com>
> Cc: Kevin Hilman <khilman@deeprootsystems.com>
> 
> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>

Thanks, queued here with Felipe's Reviewed-by: along with the other OMAP4 
PM fixes.

- Paul

Re: [PATCH] ARM: OMAP2+: PM: Fix the dt return condition in pm_late_init()

[Santosh Shilimkar]

On Friday 08 February 2013 09:04 PM, Paul Walmsley wrote:
> On Fri, 8 Feb 2013, Santosh Shilimkar wrote:
>
>> Commit 1416408d {ARM: OMAP2+: PM: share some suspend-related functions
>> across OMAP2, 3, 4} moved suspend code to common place but now with
>> that change, for DT build on OMAP4, suspend hooks are not getting
>> registered which results in no suspend support.
>>
>> The DT return condition is limited to PMIC and smartreflex
>> initialization and hence restrict it so that suspend ops gets
>> registered.
>>
>> Cc: Paul Walmsley <paul@pwsan.com>
>> Cc: Kevin Hilman <khilman@deeprootsystems.com>
>>
>> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
>
> Thanks, queued here with Felipe's Reviewed-by: along with the other OMAP4
> PM fixes.
>
Thanks Paul !!

Regards,
Santosh

[PATCH] ARM: OMAP2+: PM: Fix the dt return condition in pm_late_init()

[Santosh Shilimkar]

On Friday 08 February 2013 09:04 PM, Paul Walmsley wrote:
> On Fri, 8 Feb 2013, Santosh Shilimkar wrote:
>
>> Commit 1416408d {ARM: OMAP2+: PM: share some suspend-related functions
>> across OMAP2, 3, 4} moved suspend code to common place but now with
>> that change, for DT build on OMAP4, suspend hooks are not getting
>> registered which results in no suspend support.
>>
>> The DT return condition is limited to PMIC and smartreflex
>> initialization and hence restrict it so that suspend ops gets
>> registered.
>>
>> Cc: Paul Walmsley <paul@pwsan.com>
>> Cc: Kevin Hilman <khilman@deeprootsystems.com>
>>
>> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
>
> Thanks, queued here with Felipe's Reviewed-by: along with the other OMAP4
> PM fixes.
>
Thanks Paul !!

Regards,
Santosh