Re: [PATCH 4/6] ARM64: cpuidle: Rename cpu_init_idle to a common function name

2015-03-20 Thread Catalin Marinas
On Tue, Mar 03, 2015 at 01:29:35PM +0100, Daniel Lezcano wrote:
> With this change the cpuidle-arm64.c file calls the same function name
> for both ARM and ARM64.
> 
> Signed-off-by: Daniel Lezcano 

Acked-by: Catalin Marinas 

That said, if you find some names to avoid the arm_ prefix it's even
better but either way my ack still stands.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 4/6] ARM64: cpuidle: Rename cpu_init_idle to a common function name

2015-03-20 Thread Daniel Lezcano

On 03/03/2015 01:29 PM, Daniel Lezcano wrote:

With this change the cpuidle-arm64.c file calls the same function name
for both ARM and ARM64.

Signed-off-by: Daniel Lezcano 


Hi Catalin,

do you mind to ack the ARM64 if you agree with these changes ?

Thanks.

  -- Daniel


---
  arch/arm64/include/asm/cpuidle.h | 4 ++--
  arch/arm64/kernel/cpuidle.c  | 2 +-
  drivers/cpuidle/cpuidle-arm64.c  | 2 +-
  3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/include/asm/cpuidle.h b/arch/arm64/include/asm/cpuidle.h
index 1bee287..2bc2732 100644
--- a/arch/arm64/include/asm/cpuidle.h
+++ b/arch/arm64/include/asm/cpuidle.h
@@ -2,10 +2,10 @@
  #define __ASM_CPUIDLE_H

  #ifdef CONFIG_CPU_IDLE
-extern int cpu_init_idle(unsigned int cpu);
+extern int arm_cpuidle_init(unsigned int cpu);
  extern int cpu_suspend(unsigned long arg);
  #else
-static inline int cpu_init_idle(unsigned int cpu)
+static inline int arm_cpuidle_init(unsigned int cpu)
  {
return -EOPNOTSUPP;
  }
diff --git a/arch/arm64/kernel/cpuidle.c b/arch/arm64/kernel/cpuidle.c
index 5c08966..a78143a 100644
--- a/arch/arm64/kernel/cpuidle.c
+++ b/arch/arm64/kernel/cpuidle.c
@@ -15,7 +15,7 @@
  #include 
  #include 

