Skip to contentSkip to conversation

ARM: S3C64xx: cpuidle cleanups

PATCH New Daniel Lezcano 4 participants 10 messages May 9, 2012 – May 14, 2012

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

cover ARM: S3C64xx: cpuidle cleanups

These couple of patches use the new cpuidle core api to refactor some part of the code. The first one declares the states directly in the driver declaration and the second one use the timekeeping flag to let the cpuidle core to compute the idle time.

I don't have this board, I was not able to test these patches.

Daniel Lezcano (2):
ARM: s3c64xx: cpuidle - declare the states with the new api
ARM: s3c64xx: cpuidle - use timekeeping wrapper

 arch/arm/mach-s3c64xx/cpuidle.c |   45 +++++++++++++--------------------------
 1 files changed, 15 insertions(+), 30 deletions(-)
Signature · 1 line
1.7.5.4
#1 lore raw

PATCH 1/2 ARM: s3c64xx: cpuidle - declare the states with the new api

The states are now part of the cpuidle_driver structure, so we can declare the states in this structure directly. That saves us an extra variable declaration and a memcpy.

---
 arch/arm/mach-s3c64xx/cpuidle.c |   33 ++++++++++++++-------------------
 1 files changed, 14 insertions(+), 19 deletions(-)
arch/arm/mach-s3c64xx/cpuidle.c +14 -19
index 179460f..2750e54 100644
@@ -51,33 +51,28 @@ static int s3c64xx_enter_idle(struct cpuidle_device *dev,
 	return index;
 }
 
