[PATCH 1/4] SH: cpuidle: remove pointless initialization

[PATCH 1/4] SH: cpuidle: remove pointless initialization

[Daniel Lezcano]

The driver is a global static variable automatically initialized to zero.

Removing the useless initialization in the init function.

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 arch/sh/kernel/cpu/shmobile/cpuidle.c |    6 ------
 1 file changed, 6 deletions(-)

diff --git a/arch/sh/kernel/cpu/shmobile/cpuidle.c b/arch/sh/kernel/cpu/shmobile/cpuidle.c
index 0986f21..5b44af9 100644
--- a/arch/sh/kernel/cpu/shmobile/cpuidle.c
+++ b/arch/sh/kernel/cpu/shmobile/cpuidle.c
@@ -64,12 +64,6 @@ void sh_mobile_setup_cpuidle(void)
 	struct cpuidle_state *state;
 	int i;
 
-
-	for (i = 0; i < CPUIDLE_STATE_MAX; i++) {
-		drv->states[i].name[0] = '\0';
-		drv->states[i].desc[0] = '\0';
-	}
-
 	i = CPUIDLE_DRIVER_STATE_START;
 
 	state = &drv->states[i++];
-- 
1.7.9.5

[PATCH 2/4] SH: cpuidle: remove CPUIDLE_DRIVER_STATE_START usage

[Daniel Lezcano]

From: Daniel Lezcano <daniel.lezcano@linaro.or>

The CPUIDLE_DRIVER_STATE_START constant is only set when the kernel compilation
option CONFIG_ARCH_HAS_CPU_RELAX is set, but this is only relatated to x86, so
it is always zero.

Remove the reference to this constant in the code.

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 arch/sh/kernel/cpu/shmobile/cpuidle.c |    6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/arch/sh/kernel/cpu/shmobile/cpuidle.c b/arch/sh/kernel/cpu/shmobile/cpuidle.c
index 5b44af9..36374cc 100644
--- a/arch/sh/kernel/cpu/shmobile/cpuidle.c
+++ b/arch/sh/kernel/cpu/shmobile/cpuidle.c
@@ -62,9 +62,7 @@ void sh_mobile_setup_cpuidle(void)
 	struct cpuidle_device *dev = &cpuidle_dev;
 	struct cpuidle_driver *drv = &cpuidle_driver;
 	struct cpuidle_state *state;
-	int i;
-
-	i = CPUIDLE_DRIVER_STATE_START;
+	int i = 0;
 
 	state = &drv->states[i++];
 	snprintf(state->name, CPUIDLE_NAME_LEN, "C1");
@@ -76,7 +74,7 @@ void sh_mobile_setup_cpuidle(void)
 	state->flags |= CPUIDLE_FLAG_TIME_VALID;
 	state->enter = cpuidle_sleep_enter;
 
