[10/12] ARM: shmobile: r8a7740: Add CPUIdle

Message ID	1369645193-3595-11-git-send-email-horms+renesas@verge.net.au (mailing list archive)
State	Accepted
Commit	eb76bf238ad1e9208e35dfbc79c6dc02a829932e
Headers	show Return-Path: <linux-sh-owner@vger.kernel.org> From: Simon Horman <horms+renesas@verge.net.au> To: Arnd Bergmann <arnd@arndb.de>, Olof Johansson <olof@lixom.net> Cc: linux-sh@vger.kernel.org, arm@kernel.org, linux-arm-kernel@lists.infradead.org, Magnus Damm <magnus.damm@gmail.com>, Bastian Hecht <hechtb@gmail.com>, Bastian Hecht <hechtb+renesas@gmail.com> Subject: [PATCH 10/12] ARM: shmobile: r8a7740: Add CPUIdle Date: Mon, 27 May 2013 17:59:51 +0900 Message-Id: <1369645193-3595-11-git-send-email-horms+renesas@verge.net.au> In-Reply-To: <1369645193-3595-1-git-send-email-horms+renesas@verge.net.au> References: <1369645193-3595-1-git-send-email-horms+renesas@verge.net.au> Sender: linux-sh-owner@vger.kernel.org Precedence: bulk

Message ID

1369645193-3595-11-git-send-email-horms+renesas@verge.net.au (mailing list archive)

State

Accepted

Commit

eb76bf238ad1e9208e35dfbc79c6dc02a829932e

Headers

From: Simon Horman <horms+renesas@verge.net.au>
To: Arnd Bergmann <arnd@arndb.de>, Olof Johansson <olof@lixom.net>
Cc: linux-sh@vger.kernel.org, arm@kernel.org,
	linux-arm-kernel@lists.infradead.org,
	Magnus Damm <magnus.damm@gmail.com>, Bastian Hecht <hechtb@gmail.com>,
	Bastian Hecht <hechtb+renesas@gmail.com>
Subject: [PATCH 10/12] ARM: shmobile: r8a7740: Add CPUIdle
Date: Mon, 27 May 2013 17:59:51 +0900
Message-Id: <1369645193-3595-11-git-send-email-horms+renesas@verge.net.au>
In-Reply-To: <1369645193-3595-1-git-send-email-horms+renesas@verge.net.au>
References: <1369645193-3595-1-git-send-email-horms+renesas@verge.net.au>
Sender: linux-sh-owner@vger.kernel.org
Precedence: bulk

Commit Message

Simon Horman May 27, 2013, 8:59 a.m. UTC

From: Bastian Hecht <hechtb@gmail.com>

We make use of the r8a7740 Suspend To Ram code to plug together a
CPUIdle driver.

Signed-off-by: Bastian Hecht <hechtb+renesas@gmail.com>
Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 arch/arm/mach-shmobile/Makefile               |    2 +-
 arch/arm/mach-shmobile/cpuidle-r8a7740.c      |   62 +++++++++++++++++++++++++
 arch/arm/mach-shmobile/include/mach/r8a7740.h |    1 +
 arch/arm/mach-shmobile/pm-r8a7740.c           |    1 +
 4 files changed, 65 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm/mach-shmobile/cpuidle-r8a7740.c

Comments

Daniel Lezcano May 27, 2013, 11:03 a.m. UTC | #1

On 05/27/2013 10:59 AM, Simon Horman wrote:
> From: Bastian Hecht <hechtb@gmail.com>
> 
> We make use of the r8a7740 Suspend To Ram code to plug together a
> CPUIdle driver.
> 
> Signed-off-by: Bastian Hecht <hechtb+renesas@gmail.com>
> Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> ---

Shouldn't it go through Rafael's tree ? Or does the patch contains some
dependencies on a code only visible in the ARM tree ?

