Discussing the Linux ARM kernel

Robert Lee | 6 Dec 2011 05:38

[RFC PATCH 0/8] Add common ARM cpuidle init code

The patchset adds some cpuidle initialization functionality commonly
duplicated by many ARM platforms.  The duplicate cpuidle init code of the
various ARM platforms has been modified to use this common code.
The omap cpuidle initialization impelmentation doesn't quite fit into
this common framework so it was left alone.

All the modified ARM platforms were built against the new code and
tests were ran on a i.MX51 Babbage board using the new mx5 cpuidle
implementation included in this patchset.

Questions: 

Is arch/arm/common/ the correct location for this code?

Should the mx5 implementation be moved to a separate patchset?

Robert Lee (8):
  ARM: Add commonly used cpuidle init code
  ARM: at91: Replace init with new common ARM cpuidle init code
  ARM: kirkwood: Replace init with new common ARM cpuidle init code
  ARM: exynos: Replace init with new common ARM cpuidle init code
  ARM: davinci: Replace init with new common ARM cpuidle init code
  ARM: shmobile: Replace init with new common ARM cpuidle init code
  ARM: imx: Add mx5 clock changes necessary for low power
  ARM: imx: Add mx5 cpuidle implmentation

 arch/arm/common/Makefile            |    1 +
 arch/arm/common/cpuidle.c           |  132 +++++++++++++++++++++++++++++++++++
 arch/arm/include/asm/cpuidle.h      |   25 +++++++
 arch/arm/mach-at91/cpuidle.c        |   68 ++++++------------

(Continue reading)

Robert Lee | 6 Dec 2011 05:38

[RFC PATCH 1/8] ARM: Add commonly used cpuidle init code

Add commonly used cpuidle init code to avoid unecessary duplication.

Signed-off-by: Robert Lee <rob.lee <at> linaro.org>
---
 arch/arm/common/Makefile       |    1 +
 arch/arm/common/cpuidle.c      |  132 ++++++++++++++++++++++++++++++++++++++++
 arch/arm/include/asm/cpuidle.h |   25 ++++++++
 3 files changed, 158 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/common/cpuidle.c
 create mode 100644 arch/arm/include/asm/cpuidle.h

diff --git a/arch/arm/common/Makefile b/arch/arm/common/Makefile
index 6ea9b6f..0865f69 100644
--- a/arch/arm/common/Makefile
+++ b/arch/arm/common/Makefile
 <at>  <at>  -17,3 +17,4  <at>  <at>  obj-$(CONFIG_ARCH_IXP2000)	+= uengine.o
 obj-$(CONFIG_ARCH_IXP23XX)	+= uengine.o
 obj-$(CONFIG_PCI_HOST_ITE8152)  += it8152.o
 obj-$(CONFIG_ARM_TIMER_SP804)	+= timer-sp.o
+obj-$(CONFIG_CPU_IDLE)		+= cpuidle.o
diff --git a/arch/arm/common/cpuidle.c b/arch/arm/common/cpuidle.c
new file mode 100644
index 0000000..e9a46a3
--- /dev/null
+++ b/arch/arm/common/cpuidle.c
 <at>  <at>  -0,0 +1,132  <at>  <at> 