-static struct cpuidle_state s3c64xx_cpuidle_set[] = {
-	[0] = {
-		.enter			= s3c64xx_enter_idle,
-		.exit_latency		= 1,
-		.target_residency	= 1,
-		.flags			= CPUIDLE_FLAG_TIME_VALID,
-		.name			= "IDLE",
-		.desc			= "System active, ARM gated",
-	},
-};
+static DEFINE_PER_CPU(struct cpuidle_device, s3c64xx_cpuidle_device);
 
 static struct cpuidle_driver s3c64xx_cpuidle_driver = {
-	.name		= "s3c64xx_cpuidle",
-	.owner		= THIS_MODULE,
-	.state_count	= ARRAY_SIZE(s3c64xx_cpuidle_set),
-};
-
-static struct cpuidle_device s3c64xx_cpuidle_device = {
-	.state_count	= ARRAY_SIZE(s3c64xx_cpuidle_set),
+	.name	= "s3c64xx_cpuidle",
+	.owner  = THIS_MODULE,
+	.states = {
+		{
+			.enter            = s3c64xx_enter_idle,
+			.exit_latency     = 1,
+			.target_residency = 1,
+			.flags            = CPUIDLE_FLAG_TIME_VALID,
+			.name             = "IDLE",
+			.desc             = "System active, ARM gated",
+		},
+	},
+	.state_count = 1,
 };
 
 static int __init s3c64xx_init_cpuidle(void)
 {
 	int ret;
 
-	memcpy(s3c64xx_cpuidle_driver.states, s3c64xx_cpuidle_set,
-	       sizeof(s3c64xx_cpuidle_set));
 	cpuidle_register_driver(&s3c64xx_cpuidle_driver);
 
 	ret = cpuidle_register_device(&s3c64xx_cpuidle_device);
Signature · 1 line
1.7.5.4
#2 lore raw

PATCH 2/2 ARM: s3c64xx: cpuidle - use timekeeping wrapper

In reply to Daniel Lezcano

The timekeeping is computed from the cpuidle core if we set the .en_core_tk_irqen flag. Let's use it and remove the duplicated code.

---
 arch/arm/mach-s3c64xx/cpuidle.c |   12 +-----------
 1 files changed, 1 insertions(+), 11 deletions(-)
arch/arm/mach-s3c64xx/cpuidle.c +1 -11
index 2750e54..acb197c 100644
@@ -27,12 +27,7 @@ static int s3c64xx_enter_idle(struct cpuidle_device *dev,
 			      struct cpuidle_driver *drv,
 			      int index)
 {
-	struct timeval before, after;
 	unsigned long tmp;
-	int idle_time;
-
-	local_irq_disable();
-	do_gettimeofday(&before);
 
 	/* Setup PWRCFG to enter idle mode */
 	tmp = __raw_readl(S3C64XX_PWR_CFG);
@@ -42,12 +37,6 @@ static int s3c64xx_enter_idle(struct cpuidle_device *dev,
 
 	cpu_do_idle();
 
-	do_gettimeofday(&after);
-	local_irq_enable();
-	idle_time = (after.tv_sec - before.tv_sec) * USEC_PER_SEC +
-		    (after.tv_usec - before.tv_usec);
-
-	dev->last_residency = idle_time;
 	return index;
 }
 
@@ -56,6 +45,7 @@ static DEFINE_PER_CPU(struct cpuidle_device, s3c64xx_cpuidle_device);
 static struct cpuidle_driver s3c64xx_cpuidle_driver = {
 	.name	= "s3c64xx_cpuidle",
 	.owner  = THIS_MODULE,
+	.en_core_tk_irqen = 1,
 	.states = {
 		{
 			.enter            = s3c64xx_enter_idle,
Signature · 1 line
1.7.5.4
#3 lore raw

cover ARM: S3C64xx: cpuidle cleanups

In reply to Daniel Lezcano
Quoted 11 lines · Daniel Lezcano

These couple of patches use the new cpuidle core api to refactor some part of the code. The first one declares the states directly in the driver declaration and the second one use the timekeeping flag to let the cpuidle core to compute the idle time.

I don't have this board, I was not able to test these patches.

Daniel Lezcano (2):
ARM: s3c64xx: cpuidle - declare the states with the new api
ARM: s3c64xx: cpuidle - use timekeeping wrapper

Are these patches ok for inclusion ?

Thanks
-- Daniel

Signature · 5 lines
  <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
#4 lore raw

cover ARM: S3C64xx: cpuidle cleanups

Am Montag, 14. Mai 2012, 01:51:00 schrieb Daniel Lezcano:

Quoted 6 lines · Daniel Lezcano
Quoted 5 lines

These couple of patches use the new cpuidle core api to refactor some part of the code. The first one declares the states directly in the driver declaration and the second one use the timekeeping flag to let the cpuidle core to compute the idle time.

Quoted 7 lines
Quoted 5 lines

I don't have this board, I was not able to test these patches.

Daniel Lezcano (2):
ARM: s3c64xx: cpuidle - declare the states with the new api
ARM: s3c64xx: cpuidle - use timekeeping wrapper

Are these patches ok for inclusion ?

you might want to include the maintainer

Kukjin Kim <[email protected]>

and the samsung list

linux-samsung-soc at vger.kernel.org

into your recipients

Heiko

#5 lore raw

cover ARM: S3C64xx: cpuidle cleanups

Quoted 7 lines · Heiko St??bner

Am Montag, 14. Mai 2012, 01:51:00 schrieb Daniel Lezcano:

Quoted 3 lines · Daniel Lezcano

Are these patches ok for inclusion ?

you might want to include the maintainer

Quoted 7 lines

Kukjin Kim <[email protected]>

and the samsung list

linux-samsung-soc at vger.kernel.org

into your recipients

Also the original author (me).

#6 lore raw

cover ARM: S3C64xx: cpuidle cleanups

Quoted 10 lines · Mark Brown
Quoted 8 lines · Heiko St??bner

Am Montag, 14. Mai 2012, 01:51:00 schrieb Daniel Lezcano:

Quoted 3 lines · Daniel Lezcano

Are these patches ok for inclusion ?

you might want to include the maintainer

Kukjin Kim<[email protected]>

Quoted 7 lines
Quoted 5 lines

and the samsung list

linux-samsung-soc at vger.kernel.org

into your recipients

Also the original author (me).

Oh, ok. I thought I did it, sorry.

-- Daniel

Signature · 5 lines
  <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
#7 lore raw

cover ARM: S3C64xx: cpuidle cleanups

Quoted 13 lines · Daniel Lezcano
Quoted 12 lines · Mark Brown
Quoted 10 lines · Heiko St??bner

Am Montag, 14. Mai 2012, 01:51:00 schrieb Daniel Lezcano:

Quoted 3 lines · Daniel Lezcano

Are these patches ok for inclusion ?

you might want to include the maintainer

Kukjin Kim<[email protected]>

and the samsung list

Quoted 8 lines
Quoted 5 lines
Quoted 3 lines

linux-samsung-soc at vger.kernel.org

into your recipients

Also the original author (me).

Oh, ok. I thought I did it, sorry.

Heiko, thanks :-)

Daniel, could you please re-submit this series with adding me and Mark Brown in Cc. I couldn't find your patches in my mailbox...

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <[email protected]>, Senior Engineer, SW Solution Development Team, Samsung Electronics Co., Ltd.

#8 lore raw

cover ARM: S3C64xx: cpuidle cleanups

Quoted 18 lines · Kukjin Kim
Quoted 17 lines · Daniel Lezcano
Quoted 16 lines · Mark Brown
Quoted 14 lines · Heiko St??bner

Am Montag, 14. Mai 2012, 01:51:00 schrieb Daniel Lezcano:

Quoted 3 lines · Daniel Lezcano

Are these patches ok for inclusion ?

you might want to include the maintainer

Kukjin Kim<[email protected]>

and the samsung list

linux-samsung-soc at vger.kernel.org

into your recipients

Quoted 8 lines
Quoted 4 lines
Quoted 1 line

Also the original author (me).

Oh, ok. I thought I did it, sorry.

Heiko, thanks :-)

Daniel, could you please re-submit this series with adding me and Mark Brown in Cc. I couldn't find your patches in my mailbox...

Sure.

Signature · 5 lines
  <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
#9 lore raw

cover ARM: S3C64xx: cpuidle cleanups

In reply to Mark Brown
Quoted 10 lines · Mark Brown
Quoted 8 lines · Heiko St??bner

Am Montag, 14. Mai 2012, 01:51:00 schrieb Daniel Lezcano:

Quoted 2 lines · Daniel Lezcano

Are these patches ok for inclusion ?

you might want to include the maintainer
Kukjin Kim<[email protected]>
and the samsung list
linux-samsung-soc at vger.kernel.org
into your recipients

Also the original author (me).

Mark,

I noticed you are not in the cpuidle.c header file neither in the MAINTAINERS entry for the s3c64xx.
Just FYI.

Thanks
-- Daniel

Signature · 5 lines
  <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