>  arch/arm/mach-shmobile/Makefile               |    2 +-
>  arch/arm/mach-shmobile/cpuidle-r8a7740.c      |   62 +++++++++++++++++++++++++
>  arch/arm/mach-shmobile/include/mach/r8a7740.h |    1 +
>  arch/arm/mach-shmobile/pm-r8a7740.c           |    1 +
>  4 files changed, 65 insertions(+), 1 deletion(-)
>  create mode 100644 arch/arm/mach-shmobile/cpuidle-r8a7740.c
> 
> diff --git a/arch/arm/mach-shmobile/Makefile b/arch/arm/mach-shmobile/Makefile
> index 0568894..2852dcf 100644
> --- a/arch/arm/mach-shmobile/Makefile
> +++ b/arch/arm/mach-shmobile/Makefile
> @@ -30,7 +30,7 @@ obj-$(CONFIG_SUSPEND)		+= suspend.o
>  obj-$(CONFIG_CPU_IDLE)		+= cpuidle.o
>  obj-$(CONFIG_ARCH_SHMOBILE)	+= pm-rmobile.o
>  obj-$(CONFIG_ARCH_SH7372)	+= pm-sh7372.o sleep-sh7372.o
> -obj-$(CONFIG_ARCH_R8A7740)	+= pm-r8a7740.o sleep-r8a7740.o
> +obj-$(CONFIG_ARCH_R8A7740)	+= pm-r8a7740.o sleep-r8a7740.o cpuidle-r8a7740.o
>  obj-$(CONFIG_ARCH_R8A7779)	+= pm-r8a7779.o
>  obj-$(CONFIG_ARCH_SH73A0)	+= pm-sh73a0.o
>  
> diff --git a/arch/arm/mach-shmobile/cpuidle-r8a7740.c b/arch/arm/mach-shmobile/cpuidle-r8a7740.c
> new file mode 100644
> index 0000000..48c7a6c
> --- /dev/null
> +++ b/arch/arm/mach-shmobile/cpuidle-r8a7740.c
> @@ -0,0 +1,62 @@
> +/*
> + * CPUIdle code for SoC r8a7740
> + *
> + * Copyright (C) 2013 Bastian Hecht
> + *
> + * This file is subject to the terms and conditions of the GNU General Public
> + * License.  See the file "COPYING" in the main directory of this archive
> + * for more details.
> + */
> +#include <linux/cpuidle.h>
> +#include <linux/module.h>
> +#include <asm/cpuidle.h>
> +#include <mach/common.h>
> +#include <mach/r8a7740.h>
> +
> +#if defined(CONFIG_SUSPEND) && defined(CONFIG_CPU_IDLE)
> +static int r8a7740_enter_a3sm_pll_on(struct cpuidle_device *dev,
> +					struct cpuidle_driver *drv, int index)
> +{
> +	r8a7740_enter_a3sm_common(1);
> +	return 1;
> +}
> +
> +static int r8a7740_enter_a3sm_pll_off(struct cpuidle_device *dev,
> +					struct cpuidle_driver *drv, int index)
> +{
> +	r8a7740_enter_a3sm_common(0);
> +	return 2;
> +}
> +
> +static struct cpuidle_driver r8a7740_cpuidle_driver = {
> +	.name			= "r8a7740_cpuidle",
> +	.owner			= THIS_MODULE,
> +	.en_core_tk_irqen	= 1,
> +	.state_count		= 3,
> +	.safe_state_index	= 0, /* C1 */
> +	.states[0] = ARM_CPUIDLE_WFI_STATE,
> +	.states[1] = {
> +		.name = "C2",
> +		.desc = "A3SM PLL ON",
> +		.exit_latency = 40,
> +		.target_residency = 30 + 40,
> +		.flags = CPUIDLE_FLAG_TIME_VALID,
> +		.enter = r8a7740_enter_a3sm_pll_on,
> +	},
> +	.states[2] = {
> +		.name = "C3",
> +		.desc = "A3SM PLL OFF",
> +		.exit_latency = 120,
> +		.target_residency = 30 + 120,
> +		.flags = CPUIDLE_FLAG_TIME_VALID,
> +		.enter = r8a7740_enter_a3sm_pll_off,
> +	},
> +};
> +
> +void r8a7740_cpuidle_init(void)
> +{
> +	shmobile_cpuidle_set_driver(&r8a7740_cpuidle_driver);
> +}
> +#else
> +void r8a7740_cpuidle_init(void) {}
> +#endif
> diff --git a/arch/arm/mach-shmobile/include/mach/r8a7740.h b/arch/arm/mach-shmobile/include/mach/r8a7740.h
> index 5cfe5d9..3b538e3 100644
> --- a/arch/arm/mach-shmobile/include/mach/r8a7740.h
> +++ b/arch/arm/mach-shmobile/include/mach/r8a7740.h
> @@ -545,6 +545,7 @@ extern void r8a7740_pinmux_init(void);
>  extern void r8a7740_pm_init(void);
>  extern void r8a7740_resume(void);
>  extern void r8a7740_shutdown(void);
> +extern void r8a7740_cpuidle_init(void);
>  extern void r8a7740_enter_a3sm_common(int);
>  
>  #ifdef CONFIG_PM
> diff --git a/arch/arm/mach-shmobile/pm-r8a7740.c b/arch/arm/mach-shmobile/pm-r8a7740.c
> index fe3c867..2f196bd 100644
> --- a/arch/arm/mach-shmobile/pm-r8a7740.c
> +++ b/arch/arm/mach-shmobile/pm-r8a7740.c
> @@ -237,4 +237,5 @@ static void r8a7740_suspend_init(void) {}
>  void __init r8a7740_pm_init(void)
>  {
>  	r8a7740_suspend_init();
> +	r8a7740_cpuidle_init();
>  }
>