-	drv->safe_state_index = i-1;
+	drv->safe_state_index = 0;
 
 	if (sh_mobile_sleep_supported & SUSP_SH_SF) {
 		state = &drv->states[i++];
-- 
1.7.9.5

[PATCH 3/4] SH: cpuidle: initialize the driver's states directly

[Daniel Lezcano]

Like all the other drivers, let's initialize the structure a compile time
instead of init time.

The states #1 and #2 are not enabled by default. The init function will
check the features of the board in order to enable the state.

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 arch/sh/kernel/cpu/shmobile/cpuidle.c |   90 +++++++++++++++------------------
 1 file changed, 41 insertions(+), 49 deletions(-)

diff --git a/arch/sh/kernel/cpu/shmobile/cpuidle.c b/arch/sh/kernel/cpu/shmobile/cpuidle.c
index 36374cc..f024acf 100644
--- a/arch/sh/kernel/cpu/shmobile/cpuidle.c
+++ b/arch/sh/kernel/cpu/shmobile/cpuidle.c
@@ -53,59 +53,51 @@ static int cpuidle_sleep_enter(struct cpuidle_device *dev,
 
 static struct cpuidle_device cpuidle_dev;
 static struct cpuidle_driver cpuidle_driver = {
-	.name			= "sh_idle",
-	.owner			= THIS_MODULE,
+	.name   = "sh_idle",
+	.owner  = THIS_MODULE,
+	.states = {
+		{
+			.exit_latency = 1,
+			.target_residency = 1 * 2,
+			.power_usage = 3,
+			.flags = CPUIDLE_FLAG_TIME_VALID,
+			.enter = cpuidle_sleep_enter,
+			.name = "C1",
+			.desc = "SuperH Sleep Mode",
+		},
+		{
+			.exit_latency = 100,
+			.target_residency = 1 * 2,
+			.power_usage = 1,
+			.flags = CPUIDLE_FLAG_TIME_VALID,
+			.enter = cpuidle_sleep_enter,
+			.name = "C2",
+			.desc = "SuperH Sleep Mode [SF]",
+			.disabled = true,
+		},
+		{
+			.exit_latency = 2300,
+			.target_residency = 1 * 2,
+			.power_usage = 1,
+			.flags = CPUIDLE_FLAG_TIME_VALID,
+			.enter = cpuidle_sleep_enter,
+			.name = "C3",
+			.desc = "SuperH Mobile Standby Mode [SF]",
+			.disabled = true,
+		},
+	},
+	.safe_state_index = 0,
+	.state_count = 3,
 };
 
 void sh_mobile_setup_cpuidle(void)
 {
-	struct cpuidle_device *dev = &cpuidle_dev;
-	struct cpuidle_driver *drv = &cpuidle_driver;
-	struct cpuidle_state *state;
-	int i = 0;
+	if (sh_mobile_sleep_supported & SUSP_SH_SF)
+		cpuidle_driver.states[1].disabled = false;
 
-	state = &drv->states[i++];
-	snprintf(state->name, CPUIDLE_NAME_LEN, "C1");
-	strncpy(state->desc, "SuperH Sleep Mode", CPUIDLE_DESC_LEN);
-	state->exit_latency = 1;
-	state->target_residency = 1 * 2;
-	state->power_usage = 3;
-	state->flags = 0;
-	state->flags |= CPUIDLE_FLAG_TIME_VALID;
-	state->enter = cpuidle_sleep_enter;
+	if (sh_mobile_sleep_supported & SUSP_SH_STANDBY)
+		cpuidle_driver.states[2].disabled = false;
 
-	drv->safe_state_index = 0;
-
-	if (sh_mobile_sleep_supported & SUSP_SH_SF) {
-		state = &drv->states[i++];
-		snprintf(state->name, CPUIDLE_NAME_LEN, "C2");
-		strncpy(state->desc, "SuperH Sleep Mode [SF]",
-			CPUIDLE_DESC_LEN);
-		state->exit_latency = 100;
-		state->target_residency = 1 * 2;
-		state->power_usage = 1;
-		state->flags = 0;
-		state->flags |= CPUIDLE_FLAG_TIME_VALID;
-		state->enter = cpuidle_sleep_enter;
-	}
-
-	if (sh_mobile_sleep_supported & SUSP_SH_STANDBY) {
-		state = &drv->states[i++];
-		snprintf(state->name, CPUIDLE_NAME_LEN, "C3");
-		strncpy(state->desc, "SuperH Mobile Standby Mode [SF]",
-			CPUIDLE_DESC_LEN);
-		state->exit_latency = 2300;
-		state->target_residency = 1 * 2;
-		state->power_usage = 1;
-		state->flags = 0;
-		state->flags |= CPUIDLE_FLAG_TIME_VALID;
-		state->enter = cpuidle_sleep_enter;
-	}
-
-	drv->state_count = i;
-	dev->state_count = i;
-
-	cpuidle_register_driver(&cpuidle_driver);
-
-	cpuidle_register_device(dev);
+	if (!cpuidle_register_driver(&cpuidle_driver))
+		cpuidle_register_device(&cpuidle_dev);
 }
-- 
1.7.9.5

[PATCH 4/4] SH: cpuidle: check error code at init

[Daniel Lezcano]

Registering the driver, or the device, can fail, let's check the return code
and return the error code to the PM layer.

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 arch/sh/include/asm/suspend.h         |    4 ++--
 arch/sh/kernel/cpu/shmobile/cpuidle.c |   11 ++++++++---
 arch/sh/kernel/cpu/shmobile/pm.c      |    3 +--
 3 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/arch/sh/include/asm/suspend.h b/arch/sh/include/asm/suspend.h
index e14567a..70ae0b2 100644
--- a/arch/sh/include/asm/suspend.h
+++ b/arch/sh/include/asm/suspend.h
@@ -14,9 +14,9 @@ struct swsusp_arch_regs {
 void sh_mobile_call_standby(unsigned long mode);
 
 #ifdef CONFIG_CPU_IDLE
-void sh_mobile_setup_cpuidle(void);
+int sh_mobile_setup_cpuidle(void);
 #else
-static inline void sh_mobile_setup_cpuidle(void) {}
+static inline int sh_mobile_setup_cpuidle(void) { return 0; }
 #endif
 
 /* notifier chains for pre/post sleep hooks */
diff --git a/arch/sh/kernel/cpu/shmobile/cpuidle.c b/arch/sh/kernel/cpu/shmobile/cpuidle.c
index f024acf..fdfe57f4 100644
--- a/arch/sh/kernel/cpu/shmobile/cpuidle.c
+++ b/arch/sh/kernel/cpu/shmobile/cpuidle.c
@@ -90,14 +90,19 @@ static struct cpuidle_driver cpuidle_driver = {
 	.state_count = 3,
 };
 
-void sh_mobile_setup_cpuidle(void)
+int __init sh_mobile_setup_cpuidle(void)
 {
+	int ret;
+
 	if (sh_mobile_sleep_supported & SUSP_SH_SF)
 		cpuidle_driver.states[1].disabled = false;
 
 	if (sh_mobile_sleep_supported & SUSP_SH_STANDBY)
 		cpuidle_driver.states[2].disabled = false;
 
-	if (!cpuidle_register_driver(&cpuidle_driver))
-		cpuidle_register_device(&cpuidle_dev);
+	ret = cpuidle_register_driver(&cpuidle_driver);
+	if (ret)
+		return ret;
+
+	return cpuidle_register_device(&cpuidle_dev);
 }
diff --git a/arch/sh/kernel/cpu/shmobile/pm.c b/arch/sh/kernel/cpu/shmobile/pm.c
index 08d27fa..ac37b72 100644
--- a/arch/sh/kernel/cpu/shmobile/pm.c
+++ b/arch/sh/kernel/cpu/shmobile/pm.c
@@ -150,8 +150,7 @@ static const struct platform_suspend_ops sh_pm_ops = {
 static int __init sh_pm_init(void)
 {
 	suspend_set_ops(&sh_pm_ops);
-	sh_mobile_setup_cpuidle();
-	return 0;
+	return sh_mobile_setup_cpuidle();
 }
 
 late_initcall(sh_pm_init);
-- 
1.7.9.5

Re: [PATCH 1/4] SH: cpuidle: remove pointless initialization

[Simon Horman]

On Wed, Apr 17, 2013 at 03:32:54PM +0200, Daniel Lezcano wrote:
> The driver is a global static variable automatically initialized to zero.
> 
> Removing the useless initialization in the init function.
> 
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>

Hi Daniel,

All four patches in this series seem reasonable to me.

Acked-by: Simon Horman <horms+renesas@verge.net.au>

Please let me know if I should take them through my renesas tree.

> ---
>  arch/sh/kernel/cpu/shmobile/cpuidle.c |    6 ------
>  1 file changed, 6 deletions(-)
> 
> diff --git a/arch/sh/kernel/cpu/shmobile/cpuidle.c b/arch/sh/kernel/cpu/shmobile/cpuidle.c
> index 0986f21..5b44af9 100644
> --- a/arch/sh/kernel/cpu/shmobile/cpuidle.c
> +++ b/arch/sh/kernel/cpu/shmobile/cpuidle.c
> @@ -64,12 +64,6 @@ void sh_mobile_setup_cpuidle(void)
>  	struct cpuidle_state *state;
>  	int i;
>  
> -
> -	for (i = 0; i < CPUIDLE_STATE_MAX; i++) {
> -		drv->states[i].name[0] = '\0';
> -		drv->states[i].desc[0] = '\0';
> -	}
> -
>  	i = CPUIDLE_DRIVER_STATE_START;
>  
>  	state = &drv->states[i++];
> -- 
> 1.7.9.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 1/4] SH: cpuidle: remove pointless initialization

[Daniel Lezcano]

On 04/18/2013 03:18 AM, Simon Horman wrote:
> On Wed, Apr 17, 2013 at 03:32:54PM +0200, Daniel Lezcano wrote:
>> The driver is a global static variable automatically initialized to zero.
>>
>> Removing the useless initialization in the init function.
>>
>> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> 
> Hi Daniel,
> 
> All four patches in this series seem reasonable to me.
> 
> Acked-by: Simon Horman <horms+renesas@verge.net.au>
> 
> Please let me know if I should take them through my renesas tree.

If you are ok with that, is it possible to take them through the
linux-pm tree ? So the cpuidle drivers are synced and I can continue the
code consolidation ?

Thanks for the review.

  -- Daniel

>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-sh" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>


-- 
 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH 1/4] SH: cpuidle: remove pointless initialization

[Simon Horman]

On Thu, Apr 18, 2013 at 11:18:04AM +0200, Daniel Lezcano wrote:
> On 04/18/2013 03:18 AM, Simon Horman wrote:
> > On Wed, Apr 17, 2013 at 03:32:54PM +0200, Daniel Lezcano wrote:
> >> The driver is a global static variable automatically initialized to zero.
> >>
> >> Removing the useless initialization in the init function.
> >>
> >> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> > 
> > Hi Daniel,
> > 
> > All four patches in this series seem reasonable to me.
> > 
> > Acked-by: Simon Horman <horms+renesas@verge.net.au>
> > 
> > Please let me know if I should take them through my renesas tree.
> 
> If you are ok with that, is it possible to take them through the
> linux-pm tree ? So the cpuidle drivers are synced and I can continue the
> code consolidation ?

Yes, I am ok with them going through the linux-pm tree.

Re: [PATCH 1/4] SH: cpuidle: remove pointless initialization

[Daniel Lezcano]

On 04/18/2013 03:34 PM, Simon Horman wrote:
> On Thu, Apr 18, 2013 at 11:18:04AM +0200, Daniel Lezcano wrote:
>> On 04/18/2013 03:18 AM, Simon Horman wrote:
>>> On Wed, Apr 17, 2013 at 03:32:54PM +0200, Daniel Lezcano wrote:
>>>> The driver is a global static variable automatically initialized to zero.
>>>>
>>>> Removing the useless initialization in the init function.
>>>>
>>>> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
>>>
>>> Hi Daniel,
>>>
>>> All four patches in this series seem reasonable to me.
>>>
>>> Acked-by: Simon Horman <horms+renesas@verge.net.au>
>>>
>>> Please let me know if I should take them through my renesas tree.
>>
>> If you are ok with that, is it possible to take them through the
>> linux-pm tree ? So the cpuidle drivers are synced and I can continue the
>> code consolidation ?
> 
> Yes, I am ok with them going through the linux-pm tree.

Thanks !


-- 
 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH 1/4] SH: cpuidle: remove pointless initialization

[Rafael J. Wysocki]

On Thursday, April 18, 2013 03:35:49 PM Daniel Lezcano wrote:
> On 04/18/2013 03:34 PM, Simon Horman wrote:
> > On Thu, Apr 18, 2013 at 11:18:04AM +0200, Daniel Lezcano wrote:
> >> On 04/18/2013 03:18 AM, Simon Horman wrote:
> >>> On Wed, Apr 17, 2013 at 03:32:54PM +0200, Daniel Lezcano wrote:
> >>>> The driver is a global static variable automatically initialized to zero.
> >>>>
> >>>> Removing the useless initialization in the init function.
> >>>>
> >>>> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> >>>
> >>> Hi Daniel,
> >>>
> >>> All four patches in this series seem reasonable to me.
> >>>
> >>> Acked-by: Simon Horman <horms+renesas@verge.net.au>
> >>>
> >>> Please let me know if I should take them through my renesas tree.
> >>
> >> If you are ok with that, is it possible to take them through the
> >> linux-pm tree ? So the cpuidle drivers are synced and I can continue the
> >> code consolidation ?
> > 
> > Yes, I am ok with them going through the linux-pm tree.
> 
> Thanks !

OK, applied.

Patch [3/4] had a merge conflict against the current Linus' tree, so I've fixed
it up manually.  Please check the linux-next branch for the result (will
probably conflict with the shmobile tree in linux-next, but that's not a big
deal).

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.