-int cpu_init_idle(unsigned int cpu)
+int arm_cpuidle_init(unsigned int cpu)
  {
int ret = -EOPNOTSUPP;
struct device_node *cpu_node = of_cpu_device_node_get(cpu);
diff --git a/drivers/cpuidle/cpuidle-arm64.c b/drivers/cpuidle/cpuidle-arm64.c
index 0cea244..6ef291c7 100644
--- a/drivers/cpuidle/cpuidle-arm64.c
+++ b/drivers/cpuidle/cpuidle-arm64.c
@@ -110,7 +110,7 @@ static int __init arm64_idle_init(void)
 * idle states suspend back-end specific data
 */
for_each_possible_cpu(cpu) {
-   ret = cpu_init_idle(cpu);
+   ret = arm_cpuidle_init(cpu);
if (ret) {
pr_err("CPU %d failed to init idle CPU ops\n", cpu);
return ret;




--
  Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 4/6] ARM64: cpuidle: Rename cpu_init_idle to a common function name

2015-03-15 Thread Lorenzo Pieralisi
On Sat, Mar 14, 2015 at 11:41:09AM +, Catalin Marinas wrote:
> On Fri, Mar 13, 2015 at 06:22:46PM +, Catalin Marinas wrote:
> > On Tue, Mar 03, 2015 at 01:29:35PM +0100, Daniel Lezcano wrote:
> > > diff --git a/drivers/cpuidle/cpuidle-arm64.c 
> > > b/drivers/cpuidle/cpuidle-arm64.c
> > > index 0cea244..6ef291c7 100644
> > > --- a/drivers/cpuidle/cpuidle-arm64.c
> > > +++ b/drivers/cpuidle/cpuidle-arm64.c
> > > @@ -110,7 +110,7 @@ static int __init arm64_idle_init(void)
> > >* idle states suspend back-end specific data
> > >*/
> > >   for_each_possible_cpu(cpu) {
> > > - ret = cpu_init_idle(cpu);
> > > + ret = arm_cpuidle_init(cpu);
> > 
> > Same nitpick here about dropping the arm_ prefix (though here we already
> > have a cpuidle_init).
> 
> Actually, a question, probably for Lorenzo - why do we need to call
> cpu_init_idle() from the driver? Is there any dependency on what the
> driver had done before this call? If not, I suggest a core_initcall() in
> the arch code for cpu_init_idle(). At a quick look through the code, the
> back-end can be initialised on its own.

Because we want to register the driver if and only if both the generic
idle states parsing AND the back-end initialization succeed. If we move
the cpu_init_idle() to a core initcall() we still need a way to probe
if the back end initialization succeeded or not, we do not want to
have a driver initialized with back-end calls that fail.

It is also so, because we must guarantee the idle index mapping between
generic driver and the back end, if either initialization fails, the
driver should not be registered.

At the moment the driver is very similar to the x86 generic driver,
which is a good thing for consistency; by the way x86 driver prefixes
the function names with "intel_", I am not too fussed about the
functions naming scheme, open to suggestions.

Lorenzo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 4/6] ARM64: cpuidle: Rename cpu_init_idle to a common function name

2015-03-14 Thread Catalin Marinas
On Fri, Mar 13, 2015 at 06:22:46PM +, Catalin Marinas wrote:
> On Tue, Mar 03, 2015 at 01:29:35PM +0100, Daniel Lezcano wrote:
> > diff --git a/drivers/cpuidle/cpuidle-arm64.c 
> > b/drivers/cpuidle/cpuidle-arm64.c
> > index 0cea244..6ef291c7 100644
> > --- a/drivers/cpuidle/cpuidle-arm64.c
> > +++ b/drivers/cpuidle/cpuidle-arm64.c
> > @@ -110,7 +110,7 @@ static int __init arm64_idle_init(void)
> >  * idle states suspend back-end specific data
> >  */
> > for_each_possible_cpu(cpu) {
> > -   ret = cpu_init_idle(cpu);
> > +   ret = arm_cpuidle_init(cpu);
> 
> Same nitpick here about dropping the arm_ prefix (though here we already
> have a cpuidle_init).

Actually, a question, probably for Lorenzo - why do we need to call
cpu_init_idle() from the driver? Is there any dependency on what the
driver had done before this call? If not, I suggest a core_initcall() in
the arch code for cpu_init_idle(). At a quick look through the code, the
back-end can be initialised on its own.

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 4/6] ARM64: cpuidle: Rename cpu_init_idle to a common function name

2015-03-13 Thread Catalin Marinas
On Tue, Mar 03, 2015 at 01:29:35PM +0100, Daniel Lezcano wrote:
> diff --git a/drivers/cpuidle/cpuidle-arm64.c b/drivers/cpuidle/cpuidle-arm64.c
> index 0cea244..6ef291c7 100644
> --- a/drivers/cpuidle/cpuidle-arm64.c
> +++ b/drivers/cpuidle/cpuidle-arm64.c
> @@ -110,7 +110,7 @@ static int __init arm64_idle_init(void)
>* idle states suspend back-end specific data
>*/
>   for_each_possible_cpu(cpu) {
> - ret = cpu_init_idle(cpu);
> + ret = arm_cpuidle_init(cpu);

Same nitpick here about dropping the arm_ prefix (though here we already
have a cpuidle_init).

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH 4/6] ARM64: cpuidle: Rename cpu_init_idle to a common function name

2015-03-03 Thread Daniel Lezcano
With this change the cpuidle-arm64.c file calls the same function name
for both ARM and ARM64.

Signed-off-by: Daniel Lezcano 
---
 arch/arm64/include/asm/cpuidle.h | 4 ++--
 arch/arm64/kernel/cpuidle.c  | 2 +-
 drivers/cpuidle/cpuidle-arm64.c  | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/include/asm/cpuidle.h b/arch/arm64/include/asm/cpuidle.h
index 1bee287..2bc2732 100644
--- a/arch/arm64/include/asm/cpuidle.h
+++ b/arch/arm64/include/asm/cpuidle.h
@@ -2,10 +2,10 @@
 #define __ASM_CPUIDLE_H
 
 #ifdef CONFIG_CPU_IDLE
-extern int cpu_init_idle(unsigned int cpu);
+extern int arm_cpuidle_init(unsigned int cpu);
 extern int cpu_suspend(unsigned long arg);
 #else
-static inline int cpu_init_idle(unsigned int cpu)
+static inline int arm_cpuidle_init(unsigned int cpu)
 {
return -EOPNOTSUPP;
 }
diff --git a/arch/arm64/kernel/cpuidle.c b/arch/arm64/kernel/cpuidle.c
index 5c08966..a78143a 100644
--- a/arch/arm64/kernel/cpuidle.c
+++ b/arch/arm64/kernel/cpuidle.c
@@ -15,7 +15,7 @@
 #include 
 #include 
 
-int cpu_init_idle(unsigned int cpu)
+int arm_cpuidle_init(unsigned int cpu)
 {
int ret = -EOPNOTSUPP;
struct device_node *cpu_node = of_cpu_device_node_get(cpu);
diff --git a/drivers/cpuidle/cpuidle-arm64.c b/drivers/cpuidle/cpuidle-arm64.c
index 0cea244..6ef291c7 100644
--- a/drivers/cpuidle/cpuidle-arm64.c
+++ b/drivers/cpuidle/cpuidle-arm64.c
@@ -110,7 +110,7 @@ static int __init arm64_idle_init(void)
 * idle states suspend back-end specific data
 */
for_each_possible_cpu(cpu) {
-   ret = cpu_init_idle(cpu);
+   ret = arm_cpuidle_init(cpu);
if (ret) {
pr_err("CPU %d failed to init idle CPU ops\n", cpu);
return ret;
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/