Olof Johansson May 28, 2013, 3:54 a.m. UTC | #2

On Mon, May 27, 2013 at 01:03:31PM +0200, Daniel Lezcano wrote:
> On 05/27/2013 10:59 AM, Simon Horman wrote:
> > From: Bastian Hecht <hechtb@gmail.com>
> > 
> > We make use of the r8a7740 Suspend To Ram code to plug together a
> > CPUIdle driver.
> > 
> > Signed-off-by: Bastian Hecht <hechtb+renesas@gmail.com>
> > Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> > ---
> 
> Shouldn't it go through Rafael's tree ? Or does the patch contains some
> dependencies on a code only visible in the ARM tree ?

Missing S-o-b from Simon. But this patch clearly builds on the preceding
one in the series, so merging them independently might not make much
sense. Getting an ack from Rafael would be nice though.

I was going to say that it should probably go under drivers/cpuidle as
well, but that just seems silly -- there is practically no code to share
with any other platform in this small driver, AND there's not really
any subsystem-internal data exposed. So it might just make more sense
to keep it under arch/arm instead.

Likewise, looking at the kirkwood and calxeda drivers under drivers/cpuidle,
I'm wondering why we thought it was a good idea to merge them there, besides
getting caught up in the "nothing can live under arch/arm any more" frenzy.

-Olof
--
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

Daniel Lezcano May 28, 2013, 6:08 a.m. UTC | #3

On 05/28/2013 05:54 AM, Olof Johansson wrote:
> On Mon, May 27, 2013 at 01:03:31PM +0200, Daniel Lezcano wrote:
>> On 05/27/2013 10:59 AM, Simon Horman wrote:
>>> From: Bastian Hecht <hechtb@gmail.com>
>>>
>>> We make use of the r8a7740 Suspend To Ram code to plug together a
>>> CPUIdle driver.
>>>
>>> Signed-off-by: Bastian Hecht <hechtb+renesas@gmail.com>
>>> Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
>>> ---
>>
>> Shouldn't it go through Rafael's tree ? Or does the patch contains some
>> dependencies on a code only visible in the ARM tree ?
> 
> Missing S-o-b from Simon. But this patch clearly builds on the preceding
> one in the series, so merging them independently might not make much
> sense. Getting an ack from Rafael would be nice though.