+/*
+ * Copyright 2011 Freescale Semiconductor, Inc.
+ * Copyright 2011 Linaro Ltd.
+ *

(Continue reading)

Mark Brown | 6 Dec 2011 12:47

Re: [RFC PATCH 1/8] ARM: Add commonly used cpuidle init code

On Mon, Dec 05, 2011 at 10:38:04PM -0600, Robert Lee wrote:

> +static int arm_enter_idle(struct cpuidle_device *dev,
> +			       struct cpuidle_driver *drv, int index)
> +{
> +	ktime_t time_start, time_end;
> +
> +	local_irq_disable();
> +	local_fiq_disable();

Apart from the IRQ disables this code isn't really ARM specific at all
and I bet it'd be useful on a lot of other architectures.

> +	time_start = ktime_get();

> +	index = mach_cpuidle(dev, drv, index);

Given the number of systems that at least start off with just a call to
cpu_do_idle() here shouldn't we have a defualt mach_cpu_idle() which
does that?  Currently the code requires the caller to provide one.

Please CC me on any iterations, I've got a S3C64xx implementation I'm
pushing which could probably use this.

Rob Lee | 8 Dec 2011 18:34

Re: [RFC PATCH 1/8] ARM: Add commonly used cpuidle init code

On 6 December 2011 05:47, Mark Brown
<broonie <at> opensource.wolfsonmicro.com> wrote:
> On Mon, Dec 05, 2011 at 10:38:04PM -0600, Robert Lee wrote:
>
>> +static int arm_enter_idle(struct cpuidle_device *dev,
>> +                            struct cpuidle_driver *drv, int index)
>> +{
>> +     ktime_t time_start, time_end;
>> +
>> +     local_irq_disable();
>> +     local_fiq_disable();
>
> Apart from the IRQ disables this code isn't really ARM specific at all
> and I bet it'd be useful on a lot of other architectures.
>

I agree and had considered this as well.  Can you (or anyone else)
suggest the best/most community friendly method of doing this?  If
this code is moved to drivers/idle or drivers/cpuidle, then perhaps
just making empty fiq enable/disable functions would be ok.

>> +     time_start = ktime_get();
>
>> +     index = mach_cpuidle(dev, drv, index);
>
> Given the number of systems that at least start off with just a call to
> cpu_do_idle() here shouldn't we have a defualt mach_cpu_idle() which
> does that?  Currently the code requires the caller to provide one.
>

(Continue reading)

Mark Brown | 9 Dec 2011 09:25

Re: [RFC PATCH 1/8] ARM: Add commonly used cpuidle init code

On Thu, Dec 08, 2011 at 11:34:34AM -0600, Rob Lee wrote:

> I agree and had considered this as well.  Can you (or anyone else)
> suggest the best/most community friendly method of doing this?  If
> this code is moved to drivers/idle or drivers/cpuidle, then perhaps
> just making empty fiq enable/disable functions would be ok.

Sounds like a reasonable plan, or providing an arch hook mechanism as
well as a SoC hook mechanism and allowing them to use that.

Rob Herring | 6 Dec 2011 16:06

Picon

Re: [RFC PATCH 1/8] ARM: Add commonly used cpuidle init code

On 12/05/2011 10:38 PM, Robert Lee wrote:
> Add commonly used cpuidle init code to avoid unecessary duplication.
> 
> Signed-off-by: Robert Lee <rob.lee <at> linaro.org>
> ---
>  arch/arm/common/Makefile       |    1 +
>  arch/arm/common/cpuidle.c      |  132 ++++++++++++++++++++++++++++++++++++++++
>  arch/arm/include/asm/cpuidle.h |   25 ++++++++

Why not move cpuidle drivers to drivers/idle/ ?

>  3 files changed, 158 insertions(+), 0 deletions(-)
>  create mode 100644 arch/arm/common/cpuidle.c
>  create mode 100644 arch/arm/include/asm/cpuidle.h
> 
> diff --git a/arch/arm/common/Makefile b/arch/arm/common/Makefile
> index 6ea9b6f..0865f69 100644
> --- a/arch/arm/common/Makefile
> +++ b/arch/arm/common/Makefile
>  <at>  <at>  -17,3 +17,4  <at>  <at>  obj-$(CONFIG_ARCH_IXP2000)	+= uengine.o
>  obj-$(CONFIG_ARCH_IXP23XX)	+= uengine.o
>  obj-$(CONFIG_PCI_HOST_ITE8152)  += it8152.o
>  obj-$(CONFIG_ARM_TIMER_SP804)	+= timer-sp.o
> +obj-$(CONFIG_CPU_IDLE)		+= cpuidle.o
> diff --git a/arch/arm/common/cpuidle.c b/arch/arm/common/cpuidle.c
> new file mode 100644
> index 0000000..e9a46a3
> --- /dev/null
> +++ b/arch/arm/common/cpuidle.c
>  <at>  <at>  -0,0 +1,132  <at>  <at>

(Continue reading)

Rob Lee | 8 Dec 2011 22:46

Re: [RFC PATCH 1/8] ARM: Add commonly used cpuidle init code

Rob, thanks for your thorough review.  Comments and questions below.

On 6 December 2011 09:06, Rob Herring <robherring2 <at> gmail.com> wrote:
> On 12/05/2011 10:38 PM, Robert Lee wrote:
>> Add commonly used cpuidle init code to avoid unecessary duplication.
>>
>> Signed-off-by: Robert Lee <rob.lee <at> linaro.org>
>> ---
>>  arch/arm/common/Makefile       |    1 +
>>  arch/arm/common/cpuidle.c      |  132 ++++++++++++++++++++++++++++++++++++++++
>>  arch/arm/include/asm/cpuidle.h |   25 ++++++++
>
> Why not move cpuidle drivers to drivers/idle/ ?
>

Or to drivers/cpuidle?  I am not sure the reasoning behind a
drivers/idle directory if everything there is a cpuidle driver
implementation.  I originally did locate this common cpuidle code in
drivers/idle/arm_idle.c and put the head file in arch/arm/include/asm
but this threw a checkpatch warning so I submitted with this placement
to start with.  If the local_fiq_enable/disable calls can be handled
in a community friendly way for any architecture, then perhaps I can
just move the header file code to linux/include/cpuidle.h.

Do you have suggestions about making this functionality available for
any architecture and what is the most community friendly method of
doing this?  I suppose function declarations for
local_fiq_enable/disable could be given.  Then, one could either
define them here as empty functions or could have two idle functions,
arm_enter_idle and nonarm_enter_idle and the architecture could be

(Continue reading)

Rob Herring | 9 Dec 2011 14:54

Picon

Re: [RFC PATCH 1/8] ARM: Add commonly used cpuidle init code

On 12/08/2011 03:46 PM, Rob Lee wrote:
> Rob, thanks for your thorough review.  Comments and questions below.
> 
> On 6 December 2011 09:06, Rob Herring <robherring2 <at> gmail.com> wrote:
>> On 12/05/2011 10:38 PM, Robert Lee wrote:
>>> Add commonly used cpuidle init code to avoid unecessary duplication.
>>>
>>> Signed-off-by: Robert Lee <rob.lee <at> linaro.org>
>>> ---
>>>  arch/arm/common/Makefile       |    1 +
>>>  arch/arm/common/cpuidle.c      |  132 ++++++++++++++++++++++++++++++++++++++++
>>>  arch/arm/include/asm/cpuidle.h |   25 ++++++++
>>
>> Why not move cpuidle drivers to drivers/idle/ ?
>>
> 
> Or to drivers/cpuidle?  I am not sure the reasoning behind a

It would make sense to me that they should be combined. I'm not sure of
the history here.

> drivers/idle directory if everything there is a cpuidle driver
> implementation.  I originally did locate this common cpuidle code in
> drivers/idle/arm_idle.c and put the head file in arch/arm/include/asm
> but this threw a checkpatch warning so I submitted with this placement

What warning?

> to start with.  If the local_fiq_enable/disable calls can be handled
> in a community friendly way for any architecture, then perhaps I can

(Continue reading)

Rob Lee | 9 Dec 2011 16:55

Re: [RFC PATCH 1/8] ARM: Add commonly used cpuidle init code

On 9 December 2011 07:54, Rob Herring <robherring2 <at> gmail.com> wrote:
> On 12/08/2011 03:46 PM, Rob Lee wrote:
>> Rob, thanks for your thorough review.  Comments and questions below.
>>
>> On 6 December 2011 09:06, Rob Herring <robherring2 <at> gmail.com> wrote:
>>> On 12/05/2011 10:38 PM, Robert Lee wrote:
>>>> Add commonly used cpuidle init code to avoid unecessary duplication.
>>>>
>>>> Signed-off-by: Robert Lee <rob.lee <at> linaro.org>
>>>> ---
>>>>  arch/arm/common/Makefile       |    1 +
>>>>  arch/arm/common/cpuidle.c      |  132 ++++++++++++++++++++++++++++++++++++++++
>>>>  arch/arm/include/asm/cpuidle.h |   25 ++++++++
>>>
>>> Why not move cpuidle drivers to drivers/idle/ ?
>>>
>>
>> Or to drivers/cpuidle?  I am not sure the reasoning behind a
>
> It would make sense to me that they should be combined. I'm not sure of
> the history here.
>
>> drivers/idle directory if everything there is a cpuidle driver
>> implementation.  I originally did locate this common cpuidle code in
>> drivers/idle/arm_idle.c and put the head file in arch/arm/include/asm
>> but this threw a checkpatch warning so I submitted with this placement
>
> What warning?
>

(Continue reading)

Nicolas Pitre | 9 Dec 2011 20:48

Re: [RFC PATCH 1/8] ARM: Add commonly used cpuidle init code

On Fri, 9 Dec 2011, Rob Herring wrote:

> On 12/08/2011 03:46 PM, Rob Lee wrote:
> > to start with.  If the local_fiq_enable/disable calls can be handled
> > in a community friendly way for any architecture, then perhaps I can
> > just move the header file code to linux/include/cpuidle.h.
> 
> Maybe we should think about whether we even need to disable fiq. You
> probably don't use low latency interrupt and a high latency low power
> mode together.

Agreed.  FIQs should be invisible and normally not be interfered with by 
generic kernel code.  If some CPU specific special low power mode really 
needs FIQs to be masked out, then that should be done by the code 
dealing with that mode.  And that is valid only if you do make use of 
FIQs in the first place.

Nicolas

Robert Lee | 6 Dec 2011 05:38

[RFC PATCH 3/8] ARM: kirkwood: Replace init with new common ARM cpuidle init code

Replace duplicated cpuidle init functionality with common ARM cpuidle
init implementation.

Signed-off-by: Robert Lee <rob.lee <at> linaro.org>
---
 arch/arm/mach-kirkwood/cpuidle.c |   63 ++++++++++++-------------------------
 1 files changed, 21 insertions(+), 42 deletions(-)

diff --git a/arch/arm/mach-kirkwood/cpuidle.c b/arch/arm/mach-kirkwood/cpuidle.c
index 7088180..6913ffd 100644
--- a/arch/arm/mach-kirkwood/cpuidle.c
+++ b/arch/arm/mach-kirkwood/cpuidle.c
 <at>  <at>  -20,6 +20,7  <at>  <at> 
 #include <linux/io.h>
 #include <linux/export.h>
 #include <asm/proc-fns.h>
+#include <asm/cpuidle.h>
 #include <mach/kirkwood.h>

 #define KIRKWOOD_MAX_STATES	2
 <at>  <at>  -27,20 +28,28  <at>  <at> 
 static struct cpuidle_driver kirkwood_idle_driver = {
 	.name =         "kirkwood_idle",
 	.owner =        THIS_MODULE,
+	.states[0] = {
+		.exit_latency		= 1,
+		.target_residency	= 100000,
+		.flags			= CPUIDLE_FLAG_TIME_VALID,
+		.name			= "WFI",
+		.desc			= "Wait for interrupt",

(Continue reading)

Robert Lee | 6 Dec 2011 05:38

[RFC PATCH 2/8] ARM: at91: Replace init with new common ARM cpuidle init code

Replace duplicated cpuidle init functionality with common ARM cpuidle
init implementation.

Signed-off-by: Robert Lee <rob.lee <at> linaro.org>
---
 arch/arm/mach-at91/cpuidle.c |   68 ++++++++++++++---------------------------
 1 files changed, 23 insertions(+), 45 deletions(-)

diff --git a/arch/arm/mach-at91/cpuidle.c b/arch/arm/mach-at91/cpuidle.c
index a851e6c..bd0bbfc 100644
--- a/arch/arm/mach-at91/cpuidle.c
+++ b/arch/arm/mach-at91/cpuidle.c
 <at>  <at>  -1,6 +1,4  <at>  <at> 
 /*
- * based on arch/arm/mach-kirkwood/cpuidle.c
- *
  * CPU idle support for AT91 SoC
  *
  * This file is licensed under the terms of the GNU General Public
 <at>  <at>  -17,19 +15,32  <at>  <at> 
 #include <linux/init.h>
 #include <linux/platform_device.h>
 #include <linux/cpuidle.h>
-#include <asm/proc-fns.h>
 #include <linux/io.h>
 #include <linux/export.h>
-
+#include <asm/proc-fns.h>
+#include <asm/cpuidle.h>
 #include "pm.h"

(Continue reading)

Robert Lee | 6 Dec 2011 05:38

[RFC PATCH 4/8] ARM: exynos: Replace init with new common ARM cpuidle init code

Replace duplicated cpuidle init functionality with common ARM cpuidle
init implementation.

Signed-off-by: Robert Lee <rob.lee <at> linaro.org>
---
 arch/arm/mach-exynos/cpuidle.c |   66 +++++++--------------------------------
 1 files changed, 12 insertions(+), 54 deletions(-)

diff --git a/arch/arm/mach-exynos/cpuidle.c b/arch/arm/mach-exynos/cpuidle.c
index 4ebb382..a21b7da 100644
--- a/arch/arm/mach-exynos/cpuidle.c
+++ b/arch/arm/mach-exynos/cpuidle.c
 <at>  <at>  -13,80 +13,38  <at>  <at> 
 #include <linux/cpuidle.h>
 #include <linux/io.h>
 #include <linux/export.h>
-#include <linux/time.h>
-
 #include <asm/proc-fns.h>
+#include <asm/cpuidle.h>

-static int exynos4_enter_idle(struct cpuidle_device *dev,
-			struct cpuidle_driver *drv,
-			      int index);
-
-static struct cpuidle_state exynos4_cpuidle_set[] = {
-	[0] = {
-		.enter			= exynos4_enter_idle,
+static struct cpuidle_driver exynos4_idle_driver = {
+	.name		= "exynos4_idle",

(Continue reading)

Robert Lee | 6 Dec 2011 05:38

[RFC PATCH 5/8] ARM: davinci: Replace init with new common ARM cpuidle init code

Replace duplicated cpuidle init functionality with common ARM cpuidle
init implementation.

Signed-off-by: Robert Lee <rob.lee <at> linaro.org>
---
 arch/arm/mach-davinci/cpuidle.c |   73 ++++++++++++--------------------------
 1 files changed, 23 insertions(+), 50 deletions(-)

diff --git a/arch/arm/mach-davinci/cpuidle.c b/arch/arm/mach-davinci/cpuidle.c
index a30c7c5..d154c56 100644
--- a/arch/arm/mach-davinci/cpuidle.c
+++ b/arch/arm/mach-davinci/cpuidle.c
 <at>  <at>  -18,7 +18,7  <at>  <at> 
 #include <linux/io.h>
 #include <linux/export.h>
 #include <asm/proc-fns.h>
-
+#include <asm/cpuidle.h>
 #include <mach/cpuidle.h>
 #include <mach/ddr2.h>

 <at>  <at>  -36,9 +36,23  <at>  <at>  struct davinci_ops {
 static struct cpuidle_driver davinci_idle_driver = {
 	.name	= "cpuidle-davinci",
 	.owner	= THIS_MODULE,
+	.states[0] = {
+		.exit_latency		= 1,
+		.target_residency	= 100000,
+		.flags			= CPUIDLE_FLAG_TIME_VALID,
+		.name			= "WFI",

(Continue reading)

Robert Lee | 6 Dec 2011 05:38

[RFC PATCH 6/8] ARM: shmobile: Replace init with new common ARM cpuidle init code

Replace duplicated cpuidle init functionality with common ARM cpuidle
init implementation.

Signed-off-by: Robert Lee <rob.lee <at> linaro.org>
---
 arch/arm/mach-shmobile/cpuidle.c |   33 +++++++--------------------------
 1 files changed, 7 insertions(+), 26 deletions(-)

diff --git a/arch/arm/mach-shmobile/cpuidle.c b/arch/arm/mach-shmobile/cpuidle.c
index 1b23342..0314fa2 100644
--- a/arch/arm/mach-shmobile/cpuidle.c
+++ b/arch/arm/mach-shmobile/cpuidle.c
 <at>  <at>  -15,6 +15,7  <at>  <at> 
 #include <linux/err.h>
 #include <asm/system.h>
 #include <asm/io.h>
+#include <asm/cpuidle.h>

 static void shmobile_enter_wfi(void)
 {
 <at>  <at>  -29,25 +30,11  <at>  <at>  static int shmobile_cpuidle_enter(struct cpuidle_device *dev,
 				  struct cpuidle_driver *drv,
 				  int index)
 {
-	ktime_t before, after;
-
-	before = ktime_get();
-
-	local_irq_disable();
-	local_fiq_disable();

(Continue reading)

Robert Lee | 6 Dec 2011 05:38

[RFC PATCH 7/8] ARM: imx: Add mx5 clock changes necessary for low power

Add mx5 clock changes necessary to enter low power operating modes.

Signed-off-by: Robert Lee <rob.lee <at> linaro.org>
---
 arch/arm/mach-mx5/clock-mx51-mx53.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-mx5/clock-mx51-mx53.c b/arch/arm/mach-mx5/clock-mx51-mx53.c
index 4cb2769..12c8a2b 100644
--- a/arch/arm/mach-mx5/clock-mx51-mx53.c
+++ b/arch/arm/mach-mx5/clock-mx51-mx53.c
 <at>  <at>  -1533,6 +1533,7  <at>  <at>  static struct clk_lookup mx53_lookups[] = {
 	_REGISTER_CLOCK("imx53-ahci.0", "ahci", sata_clk)
 	_REGISTER_CLOCK("imx53-ahci.0", "ahci_phy", ahci_phy_clk)
 	_REGISTER_CLOCK("imx53-ahci.0", "ahci_dma", ahci_dma_clk)
+	_REGISTER_CLOCK(NULL, "gpc_dvfs", gpc_dvfs_clk)
 };

 static void clk_tree_init(void)
 <at>  <at>  -1572,6 +1573,7  <at>  <at>  int __init mx51_clocks_init(unsigned long ckil, unsigned long osc,

 	clk_enable(&cpu_clk);
 	clk_enable(&main_bus_clk);
+	clk_enable(&gpc_dvfs_clk);

 	clk_enable(&iim_clk);
 	imx_print_silicon_rev("i.MX51", mx51_revision());
 <at>  <at>  -1615,6 +1617,7  <at>  <at>  int __init mx53_clocks_init(unsigned long ckil, unsigned long osc,
 	clk_set_parent(&uart_root_clk, &pll3_sw_clk);
 	clk_enable(&cpu_clk);

(Continue reading)

Robert Lee | 6 Dec 2011 05:38

[RFC PATCH 8/8] ARM: imx: Add mx5 cpuidle implmentation

Add mx5 cpuidle implmentation based upon the common ARM cpuidle
initialization code.

Signed-off-by: Robert Lee <rob.lee <at> linaro.org>
---
 arch/arm/mach-mx5/Makefile  |    3 +-
 arch/arm/mach-mx5/cpuidle.c |   59 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 61 insertions(+), 1 deletions(-)
 create mode 100644 arch/arm/mach-mx5/cpuidle.c

diff --git a/arch/arm/mach-mx5/Makefile b/arch/arm/mach-mx5/Makefile
index 0fc6080..2c348b4 100644
--- a/arch/arm/mach-mx5/Makefile
+++ b/arch/arm/mach-mx5/Makefile
 <at>  <at>  -3,8 +3,9  <at>  <at> 
 #

 # Object file lists.
-obj-y   := cpu.o mm.o clock-mx51-mx53.o ehci.o system.o

+obj-y   := cpu.o mm.o clock-mx51-mx53.o ehci.o system.o
+obj-$(CONFIG_CPU_IDLE) += cpuidle.o
 obj-$(CONFIG_PM) += pm-imx5.o
 obj-$(CONFIG_CPU_FREQ_IMX)    += cpu_op-mx51.o
 obj-$(CONFIG_MACH_MX51_BABBAGE) += board-mx51_babbage.o
diff --git a/arch/arm/mach-mx5/cpuidle.c b/arch/arm/mach-mx5/cpuidle.c
new file mode 100644
index 0000000..3584df5
--- /dev/null
+++ b/arch/arm/mach-mx5/cpuidle.c

(Continue reading)

Shawn Guo | 8 Dec 2011 15:56

Re: [RFC PATCH 0/8] Add common ARM cpuidle init code

On Mon, Dec 05, 2011 at 10:38:03PM -0600, Robert Lee wrote:
> The patchset adds some cpuidle initialization functionality commonly
> duplicated by many ARM platforms.  The duplicate cpuidle init code of the
> various ARM platforms has been modified to use this common code.
> The omap cpuidle initialization impelmentation doesn't quite fit into
> this common framework so it was left alone.
> 
> All the modified ARM platforms were built against the new code and
> tests were ran on a i.MX51 Babbage board using the new mx5 cpuidle
> implementation included in this patchset.
> 
> Questions: 
> 
> Is arch/arm/common/ the correct location for this code?
> 
With many drivers are being moved into drivers/* from arch/arm/mach-*
and arch/arm/plat-*, I'm wondering if we have any problem with doing
the same thing on cpuidle.  If we can move these cpuidle drivers into
drivers/cpuidle (or drivers/idle, I'm not sure why two folders), that
will be the best place for such consolidation to me.

Cc-ed linux-pm and Len Brown for helping understand.

Regards,
Shawn

> Should the mx5 implementation be moved to a separate patchset?
> 
> Robert Lee (8):
>   ARM: Add commonly used cpuidle init code

(Continue reading)

Arnd Bergmann | 8 Dec 2011 16:37

Re: [RFC PATCH 0/8] Add common ARM cpuidle init code

On Thursday 08 December 2011, Shawn Guo wrote:
> > 
> With many drivers are being moved into drivers/* from arch/arm/mach-*
> and arch/arm/plat-*, I'm wondering if we have any problem with doing
> the same thing on cpuidle.  If we can move these cpuidle drivers into
> drivers/cpuidle (or drivers/idle, I'm not sure why two folders), that
> will be the best place for such consolidation to me.
> 
> Cc-ed linux-pm and Len Brown for helping understand.

I think we should definitely move them into one of the two directories.
The one thing making this a bit nasty is that many SoC implementation
need to access some global registers for this and don't have a "device"
that can be used for cpuidle with its own register ranges, but it's
still better to have the code in a single place IMHO.

	Arnd

Daniel Lezcano | 3 Jan 2012 15:54

Re: [RFC PATCH 0/8] Add common ARM cpuidle init code

On 12/08/2011 04:37 PM, Arnd Bergmann wrote:
> On Thursday 08 December 2011, Shawn Guo wrote:
>>>
>> With many drivers are being moved into drivers/* from arch/arm/mach-*
>> and arch/arm/plat-*, I'm wondering if we have any problem with doing
>> the same thing on cpuidle.  If we can move these cpuidle drivers into
>> drivers/cpuidle (or drivers/idle, I'm not sure why two folders), that
>> will be the best place for such consolidation to me.
>>
>> Cc-ed linux-pm and Len Brown for helping understand.
>
> I think we should definitely move them into one of the two directories.
> The one thing making this a bit nasty is that many SoC implementation
> need to access some global registers for this and don't have a "device"
> that can be used for cpuidle with its own register ranges, but it's
> still better to have the code in a single place IMHO.

Yes, I agree. Moreover that will facilitate to factor out the code which 
is spread all around the different architecture.

The places where there is the drivers cpuidle code are at drivers/idle, 
driver/acpi, arch/arm, arch/sh/kernel/cpu/mobile/cpuidle, etc ...

IMHO, before doing some code factoring out, we should move all these 
drivers to the cpuidle directory, no ?

drivers/idle/intel_idle.c => drivers/cpuidle/intel.c
drivers/acpi/processor_idle.c => drivers/cpuidle/acpi.c

arch/arm/mach-omap2/cpuidle34xx.c => drivers/cpuidle/omap-34xx.c

(Continue reading)

Arnd Bergmann | 3 Jan 2012 17:02

Re: [RFC PATCH 0/8] Add common ARM cpuidle init code

On Tuesday 03 January 2012, Daniel Lezcano wrote:
> drivers/idle/intel_idle.c => drivers/cpuidle/intel.c
> drivers/acpi/processor_idle.c => drivers/cpuidle/acpi.c
> 
> arch/arm/mach-omap2/cpuidle34xx.c => drivers/cpuidle/omap-34xx.c
> arch/arm/mach-omap2/cpuidle44xx.c => drivers/cpuidle/omap-44xx.c
> arch/arm/mach-shmobile/cpuidle.c  => drivers/cpuidle/arm-shmobile.c
> arch/arm/mach-davinci/cpuidle.c => drivers/cpuidle/davinci.c
> arch/arm/mach-at91/cpuidle.c => drivers/cpuidle/at91.c
> arch/arm/mach-s3c64xx/cpuidle.c => drivers/cpuidle/s3c64xx.c
> arch/arm/mach-exynos/cpuidle.c => drivers/cpuidle/exynos.c
> arch/arm/mach-kirkwood/cpuidle.c => drivers/cpuidle/kirkwood.c
> arch/arcm/mach-msm/idle.S => drivers/cpuidle/msm.S
> 
> That could be a first step and then we move the other archs which could 
> be a bit more tricky like the powerpc.
> 
> Does it make sense ?

Sounds good to me. However, if any of the drivers can be built as
loadable modules, or if there is any intention to make them so,
the file name should be globally unique and follow a common naming
scheme, e.g. idle-intel.c, idle-acpi.c, ...

	Arnd

Daniel Lezcano | 4 Jan 2012 10:17

Re: [RFC PATCH 0/8] Add common ARM cpuidle init code


On 01/03/2012 05:02 PM, Arnd Bergmann wrote:
> On Tuesday 03 January 2012, Daniel Lezcano wrote:
>> drivers/idle/intel_idle.c => drivers/cpuidle/intel.c
>> drivers/acpi/processor_idle.c => drivers/cpuidle/acpi.c
>>
>> arch/arm/mach-omap2/cpuidle34xx.c => drivers/cpuidle/omap-34xx.c
>> arch/arm/mach-omap2/cpuidle44xx.c => drivers/cpuidle/omap-44xx.c
>> arch/arm/mach-shmobile/cpuidle.c  => drivers/cpuidle/arm-shmobile.c
>> arch/arm/mach-davinci/cpuidle.c => drivers/cpuidle/davinci.c
>> arch/arm/mach-at91/cpuidle.c => drivers/cpuidle/at91.c
>> arch/arm/mach-s3c64xx/cpuidle.c => drivers/cpuidle/s3c64xx.c
>> arch/arm/mach-exynos/cpuidle.c => drivers/cpuidle/exynos.c
>> arch/arm/mach-kirkwood/cpuidle.c => drivers/cpuidle/kirkwood.c
>> arch/arcm/mach-msm/idle.S => drivers/cpuidle/msm.S
>>
>> That could be a first step and then we move the other archs which could 
>> be a bit more tricky like the powerpc.
>>
>> Does it make sense ?
> 
> Sounds good to me. However, if any of the drivers can be built as
> loadable modules, or if there is any intention to make them so,
> the file name should be globally unique and follow a common naming
> scheme, e.g. idle-intel.c, idle-acpi.c, ...

Makes sense.

Thanks
  -- Daniel

(Continue reading)

Rob Lee | 4 Jan 2012 19:05

Re: [RFC PATCH 0/8] Add common ARM cpuidle init code

Shawn and Arnd,

Sorry for my late reply.  For some reason, I didn't see this thread
until after Daniel's response.

Per an offline conversation Daniel and I had, since Daniel has
volunteered to move the other drivers and has already submitted a RFC
for at91 cpuidle, I will submit a v3 of the common cpuidle code which
will include moving the i.MX5 cpuidle code to drivers/cpuidle to have
the best chance of getting this code and an i.MX5 cpuidle driver into
3.3.  If my code get's merged, then Daniel will re-base his changes
against the common code.

Return

Return to gmane.linux.ports.arm.kernel.

Search Archive

Language

Change language

Options

Current view: Threads only / Showing only 20 lines / Not hiding cited text.
Change to All messages, whole messages, or hide cited text.

Post a message
NNTP Newsgroup
Classic Gmane web interface
XML

XML

RSS Feed
List Information

About Gmane