I was not suggesting to put the driver in the drivers/cpuidle directory
but to merge the driver through Rafael's tree as we decided some weeks
ago [1]. Although having the drivers all over the place does not help to
consolidate the code, so moving them little by little to drivers/cpuidle
makes sense but this is part of another work.

> I was going to say that it should probably go under drivers/cpuidle as
> well, but that just seems silly -- there is practically no code to share
> with any other platform in this small driver, AND there's not really
> any subsystem-internal data exposed. So it might just make more sense
> to keep it under arch/arm instead.
> 
> Likewise, looking at the kirkwood and calxeda drivers under drivers/cpuidle,
> I'm wondering why we thought it was a good idea to merge them there, besides
> getting caught up in the "nothing can live under arch/arm any more" frenzy.

Having the drivers in the drivers/cpuidle directory like drivers/cpufreq
will help to keep a consistency with the code and a single entry point
for upstream and review.

Thanks.
  -- Daniel

[1] https://patchwork.kernel.org/patch/2492841/

Simon Horman June 4, 2013, 5:10 a.m. UTC | #4

On Mon, May 27, 2013 at 08:54:46PM -0700, Olof Johansson wrote:
> On Mon, May 27, 2013 at 01:03:31PM +0200, Daniel Lezcano wrote:
> > On 05/27/2013 10:59 AM, Simon Horman wrote:
> > > From: Bastian Hecht <hechtb@gmail.com>
> > > 
> > > We make use of the r8a7740 Suspend To Ram code to plug together a
> > > CPUIdle driver.
> > > 
> > > Signed-off-by: Bastian Hecht <hechtb+renesas@gmail.com>
> > > Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> > > ---
> > 
> > Shouldn't it go through Rafael's tree ? Or does the patch contains some
> > dependencies on a code only visible in the ARM tree ?
> 
> Missing S-o-b from Simon. But this patch clearly builds on the preceding
> one in the series, so merging them independently might not make much
> sense. Getting an ack from Rafael would be nice though.
> 
> I was going to say that it should probably go under drivers/cpuidle as
> well, but that just seems silly -- there is practically no code to share
> with any other platform in this small driver, AND there's not really
> any subsystem-internal data exposed. So it might just make more sense
> to keep it under arch/arm instead.
> 
> Likewise, looking at the kirkwood and calxeda drivers under drivers/cpuidle,
> I'm wondering why we thought it was a good idea to merge them there, besides
> getting caught up in the "nothing can live under arch/arm any more" frenzy.

After discussion with Magnus I have decided to drop these changes.
We can revisit them for a future release.
--
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

diff --git a/arch/arm/mach-shmobile/Makefile b/arch/arm/mach-shmobile/Makefile
index 0568894..2852dcf 100644
--- a/arch/arm/mach-shmobile/Makefile
+++ b/arch/arm/mach-shmobile/Makefile
@@ -30,7 +30,7 @@  obj-$(CONFIG_SUSPEND)		+= suspend.o
 obj-$(CONFIG_CPU_IDLE)		+= cpuidle.o
 obj-$(CONFIG_ARCH_SHMOBILE)	+= pm-rmobile.o
 obj-$(CONFIG_ARCH_SH7372)	+= pm-sh7372.o sleep-sh7372.o
-obj-$(CONFIG_ARCH_R8A7740)	+= pm-r8a7740.o sleep-r8a7740.o
+obj-$(CONFIG_ARCH_R8A7740)	+= pm-r8a7740.o sleep-r8a7740.o cpuidle-r8a7740.o
 obj-$(CONFIG_ARCH_R8A7779)	+= pm-r8a7779.o
 obj-$(CONFIG_ARCH_SH73A0)	+= pm-sh73a0.o
 
diff --git a/arch/arm/mach-shmobile/cpuidle-r8a7740.c b/arch/arm/mach-shmobile/cpuidle-r8a7740.c
new file mode 100644
index 0000000..48c7a6c
--- /dev/null
+++ b/arch/arm/mach-shmobile/cpuidle-r8a7740.c
@@ -0,0 +1,62 @@ 
+/*
+ * CPUIdle code for SoC r8a7740
+ *
+ * Copyright (C) 2013 Bastian Hecht
+ *
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License.  See the file "COPYING" in the main directory of this archive
+ * for more details.
+ */
+#include <linux/cpuidle.h>
+#include <linux/module.h>
+#include <asm/cpuidle.h>
+#include <mach/common.h>
+#include <mach/r8a7740.h>
+
+#if defined(CONFIG_SUSPEND) && defined(CONFIG_CPU_IDLE)
+static int r8a7740_enter_a3sm_pll_on(struct cpuidle_device *dev,
+					struct cpuidle_driver *drv, int index)
+{
+	r8a7740_enter_a3sm_common(1);
+	return 1;
+}
+
+static int r8a7740_enter_a3sm_pll_off(struct cpuidle_device *dev,
+					struct cpuidle_driver *drv, int index)
+{
+	r8a7740_enter_a3sm_common(0);
+	return 2;
+}
+
+static struct cpuidle_driver r8a7740_cpuidle_driver = {
+	.name			= "r8a7740_cpuidle",
+	.owner			= THIS_MODULE,
+	.en_core_tk_irqen	= 1,
+	.state_count		= 3,
+	.safe_state_index	= 0, /* C1 */
+	.states[0] = ARM_CPUIDLE_WFI_STATE,
+	.states[1] = {
+		.name = "C2",
+		.desc = "A3SM PLL ON",
+		.exit_latency = 40,
+		.target_residency = 30 + 40,
+		.flags = CPUIDLE_FLAG_TIME_VALID,
+		.enter = r8a7740_enter_a3sm_pll_on,
+	},
+	.states[2] = {
+		.name = "C3",
+		.desc = "A3SM PLL OFF",
+		.exit_latency = 120,
+		.target_residency = 30 + 120,
+		.flags = CPUIDLE_FLAG_TIME_VALID,
+		.enter = r8a7740_enter_a3sm_pll_off,
+	},
+};
+
+void r8a7740_cpuidle_init(void)
+{
+	shmobile_cpuidle_set_driver(&r8a7740_cpuidle_driver);
+}
+#else
+void r8a7740_cpuidle_init(void) {}
+#endif
diff --git a/arch/arm/mach-shmobile/include/mach/r8a7740.h b/arch/arm/mach-shmobile/include/mach/r8a7740.h
index 5cfe5d9..3b538e3 100644
--- a/arch/arm/mach-shmobile/include/mach/r8a7740.h
+++ b/arch/arm/mach-shmobile/include/mach/r8a7740.h
@@ -545,6 +545,7 @@  extern void r8a7740_pinmux_init(void);
 extern void r8a7740_pm_init(void);
 extern void r8a7740_resume(void);
 extern void r8a7740_shutdown(void);
+extern void r8a7740_cpuidle_init(void);
 extern void r8a7740_enter_a3sm_common(int);
 
 #ifdef CONFIG_PM
diff --git a/arch/arm/mach-shmobile/pm-r8a7740.c b/arch/arm/mach-shmobile/pm-r8a7740.c
index fe3c867..2f196bd 100644
--- a/arch/arm/mach-shmobile/pm-r8a7740.c
+++ b/arch/arm/mach-shmobile/pm-r8a7740.c
@@ -237,4 +237,5 @@  static void r8a7740_suspend_init(void) {}
 void __init r8a7740_pm_init(void)
 {
 	r8a7740_suspend_init();
+	r8a7740_cpuidle_init();
 }

[10/12] ARM: shmobile: r8a7740: Add CPUIdle

Commit Message

Comments

Patch