[PATCH V2 0/2] ARM: EXYNOS: cpuidle: fixes in exynos cpuidle/PM

[PATCH V2 0/2] ARM: EXYNOS: cpuidle: fixes in exynos cpuidle/PM

[Amit Daniel Kachhap]

This patch series contains a couple of fixes in cpuidle/PM due to
1) exynos5440 platform which does not support cpuidle C1 state.
2) Compilations error when generic PM options are enabled.

This patch series depends on earlier posted patch
http://www.spinics.net/lists/arm-kernel/msg248831.html

Changes in V2:
* Rebase against samsung maintainer's for next tree.

Amit Daniel Kachhap (2):
  ARM: SAMSUNG: Add SAMSUNG_PM config option to select pm
  ARM: EXYNOS: cpuidle: Allow C1 state only in supported SOC's.

 arch/arm/mach-exynos/Makefile           |    2 +-
 arch/arm/mach-exynos/common.c           |    2 +-
 arch/arm/mach-exynos/common.h           |    1 -
 arch/arm/mach-exynos/cpuidle.c          |    5 ++++-
 arch/arm/plat-samsung/Kconfig           |    7 +++++++
 arch/arm/plat-samsung/Makefile          |    3 +--
 arch/arm/plat-samsung/include/plat/pm.h |    8 ++++----
 7 files changed, 18 insertions(+), 10 deletions(-)

[PATCH V2 0/2] ARM: EXYNOS: cpuidle: fixes in exynos cpuidle/PM

[Amit Daniel Kachhap]

This patch series contains a couple of fixes in cpuidle/PM due to
1) exynos5440 platform which does not support cpuidle C1 state.
2) Compilations error when generic PM options are enabled.

This patch series depends on earlier posted patch
http://www.spinics.net/lists/arm-kernel/msg248831.html

Changes in V2:
* Rebase against samsung maintainer's for next tree.

Amit Daniel Kachhap (2):
  ARM: SAMSUNG: Add SAMSUNG_PM config option to select pm
  ARM: EXYNOS: cpuidle: Allow C1 state only in supported SOC's.

 arch/arm/mach-exynos/Makefile           |    2 +-
 arch/arm/mach-exynos/common.c           |    2 +-
 arch/arm/mach-exynos/common.h           |    1 -
 arch/arm/mach-exynos/cpuidle.c          |    5 ++++-
 arch/arm/plat-samsung/Kconfig           |    7 +++++++
 arch/arm/plat-samsung/Makefile          |    3 +--
 arch/arm/plat-samsung/include/plat/pm.h |    8 ++++----
 7 files changed, 18 insertions(+), 10 deletions(-)

[PATCH V2 1/2] ARM: SAMSUNG: Add SAMSUNG_PM config option to select pm

[Amit Daniel Kachhap]

This patch enables the selection of samsung pm related stuffs
when SAMSUNG_PM config is enabled and not just when generic PM
config is enabled. Power management for s3c64XX and s3c24XX
is enabled by default and for other platform depends on S5P_PM.
This patch also fixes the following compilation error's when compiling
a platform like exynos5440 which does not select pm stuffs.

arch/arm/mach-exynos/built-in.o: In function `__virt_to_phys':
linux/arch/arm/include/asm/memory.h:175: undefined reference to `s3c_cpu_resume'
linux/arch/arm/include/asm/memory.h:175: undefined reference to `s3c_cpu_resume'
linux/arch/arm/include/asm/memory.h:175: undefined reference to `s3c_cpu_resume'
linux/arch/arm/include/asm/memory.h:175: undefined reference to `s3c_cpu_resume'
arch/arm/mach-exynos/built-in.o: In function `exynos5_init_irq':
linux/arch/arm/mach-exynos/common.c:492: undefined reference to `s3c_irq_wake'
linux/arch/arm/mach-exynos/common.c:492: undefined reference to `s3c_irq_wake'
arch/arm/mach-exynos/built-in.o: In function `exynos4_init_irq':
linux/arch/arm/mach-exynos/common.c:476: undefined reference to `s3c_irq_wake'
linux/arch/arm/mach-exynos/common.c:476: undefined reference to `s3c_irq_wake'
arch/arm/plat-samsung/built-in.o: In function `s3c_irqext_wake':
linux/arch/arm/plat-samsung/pm.c:144: undefined reference to `s3c_irqwake_eintallow'
linux/arch/arm/plat-samsung/pm.c:144: undefined reference to `s3c_irqwake_eintallow'
arch/arm/plat-samsung/built-in.o: In function `s3c_pm_enter':
linux/arch/arm/plat-samsung/pm.c:263: undefined reference to `s3c_irqwake_intallow'
linux/arch/arm/plat-samsung/pm.c:263: undefined reference to `s3c_irqwake_intallow'
linux/arch/arm/plat-samsung/pm.c:264: undefined reference to `s3c_irqwake_eintallow'
linux/arch/arm/plat-samsung/pm.c:264: undefined reference to `s3c_irqwake_eintallow'
linux/arch/arm/plat-samsung/pm.c:275: undefined reference to `s3c_pm_save_core'
linux/arch/arm/plat-samsung/pm.c:279: undefined reference to `s3c_pm_configure_extint'
linux/arch/arm/plat-samsung/pm.c:310: undefined reference to `s3c_pm_restore_core'
make: *** [vmlinux] Error 1

Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
---
 arch/arm/mach-exynos/Makefile           |    2 +-
 arch/arm/mach-exynos/common.c           |    2 +-
 arch/arm/mach-exynos/common.h           |    1 -
 arch/arm/mach-exynos/cpuidle.c          |    1 +
 arch/arm/plat-samsung/Kconfig           |    7 +++++++
 arch/arm/plat-samsung/Makefile          |    3 +--
 arch/arm/plat-samsung/include/plat/pm.h |    8 ++++----
 7 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/arch/arm/mach-exynos/Makefile b/arch/arm/mach-exynos/Makefile
index 9811f87..3fa277a 100644
--- a/arch/arm/mach-exynos/Makefile
+++ b/arch/arm/mach-exynos/Makefile
@@ -14,7 +14,7 @@ obj-				:=
 
 obj-$(CONFIG_ARCH_EXYNOS)	+= common.o
 
-obj-$(CONFIG_PM)		+= pm.o
+obj-$(CONFIG_S5P_PM)		+= pm.o
 obj-$(CONFIG_PM_GENERIC_DOMAINS) += pm_domains.o
 obj-$(CONFIG_CPU_IDLE)		+= cpuidle.o
 
diff --git a/arch/arm/mach-exynos/common.c b/arch/arm/mach-exynos/common.c
index 9834357..d2b4f54 100644
--- a/arch/arm/mach-exynos/common.c
+++ b/arch/arm/mach-exynos/common.c
@@ -799,7 +799,7 @@ static struct irq_chip exynos_irq_eint = {
 	.irq_mask_ack	= exynos_irq_eint_maskack,
 	.irq_ack	= exynos_irq_eint_ack,
 	.irq_set_type	= exynos_irq_eint_set_type,
-#ifdef CONFIG_PM
+#ifdef CONFIG_S5P_PM
 	.irq_set_wake	= s3c_irqext_wake,
 #endif
 };
diff --git a/arch/arm/mach-exynos/common.h b/arch/arm/mach-exynos/common.h
index 11fc1e2..f2b2e23 100644
--- a/arch/arm/mach-exynos/common.h
+++ b/arch/arm/mach-exynos/common.h
@@ -98,6 +98,5 @@ struct exynos_pmu_conf {
 };
 
 extern void exynos_sys_powerdown_conf(enum sys_powerdown mode);
-extern void s3c_cpu_resume(void);
 
 #endif /* __ARCH_ARM_MACH_EXYNOS_COMMON_H */
diff --git a/arch/arm/mach-exynos/cpuidle.c b/arch/arm/mach-exynos/cpuidle.c
index 4667907..2d071c6 100644
--- a/arch/arm/mach-exynos/cpuidle.c
+++ b/arch/arm/mach-exynos/cpuidle.c
@@ -25,6 +25,7 @@
 #include <mach/regs-pmu.h>
 
 #include <plat/cpu.h>
+#include <plat/pm.h>
 
 #include "common.h"
 
diff --git a/arch/arm/plat-samsung/Kconfig b/arch/arm/plat-samsung/Kconfig
index f8ed2de..6c25aa6 100644
--- a/arch/arm/plat-samsung/Kconfig
+++ b/arch/arm/plat-samsung/Kconfig
@@ -31,6 +31,13 @@ config PLAT_S5P
 	help
 	  Base platform code for Samsung's S5P series SoC.
 
+config SAMSUNG_PM
+	bool
+	depends on PM && (PLAT_S3C24XX || ARCH_S3C64XX || S5P_PM)
+	default y
+	help
+	  Base platform power management code for samsung code
+
 if PLAT_SAMSUNG
 
 # boot configurations
diff --git a/arch/arm/plat-samsung/Makefile b/arch/arm/plat-samsung/Makefile
index a23c460..1c406b3 100644
--- a/arch/arm/plat-samsung/Makefile
+++ b/arch/arm/plat-samsung/Makefile
@@ -51,8 +51,7 @@ obj-$(CONFIG_SAMSUNG_DMADEV)	+= dma-ops.o
 
 # PM support
 
-obj-$(CONFIG_PM)		+= pm.o
-obj-$(CONFIG_PM)		+= pm-gpio.o
+obj-$(CONFIG_SAMSUNG_PM)	+= pm.o pm-gpio.o
 obj-$(CONFIG_SAMSUNG_PM_CHECK)	+= pm-check.o
 
 obj-$(CONFIG_SAMSUNG_WAKEMASK)	+= wakeup-mask.o
diff --git a/arch/arm/plat-samsung/include/plat/pm.h b/arch/arm/plat-samsung/include/plat/pm.h
index f6fcade..46e2c82 100644
--- a/arch/arm/plat-samsung/include/plat/pm.h
+++ b/arch/arm/plat-samsung/include/plat/pm.h
@@ -19,7 +19,7 @@
 
 struct device;
 
-#ifdef CONFIG_PM
+#ifdef CONFIG_SAMSUNG_PM
 
 extern __init int s3c_pm_init(void);
 extern __init int s3c64xx_pm_init(void);
@@ -58,8 +58,6 @@ extern unsigned char pm_uart_udivslot;  /* true to save UART UDIVSLOT */
 
 /* from sleep.S */
 
-extern void s3c_cpu_resume(void);
-
 extern int s3c2410_cpu_suspend(unsigned long);
 
 /* sleep save info */
@@ -106,12 +104,14 @@ extern void s3c_pm_do_save(struct sleep_save *ptr, int count);
 extern void s3c_pm_do_restore(struct sleep_save *ptr, int count);
 extern void s3c_pm_do_restore_core(struct sleep_save *ptr, int count);
 
-#ifdef CONFIG_PM
+#ifdef CONFIG_SAMSUNG_PM
 extern int s3c_irq_wake(struct irq_data *data, unsigned int state);
 extern int s3c_irqext_wake(struct irq_data *data, unsigned int state);
+extern void s3c_cpu_resume(void);
 #else
 #define s3c_irq_wake NULL
 #define s3c_irqext_wake NULL
+#define s3c_cpu_resume NULL
 #endif
 
 /* PM debug functions */
-- 
1.7.1

[PATCH V2 1/2] ARM: SAMSUNG: Add SAMSUNG_PM config option to select pm

[Amit Daniel Kachhap]

This patch enables the selection of samsung pm related stuffs
when SAMSUNG_PM config is enabled and not just when generic PM
config is enabled. Power management for s3c64XX and s3c24XX
is enabled by default and for other platform depends on S5P_PM.
This patch also fixes the following compilation error's when compiling
a platform like exynos5440 which does not select pm stuffs.

arch/arm/mach-exynos/built-in.o: In function `__virt_to_phys':
linux/arch/arm/include/asm/memory.h:175: undefined reference to `s3c_cpu_resume'
linux/arch/arm/include/asm/memory.h:175: undefined reference to `s3c_cpu_resume'
linux/arch/arm/include/asm/memory.h:175: undefined reference to `s3c_cpu_resume'
linux/arch/arm/include/asm/memory.h:175: undefined reference to `s3c_cpu_resume'
arch/arm/mach-exynos/built-in.o: In function `exynos5_init_irq':
linux/arch/arm/mach-exynos/common.c:492: undefined reference to `s3c_irq_wake'
linux/arch/arm/mach-exynos/common.c:492: undefined reference to `s3c_irq_wake'
arch/arm/mach-exynos/built-in.o: In function `exynos4_init_irq':
linux/arch/arm/mach-exynos/common.c:476: undefined reference to `s3c_irq_wake'
linux/arch/arm/mach-exynos/common.c:476: undefined reference to `s3c_irq_wake'
arch/arm/plat-samsung/built-in.o: In function `s3c_irqext_wake':
linux/arch/arm/plat-samsung/pm.c:144: undefined reference to `s3c_irqwake_eintallow'
linux/arch/arm/plat-samsung/pm.c:144: undefined reference to `s3c_irqwake_eintallow'
arch/arm/plat-samsung/built-in.o: In function `s3c_pm_enter':
linux/arch/arm/plat-samsung/pm.c:263: undefined reference to `s3c_irqwake_intallow'
linux/arch/arm/plat-samsung/pm.c:263: undefined reference to `s3c_irqwake_intallow'
linux/arch/arm/plat-samsung/pm.c:264: undefined reference to `s3c_irqwake_eintallow'
linux/arch/arm/plat-samsung/pm.c:264: undefined reference to `s3c_irqwake_eintallow'
linux/arch/arm/plat-samsung/pm.c:275: undefined reference to `s3c_pm_save_core'
linux/arch/arm/plat-samsung/pm.c:279: undefined reference to `s3c_pm_configure_extint'
linux/arch/arm/plat-samsung/pm.c:310: undefined reference to `s3c_pm_restore_core'
make: *** [vmlinux] Error 1

Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
---
 arch/arm/mach-exynos/Makefile           |    2 +-
 arch/arm/mach-exynos/common.c           |    2 +-
 arch/arm/mach-exynos/common.h           |    1 -
 arch/arm/mach-exynos/cpuidle.c          |    1 +
 arch/arm/plat-samsung/Kconfig           |    7 +++++++
 arch/arm/plat-samsung/Makefile          |    3 +--
 arch/arm/plat-samsung/include/plat/pm.h |    8 ++++----
 7 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/arch/arm/mach-exynos/Makefile b/arch/arm/mach-exynos/Makefile
index 9811f87..3fa277a 100644
--- a/arch/arm/mach-exynos/Makefile
+++ b/arch/arm/mach-exynos/Makefile
@@ -14,7 +14,7 @@ obj-				:=
 
 obj-$(CONFIG_ARCH_EXYNOS)	+= common.o
 
-obj-$(CONFIG_PM)		+= pm.o
+obj-$(CONFIG_S5P_PM)		+= pm.o
 obj-$(CONFIG_PM_GENERIC_DOMAINS) += pm_domains.o
 obj-$(CONFIG_CPU_IDLE)		+= cpuidle.o
 
diff --git a/arch/arm/mach-exynos/common.c b/arch/arm/mach-exynos/common.c
index 9834357..d2b4f54 100644
--- a/arch/arm/mach-exynos/common.c
+++ b/arch/arm/mach-exynos/common.c
@@ -799,7 +799,7 @@ static struct irq_chip exynos_irq_eint = {
 	.irq_mask_ack	= exynos_irq_eint_maskack,
 	.irq_ack	= exynos_irq_eint_ack,
 	.irq_set_type	= exynos_irq_eint_set_type,
-#ifdef CONFIG_PM
+#ifdef CONFIG_S5P_PM
 	.irq_set_wake	= s3c_irqext_wake,
 #endif
 };
diff --git a/arch/arm/mach-exynos/common.h b/arch/arm/mach-exynos/common.h
index 11fc1e2..f2b2e23 100644
--- a/arch/arm/mach-exynos/common.h
+++ b/arch/arm/mach-exynos/common.h
@@ -98,6 +98,5 @@ struct exynos_pmu_conf {
 };
 
 extern void exynos_sys_powerdown_conf(enum sys_powerdown mode);
-extern void s3c_cpu_resume(void);
 
 #endif /* __ARCH_ARM_MACH_EXYNOS_COMMON_H */
diff --git a/arch/arm/mach-exynos/cpuidle.c b/arch/arm/mach-exynos/cpuidle.c
index 4667907..2d071c6 100644
--- a/arch/arm/mach-exynos/cpuidle.c
+++ b/arch/arm/mach-exynos/cpuidle.c
@@ -25,6 +25,7 @@
 #include <mach/regs-pmu.h>
 
 #include <plat/cpu.h>
+#include <plat/pm.h>
 
 #include "common.h"
 
diff --git a/arch/arm/plat-samsung/Kconfig b/arch/arm/plat-samsung/Kconfig
index f8ed2de..6c25aa6 100644
--- a/arch/arm/plat-samsung/Kconfig
+++ b/arch/arm/plat-samsung/Kconfig
@@ -31,6 +31,13 @@ config PLAT_S5P
 	help
 	  Base platform code for Samsung's S5P series SoC.
 
+config SAMSUNG_PM
+	bool
+	depends on PM && (PLAT_S3C24XX || ARCH_S3C64XX || S5P_PM)
+	default y
+	help
+	  Base platform power management code for samsung code
+
 if PLAT_SAMSUNG
 
 # boot configurations
diff --git a/arch/arm/plat-samsung/Makefile b/arch/arm/plat-samsung/Makefile
index a23c460..1c406b3 100644
--- a/arch/arm/plat-samsung/Makefile
+++ b/arch/arm/plat-samsung/Makefile
@@ -51,8 +51,7 @@ obj-$(CONFIG_SAMSUNG_DMADEV)	+= dma-ops.o
 
 # PM support
 
-obj-$(CONFIG_PM)		+= pm.o
-obj-$(CONFIG_PM)		+= pm-gpio.o
+obj-$(CONFIG_SAMSUNG_PM)	+= pm.o pm-gpio.o
 obj-$(CONFIG_SAMSUNG_PM_CHECK)	+= pm-check.o
 
 obj-$(CONFIG_SAMSUNG_WAKEMASK)	+= wakeup-mask.o
diff --git a/arch/arm/plat-samsung/include/plat/pm.h b/arch/arm/plat-samsung/include/plat/pm.h
index f6fcade..46e2c82 100644
--- a/arch/arm/plat-samsung/include/plat/pm.h
+++ b/arch/arm/plat-samsung/include/plat/pm.h
@@ -19,7 +19,7 @@
 
 struct device;
 
-#ifdef CONFIG_PM
+#ifdef CONFIG_SAMSUNG_PM
 
 extern __init int s3c_pm_init(void);
 extern __init int s3c64xx_pm_init(void);
@@ -58,8 +58,6 @@ extern unsigned char pm_uart_udivslot;  /* true to save UART UDIVSLOT */
 
 /* from sleep.S */
 
-extern void s3c_cpu_resume(void);
-
 extern int s3c2410_cpu_suspend(unsigned long);
 
 /* sleep save info */
@@ -106,12 +104,14 @@ extern void s3c_pm_do_save(struct sleep_save *ptr, int count);
 extern void s3c_pm_do_restore(struct sleep_save *ptr, int count);
 extern void s3c_pm_do_restore_core(struct sleep_save *ptr, int count);
 
-#ifdef CONFIG_PM
+#ifdef CONFIG_SAMSUNG_PM
 extern int s3c_irq_wake(struct irq_data *data, unsigned int state);
 extern int s3c_irqext_wake(struct irq_data *data, unsigned int state);
+extern void s3c_cpu_resume(void);
 #else
 #define s3c_irq_wake NULL
 #define s3c_irqext_wake NULL
+#define s3c_cpu_resume NULL
 #endif
 
 /* PM debug functions */
-- 
1.7.1

RE: [PATCH V2 1/2] ARM: SAMSUNG: Add SAMSUNG_PM config option to select pm

[Kukjin Kim]

Amit Daniel wrote:
> 
> This patch enables the selection of samsung pm related stuffs
> when SAMSUNG_PM config is enabled and not just when generic PM
> config is enabled. Power management for s3c64XX and s3c24XX
> is enabled by default and for other platform depends on S5P_PM.
> This patch also fixes the following compilation error's when compiling
> a platform like exynos5440 which does not select pm stuffs.
> 
> arch/arm/mach-exynos/built-in.o: In function `__virt_to_phys':
> linux/arch/arm/include/asm/memory.h:175: undefined reference to
> `s3c_cpu_resume'
> linux/arch/arm/include/asm/memory.h:175: undefined reference to
> `s3c_cpu_resume'
> linux/arch/arm/include/asm/memory.h:175: undefined reference to
> `s3c_cpu_resume'
> linux/arch/arm/include/asm/memory.h:175: undefined reference to
> `s3c_cpu_resume'
> arch/arm/mach-exynos/built-in.o: In function `exynos5_init_irq':
> linux/arch/arm/mach-exynos/common.c:492: undefined reference to
> `s3c_irq_wake'
> linux/arch/arm/mach-exynos/common.c:492: undefined reference to
> `s3c_irq_wake'
> arch/arm/mach-exynos/built-in.o: In function `exynos4_init_irq':
> linux/arch/arm/mach-exynos/common.c:476: undefined reference to
> `s3c_irq_wake'
> linux/arch/arm/mach-exynos/common.c:476: undefined reference to
> `s3c_irq_wake'
> arch/arm/plat-samsung/built-in.o: In function `s3c_irqext_wake':
> linux/arch/arm/plat-samsung/pm.c:144: undefined reference to
> `s3c_irqwake_eintallow'
> linux/arch/arm/plat-samsung/pm.c:144: undefined reference to
> `s3c_irqwake_eintallow'
> arch/arm/plat-samsung/built-in.o: In function `s3c_pm_enter':
> linux/arch/arm/plat-samsung/pm.c:263: undefined reference to
> `s3c_irqwake_intallow'
> linux/arch/arm/plat-samsung/pm.c:263: undefined reference to
> `s3c_irqwake_intallow'
> linux/arch/arm/plat-samsung/pm.c:264: undefined reference to
> `s3c_irqwake_eintallow'
> linux/arch/arm/plat-samsung/pm.c:264: undefined reference to
> `s3c_irqwake_eintallow'
> linux/arch/arm/plat-samsung/pm.c:275: undefined reference to
> `s3c_pm_save_core'
> linux/arch/arm/plat-samsung/pm.c:279: undefined reference to
> `s3c_pm_configure_extint'
> linux/arch/arm/plat-samsung/pm.c:310: undefined reference to
> `s3c_pm_restore_core'
> make: *** [vmlinux] Error 1
> 
> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
> ---
>  arch/arm/mach-exynos/Makefile           |    2 +-
>  arch/arm/mach-exynos/common.c           |    2 +-
>  arch/arm/mach-exynos/common.h           |    1 -
>  arch/arm/mach-exynos/cpuidle.c          |    1 +
>  arch/arm/plat-samsung/Kconfig           |    7 +++++++
>  arch/arm/plat-samsung/Makefile          |    3 +--
>  arch/arm/plat-samsung/include/plat/pm.h |    8 ++++----
>  7 files changed, 15 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/arm/mach-exynos/Makefile b/arch/arm/mach-exynos/Makefile
> index 9811f87..3fa277a 100644
> --- a/arch/arm/mach-exynos/Makefile
> +++ b/arch/arm/mach-exynos/Makefile
> @@ -14,7 +14,7 @@ obj-				:=
> 
>  obj-$(CONFIG_ARCH_EXYNOS)	+= common.o
> 
> -obj-$(CONFIG_PM)		+= pm.o
> +obj-$(CONFIG_S5P_PM)		+= pm.o
>  obj-$(CONFIG_PM_GENERIC_DOMAINS) += pm_domains.o
>  obj-$(CONFIG_CPU_IDLE)		+= cpuidle.o
> 
> diff --git a/arch/arm/mach-exynos/common.c b/arch/arm/mach-exynos/common.c
> index 9834357..d2b4f54 100644
> --- a/arch/arm/mach-exynos/common.c
> +++ b/arch/arm/mach-exynos/common.c
> @@ -799,7 +799,7 @@ static struct irq_chip exynos_irq_eint = {
>  	.irq_mask_ack	= exynos_irq_eint_maskack,
>  	.irq_ack	= exynos_irq_eint_ack,
>  	.irq_set_type	= exynos_irq_eint_set_type,
> -#ifdef CONFIG_PM
> +#ifdef CONFIG_S5P_PM

The 'S5P_PM' is used for plat-samsung/s5p-pm.c and s5p-irq-pm.c for s5pv210.

So there is no problem to use same name for exynos and s5pv210?...

- Kukjin

[PATCH V2 1/2] ARM: SAMSUNG: Add SAMSUNG_PM config option to select pm

[Kukjin Kim]

Amit Daniel wrote:
> 
> This patch enables the selection of samsung pm related stuffs
> when SAMSUNG_PM config is enabled and not just when generic PM
> config is enabled. Power management for s3c64XX and s3c24XX
> is enabled by default and for other platform depends on S5P_PM.
> This patch also fixes the following compilation error's when compiling
> a platform like exynos5440 which does not select pm stuffs.
> 
> arch/arm/mach-exynos/built-in.o: In function `__virt_to_phys':
> linux/arch/arm/include/asm/memory.h:175: undefined reference to
> `s3c_cpu_resume'
> linux/arch/arm/include/asm/memory.h:175: undefined reference to
> `s3c_cpu_resume'
> linux/arch/arm/include/asm/memory.h:175: undefined reference to
> `s3c_cpu_resume'
> linux/arch/arm/include/asm/memory.h:175: undefined reference to
> `s3c_cpu_resume'
> arch/arm/mach-exynos/built-in.o: In function `exynos5_init_irq':
> linux/arch/arm/mach-exynos/common.c:492: undefined reference to
> `s3c_irq_wake'
> linux/arch/arm/mach-exynos/common.c:492: undefined reference to
> `s3c_irq_wake'
> arch/arm/mach-exynos/built-in.o: In function `exynos4_init_irq':
> linux/arch/arm/mach-exynos/common.c:476: undefined reference to
> `s3c_irq_wake'
> linux/arch/arm/mach-exynos/common.c:476: undefined reference to
> `s3c_irq_wake'
> arch/arm/plat-samsung/built-in.o: In function `s3c_irqext_wake':
> linux/arch/arm/plat-samsung/pm.c:144: undefined reference to
> `s3c_irqwake_eintallow'
> linux/arch/arm/plat-samsung/pm.c:144: undefined reference to
> `s3c_irqwake_eintallow'
> arch/arm/plat-samsung/built-in.o: In function `s3c_pm_enter':
> linux/arch/arm/plat-samsung/pm.c:263: undefined reference to
> `s3c_irqwake_intallow'
> linux/arch/arm/plat-samsung/pm.c:263: undefined reference to
> `s3c_irqwake_intallow'
> linux/arch/arm/plat-samsung/pm.c:264: undefined reference to
> `s3c_irqwake_eintallow'
> linux/arch/arm/plat-samsung/pm.c:264: undefined reference to
> `s3c_irqwake_eintallow'
> linux/arch/arm/plat-samsung/pm.c:275: undefined reference to
> `s3c_pm_save_core'
> linux/arch/arm/plat-samsung/pm.c:279: undefined reference to
> `s3c_pm_configure_extint'
> linux/arch/arm/plat-samsung/pm.c:310: undefined reference to
> `s3c_pm_restore_core'
> make: *** [vmlinux] Error 1
> 
> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
> ---
>  arch/arm/mach-exynos/Makefile           |    2 +-
>  arch/arm/mach-exynos/common.c           |    2 +-
>  arch/arm/mach-exynos/common.h           |    1 -
>  arch/arm/mach-exynos/cpuidle.c          |    1 +
>  arch/arm/plat-samsung/Kconfig           |    7 +++++++
>  arch/arm/plat-samsung/Makefile          |    3 +--
>  arch/arm/plat-samsung/include/plat/pm.h |    8 ++++----
>  7 files changed, 15 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/arm/mach-exynos/Makefile b/arch/arm/mach-exynos/Makefile
> index 9811f87..3fa277a 100644
> --- a/arch/arm/mach-exynos/Makefile
> +++ b/arch/arm/mach-exynos/Makefile
> @@ -14,7 +14,7 @@ obj-				:=
> 
>  obj-$(CONFIG_ARCH_EXYNOS)	+= common.o
> 
> -obj-$(CONFIG_PM)		+= pm.o
> +obj-$(CONFIG_S5P_PM)		+= pm.o
>  obj-$(CONFIG_PM_GENERIC_DOMAINS) += pm_domains.o
>  obj-$(CONFIG_CPU_IDLE)		+= cpuidle.o
> 
> diff --git a/arch/arm/mach-exynos/common.c b/arch/arm/mach-exynos/common.c
> index 9834357..d2b4f54 100644
> --- a/arch/arm/mach-exynos/common.c
> +++ b/arch/arm/mach-exynos/common.c
> @@ -799,7 +799,7 @@ static struct irq_chip exynos_irq_eint = {
>  	.irq_mask_ack	= exynos_irq_eint_maskack,
>  	.irq_ack	= exynos_irq_eint_ack,
>  	.irq_set_type	= exynos_irq_eint_set_type,
> -#ifdef CONFIG_PM
> +#ifdef CONFIG_S5P_PM

The 'S5P_PM' is used for plat-samsung/s5p-pm.c and s5p-irq-pm.c for s5pv210.

So there is no problem to use same name for exynos and s5pv210?...

- Kukjin

Re: [PATCH V2 1/2] ARM: SAMSUNG: Add SAMSUNG_PM config option to select pm

[amit daniel kachhap]

Hi,

Thanks for your review comments.

On Thu, Jun 13, 2013 at 6:12 PM, Kukjin Kim <kgene.kim@samsung.com> wrote:
> Amit Daniel wrote:
>>
>> This patch enables the selection of samsung pm related stuffs
>> when SAMSUNG_PM config is enabled and not just when generic PM
>> config is enabled. Power management for s3c64XX and s3c24XX
>> is enabled by default and for other platform depends on S5P_PM.
>> This patch also fixes the following compilation error's when compiling
>> a platform like exynos5440 which does not select pm stuffs.
>>
>> arch/arm/mach-exynos/built-in.o: In function `__virt_to_phys':
>> linux/arch/arm/include/asm/memory.h:175: undefined reference to
>> `s3c_cpu_resume'
>> linux/arch/arm/include/asm/memory.h:175: undefined reference to
>> `s3c_cpu_resume'
>> linux/arch/arm/include/asm/memory.h:175: undefined reference to
>> `s3c_cpu_resume'
>> linux/arch/arm/include/asm/memory.h:175: undefined reference to
>> `s3c_cpu_resume'
>> arch/arm/mach-exynos/built-in.o: In function `exynos5_init_irq':
>> linux/arch/arm/mach-exynos/common.c:492: undefined reference to
>> `s3c_irq_wake'
>> linux/arch/arm/mach-exynos/common.c:492: undefined reference to
>> `s3c_irq_wake'
>> arch/arm/mach-exynos/built-in.o: In function `exynos4_init_irq':
>> linux/arch/arm/mach-exynos/common.c:476: undefined reference to
>> `s3c_irq_wake'
>> linux/arch/arm/mach-exynos/common.c:476: undefined reference to
>> `s3c_irq_wake'
>> arch/arm/plat-samsung/built-in.o: In function `s3c_irqext_wake':
>> linux/arch/arm/plat-samsung/pm.c:144: undefined reference to
>> `s3c_irqwake_eintallow'
>> linux/arch/arm/plat-samsung/pm.c:144: undefined reference to
>> `s3c_irqwake_eintallow'
>> arch/arm/plat-samsung/built-in.o: In function `s3c_pm_enter':
>> linux/arch/arm/plat-samsung/pm.c:263: undefined reference to
>> `s3c_irqwake_intallow'
>> linux/arch/arm/plat-samsung/pm.c:263: undefined reference to
>> `s3c_irqwake_intallow'
>> linux/arch/arm/plat-samsung/pm.c:264: undefined reference to
>> `s3c_irqwake_eintallow'
>> linux/arch/arm/plat-samsung/pm.c:264: undefined reference to
>> `s3c_irqwake_eintallow'
>> linux/arch/arm/plat-samsung/pm.c:275: undefined reference to
>> `s3c_pm_save_core'
>> linux/arch/arm/plat-samsung/pm.c:279: undefined reference to
>> `s3c_pm_configure_extint'
>> linux/arch/arm/plat-samsung/pm.c:310: undefined reference to
>> `s3c_pm_restore_core'
>> make: *** [vmlinux] Error 1
>>
>> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
>> ---
>>  arch/arm/mach-exynos/Makefile           |    2 +-
>>  arch/arm/mach-exynos/common.c           |    2 +-
>>  arch/arm/mach-exynos/common.h           |    1 -
>>  arch/arm/mach-exynos/cpuidle.c          |    1 +
>>  arch/arm/plat-samsung/Kconfig           |    7 +++++++
>>  arch/arm/plat-samsung/Makefile          |    3 +--
>>  arch/arm/plat-samsung/include/plat/pm.h |    8 ++++----
>>  7 files changed, 15 insertions(+), 9 deletions(-)
>>
>> diff --git a/arch/arm/mach-exynos/Makefile b/arch/arm/mach-exynos/Makefile
>> index 9811f87..3fa277a 100644
>> --- a/arch/arm/mach-exynos/Makefile
>> +++ b/arch/arm/mach-exynos/Makefile
>> @@ -14,7 +14,7 @@ obj-                                :=
>>
>>  obj-$(CONFIG_ARCH_EXYNOS)    += common.o
>>
>> -obj-$(CONFIG_PM)             += pm.o
>> +obj-$(CONFIG_S5P_PM)         += pm.o
>>  obj-$(CONFIG_PM_GENERIC_DOMAINS) += pm_domains.o
>>  obj-$(CONFIG_CPU_IDLE)               += cpuidle.o
>>
>> diff --git a/arch/arm/mach-exynos/common.c b/arch/arm/mach-exynos/common.c
>> index 9834357..d2b4f54 100644
>> --- a/arch/arm/mach-exynos/common.c
>> +++ b/arch/arm/mach-exynos/common.c
>> @@ -799,7 +799,7 @@ static struct irq_chip exynos_irq_eint = {
>>       .irq_mask_ack   = exynos_irq_eint_maskack,
>>       .irq_ack        = exynos_irq_eint_ack,
>>       .irq_set_type   = exynos_irq_eint_set_type,
>> -#ifdef CONFIG_PM
>> +#ifdef CONFIG_S5P_PM
>
> The 'S5P_PM' is used for plat-samsung/s5p-pm.c and s5p-irq-pm.c for s5pv210.
>
> So there is no problem to use same name for exynos and s5pv210?...
Basically I looked into arch/arm/mach-exynos/Kconfig and there  select
S5P_PM is done for exynos 5250, 4210, 4212 etc. so I used the same
config macro to enable other PM stuffs inside mach-exynos folder.

Thanks,
Amit Daniel
>
> - Kukjin
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH V2 1/2] ARM: SAMSUNG: Add SAMSUNG_PM config option to select pm

[amit daniel kachhap]

Hi,

Thanks for your review comments.

On Thu, Jun 13, 2013 at 6:12 PM, Kukjin Kim <kgene.kim@samsung.com> wrote:
> Amit Daniel wrote:
>>
>> This patch enables the selection of samsung pm related stuffs
>> when SAMSUNG_PM config is enabled and not just when generic PM
>> config is enabled. Power management for s3c64XX and s3c24XX
>> is enabled by default and for other platform depends on S5P_PM.
>> This patch also fixes the following compilation error's when compiling
>> a platform like exynos5440 which does not select pm stuffs.
>>
>> arch/arm/mach-exynos/built-in.o: In function `__virt_to_phys':
>> linux/arch/arm/include/asm/memory.h:175: undefined reference to
>> `s3c_cpu_resume'
>> linux/arch/arm/include/asm/memory.h:175: undefined reference to
>> `s3c_cpu_resume'
>> linux/arch/arm/include/asm/memory.h:175: undefined reference to
>> `s3c_cpu_resume'
>> linux/arch/arm/include/asm/memory.h:175: undefined reference to
>> `s3c_cpu_resume'
>> arch/arm/mach-exynos/built-in.o: In function `exynos5_init_irq':
>> linux/arch/arm/mach-exynos/common.c:492: undefined reference to
>> `s3c_irq_wake'
>> linux/arch/arm/mach-exynos/common.c:492: undefined reference to
>> `s3c_irq_wake'
>> arch/arm/mach-exynos/built-in.o: In function `exynos4_init_irq':
>> linux/arch/arm/mach-exynos/common.c:476: undefined reference to
>> `s3c_irq_wake'
>> linux/arch/arm/mach-exynos/common.c:476: undefined reference to
>> `s3c_irq_wake'
>> arch/arm/plat-samsung/built-in.o: In function `s3c_irqext_wake':
>> linux/arch/arm/plat-samsung/pm.c:144: undefined reference to
>> `s3c_irqwake_eintallow'
>> linux/arch/arm/plat-samsung/pm.c:144: undefined reference to
>> `s3c_irqwake_eintallow'
>> arch/arm/plat-samsung/built-in.o: In function `s3c_pm_enter':
>> linux/arch/arm/plat-samsung/pm.c:263: undefined reference to
>> `s3c_irqwake_intallow'
>> linux/arch/arm/plat-samsung/pm.c:263: undefined reference to
>> `s3c_irqwake_intallow'
>> linux/arch/arm/plat-samsung/pm.c:264: undefined reference to
>> `s3c_irqwake_eintallow'
>> linux/arch/arm/plat-samsung/pm.c:264: undefined reference to
>> `s3c_irqwake_eintallow'
>> linux/arch/arm/plat-samsung/pm.c:275: undefined reference to
>> `s3c_pm_save_core'
>> linux/arch/arm/plat-samsung/pm.c:279: undefined reference to
>> `s3c_pm_configure_extint'
>> linux/arch/arm/plat-samsung/pm.c:310: undefined reference to
>> `s3c_pm_restore_core'
>> make: *** [vmlinux] Error 1
>>
>> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
>> ---
>>  arch/arm/mach-exynos/Makefile           |    2 +-
>>  arch/arm/mach-exynos/common.c           |    2 +-
>>  arch/arm/mach-exynos/common.h           |    1 -
>>  arch/arm/mach-exynos/cpuidle.c          |    1 +
>>  arch/arm/plat-samsung/Kconfig           |    7 +++++++
>>  arch/arm/plat-samsung/Makefile          |    3 +--
>>  arch/arm/plat-samsung/include/plat/pm.h |    8 ++++----
>>  7 files changed, 15 insertions(+), 9 deletions(-)
>>
>> diff --git a/arch/arm/mach-exynos/Makefile b/arch/arm/mach-exynos/Makefile
>> index 9811f87..3fa277a 100644
>> --- a/arch/arm/mach-exynos/Makefile
>> +++ b/arch/arm/mach-exynos/Makefile
>> @@ -14,7 +14,7 @@ obj-                                :=
>>
>>  obj-$(CONFIG_ARCH_EXYNOS)    += common.o
>>
>> -obj-$(CONFIG_PM)             += pm.o
>> +obj-$(CONFIG_S5P_PM)         += pm.o
>>  obj-$(CONFIG_PM_GENERIC_DOMAINS) += pm_domains.o
>>  obj-$(CONFIG_CPU_IDLE)               += cpuidle.o
>>
>> diff --git a/arch/arm/mach-exynos/common.c b/arch/arm/mach-exynos/common.c
>> index 9834357..d2b4f54 100644
>> --- a/arch/arm/mach-exynos/common.c
>> +++ b/arch/arm/mach-exynos/common.c
>> @@ -799,7 +799,7 @@ static struct irq_chip exynos_irq_eint = {
>>       .irq_mask_ack   = exynos_irq_eint_maskack,
>>       .irq_ack        = exynos_irq_eint_ack,
>>       .irq_set_type   = exynos_irq_eint_set_type,
>> -#ifdef CONFIG_PM
>> +#ifdef CONFIG_S5P_PM
>
> The 'S5P_PM' is used for plat-samsung/s5p-pm.c and s5p-irq-pm.c for s5pv210.
>
> So there is no problem to use same name for exynos and s5pv210?...
Basically I looked into arch/arm/mach-exynos/Kconfig and there  select
S5P_PM is done for exynos 5250, 4210, 4212 etc. so I used the same
config macro to enable other PM stuffs inside mach-exynos folder.

Thanks,
Amit Daniel
>
> - Kukjin
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH V2 1/2] ARM: SAMSUNG: Add SAMSUNG_PM config option to select pm

[Kukjin Kim]

Amit Daniel Kachhap wrote:
> 
> This patch enables the selection of samsung pm related stuffs
> when SAMSUNG_PM config is enabled and not just when generic PM
> config is enabled. Power management for s3c64XX and s3c24XX
> is enabled by default and for other platform depends on S5P_PM.
> This patch also fixes the following compilation error's when compiling
> a platform like exynos5440 which does not select pm stuffs.
> 
> arch/arm/mach-exynos/built-in.o: In function `__virt_to_phys':
> linux/arch/arm/include/asm/memory.h:175: undefined reference to
> `s3c_cpu_resume'
> linux/arch/arm/include/asm/memory.h:175: undefined reference to
> `s3c_cpu_resume'
> linux/arch/arm/include/asm/memory.h:175: undefined reference to
> `s3c_cpu_resume'
> linux/arch/arm/include/asm/memory.h:175: undefined reference to
> `s3c_cpu_resume'
> arch/arm/mach-exynos/built-in.o: In function `exynos5_init_irq':
> linux/arch/arm/mach-exynos/common.c:492: undefined reference to
> `s3c_irq_wake'
> linux/arch/arm/mach-exynos/common.c:492: undefined reference to
> `s3c_irq_wake'
> arch/arm/mach-exynos/built-in.o: In function `exynos4_init_irq':
> linux/arch/arm/mach-exynos/common.c:476: undefined reference to
> `s3c_irq_wake'
> linux/arch/arm/mach-exynos/common.c:476: undefined reference to
> `s3c_irq_wake'
> arch/arm/plat-samsung/built-in.o: In function `s3c_irqext_wake':
> linux/arch/arm/plat-samsung/pm.c:144: undefined reference to
> `s3c_irqwake_eintallow'
> linux/arch/arm/plat-samsung/pm.c:144: undefined reference to
> `s3c_irqwake_eintallow'
> arch/arm/plat-samsung/built-in.o: In function `s3c_pm_enter':
> linux/arch/arm/plat-samsung/pm.c:263: undefined reference to
> `s3c_irqwake_intallow'
> linux/arch/arm/plat-samsung/pm.c:263: undefined reference to
> `s3c_irqwake_intallow'
> linux/arch/arm/plat-samsung/pm.c:264: undefined reference to
> `s3c_irqwake_eintallow'
> linux/arch/arm/plat-samsung/pm.c:264: undefined reference to
> `s3c_irqwake_eintallow'
> linux/arch/arm/plat-samsung/pm.c:275: undefined reference to
> `s3c_pm_save_core'
> linux/arch/arm/plat-samsung/pm.c:279: undefined reference to
> `s3c_pm_configure_extint'
> linux/arch/arm/plat-samsung/pm.c:310: undefined reference to
> `s3c_pm_restore_core'
> make: *** [vmlinux] Error 1
> 
> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
> ---
>  arch/arm/mach-exynos/Makefile           |    2 +-
>  arch/arm/mach-exynos/common.c           |    2 +-
>  arch/arm/mach-exynos/common.h           |    1 -
>  arch/arm/mach-exynos/cpuidle.c          |    1 +
>  arch/arm/plat-samsung/Kconfig           |    7 +++++++
>  arch/arm/plat-samsung/Makefile          |    3 +--
>  arch/arm/plat-samsung/include/plat/pm.h |    8 ++++----
>  7 files changed, 15 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/arm/mach-exynos/Makefile b/arch/arm/mach-exynos/Makefile
> index 9811f87..3fa277a 100644
> --- a/arch/arm/mach-exynos/Makefile
> +++ b/arch/arm/mach-exynos/Makefile
> @@ -14,7 +14,7 @@ obj-				:=
> 
>  obj-$(CONFIG_ARCH_EXYNOS)	+= common.o
> 
> -obj-$(CONFIG_PM)		+= pm.o
> +obj-$(CONFIG_S5P_PM)		+= pm.o
>  obj-$(CONFIG_PM_GENERIC_DOMAINS) += pm_domains.o
>  obj-$(CONFIG_CPU_IDLE)		+= cpuidle.o
> 
> diff --git a/arch/arm/mach-exynos/common.c b/arch/arm/mach-exynos/common.c
> index 9834357..d2b4f54 100644
> --- a/arch/arm/mach-exynos/common.c
> +++ b/arch/arm/mach-exynos/common.c
> @@ -799,7 +799,7 @@ static struct irq_chip exynos_irq_eint = {
>  	.irq_mask_ack	= exynos_irq_eint_maskack,
>  	.irq_ack	= exynos_irq_eint_ack,
>  	.irq_set_type	= exynos_irq_eint_set_type,
> -#ifdef CONFIG_PM
> +#ifdef CONFIG_S5P_PM
>  	.irq_set_wake	= s3c_irqext_wake,
>  #endif
>  };
> diff --git a/arch/arm/mach-exynos/common.h b/arch/arm/mach-exynos/common.h
> index 11fc1e2..f2b2e23 100644
> --- a/arch/arm/mach-exynos/common.h
> +++ b/arch/arm/mach-exynos/common.h
> @@ -98,6 +98,5 @@ struct exynos_pmu_conf {
>  };
> 
>  extern void exynos_sys_powerdown_conf(enum sys_powerdown mode);
> -extern void s3c_cpu_resume(void);
> 
>  #endif /* __ARCH_ARM_MACH_EXYNOS_COMMON_H */
> diff --git a/arch/arm/mach-exynos/cpuidle.c b/arch/arm/mach-
> exynos/cpuidle.c
> index 4667907..2d071c6 100644
> --- a/arch/arm/mach-exynos/cpuidle.c
> +++ b/arch/arm/mach-exynos/cpuidle.c
> @@ -25,6 +25,7 @@
>  #include <mach/regs-pmu.h>
> 
>  #include <plat/cpu.h>
> +#include <plat/pm.h>
> 
>  #include "common.h"
> 
> diff --git a/arch/arm/plat-samsung/Kconfig b/arch/arm/plat-samsung/Kconfig
> index f8ed2de..6c25aa6 100644
> --- a/arch/arm/plat-samsung/Kconfig
> +++ b/arch/arm/plat-samsung/Kconfig
> @@ -31,6 +31,13 @@ config PLAT_S5P
>  	help
>  	  Base platform code for Samsung's S5P series SoC.
> 
> +config SAMSUNG_PM
> +	bool
> +	depends on PM && (PLAT_S3C24XX || ARCH_S3C64XX || S5P_PM)

As we discussed before, this should be:

+	depends on PM && (PLAT_S3C24XX || ARCH_S3C64XX || ARCH_S5P64X0 ||
S5P_PM)

If not, build error happens with s5p64x0_defconfig. So I fixed.

> +	default y
> +	help
> +	  Base platform power management code for samsung code
> +
>  if PLAT_SAMSUNG
> 
>  # boot configurations
> diff --git a/arch/arm/plat-samsung/Makefile b/arch/arm/plat-
> samsung/Makefile
> index a23c460..1c406b3 100644
> --- a/arch/arm/plat-samsung/Makefile
> +++ b/arch/arm/plat-samsung/Makefile
> @@ -51,8 +51,7 @@ obj-$(CONFIG_SAMSUNG_DMADEV)	+= dma-ops.o
> 
>  # PM support
> 
> -obj-$(CONFIG_PM)		+= pm.o
> -obj-$(CONFIG_PM)		+= pm-gpio.o
> +obj-$(CONFIG_SAMSUNG_PM)	+= pm.o pm-gpio.o
>  obj-$(CONFIG_SAMSUNG_PM_CHECK)	+= pm-check.o
> 
>  obj-$(CONFIG_SAMSUNG_WAKEMASK)	+= wakeup-mask.o
> diff --git a/arch/arm/plat-samsung/include/plat/pm.h b/arch/arm/plat-
> samsung/include/plat/pm.h
> index f6fcade..46e2c82 100644
> --- a/arch/arm/plat-samsung/include/plat/pm.h
> +++ b/arch/arm/plat-samsung/include/plat/pm.h
> @@ -19,7 +19,7 @@
> 
>  struct device;
> 
> -#ifdef CONFIG_PM
> +#ifdef CONFIG_SAMSUNG_PM
> 
>  extern __init int s3c_pm_init(void);
>  extern __init int s3c64xx_pm_init(void);
> @@ -58,8 +58,6 @@ extern unsigned char pm_uart_udivslot;  /* true to save
> UART UDIVSLOT */
> 
>  /* from sleep.S */
> 
> -extern void s3c_cpu_resume(void);
> -
>  extern int s3c2410_cpu_suspend(unsigned long);
> 
>  /* sleep save info */
> @@ -106,12 +104,14 @@ extern void s3c_pm_do_save(struct sleep_save *ptr,
> int count);
>  extern void s3c_pm_do_restore(struct sleep_save *ptr, int count);
>  extern void s3c_pm_do_restore_core(struct sleep_save *ptr, int count);
> 
> -#ifdef CONFIG_PM
> +#ifdef CONFIG_SAMSUNG_PM
>  extern int s3c_irq_wake(struct irq_data *data, unsigned int state);
>  extern int s3c_irqext_wake(struct irq_data *data, unsigned int state);
> +extern void s3c_cpu_resume(void);
>  #else
>  #define s3c_irq_wake NULL
>  #define s3c_irqext_wake NULL
> +#define s3c_cpu_resume NULL
>  #endif
> 
>  /* PM debug functions */
> --
> 1.7.1

Applied into -fixes, thanks.

- Kukjin

[PATCH V2 1/2] ARM: SAMSUNG: Add SAMSUNG_PM config option to select pm

[Kukjin Kim]

Amit Daniel Kachhap wrote:
> 
> This patch enables the selection of samsung pm related stuffs
> when SAMSUNG_PM config is enabled and not just when generic PM
> config is enabled. Power management for s3c64XX and s3c24XX
> is enabled by default and for other platform depends on S5P_PM.
> This patch also fixes the following compilation error's when compiling
> a platform like exynos5440 which does not select pm stuffs.
> 
> arch/arm/mach-exynos/built-in.o: In function `__virt_to_phys':
> linux/arch/arm/include/asm/memory.h:175: undefined reference to
> `s3c_cpu_resume'
> linux/arch/arm/include/asm/memory.h:175: undefined reference to
> `s3c_cpu_resume'
> linux/arch/arm/include/asm/memory.h:175: undefined reference to
> `s3c_cpu_resume'
> linux/arch/arm/include/asm/memory.h:175: undefined reference to
> `s3c_cpu_resume'
> arch/arm/mach-exynos/built-in.o: In function `exynos5_init_irq':
> linux/arch/arm/mach-exynos/common.c:492: undefined reference to
> `s3c_irq_wake'
> linux/arch/arm/mach-exynos/common.c:492: undefined reference to
> `s3c_irq_wake'
> arch/arm/mach-exynos/built-in.o: In function `exynos4_init_irq':
> linux/arch/arm/mach-exynos/common.c:476: undefined reference to
> `s3c_irq_wake'
> linux/arch/arm/mach-exynos/common.c:476: undefined reference to
> `s3c_irq_wake'
> arch/arm/plat-samsung/built-in.o: In function `s3c_irqext_wake':
> linux/arch/arm/plat-samsung/pm.c:144: undefined reference to
> `s3c_irqwake_eintallow'
> linux/arch/arm/plat-samsung/pm.c:144: undefined reference to
> `s3c_irqwake_eintallow'
> arch/arm/plat-samsung/built-in.o: In function `s3c_pm_enter':
> linux/arch/arm/plat-samsung/pm.c:263: undefined reference to
> `s3c_irqwake_intallow'
> linux/arch/arm/plat-samsung/pm.c:263: undefined reference to
> `s3c_irqwake_intallow'
> linux/arch/arm/plat-samsung/pm.c:264: undefined reference to
> `s3c_irqwake_eintallow'
> linux/arch/arm/plat-samsung/pm.c:264: undefined reference to
> `s3c_irqwake_eintallow'
> linux/arch/arm/plat-samsung/pm.c:275: undefined reference to
> `s3c_pm_save_core'
> linux/arch/arm/plat-samsung/pm.c:279: undefined reference to
> `s3c_pm_configure_extint'
> linux/arch/arm/plat-samsung/pm.c:310: undefined reference to
> `s3c_pm_restore_core'
> make: *** [vmlinux] Error 1
> 
> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
> ---
>  arch/arm/mach-exynos/Makefile           |    2 +-
>  arch/arm/mach-exynos/common.c           |    2 +-
>  arch/arm/mach-exynos/common.h           |    1 -
>  arch/arm/mach-exynos/cpuidle.c          |    1 +
>  arch/arm/plat-samsung/Kconfig           |    7 +++++++
>  arch/arm/plat-samsung/Makefile          |    3 +--
>  arch/arm/plat-samsung/include/plat/pm.h |    8 ++++----
>  7 files changed, 15 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/arm/mach-exynos/Makefile b/arch/arm/mach-exynos/Makefile
> index 9811f87..3fa277a 100644
> --- a/arch/arm/mach-exynos/Makefile
> +++ b/arch/arm/mach-exynos/Makefile
> @@ -14,7 +14,7 @@ obj-				:=
> 
>  obj-$(CONFIG_ARCH_EXYNOS)	+= common.o
> 
> -obj-$(CONFIG_PM)		+= pm.o
> +obj-$(CONFIG_S5P_PM)		+= pm.o
>  obj-$(CONFIG_PM_GENERIC_DOMAINS) += pm_domains.o
>  obj-$(CONFIG_CPU_IDLE)		+= cpuidle.o
> 
> diff --git a/arch/arm/mach-exynos/common.c b/arch/arm/mach-exynos/common.c
> index 9834357..d2b4f54 100644
> --- a/arch/arm/mach-exynos/common.c
> +++ b/arch/arm/mach-exynos/common.c
> @@ -799,7 +799,7 @@ static struct irq_chip exynos_irq_eint = {
>  	.irq_mask_ack	= exynos_irq_eint_maskack,
>  	.irq_ack	= exynos_irq_eint_ack,
>  	.irq_set_type	= exynos_irq_eint_set_type,
> -#ifdef CONFIG_PM
> +#ifdef CONFIG_S5P_PM
>  	.irq_set_wake	= s3c_irqext_wake,
>  #endif
>  };
> diff --git a/arch/arm/mach-exynos/common.h b/arch/arm/mach-exynos/common.h
> index 11fc1e2..f2b2e23 100644
> --- a/arch/arm/mach-exynos/common.h
> +++ b/arch/arm/mach-exynos/common.h
> @@ -98,6 +98,5 @@ struct exynos_pmu_conf {
>  };
> 
>  extern void exynos_sys_powerdown_conf(enum sys_powerdown mode);
> -extern void s3c_cpu_resume(void);
> 
>  #endif /* __ARCH_ARM_MACH_EXYNOS_COMMON_H */
> diff --git a/arch/arm/mach-exynos/cpuidle.c b/arch/arm/mach-
> exynos/cpuidle.c
> index 4667907..2d071c6 100644
> --- a/arch/arm/mach-exynos/cpuidle.c
> +++ b/arch/arm/mach-exynos/cpuidle.c
> @@ -25,6 +25,7 @@
>  #include <mach/regs-pmu.h>
> 
>  #include <plat/cpu.h>
> +#include <plat/pm.h>
> 
>  #include "common.h"
> 
> diff --git a/arch/arm/plat-samsung/Kconfig b/arch/arm/plat-samsung/Kconfig
> index f8ed2de..6c25aa6 100644
> --- a/arch/arm/plat-samsung/Kconfig
> +++ b/arch/arm/plat-samsung/Kconfig
> @@ -31,6 +31,13 @@ config PLAT_S5P
>  	help
>  	  Base platform code for Samsung's S5P series SoC.
> 
> +config SAMSUNG_PM
> +	bool
> +	depends on PM && (PLAT_S3C24XX || ARCH_S3C64XX || S5P_PM)

As we discussed before, this should be:

+	depends on PM && (PLAT_S3C24XX || ARCH_S3C64XX || ARCH_S5P64X0 ||
S5P_PM)

If not, build error happens with s5p64x0_defconfig. So I fixed.

> +	default y
> +	help
> +	  Base platform power management code for samsung code
> +
>  if PLAT_SAMSUNG
> 
>  # boot configurations
> diff --git a/arch/arm/plat-samsung/Makefile b/arch/arm/plat-
> samsung/Makefile
> index a23c460..1c406b3 100644
> --- a/arch/arm/plat-samsung/Makefile
> +++ b/arch/arm/plat-samsung/Makefile
> @@ -51,8 +51,7 @@ obj-$(CONFIG_SAMSUNG_DMADEV)	+= dma-ops.o
> 
>  # PM support
> 
> -obj-$(CONFIG_PM)		+= pm.o
> -obj-$(CONFIG_PM)		+= pm-gpio.o
> +obj-$(CONFIG_SAMSUNG_PM)	+= pm.o pm-gpio.o
>  obj-$(CONFIG_SAMSUNG_PM_CHECK)	+= pm-check.o
> 
>  obj-$(CONFIG_SAMSUNG_WAKEMASK)	+= wakeup-mask.o
> diff --git a/arch/arm/plat-samsung/include/plat/pm.h b/arch/arm/plat-
> samsung/include/plat/pm.h
> index f6fcade..46e2c82 100644
> --- a/arch/arm/plat-samsung/include/plat/pm.h
> +++ b/arch/arm/plat-samsung/include/plat/pm.h
> @@ -19,7 +19,7 @@
> 
>  struct device;
> 
> -#ifdef CONFIG_PM
> +#ifdef CONFIG_SAMSUNG_PM
> 
>  extern __init int s3c_pm_init(void);
>  extern __init int s3c64xx_pm_init(void);
> @@ -58,8 +58,6 @@ extern unsigned char pm_uart_udivslot;  /* true to save
> UART UDIVSLOT */
> 
>  /* from sleep.S */
> 
> -extern void s3c_cpu_resume(void);
> -
>  extern int s3c2410_cpu_suspend(unsigned long);
> 
>  /* sleep save info */
> @@ -106,12 +104,14 @@ extern void s3c_pm_do_save(struct sleep_save *ptr,
> int count);
>  extern void s3c_pm_do_restore(struct sleep_save *ptr, int count);
>  extern void s3c_pm_do_restore_core(struct sleep_save *ptr, int count);
> 
> -#ifdef CONFIG_PM
> +#ifdef CONFIG_SAMSUNG_PM
>  extern int s3c_irq_wake(struct irq_data *data, unsigned int state);
>  extern int s3c_irqext_wake(struct irq_data *data, unsigned int state);
> +extern void s3c_cpu_resume(void);
>  #else
>  #define s3c_irq_wake NULL
>  #define s3c_irqext_wake NULL
> +#define s3c_cpu_resume NULL
>  #endif
> 
>  /* PM debug functions */
> --
> 1.7.1

Applied into -fixes, thanks.

- Kukjin

Re: [PATCH V2 1/2] ARM: SAMSUNG: Add SAMSUNG_PM config option to select pm

[amit daniel kachhap]

On Wed, Jul 24, 2013 at 10:24 AM, Kukjin Kim <kgene@kernel.org> wrote:
> Amit Daniel Kachhap wrote:
>>
>> This patch enables the selection of samsung pm related stuffs
>> when SAMSUNG_PM config is enabled and not just when generic PM
>> config is enabled. Power management for s3c64XX and s3c24XX
>> is enabled by default and for other platform depends on S5P_PM.
>> This patch also fixes the following compilation error's when compiling
>> a platform like exynos5440 which does not select pm stuffs.
>>
>> arch/arm/mach-exynos/built-in.o: In function `__virt_to_phys':
>> linux/arch/arm/include/asm/memory.h:175: undefined reference to
>> `s3c_cpu_resume'
>> linux/arch/arm/include/asm/memory.h:175: undefined reference to
>> `s3c_cpu_resume'
>> linux/arch/arm/include/asm/memory.h:175: undefined reference to
>> `s3c_cpu_resume'
>> linux/arch/arm/include/asm/memory.h:175: undefined reference to
>> `s3c_cpu_resume'
>> arch/arm/mach-exynos/built-in.o: In function `exynos5_init_irq':
>> linux/arch/arm/mach-exynos/common.c:492: undefined reference to
>> `s3c_irq_wake'
>> linux/arch/arm/mach-exynos/common.c:492: undefined reference to
>> `s3c_irq_wake'
>> arch/arm/mach-exynos/built-in.o: In function `exynos4_init_irq':
>> linux/arch/arm/mach-exynos/common.c:476: undefined reference to
>> `s3c_irq_wake'
>> linux/arch/arm/mach-exynos/common.c:476: undefined reference to
>> `s3c_irq_wake'
>> arch/arm/plat-samsung/built-in.o: In function `s3c_irqext_wake':
>> linux/arch/arm/plat-samsung/pm.c:144: undefined reference to
>> `s3c_irqwake_eintallow'
>> linux/arch/arm/plat-samsung/pm.c:144: undefined reference to
>> `s3c_irqwake_eintallow'
>> arch/arm/plat-samsung/built-in.o: In function `s3c_pm_enter':
>> linux/arch/arm/plat-samsung/pm.c:263: undefined reference to
>> `s3c_irqwake_intallow'
>> linux/arch/arm/plat-samsung/pm.c:263: undefined reference to
>> `s3c_irqwake_intallow'
>> linux/arch/arm/plat-samsung/pm.c:264: undefined reference to
>> `s3c_irqwake_eintallow'
>> linux/arch/arm/plat-samsung/pm.c:264: undefined reference to
>> `s3c_irqwake_eintallow'
>> linux/arch/arm/plat-samsung/pm.c:275: undefined reference to
>> `s3c_pm_save_core'
>> linux/arch/arm/plat-samsung/pm.c:279: undefined reference to
>> `s3c_pm_configure_extint'
>> linux/arch/arm/plat-samsung/pm.c:310: undefined reference to
>> `s3c_pm_restore_core'
>> make: *** [vmlinux] Error 1
>>
>> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
>> ---
>>  arch/arm/mach-exynos/Makefile           |    2 +-
>>  arch/arm/mach-exynos/common.c           |    2 +-
>>  arch/arm/mach-exynos/common.h           |    1 -
>>  arch/arm/mach-exynos/cpuidle.c          |    1 +
>>  arch/arm/plat-samsung/Kconfig           |    7 +++++++
>>  arch/arm/plat-samsung/Makefile          |    3 +--
>>  arch/arm/plat-samsung/include/plat/pm.h |    8 ++++----
>>  7 files changed, 15 insertions(+), 9 deletions(-)
>>
>> diff --git a/arch/arm/mach-exynos/Makefile b/arch/arm/mach-exynos/Makefile
>> index 9811f87..3fa277a 100644
>> --- a/arch/arm/mach-exynos/Makefile
>> +++ b/arch/arm/mach-exynos/Makefile
>> @@ -14,7 +14,7 @@ obj-                                :=
>>
>>  obj-$(CONFIG_ARCH_EXYNOS)    += common.o
>>
>> -obj-$(CONFIG_PM)             += pm.o
>> +obj-$(CONFIG_S5P_PM)         += pm.o
>>  obj-$(CONFIG_PM_GENERIC_DOMAINS) += pm_domains.o
>>  obj-$(CONFIG_CPU_IDLE)               += cpuidle.o
>>
>> diff --git a/arch/arm/mach-exynos/common.c b/arch/arm/mach-exynos/common.c
>> index 9834357..d2b4f54 100644
>> --- a/arch/arm/mach-exynos/common.c
>> +++ b/arch/arm/mach-exynos/common.c
>> @@ -799,7 +799,7 @@ static struct irq_chip exynos_irq_eint = {
>>       .irq_mask_ack   = exynos_irq_eint_maskack,
>>       .irq_ack        = exynos_irq_eint_ack,
>>       .irq_set_type   = exynos_irq_eint_set_type,
>> -#ifdef CONFIG_PM
>> +#ifdef CONFIG_S5P_PM
>>       .irq_set_wake   = s3c_irqext_wake,
>>  #endif
>>  };
>> diff --git a/arch/arm/mach-exynos/common.h b/arch/arm/mach-exynos/common.h
>> index 11fc1e2..f2b2e23 100644
>> --- a/arch/arm/mach-exynos/common.h
>> +++ b/arch/arm/mach-exynos/common.h
>> @@ -98,6 +98,5 @@ struct exynos_pmu_conf {
>>  };
>>
>>  extern void exynos_sys_powerdown_conf(enum sys_powerdown mode);
>> -extern void s3c_cpu_resume(void);
>>
>>  #endif /* __ARCH_ARM_MACH_EXYNOS_COMMON_H */
>> diff --git a/arch/arm/mach-exynos/cpuidle.c b/arch/arm/mach-
>> exynos/cpuidle.c
>> index 4667907..2d071c6 100644
>> --- a/arch/arm/mach-exynos/cpuidle.c
>> +++ b/arch/arm/mach-exynos/cpuidle.c
>> @@ -25,6 +25,7 @@
>>  #include <mach/regs-pmu.h>
>>
>>  #include <plat/cpu.h>
>> +#include <plat/pm.h>
>>
>>  #include "common.h"
>>
>> diff --git a/arch/arm/plat-samsung/Kconfig b/arch/arm/plat-samsung/Kconfig
>> index f8ed2de..6c25aa6 100644
>> --- a/arch/arm/plat-samsung/Kconfig
>> +++ b/arch/arm/plat-samsung/Kconfig
>> @@ -31,6 +31,13 @@ config PLAT_S5P
>>       help
>>         Base platform code for Samsung's S5P series SoC.
>>
>> +config SAMSUNG_PM
>> +     bool
>> +     depends on PM && (PLAT_S3C24XX || ARCH_S3C64XX || S5P_PM)
>
> As we discussed before, this should be:
>
> +       depends on PM && (PLAT_S3C24XX || ARCH_S3C64XX || ARCH_S5P64X0 ||
> S5P_PM)
>
> If not, build error happens with s5p64x0_defconfig. So I fixed.
Ok, Thanks
>
>> +     default y
>> +     help
>> +       Base platform power management code for samsung code
>> +
>>  if PLAT_SAMSUNG
>>
>>  # boot configurations
>> diff --git a/arch/arm/plat-samsung/Makefile b/arch/arm/plat-
>> samsung/Makefile
>> index a23c460..1c406b3 100644
>> --- a/arch/arm/plat-samsung/Makefile
>> +++ b/arch/arm/plat-samsung/Makefile
>> @@ -51,8 +51,7 @@ obj-$(CONFIG_SAMSUNG_DMADEV)        += dma-ops.o
>>
>>  # PM support
>>
>> -obj-$(CONFIG_PM)             += pm.o
>> -obj-$(CONFIG_PM)             += pm-gpio.o
>> +obj-$(CONFIG_SAMSUNG_PM)     += pm.o pm-gpio.o
>>  obj-$(CONFIG_SAMSUNG_PM_CHECK)       += pm-check.o
>>
>>  obj-$(CONFIG_SAMSUNG_WAKEMASK)       += wakeup-mask.o
>> diff --git a/arch/arm/plat-samsung/include/plat/pm.h b/arch/arm/plat-
>> samsung/include/plat/pm.h
>> index f6fcade..46e2c82 100644
>> --- a/arch/arm/plat-samsung/include/plat/pm.h
>> +++ b/arch/arm/plat-samsung/include/plat/pm.h
>> @@ -19,7 +19,7 @@
>>
>>  struct device;
>>
>> -#ifdef CONFIG_PM
>> +#ifdef CONFIG_SAMSUNG_PM
>>
>>  extern __init int s3c_pm_init(void);
>>  extern __init int s3c64xx_pm_init(void);
>> @@ -58,8 +58,6 @@ extern unsigned char pm_uart_udivslot;  /* true to save
>> UART UDIVSLOT */
>>
>>  /* from sleep.S */
>>
>> -extern void s3c_cpu_resume(void);
>> -
>>  extern int s3c2410_cpu_suspend(unsigned long);
>>
>>  /* sleep save info */
>> @@ -106,12 +104,14 @@ extern void s3c_pm_do_save(struct sleep_save *ptr,
>> int count);
>>  extern void s3c_pm_do_restore(struct sleep_save *ptr, int count);
>>  extern void s3c_pm_do_restore_core(struct sleep_save *ptr, int count);
>>
>> -#ifdef CONFIG_PM
>> +#ifdef CONFIG_SAMSUNG_PM
>>  extern int s3c_irq_wake(struct irq_data *data, unsigned int state);
>>  extern int s3c_irqext_wake(struct irq_data *data, unsigned int state);
>> +extern void s3c_cpu_resume(void);
>>  #else
>>  #define s3c_irq_wake NULL
>>  #define s3c_irqext_wake NULL
>> +#define s3c_cpu_resume NULL
>>  #endif
>>
>>  /* PM debug functions */
>> --
>> 1.7.1
>
> Applied into -fixes, thanks.
>
> - Kukjin
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH V2 1/2] ARM: SAMSUNG: Add SAMSUNG_PM config option to select pm

[amit daniel kachhap]

On Wed, Jul 24, 2013 at 10:24 AM, Kukjin Kim <kgene@kernel.org> wrote:
> Amit Daniel Kachhap wrote:
>>
>> This patch enables the selection of samsung pm related stuffs
>> when SAMSUNG_PM config is enabled and not just when generic PM
>> config is enabled. Power management for s3c64XX and s3c24XX
>> is enabled by default and for other platform depends on S5P_PM.
>> This patch also fixes the following compilation error's when compiling
>> a platform like exynos5440 which does not select pm stuffs.
>>
>> arch/arm/mach-exynos/built-in.o: In function `__virt_to_phys':
>> linux/arch/arm/include/asm/memory.h:175: undefined reference to
>> `s3c_cpu_resume'
>> linux/arch/arm/include/asm/memory.h:175: undefined reference to
>> `s3c_cpu_resume'
>> linux/arch/arm/include/asm/memory.h:175: undefined reference to
>> `s3c_cpu_resume'
>> linux/arch/arm/include/asm/memory.h:175: undefined reference to
>> `s3c_cpu_resume'
>> arch/arm/mach-exynos/built-in.o: In function `exynos5_init_irq':
>> linux/arch/arm/mach-exynos/common.c:492: undefined reference to
>> `s3c_irq_wake'
>> linux/arch/arm/mach-exynos/common.c:492: undefined reference to
>> `s3c_irq_wake'
>> arch/arm/mach-exynos/built-in.o: In function `exynos4_init_irq':
>> linux/arch/arm/mach-exynos/common.c:476: undefined reference to
>> `s3c_irq_wake'
>> linux/arch/arm/mach-exynos/common.c:476: undefined reference to
>> `s3c_irq_wake'
>> arch/arm/plat-samsung/built-in.o: In function `s3c_irqext_wake':
>> linux/arch/arm/plat-samsung/pm.c:144: undefined reference to
>> `s3c_irqwake_eintallow'
>> linux/arch/arm/plat-samsung/pm.c:144: undefined reference to
>> `s3c_irqwake_eintallow'
>> arch/arm/plat-samsung/built-in.o: In function `s3c_pm_enter':
>> linux/arch/arm/plat-samsung/pm.c:263: undefined reference to
>> `s3c_irqwake_intallow'
>> linux/arch/arm/plat-samsung/pm.c:263: undefined reference to
>> `s3c_irqwake_intallow'
>> linux/arch/arm/plat-samsung/pm.c:264: undefined reference to
>> `s3c_irqwake_eintallow'
>> linux/arch/arm/plat-samsung/pm.c:264: undefined reference to
>> `s3c_irqwake_eintallow'
>> linux/arch/arm/plat-samsung/pm.c:275: undefined reference to
>> `s3c_pm_save_core'
>> linux/arch/arm/plat-samsung/pm.c:279: undefined reference to
>> `s3c_pm_configure_extint'
>> linux/arch/arm/plat-samsung/pm.c:310: undefined reference to
>> `s3c_pm_restore_core'
>> make: *** [vmlinux] Error 1
>>
>> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
>> ---
>>  arch/arm/mach-exynos/Makefile           |    2 +-
>>  arch/arm/mach-exynos/common.c           |    2 +-
>>  arch/arm/mach-exynos/common.h           |    1 -
>>  arch/arm/mach-exynos/cpuidle.c          |    1 +
>>  arch/arm/plat-samsung/Kconfig           |    7 +++++++
>>  arch/arm/plat-samsung/Makefile          |    3 +--
>>  arch/arm/plat-samsung/include/plat/pm.h |    8 ++++----
>>  7 files changed, 15 insertions(+), 9 deletions(-)
>>
>> diff --git a/arch/arm/mach-exynos/Makefile b/arch/arm/mach-exynos/Makefile
>> index 9811f87..3fa277a 100644
>> --- a/arch/arm/mach-exynos/Makefile
>> +++ b/arch/arm/mach-exynos/Makefile
>> @@ -14,7 +14,7 @@ obj-                                :=
>>
>>  obj-$(CONFIG_ARCH_EXYNOS)    += common.o
>>
>> -obj-$(CONFIG_PM)             += pm.o
>> +obj-$(CONFIG_S5P_PM)         += pm.o
>>  obj-$(CONFIG_PM_GENERIC_DOMAINS) += pm_domains.o
>>  obj-$(CONFIG_CPU_IDLE)               += cpuidle.o
>>
>> diff --git a/arch/arm/mach-exynos/common.c b/arch/arm/mach-exynos/common.c
>> index 9834357..d2b4f54 100644
>> --- a/arch/arm/mach-exynos/common.c
>> +++ b/arch/arm/mach-exynos/common.c
>> @@ -799,7 +799,7 @@ static struct irq_chip exynos_irq_eint = {
>>       .irq_mask_ack   = exynos_irq_eint_maskack,
>>       .irq_ack        = exynos_irq_eint_ack,
>>       .irq_set_type   = exynos_irq_eint_set_type,
>> -#ifdef CONFIG_PM
>> +#ifdef CONFIG_S5P_PM
>>       .irq_set_wake   = s3c_irqext_wake,
>>  #endif
>>  };
>> diff --git a/arch/arm/mach-exynos/common.h b/arch/arm/mach-exynos/common.h
>> index 11fc1e2..f2b2e23 100644
>> --- a/arch/arm/mach-exynos/common.h
>> +++ b/arch/arm/mach-exynos/common.h
>> @@ -98,6 +98,5 @@ struct exynos_pmu_conf {
>>  };
>>
>>  extern void exynos_sys_powerdown_conf(enum sys_powerdown mode);
>> -extern void s3c_cpu_resume(void);
>>
>>  #endif /* __ARCH_ARM_MACH_EXYNOS_COMMON_H */
>> diff --git a/arch/arm/mach-exynos/cpuidle.c b/arch/arm/mach-
>> exynos/cpuidle.c
>> index 4667907..2d071c6 100644
>> --- a/arch/arm/mach-exynos/cpuidle.c
>> +++ b/arch/arm/mach-exynos/cpuidle.c
>> @@ -25,6 +25,7 @@
>>  #include <mach/regs-pmu.h>
>>
>>  #include <plat/cpu.h>
>> +#include <plat/pm.h>
>>
>>  #include "common.h"
>>
>> diff --git a/arch/arm/plat-samsung/Kconfig b/arch/arm/plat-samsung/Kconfig
>> index f8ed2de..6c25aa6 100644
>> --- a/arch/arm/plat-samsung/Kconfig
>> +++ b/arch/arm/plat-samsung/Kconfig
>> @@ -31,6 +31,13 @@ config PLAT_S5P
>>       help
>>         Base platform code for Samsung's S5P series SoC.
>>
>> +config SAMSUNG_PM
>> +     bool
>> +     depends on PM && (PLAT_S3C24XX || ARCH_S3C64XX || S5P_PM)
>
> As we discussed before, this should be:
>
> +       depends on PM && (PLAT_S3C24XX || ARCH_S3C64XX || ARCH_S5P64X0 ||
> S5P_PM)
>
> If not, build error happens with s5p64x0_defconfig. So I fixed.
Ok, Thanks
>
>> +     default y
>> +     help
>> +       Base platform power management code for samsung code
>> +
>>  if PLAT_SAMSUNG
>>
>>  # boot configurations
>> diff --git a/arch/arm/plat-samsung/Makefile b/arch/arm/plat-
>> samsung/Makefile
>> index a23c460..1c406b3 100644
>> --- a/arch/arm/plat-samsung/Makefile
>> +++ b/arch/arm/plat-samsung/Makefile
>> @@ -51,8 +51,7 @@ obj-$(CONFIG_SAMSUNG_DMADEV)        += dma-ops.o
>>
>>  # PM support
>>
>> -obj-$(CONFIG_PM)             += pm.o
>> -obj-$(CONFIG_PM)             += pm-gpio.o
>> +obj-$(CONFIG_SAMSUNG_PM)     += pm.o pm-gpio.o
>>  obj-$(CONFIG_SAMSUNG_PM_CHECK)       += pm-check.o
>>
>>  obj-$(CONFIG_SAMSUNG_WAKEMASK)       += wakeup-mask.o
>> diff --git a/arch/arm/plat-samsung/include/plat/pm.h b/arch/arm/plat-
>> samsung/include/plat/pm.h
>> index f6fcade..46e2c82 100644
>> --- a/arch/arm/plat-samsung/include/plat/pm.h
>> +++ b/arch/arm/plat-samsung/include/plat/pm.h
>> @@ -19,7 +19,7 @@
>>
>>  struct device;
>>
>> -#ifdef CONFIG_PM
>> +#ifdef CONFIG_SAMSUNG_PM
>>
>>  extern __init int s3c_pm_init(void);
>>  extern __init int s3c64xx_pm_init(void);
>> @@ -58,8 +58,6 @@ extern unsigned char pm_uart_udivslot;  /* true to save
>> UART UDIVSLOT */
>>
>>  /* from sleep.S */
>>
>> -extern void s3c_cpu_resume(void);
>> -
>>  extern int s3c2410_cpu_suspend(unsigned long);
>>
>>  /* sleep save info */
>> @@ -106,12 +104,14 @@ extern void s3c_pm_do_save(struct sleep_save *ptr,
>> int count);
>>  extern void s3c_pm_do_restore(struct sleep_save *ptr, int count);
>>  extern void s3c_pm_do_restore_core(struct sleep_save *ptr, int count);
>>
>> -#ifdef CONFIG_PM
>> +#ifdef CONFIG_SAMSUNG_PM
>>  extern int s3c_irq_wake(struct irq_data *data, unsigned int state);
>>  extern int s3c_irqext_wake(struct irq_data *data, unsigned int state);
>> +extern void s3c_cpu_resume(void);
>>  #else
>>  #define s3c_irq_wake NULL
>>  #define s3c_irqext_wake NULL
>> +#define s3c_cpu_resume NULL
>>  #endif
>>
>>  /* PM debug functions */
>> --
>> 1.7.1
>
> Applied into -fixes, thanks.
>
> - Kukjin
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH V2 2/2] ARM: EXYNOS: cpuidle: Allow C1 state only in supported SOC's.

[Amit Daniel Kachhap]

This patch registers the basic C0 state for all exynos SOC's but
limits the C1(AFTR -Arm off top running) state in only the supported
SOC's(ie. EXYNOS 4210, 4212, 4412 and 5250).

Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
---
 arch/arm/mach-exynos/cpuidle.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-exynos/cpuidle.c b/arch/arm/mach-exynos/cpuidle.c
index 2d071c6..ccb2b48 100644
--- a/arch/arm/mach-exynos/cpuidle.c
+++ b/arch/arm/mach-exynos/cpuidle.c
@@ -176,7 +176,9 @@ static int __init exynos4_init_cpuidle(void)
 		device->cpu = cpu_id;
 
 		/* Support IDLE only */
-		if (cpu_id != 0)
+		if (!(soc_is_exynos4210() || soc_is_exynos4212() ||
+			soc_is_exynos4412() || soc_is_exynos5250()) ||
+			cpu_id != 0)
 			device->state_count = 1;
 
 		ret = cpuidle_register_device(device);
-- 
1.7.1

[PATCH V2 2/2] ARM: EXYNOS: cpuidle: Allow C1 state only in supported SOC's.

[Amit Daniel Kachhap]

This patch registers the basic C0 state for all exynos SOC's but
limits the C1(AFTR -Arm off top running) state in only the supported
SOC's(ie. EXYNOS 4210, 4212, 4412 and 5250).

Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
---
 arch/arm/mach-exynos/cpuidle.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-exynos/cpuidle.c b/arch/arm/mach-exynos/cpuidle.c
index 2d071c6..ccb2b48 100644
--- a/arch/arm/mach-exynos/cpuidle.c
+++ b/arch/arm/mach-exynos/cpuidle.c
@@ -176,7 +176,9 @@ static int __init exynos4_init_cpuidle(void)
 		device->cpu = cpu_id;
 
 		/* Support IDLE only */
-		if (cpu_id != 0)
+		if (!(soc_is_exynos4210() || soc_is_exynos4212() ||
+			soc_is_exynos4412() || soc_is_exynos5250()) ||
+			cpu_id != 0)
 			device->state_count = 1;
 
 		ret = cpuidle_register_device(device);
-- 
1.7.1

RE: [PATCH V2 2/2] ARM: EXYNOS: cpuidle: Allow C1 state only in supported SOC's.

[Kukjin Kim]

Amit Daniel Kachhap wrote:
> 
> This patch registers the basic C0 state for all exynos SOC's but
> limits the C1(AFTR -Arm off top running) state in only the supported
> SOC's(ie. EXYNOS 4210, 4212, 4412 and 5250).
> 
> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
> ---
>  arch/arm/mach-exynos/cpuidle.c |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm/mach-exynos/cpuidle.c b/arch/arm/mach-
> exynos/cpuidle.c
> index 2d071c6..ccb2b48 100644
> --- a/arch/arm/mach-exynos/cpuidle.c
> +++ b/arch/arm/mach-exynos/cpuidle.c
> @@ -176,7 +176,9 @@ static int __init exynos4_init_cpuidle(void)
>  		device->cpu = cpu_id;
> 
>  		/* Support IDLE only */
> -		if (cpu_id != 0)
> +		if (!(soc_is_exynos4210() || soc_is_exynos4212() ||
> +			soc_is_exynos4412() || soc_is_exynos5250()) ||
> +			cpu_id != 0)

How about exynos5420?

So...

+		if (soc_is_exynos5440() || cpu_id !=0) ?

>  			device->state_count = 1;
> 
>  		ret = cpuidle_register_device(device);
> --
> 1.7.1

- Kukjin

[PATCH V2 2/2] ARM: EXYNOS: cpuidle: Allow C1 state only in supported SOC's.

[Kukjin Kim]

Amit Daniel Kachhap wrote:
> 
> This patch registers the basic C0 state for all exynos SOC's but
> limits the C1(AFTR -Arm off top running) state in only the supported
> SOC's(ie. EXYNOS 4210, 4212, 4412 and 5250).
> 
> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
> ---
>  arch/arm/mach-exynos/cpuidle.c |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm/mach-exynos/cpuidle.c b/arch/arm/mach-
> exynos/cpuidle.c
> index 2d071c6..ccb2b48 100644
> --- a/arch/arm/mach-exynos/cpuidle.c
> +++ b/arch/arm/mach-exynos/cpuidle.c
> @@ -176,7 +176,9 @@ static int __init exynos4_init_cpuidle(void)
>  		device->cpu = cpu_id;
> 
>  		/* Support IDLE only */
> -		if (cpu_id != 0)
> +		if (!(soc_is_exynos4210() || soc_is_exynos4212() ||
> +			soc_is_exynos4412() || soc_is_exynos5250()) ||
> +			cpu_id != 0)

How about exynos5420?

So...

+		if (soc_is_exynos5440() || cpu_id !=0) ?

>  			device->state_count = 1;
> 
>  		ret = cpuidle_register_device(device);
> --
> 1.7.1

- Kukjin

Re: [PATCH V2 2/2] ARM: EXYNOS: cpuidle: Allow C1 state only in supported SOC's.

[amit daniel kachhap]

Hi,

On Wed, Jul 24, 2013 at 10:45 AM, Kukjin Kim <kgene@kernel.org> wrote:
> Amit Daniel Kachhap wrote:
>>
>> This patch registers the basic C0 state for all exynos SOC's but
>> limits the C1(AFTR -Arm off top running) state in only the supported
>> SOC's(ie. EXYNOS 4210, 4212, 4412 and 5250).
>>
>> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
>> ---
>>  arch/arm/mach-exynos/cpuidle.c |    4 +++-
>>  1 files changed, 3 insertions(+), 1 deletions(-)
>>
>> diff --git a/arch/arm/mach-exynos/cpuidle.c b/arch/arm/mach-
>> exynos/cpuidle.c
>> index 2d071c6..ccb2b48 100644
>> --- a/arch/arm/mach-exynos/cpuidle.c
>> +++ b/arch/arm/mach-exynos/cpuidle.c
>> @@ -176,7 +176,9 @@ static int __init exynos4_init_cpuidle(void)
>>               device->cpu = cpu_id;
>>
>>               /* Support IDLE only */
>> -             if (cpu_id != 0)
>> +             if (!(soc_is_exynos4210() || soc_is_exynos4212() ||
>> +                     soc_is_exynos4412() || soc_is_exynos5250()) ||
>> +                     cpu_id != 0)
>
> How about exynos5420?
>
> So...
>
> +               if (soc_is_exynos5440() || cpu_id !=0) ?
This is fine.

Thanks,
Amit
>
>>                       device->state_count = 1;
>>
>>               ret = cpuidle_register_device(device);
>> --
>> 1.7.1
>
> - Kukjin
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH V2 2/2] ARM: EXYNOS: cpuidle: Allow C1 state only in supported SOC's.

[amit daniel kachhap]

Hi,

On Wed, Jul 24, 2013 at 10:45 AM, Kukjin Kim <kgene@kernel.org> wrote:
> Amit Daniel Kachhap wrote:
>>
>> This patch registers the basic C0 state for all exynos SOC's but
>> limits the C1(AFTR -Arm off top running) state in only the supported
>> SOC's(ie. EXYNOS 4210, 4212, 4412 and 5250).
>>
>> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
>> ---
>>  arch/arm/mach-exynos/cpuidle.c |    4 +++-
>>  1 files changed, 3 insertions(+), 1 deletions(-)
>>
>> diff --git a/arch/arm/mach-exynos/cpuidle.c b/arch/arm/mach-
>> exynos/cpuidle.c
>> index 2d071c6..ccb2b48 100644
>> --- a/arch/arm/mach-exynos/cpuidle.c
>> +++ b/arch/arm/mach-exynos/cpuidle.c
>> @@ -176,7 +176,9 @@ static int __init exynos4_init_cpuidle(void)
>>               device->cpu = cpu_id;
>>
>>               /* Support IDLE only */
>> -             if (cpu_id != 0)
>> +             if (!(soc_is_exynos4210() || soc_is_exynos4212() ||
>> +                     soc_is_exynos4412() || soc_is_exynos5250()) ||
>> +                     cpu_id != 0)
>
> How about exynos5420?
>
> So...
>
> +               if (soc_is_exynos5440() || cpu_id !=0) ?
This is fine.

Thanks,
Amit
>
>>                       device->state_count = 1;
>>
>>               ret = cpuidle_register_device(device);
>> --
>> 1.7.1
>
> - Kukjin
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH V2 2/2] ARM: EXYNOS: cpuidle: Allow C1 state only in supported SOC's.

[Kukjin Kim]

amit daniel kachhap wrote:

[...]

> >>               /* Support IDLE only */
> >> -             if (cpu_id != 0)
> >> +             if (!(soc_is_exynos4210() || soc_is_exynos4212() ||
> >> +                     soc_is_exynos4412() || soc_is_exynos5250()) ||
> >> +                     cpu_id != 0)
> >
> > How about exynos5420?
> >
> > So...
> >
> > +               if (soc_is_exynos5440() || cpu_id !=0) ?
> This is fine.
> 
So, you will re-send?

- Kukjin

[PATCH V2 2/2] ARM: EXYNOS: cpuidle: Allow C1 state only in supported SOC's.

[Kukjin Kim]

amit daniel kachhap wrote:

[...]

> >>               /* Support IDLE only */
> >> -             if (cpu_id != 0)
> >> +             if (!(soc_is_exynos4210() || soc_is_exynos4212() ||
> >> +                     soc_is_exynos4412() || soc_is_exynos5250()) ||
> >> +                     cpu_id != 0)
> >
> > How about exynos5420?
> >
> > So...
> >
> > +               if (soc_is_exynos5440() || cpu_id !=0) ?
> This is fine.
> 
So, you will re-send?

- Kukjin

[PATCH] ARM: EXYNOS: cpuidle: Skip C1 cpuidle state for exynos5440

[Amit Daniel Kachhap]

This patch skips the deep C1(AFTR -Arm off top running) state for exynos5440
soc as this soc does not support this state. All the cpu's only allows the basic
C0 state.

Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
---
 arch/arm/mach-exynos/cpuidle.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-exynos/cpuidle.c b/arch/arm/mach-exynos/cpuidle.c
index 17a18ff..9a776a1 100644
--- a/arch/arm/mach-exynos/cpuidle.c
+++ b/arch/arm/mach-exynos/cpuidle.c
@@ -210,7 +210,7 @@ static int __init exynos4_init_cpuidle(void)
 		device->cpu = cpu_id;
 
 		/* Support IDLE only */
-		if (cpu_id != 0)
+		if (soc_is_exynos5440() || cpu_id != 0)
 			device->state_count = 1;
 
 		ret = cpuidle_register_device(device);
-- 
1.7.1

[PATCH] ARM: EXYNOS: cpuidle: Skip C1 cpuidle state for exynos5440

[Amit Daniel Kachhap]

This patch skips the deep C1(AFTR -Arm off top running) state for exynos5440
soc as this soc does not support this state. All the cpu's only allows the basic
C0 state.

Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
---
 arch/arm/mach-exynos/cpuidle.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-exynos/cpuidle.c b/arch/arm/mach-exynos/cpuidle.c
index 17a18ff..9a776a1 100644
--- a/arch/arm/mach-exynos/cpuidle.c
+++ b/arch/arm/mach-exynos/cpuidle.c
@@ -210,7 +210,7 @@ static int __init exynos4_init_cpuidle(void)
 		device->cpu = cpu_id;
 
 		/* Support IDLE only */
-		if (cpu_id != 0)
+		if (soc_is_exynos5440() || cpu_id != 0)
 			device->state_count = 1;
 
 		ret = cpuidle_register_device(device);
-- 
1.7.1

RE: [PATCH] ARM: EXYNOS: cpuidle: Skip C1 cpuidle state for exynos5440

[Kukjin Kim]

Amit Daniel Kachhap wrote:
> 
> This patch skips the deep C1(AFTR -Arm off top running) state for
> exynos5440
> soc as this soc does not support this state. All the cpu's only allows the
> basic
> C0 state.
> 
> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
> ---
>  arch/arm/mach-exynos/cpuidle.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm/mach-exynos/cpuidle.c b/arch/arm/mach-
> exynos/cpuidle.c
> index 17a18ff..9a776a1 100644
> --- a/arch/arm/mach-exynos/cpuidle.c
> +++ b/arch/arm/mach-exynos/cpuidle.c
> @@ -210,7 +210,7 @@ static int __init exynos4_init_cpuidle(void)
>  		device->cpu = cpu_id;
> 
>  		/* Support IDLE only */
> -		if (cpu_id != 0)
> +		if (soc_is_exynos5440() || cpu_id != 0)
>  			device->state_count = 1;
> 
>  		ret = cpuidle_register_device(device);
> --
> 1.7.1

Applied, thanks.
- Kukjin

[PATCH] ARM: EXYNOS: cpuidle: Skip C1 cpuidle state for exynos5440

[Kukjin Kim]

Amit Daniel Kachhap wrote:
> 
> This patch skips the deep C1(AFTR -Arm off top running) state for
> exynos5440
> soc as this soc does not support this state. All the cpu's only allows the
> basic
> C0 state.
> 
> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
> ---
>  arch/arm/mach-exynos/cpuidle.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm/mach-exynos/cpuidle.c b/arch/arm/mach-
> exynos/cpuidle.c
> index 17a18ff..9a776a1 100644
> --- a/arch/arm/mach-exynos/cpuidle.c
> +++ b/arch/arm/mach-exynos/cpuidle.c
> @@ -210,7 +210,7 @@ static int __init exynos4_init_cpuidle(void)
>  		device->cpu = cpu_id;
> 
>  		/* Support IDLE only */
> -		if (cpu_id != 0)
> +		if (soc_is_exynos5440() || cpu_id != 0)
>  			device->state_count = 1;
> 
>  		ret = cpuidle_register_device(device);
> --
> 1.7.1

Applied, thanks.
- Kukjin

Re: [PATCH] ARM: EXYNOS: cpuidle: Skip C1 cpuidle state for exynos5440

[Daniel Lezcano]

On 07/24/2013 01:47 PM, Kukjin Kim wrote:
> Amit Daniel Kachhap wrote:
>>
>> This patch skips the deep C1(AFTR -Arm off top running) state for
>> exynos5440
>> soc as this soc does not support this state. All the cpu's only allows the
>> basic
>> C0 state.
>>
>> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
>> ---
>>  arch/arm/mach-exynos/cpuidle.c |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/arch/arm/mach-exynos/cpuidle.c b/arch/arm/mach-
>> exynos/cpuidle.c
>> index 17a18ff..9a776a1 100644
>> --- a/arch/arm/mach-exynos/cpuidle.c
>> +++ b/arch/arm/mach-exynos/cpuidle.c
>> @@ -210,7 +210,7 @@ static int __init exynos4_init_cpuidle(void)
>>  		device->cpu = cpu_id;
>>
>>  		/* Support IDLE only */
>> -		if (cpu_id != 0)
>> +		if (soc_is_exynos5440() || cpu_id != 0)
>>  			device->state_count = 1;
>>
>>  		ret = cpuidle_register_device(device);
>> --
>> 1.7.1
> 
> Applied, thanks.

You shouldn't have. This patch means exynos5540 has no cpuidle driver at
all. It should be fixed in the Kconfig to unselect CONFIG_CPU_IDLE for
an exynos5540.


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

[PATCH] ARM: EXYNOS: cpuidle: Skip C1 cpuidle state for exynos5440

[Daniel Lezcano]

On 07/24/2013 01:47 PM, Kukjin Kim wrote:
> Amit Daniel Kachhap wrote:
>>
>> This patch skips the deep C1(AFTR -Arm off top running) state for
>> exynos5440
>> soc as this soc does not support this state. All the cpu's only allows the
>> basic
>> C0 state.
>>
>> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
>> ---
>>  arch/arm/mach-exynos/cpuidle.c |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/arch/arm/mach-exynos/cpuidle.c b/arch/arm/mach-
>> exynos/cpuidle.c
>> index 17a18ff..9a776a1 100644
>> --- a/arch/arm/mach-exynos/cpuidle.c
>> +++ b/arch/arm/mach-exynos/cpuidle.c
>> @@ -210,7 +210,7 @@ static int __init exynos4_init_cpuidle(void)
>>  		device->cpu = cpu_id;
>>
>>  		/* Support IDLE only */
>> -		if (cpu_id != 0)
>> +		if (soc_is_exynos5440() || cpu_id != 0)
>>  			device->state_count = 1;
>>
>>  		ret = cpuidle_register_device(device);
>> --
>> 1.7.1
> 
> Applied, thanks.

You shouldn't have. This patch means exynos5540 has no cpuidle driver at
all. It should be fixed in the Kconfig to unselect CONFIG_CPU_IDLE for
an exynos5540.


-- 
 <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] ARM: EXYNOS: cpuidle: Skip C1 cpuidle state for exynos5440

[Tomasz Figa]

On Sunday 28 of July 2013 09:10:09 Daniel Lezcano wrote:
> On 07/24/2013 01:47 PM, Kukjin Kim wrote:
> > Amit Daniel Kachhap wrote:
> >> This patch skips the deep C1(AFTR -Arm off top running) state for
> >> exynos5440
> >> soc as this soc does not support this state. All the cpu's only
> >> allows the basic
> >> C0 state.
> >> 
> >> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
> >> ---
> >> 
> >>  arch/arm/mach-exynos/cpuidle.c |    2 +-
> >>  1 files changed, 1 insertions(+), 1 deletions(-)
> >> 
> >> diff --git a/arch/arm/mach-exynos/cpuidle.c b/arch/arm/mach-
> >> exynos/cpuidle.c
> >> index 17a18ff..9a776a1 100644
> >> --- a/arch/arm/mach-exynos/cpuidle.c
> >> +++ b/arch/arm/mach-exynos/cpuidle.c
> >> @@ -210,7 +210,7 @@ static int __init exynos4_init_cpuidle(void)
> >> 
> >>  		device->cpu = cpu_id;
> >>  		
> >>  		/* Support IDLE only */
> >> 
> >> -		if (cpu_id != 0)
> >> +		if (soc_is_exynos5440() || cpu_id != 0)
> >> 
> >>  			device->state_count = 1;
> >>  		
> >>  		ret = cpuidle_register_device(device);
> >> 
> >> --
> >> 1.7.1
> > 
> > Applied, thanks.
> 
> You shouldn't have. This patch means exynos5540 has no cpuidle driver at
> all. It should be fixed in the Kconfig to unselect CONFIG_CPU_IDLE for
> an exynos5540.

To shed more light on this, let me add that you need to register a cpuidle 
driver only if you have more states than a simple WFI or you need some 
crazy steps to enter WFI. Default setup falls back to generic ARM WFI. 
(Daniel, do we get the nice idle stats as provided by cpuidle core then?)

Anyway, Exynos cpuidle is using an initcall to initialize and we support 
multiple Exynos SoCs in single zImage, so deselecting CONFIG_CPU_IDLE is 
not an option. Considering multiplatform requirements, the driver has to 
be modified to initialize only on supported platforms, either by:

 a) dropping the initcall and calling the init function directly from 
arch/arm/mach-exynos

 or

 b) checking if machine we are running on is supported, which would mean a 
long list of all Exynos SoCs that needs to be checked.

An evolution of option a) is registering a platform device somewhere in 
arch/arm/mach-exynos and making exynos-cpuidle a platform driver. The 
problem is that you must register a static platform device from arch code, 
because cpuidle is not a real hardware block that can be put into device 
tree.

Best regards,
Tomasz

[PATCH] ARM: EXYNOS: cpuidle: Skip C1 cpuidle state for exynos5440

[Tomasz Figa]

On Sunday 28 of July 2013 09:10:09 Daniel Lezcano wrote:
> On 07/24/2013 01:47 PM, Kukjin Kim wrote:
> > Amit Daniel Kachhap wrote:
> >> This patch skips the deep C1(AFTR -Arm off top running) state for
> >> exynos5440
> >> soc as this soc does not support this state. All the cpu's only
> >> allows the basic
> >> C0 state.
> >> 
> >> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
> >> ---
> >> 
> >>  arch/arm/mach-exynos/cpuidle.c |    2 +-
> >>  1 files changed, 1 insertions(+), 1 deletions(-)
> >> 
> >> diff --git a/arch/arm/mach-exynos/cpuidle.c b/arch/arm/mach-
> >> exynos/cpuidle.c
> >> index 17a18ff..9a776a1 100644
> >> --- a/arch/arm/mach-exynos/cpuidle.c
> >> +++ b/arch/arm/mach-exynos/cpuidle.c
> >> @@ -210,7 +210,7 @@ static int __init exynos4_init_cpuidle(void)
> >> 
> >>  		device->cpu = cpu_id;
> >>  		
> >>  		/* Support IDLE only */
> >> 
> >> -		if (cpu_id != 0)
> >> +		if (soc_is_exynos5440() || cpu_id != 0)
> >> 
> >>  			device->state_count = 1;
> >>  		
> >>  		ret = cpuidle_register_device(device);
> >> 
> >> --
> >> 1.7.1
> > 
> > Applied, thanks.
> 
> You shouldn't have. This patch means exynos5540 has no cpuidle driver at
> all. It should be fixed in the Kconfig to unselect CONFIG_CPU_IDLE for
> an exynos5540.

To shed more light on this, let me add that you need to register a cpuidle 
driver only if you have more states than a simple WFI or you need some 
crazy steps to enter WFI. Default setup falls back to generic ARM WFI. 
(Daniel, do we get the nice idle stats as provided by cpuidle core then?)

Anyway, Exynos cpuidle is using an initcall to initialize and we support 
multiple Exynos SoCs in single zImage, so deselecting CONFIG_CPU_IDLE is 
not an option. Considering multiplatform requirements, the driver has to 
be modified to initialize only on supported platforms, either by:

 a) dropping the initcall and calling the init function directly from 
arch/arm/mach-exynos

 or

 b) checking if machine we are running on is supported, which would mean a 
long list of all Exynos SoCs that needs to be checked.

An evolution of option a) is registering a platform device somewhere in 
arch/arm/mach-exynos and making exynos-cpuidle a platform driver. The 
problem is that you must register a static platform device from arch code, 
because cpuidle is not a real hardware block that can be put into device 
tree.

Best regards,
Tomasz

Re: [PATCH] ARM: EXYNOS: cpuidle: Skip C1 cpuidle state for exynos5440

[Daniel Lezcano]

On 07/28/2013 11:22 AM, Tomasz Figa wrote:
> On Sunday 28 of July 2013 09:10:09 Daniel Lezcano wrote:
>> On 07/24/2013 01:47 PM, Kukjin Kim wrote:
>>> Amit Daniel Kachhap wrote:
>>>> This patch skips the deep C1(AFTR -Arm off top running) state for
>>>> exynos5440
>>>> soc as this soc does not support this state. All the cpu's only
>>>> allows the basic
>>>> C0 state.
>>>>
>>>> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
>>>> ---
>>>>
>>>>  arch/arm/mach-exynos/cpuidle.c |    2 +-
>>>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>>>
>>>> diff --git a/arch/arm/mach-exynos/cpuidle.c b/arch/arm/mach-
>>>> exynos/cpuidle.c
>>>> index 17a18ff..9a776a1 100644
>>>> --- a/arch/arm/mach-exynos/cpuidle.c
>>>> +++ b/arch/arm/mach-exynos/cpuidle.c
>>>> @@ -210,7 +210,7 @@ static int __init exynos4_init_cpuidle(void)
>>>>
>>>>  		device->cpu = cpu_id;
>>>>  		
>>>>  		/* Support IDLE only */
>>>>
>>>> -		if (cpu_id != 0)
>>>> +		if (soc_is_exynos5440() || cpu_id != 0)
>>>>
>>>>  			device->state_count = 1;
>>>>  		
>>>>  		ret = cpuidle_register_device(device);
>>>>
>>>> --
>>>> 1.7.1
>>>
>>> Applied, thanks.
>>
>> You shouldn't have. This patch means exynos5540 has no cpuidle driver at
>> all. It should be fixed in the Kconfig to unselect CONFIG_CPU_IDLE for
>> an exynos5540.
> 
> To shed more light on this, let me add that you need to register a cpuidle 
> driver only if you have more states than a simple WFI or you need some 
> crazy steps to enter WFI. Default setup falls back to generic ARM WFI. 
> (Daniel, do we get the nice idle stats as provided by cpuidle core then?)

Nope, but with one state, idle vs busy stats do the trick.

BTW, I am writing a tool to do some stats based on the idle events [1].
It is still at a very early development stage but we can get some
interesting informations.


> Anyway, Exynos cpuidle is using an initcall to initialize and we support 
> multiple Exynos SoCs in single zImage, so deselecting CONFIG_CPU_IDLE is 
> not an option. 

Good point.

> Considering multiplatform requirements, the driver has to 
> be modified to initialize only on supported platforms, either by:
> 
>  a) dropping the initcall and calling the init function directly from 
> arch/arm/mach-exynos
> 
>  or
> 
>  b) checking if machine we are running on is supported, which would mean a 
> long list of all Exynos SoCs that needs to be checked.
> 
> An evolution of option a) is registering a platform device somewhere in 
> arch/arm/mach-exynos and making exynos-cpuidle a platform driver.

Yes, I am favorable to this solution [2].

> The 
> problem is that you must register a static platform device from arch code, 
> because cpuidle is not a real hardware block that can be put into device 
> tree.

Thanks
  -- Daniel

[1] https://git.linaro.org/gitweb?p=people/dlezcano/idlestat.git;a=summary
[2] http://patches.linaro.org/18368/

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

[PATCH] ARM: EXYNOS: cpuidle: Skip C1 cpuidle state for exynos5440

[Daniel Lezcano]

On 07/28/2013 11:22 AM, Tomasz Figa wrote:
> On Sunday 28 of July 2013 09:10:09 Daniel Lezcano wrote:
>> On 07/24/2013 01:47 PM, Kukjin Kim wrote:
>>> Amit Daniel Kachhap wrote:
>>>> This patch skips the deep C1(AFTR -Arm off top running) state for
>>>> exynos5440
>>>> soc as this soc does not support this state. All the cpu's only
>>>> allows the basic
>>>> C0 state.
>>>>
>>>> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
>>>> ---
>>>>
>>>>  arch/arm/mach-exynos/cpuidle.c |    2 +-
>>>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>>>
>>>> diff --git a/arch/arm/mach-exynos/cpuidle.c b/arch/arm/mach-
>>>> exynos/cpuidle.c
>>>> index 17a18ff..9a776a1 100644
>>>> --- a/arch/arm/mach-exynos/cpuidle.c
>>>> +++ b/arch/arm/mach-exynos/cpuidle.c
>>>> @@ -210,7 +210,7 @@ static int __init exynos4_init_cpuidle(void)
>>>>
>>>>  		device->cpu = cpu_id;
>>>>  		
>>>>  		/* Support IDLE only */
>>>>
>>>> -		if (cpu_id != 0)
>>>> +		if (soc_is_exynos5440() || cpu_id != 0)
>>>>
>>>>  			device->state_count = 1;
>>>>  		
>>>>  		ret = cpuidle_register_device(device);
>>>>
>>>> --
>>>> 1.7.1
>>>
>>> Applied, thanks.
>>
>> You shouldn't have. This patch means exynos5540 has no cpuidle driver at
>> all. It should be fixed in the Kconfig to unselect CONFIG_CPU_IDLE for
>> an exynos5540.
> 
> To shed more light on this, let me add that you need to register a cpuidle 
> driver only if you have more states than a simple WFI or you need some 
> crazy steps to enter WFI. Default setup falls back to generic ARM WFI. 
> (Daniel, do we get the nice idle stats as provided by cpuidle core then?)

Nope, but with one state, idle vs busy stats do the trick.

BTW, I am writing a tool to do some stats based on the idle events [1].
It is still at a very early development stage but we can get some
interesting informations.


> Anyway, Exynos cpuidle is using an initcall to initialize and we support 
> multiple Exynos SoCs in single zImage, so deselecting CONFIG_CPU_IDLE is 
> not an option. 

Good point.

> Considering multiplatform requirements, the driver has to 
> be modified to initialize only on supported platforms, either by:
> 
>  a) dropping the initcall and calling the init function directly from 
> arch/arm/mach-exynos
> 
>  or
> 
>  b) checking if machine we are running on is supported, which would mean a 
> long list of all Exynos SoCs that needs to be checked.
> 
> An evolution of option a) is registering a platform device somewhere in 
> arch/arm/mach-exynos and making exynos-cpuidle a platform driver.

Yes, I am favorable to this solution [2].

> The 
> problem is that you must register a static platform device from arch code, 
> because cpuidle is not a real hardware block that can be put into device 
> tree.

Thanks
  -- Daniel

[1] https://git.linaro.org/gitweb?p=people/dlezcano/idlestat.git;a=summary
[2] http://patches.linaro.org/18368/

-- 
 <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] ARM: EXYNOS: cpuidle: Skip C1 cpuidle state for exynos5440

[Tomasz Figa]

On Sunday 28 of July 2013 12:31:51 Daniel Lezcano wrote:
> On 07/28/2013 11:22 AM, Tomasz Figa wrote:
> > On Sunday 28 of July 2013 09:10:09 Daniel Lezcano wrote:
> >> On 07/24/2013 01:47 PM, Kukjin Kim wrote:
> >>> Amit Daniel Kachhap wrote:
> >>>> This patch skips the deep C1(AFTR -Arm off top running) state for
> >>>> exynos5440
> >>>> soc as this soc does not support this state. All the cpu's only
> >>>> allows the basic
> >>>> C0 state.
> >>>> 
> >>>> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
> >>>> ---
> >>>> 
> >>>>  arch/arm/mach-exynos/cpuidle.c |    2 +-
> >>>>  1 files changed, 1 insertions(+), 1 deletions(-)
> >>>> 
> >>>> diff --git a/arch/arm/mach-exynos/cpuidle.c b/arch/arm/mach-
> >>>> exynos/cpuidle.c
> >>>> index 17a18ff..9a776a1 100644
> >>>> --- a/arch/arm/mach-exynos/cpuidle.c
> >>>> +++ b/arch/arm/mach-exynos/cpuidle.c
> >>>> @@ -210,7 +210,7 @@ static int __init exynos4_init_cpuidle(void)
> >>>> 
> >>>>  		device->cpu = cpu_id;
> >>>>  		
> >>>>  		/* Support IDLE only */
> >>>> 
> >>>> -		if (cpu_id != 0)
> >>>> +		if (soc_is_exynos5440() || cpu_id != 0)
> >>>> 
> >>>>  			device->state_count = 1;
> >>>>  		
> >>>>  		ret = cpuidle_register_device(device);
> >>>> 
> >>>> --
> >>>> 1.7.1
> >>> 
> >>> Applied, thanks.
> >> 
> >> You shouldn't have. This patch means exynos5540 has no cpuidle driver
> >> at all. It should be fixed in the Kconfig to unselect
> >> CONFIG_CPU_IDLE for an exynos5540.
> > 
> > To shed more light on this, let me add that you need to register a
> > cpuidle driver only if you have more states than a simple WFI or you
> > need some crazy steps to enter WFI. Default setup falls back to
> > generic ARM WFI. (Daniel, do we get the nice idle stats as provided
> > by cpuidle core then?)
> Nope, but with one state, idle vs busy stats do the trick.
> 
> BTW, I am writing a tool to do some stats based on the idle events [1].
> It is still at a very early development stage but we can get some
> interesting informations.

Sounds good.

By the way, a random idea that just came to my mind is that since we 
already support multiple cpuidle drivers, what about providing a default 
cpuidle-wfi driver that gets used in case no other driver is available?

This would give us cpuidle stats at the same place as with other cpuidle 
drivers and might also unify some code paths.

As I said, it's just a random idea, so feel free to redirect it to 
/dev/null.

> > Anyway, Exynos cpuidle is using an initcall to initialize and we
> > support multiple Exynos SoCs in single zImage, so deselecting
> > CONFIG_CPU_IDLE is not an option.
> 
> Good point.
> 
> > Considering multiplatform requirements, the driver has to
> > 
> > be modified to initialize only on supported platforms, either by:
> >  a) dropping the initcall and calling the init function directly from
> > 
> > arch/arm/mach-exynos
> > 
> >  or
> >  
> >  b) checking if machine we are running on is supported, which would
> >  mean a> 
> > long list of all Exynos SoCs that needs to be checked.
> > 
> > An evolution of option a) is registering a platform device somewhere
> > in
> > arch/arm/mach-exynos and making exynos-cpuidle a platform driver.
> 
> Yes, I am favorable to this solution [2].

OK, looks good.

Best regards,
Tomasz

[PATCH] ARM: EXYNOS: cpuidle: Skip C1 cpuidle state for exynos5440

[Tomasz Figa]

On Sunday 28 of July 2013 12:31:51 Daniel Lezcano wrote:
> On 07/28/2013 11:22 AM, Tomasz Figa wrote:
> > On Sunday 28 of July 2013 09:10:09 Daniel Lezcano wrote:
> >> On 07/24/2013 01:47 PM, Kukjin Kim wrote:
> >>> Amit Daniel Kachhap wrote:
> >>>> This patch skips the deep C1(AFTR -Arm off top running) state for
> >>>> exynos5440
> >>>> soc as this soc does not support this state. All the cpu's only
> >>>> allows the basic
> >>>> C0 state.
> >>>> 
> >>>> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
> >>>> ---
> >>>> 
> >>>>  arch/arm/mach-exynos/cpuidle.c |    2 +-
> >>>>  1 files changed, 1 insertions(+), 1 deletions(-)
> >>>> 
> >>>> diff --git a/arch/arm/mach-exynos/cpuidle.c b/arch/arm/mach-
> >>>> exynos/cpuidle.c
> >>>> index 17a18ff..9a776a1 100644
> >>>> --- a/arch/arm/mach-exynos/cpuidle.c
> >>>> +++ b/arch/arm/mach-exynos/cpuidle.c
> >>>> @@ -210,7 +210,7 @@ static int __init exynos4_init_cpuidle(void)
> >>>> 
> >>>>  		device->cpu = cpu_id;
> >>>>  		
> >>>>  		/* Support IDLE only */
> >>>> 
> >>>> -		if (cpu_id != 0)
> >>>> +		if (soc_is_exynos5440() || cpu_id != 0)
> >>>> 
> >>>>  			device->state_count = 1;
> >>>>  		
> >>>>  		ret = cpuidle_register_device(device);
> >>>> 
> >>>> --
> >>>> 1.7.1
> >>> 
> >>> Applied, thanks.
> >> 
> >> You shouldn't have. This patch means exynos5540 has no cpuidle driver
> >> at all. It should be fixed in the Kconfig to unselect
> >> CONFIG_CPU_IDLE for an exynos5540.
> > 
> > To shed more light on this, let me add that you need to register a
> > cpuidle driver only if you have more states than a simple WFI or you
> > need some crazy steps to enter WFI. Default setup falls back to
> > generic ARM WFI. (Daniel, do we get the nice idle stats as provided
> > by cpuidle core then?)
> Nope, but with one state, idle vs busy stats do the trick.
> 
> BTW, I am writing a tool to do some stats based on the idle events [1].
> It is still at a very early development stage but we can get some
> interesting informations.

Sounds good.

By the way, a random idea that just came to my mind is that since we 
already support multiple cpuidle drivers, what about providing a default 
cpuidle-wfi driver that gets used in case no other driver is available?

This would give us cpuidle stats at the same place as with other cpuidle 
drivers and might also unify some code paths.

As I said, it's just a random idea, so feel free to redirect it to 
/dev/null.

> > Anyway, Exynos cpuidle is using an initcall to initialize and we
> > support multiple Exynos SoCs in single zImage, so deselecting
> > CONFIG_CPU_IDLE is not an option.
> 
> Good point.
> 
> > Considering multiplatform requirements, the driver has to
> > 
> > be modified to initialize only on supported platforms, either by:
> >  a) dropping the initcall and calling the init function directly from
> > 
> > arch/arm/mach-exynos
> > 
> >  or
> >  
> >  b) checking if machine we are running on is supported, which would
> >  mean a> 
> > long list of all Exynos SoCs that needs to be checked.
> > 
> > An evolution of option a) is registering a platform device somewhere
> > in
> > arch/arm/mach-exynos and making exynos-cpuidle a platform driver.
> 
> Yes, I am favorable to this solution [2].

OK, looks good.

Best regards,
Tomasz

Re: [PATCH] ARM: EXYNOS: cpuidle: Skip C1 cpuidle state for exynos5440

[amit daniel kachhap]

Hi Daniel/Tomasz,

From the discussion I can conclude that SOC check is needed in the
cpuidle driver for deeper C states. Only the question is where to
insert this.
Also to perform the SOC there can be 2 ways like
1) each SOC check 4120, 4412, 5250 etc (long list)
2) negate the nonsupporting SOC's like 5440 (small list like current patch)
Any opinion?
 As Bartlomiej suggested that this patch conflicts with Daniel's
earlier patch http://marc.info/?l=linux-arm-kernel&m=137467935712513&w=2
So I can re-base my patch on top of this one if needed.

Thanks,
Amit Daniel

On Sun, Jul 28, 2013 at 4:01 PM, Daniel Lezcano
<daniel.lezcano@linaro.org> wrote:
> On 07/28/2013 11:22 AM, Tomasz Figa wrote:
>> On Sunday 28 of July 2013 09:10:09 Daniel Lezcano wrote:
>>> On 07/24/2013 01:47 PM, Kukjin Kim wrote:
>>>> Amit Daniel Kachhap wrote:
>>>>> This patch skips the deep C1(AFTR -Arm off top running) state for
>>>>> exynos5440
>>>>> soc as this soc does not support this state. All the cpu's only
>>>>> allows the basic
>>>>> C0 state.
>>>>>
>>>>> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
>>>>> ---
>>>>>
>>>>>  arch/arm/mach-exynos/cpuidle.c |    2 +-
>>>>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>>>>
>>>>> diff --git a/arch/arm/mach-exynos/cpuidle.c b/arch/arm/mach-
>>>>> exynos/cpuidle.c
>>>>> index 17a18ff..9a776a1 100644
>>>>> --- a/arch/arm/mach-exynos/cpuidle.c
>>>>> +++ b/arch/arm/mach-exynos/cpuidle.c
>>>>> @@ -210,7 +210,7 @@ static int __init exynos4_init_cpuidle(void)
>>>>>
>>>>>            device->cpu = cpu_id;
>>>>>
>>>>>            /* Support IDLE only */
>>>>>
>>>>> -          if (cpu_id != 0)
>>>>> +          if (soc_is_exynos5440() || cpu_id != 0)
>>>>>
>>>>>                    device->state_count = 1;
>>>>>
>>>>>            ret = cpuidle_register_device(device);
>>>>>
>>>>> --
>>>>> 1.7.1
>>>>
>>>> Applied, thanks.
>>>
>>> You shouldn't have. This patch means exynos5540 has no cpuidle driver at
>>> all. It should be fixed in the Kconfig to unselect CONFIG_CPU_IDLE for
>>> an exynos5540.
>>
>> To shed more light on this, let me add that you need to register a cpuidle
>> driver only if you have more states than a simple WFI or you need some
>> crazy steps to enter WFI. Default setup falls back to generic ARM WFI.
>> (Daniel, do we get the nice idle stats as provided by cpuidle core then?)
>
> Nope, but with one state, idle vs busy stats do the trick.
>
> BTW, I am writing a tool to do some stats based on the idle events [1].
> It is still at a very early development stage but we can get some
> interesting informations.
>
>
>> Anyway, Exynos cpuidle is using an initcall to initialize and we support
>> multiple Exynos SoCs in single zImage, so deselecting CONFIG_CPU_IDLE is
>> not an option.
>
> Good point.
>
>> Considering multiplatform requirements, the driver has to
>> be modified to initialize only on supported platforms, either by:
>>
>>  a) dropping the initcall and calling the init function directly from
>> arch/arm/mach-exynos
>>
>>  or
>>
>>  b) checking if machine we are running on is supported, which would mean a
>> long list of all Exynos SoCs that needs to be checked.
>>
>> An evolution of option a) is registering a platform device somewhere in
>> arch/arm/mach-exynos and making exynos-cpuidle a platform driver.
>
> Yes, I am favorable to this solution [2].
>
>> The
>> problem is that you must register a static platform device from arch code,
>> because cpuidle is not a real hardware block that can be put into device
>> tree.
>
> Thanks
>   -- Daniel
>
> [1] https://git.linaro.org/gitweb?p=people/dlezcano/idlestat.git;a=summary
> [2] http://patches.linaro.org/18368/
>
> --
>  <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
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] ARM: EXYNOS: cpuidle: Skip C1 cpuidle state for exynos5440

[amit daniel kachhap]

Hi Daniel/Tomasz,

>From the discussion I can conclude that SOC check is needed in the
cpuidle driver for deeper C states. Only the question is where to
insert this.
Also to perform the SOC there can be 2 ways like
1) each SOC check 4120, 4412, 5250 etc (long list)
2) negate the nonsupporting SOC's like 5440 (small list like current patch)
Any opinion?
 As Bartlomiej suggested that this patch conflicts with Daniel's
earlier patch http://marc.info/?l=linux-arm-kernel&m=137467935712513&w=2
So I can re-base my patch on top of this one if needed.

Thanks,
Amit Daniel

On Sun, Jul 28, 2013 at 4:01 PM, Daniel Lezcano
<daniel.lezcano@linaro.org> wrote:
> On 07/28/2013 11:22 AM, Tomasz Figa wrote:
>> On Sunday 28 of July 2013 09:10:09 Daniel Lezcano wrote:
>>> On 07/24/2013 01:47 PM, Kukjin Kim wrote:
>>>> Amit Daniel Kachhap wrote:
>>>>> This patch skips the deep C1(AFTR -Arm off top running) state for
>>>>> exynos5440
>>>>> soc as this soc does not support this state. All the cpu's only
>>>>> allows the basic
>>>>> C0 state.
>>>>>
>>>>> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
>>>>> ---
>>>>>
>>>>>  arch/arm/mach-exynos/cpuidle.c |    2 +-
>>>>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>>>>
>>>>> diff --git a/arch/arm/mach-exynos/cpuidle.c b/arch/arm/mach-
>>>>> exynos/cpuidle.c
>>>>> index 17a18ff..9a776a1 100644
>>>>> --- a/arch/arm/mach-exynos/cpuidle.c
>>>>> +++ b/arch/arm/mach-exynos/cpuidle.c
>>>>> @@ -210,7 +210,7 @@ static int __init exynos4_init_cpuidle(void)
>>>>>
>>>>>            device->cpu = cpu_id;
>>>>>
>>>>>            /* Support IDLE only */
>>>>>
>>>>> -          if (cpu_id != 0)
>>>>> +          if (soc_is_exynos5440() || cpu_id != 0)
>>>>>
>>>>>                    device->state_count = 1;
>>>>>
>>>>>            ret = cpuidle_register_device(device);
>>>>>
>>>>> --
>>>>> 1.7.1
>>>>
>>>> Applied, thanks.
>>>
>>> You shouldn't have. This patch means exynos5540 has no cpuidle driver at
>>> all. It should be fixed in the Kconfig to unselect CONFIG_CPU_IDLE for
>>> an exynos5540.
>>
>> To shed more light on this, let me add that you need to register a cpuidle
>> driver only if you have more states than a simple WFI or you need some
>> crazy steps to enter WFI. Default setup falls back to generic ARM WFI.
>> (Daniel, do we get the nice idle stats as provided by cpuidle core then?)
>
> Nope, but with one state, idle vs busy stats do the trick.
>
> BTW, I am writing a tool to do some stats based on the idle events [1].
> It is still at a very early development stage but we can get some
> interesting informations.
>
>
>> Anyway, Exynos cpuidle is using an initcall to initialize and we support
>> multiple Exynos SoCs in single zImage, so deselecting CONFIG_CPU_IDLE is
>> not an option.
>
> Good point.
>
>> Considering multiplatform requirements, the driver has to
>> be modified to initialize only on supported platforms, either by:
>>
>>  a) dropping the initcall and calling the init function directly from
>> arch/arm/mach-exynos
>>
>>  or
>>
>>  b) checking if machine we are running on is supported, which would mean a
>> long list of all Exynos SoCs that needs to be checked.
>>
>> An evolution of option a) is registering a platform device somewhere in
>> arch/arm/mach-exynos and making exynos-cpuidle a platform driver.
>
> Yes, I am favorable to this solution [2].
>
>> The
>> problem is that you must register a static platform device from arch code,
>> because cpuidle is not a real hardware block that can be put into device
>> tree.
>
> Thanks
>   -- Daniel
>
> [1] https://git.linaro.org/gitweb?p=people/dlezcano/idlestat.git;a=summary
> [2] http://patches.linaro.org/18368/
>
> --
>  <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
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH] ARM: EXYNOS: cpuidle: Skip C1 cpuidle state for exynos5440

[Kukjin Kim]

amit daniel kachhap wrote:
> 
> Hi Daniel/Tomasz,
> 
> From the discussion I can conclude that SOC check is needed in the
> cpuidle driver for deeper C states. Only the question is where to
> insert this.
> Also to perform the SOC there can be 2 ways like
> 1) each SOC check 4120, 4412, 5250 etc (long list)
> 2) negate the nonsupporting SOC's like 5440 (small list like current patch)
> Any opinion?

I’d preferred to use 2nd :)

>  As Bartlomiej suggested that this patch conflicts with Daniel's
> earlier patch http://marc.info/?l=linux-arm-kernel&m=137467935712513&w=2
> So I can re-base my patch on top of this one if needed.
> 
Sounds good to me.

Thanks,
Kukjin

> Thanks,
> Amit Daniel
> 
> On Sun, Jul 28, 2013 at 4:01 PM, Daniel Lezcano
> <daniel.lezcano@linaro.org> wrote:
> > On 07/28/2013 11:22 AM, Tomasz Figa wrote:
> >> On Sunday 28 of July 2013 09:10:09 Daniel Lezcano wrote:
> >>> On 07/24/2013 01:47 PM, Kukjin Kim wrote:
> >>>> Amit Daniel Kachhap wrote:
> >>>>> This patch skips the deep C1(AFTR -Arm off top running) state for
> >>>>> exynos5440
> >>>>> soc as this soc does not support this state. All the cpu's only
> >>>>> allows the basic
> >>>>> C0 state.
> >>>>>
> >>>>> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
> >>>>> ---
> >>>>>
> >>>>>  arch/arm/mach-exynos/cpuidle.c |    2 +-
> >>>>>  1 files changed, 1 insertions(+), 1 deletions(-)
> >>>>>
> >>>>> diff --git a/arch/arm/mach-exynos/cpuidle.c b/arch/arm/mach-
> >>>>> exynos/cpuidle.c
> >>>>> index 17a18ff..9a776a1 100644
> >>>>> --- a/arch/arm/mach-exynos/cpuidle.c
> >>>>> +++ b/arch/arm/mach-exynos/cpuidle.c
> >>>>> @@ -210,7 +210,7 @@ static int __init exynos4_init_cpuidle(void)
> >>>>>
> >>>>>            device->cpu = cpu_id;
> >>>>>
> >>>>>            /* Support IDLE only */
> >>>>>
> >>>>> -          if (cpu_id != 0)
> >>>>> +          if (soc_is_exynos5440() || cpu_id != 0)
> >>>>>
> >>>>>                    device->state_count = 1;
> >>>>>
> >>>>>            ret = cpuidle_register_device(device);
> >>>>>
> >>>>> --
> >>>>> 1.7.1
> >>>>
> >>>> Applied, thanks.
> >>>
> >>> You shouldn't have. This patch means exynos5540 has no cpuidle driver
> at
> >>> all. It should be fixed in the Kconfig to unselect CONFIG_CPU_IDLE for
> >>> an exynos5540.
> >>
> >> To shed more light on this, let me add that you need to register a
> cpuidle
> >> driver only if you have more states than a simple WFI or you need some
> >> crazy steps to enter WFI. Default setup falls back to generic ARM WFI.
> >> (Daniel, do we get the nice idle stats as provided by cpuidle core
> then?)
> >
> > Nope, but with one state, idle vs busy stats do the trick.
> >
> > BTW, I am writing a tool to do some stats based on the idle events [1].
> > It is still at a very early development stage but we can get some
> > interesting informations.
> >
> >
> >> Anyway, Exynos cpuidle is using an initcall to initialize and we
> support
> >> multiple Exynos SoCs in single zImage, so deselecting CONFIG_CPU_IDLE
> is
> >> not an option.
> >
> > Good point.
> >
> >> Considering multiplatform requirements, the driver has to
> >> be modified to initialize only on supported platforms, either by:
> >>
> >>  a) dropping the initcall and calling the init function directly from
> >> arch/arm/mach-exynos
> >>
> >>  or
> >>
> >>  b) checking if machine we are running on is supported, which would
> mean a
> >> long list of all Exynos SoCs that needs to be checked.
> >>
> >> An evolution of option a) is registering a platform device somewhere in
> >> arch/arm/mach-exynos and making exynos-cpuidle a platform driver.
> >
> > Yes, I am favorable to this solution [2].
> >
> >> The
> >> problem is that you must register a static platform device from arch
> code,
> >> because cpuidle is not a real hardware block that can be put into
> device
> >> tree.

[PATCH] ARM: EXYNOS: cpuidle: Skip C1 cpuidle state for exynos5440

[Kukjin Kim]

amit daniel kachhap wrote:
> 
> Hi Daniel/Tomasz,
> 
> From the discussion I can conclude that SOC check is needed in the
> cpuidle driver for deeper C states. Only the question is where to
> insert this.
> Also to perform the SOC there can be 2 ways like
> 1) each SOC check 4120, 4412, 5250 etc (long list)
> 2) negate the nonsupporting SOC's like 5440 (small list like current patch)
> Any opinion?

I?d preferred to use 2nd :)

>  As Bartlomiej suggested that this patch conflicts with Daniel's
> earlier patch http://marc.info/?l=linux-arm-kernel&m=137467935712513&w=2
> So I can re-base my patch on top of this one if needed.
> 
Sounds good to me.

Thanks,
Kukjin

> Thanks,
> Amit Daniel
> 
> On Sun, Jul 28, 2013 at 4:01 PM, Daniel Lezcano
> <daniel.lezcano@linaro.org> wrote:
> > On 07/28/2013 11:22 AM, Tomasz Figa wrote:
> >> On Sunday 28 of July 2013 09:10:09 Daniel Lezcano wrote:
> >>> On 07/24/2013 01:47 PM, Kukjin Kim wrote:
> >>>> Amit Daniel Kachhap wrote:
> >>>>> This patch skips the deep C1(AFTR -Arm off top running) state for
> >>>>> exynos5440
> >>>>> soc as this soc does not support this state. All the cpu's only
> >>>>> allows the basic
> >>>>> C0 state.
> >>>>>
> >>>>> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
> >>>>> ---
> >>>>>
> >>>>>  arch/arm/mach-exynos/cpuidle.c |    2 +-
> >>>>>  1 files changed, 1 insertions(+), 1 deletions(-)
> >>>>>
> >>>>> diff --git a/arch/arm/mach-exynos/cpuidle.c b/arch/arm/mach-
> >>>>> exynos/cpuidle.c
> >>>>> index 17a18ff..9a776a1 100644
> >>>>> --- a/arch/arm/mach-exynos/cpuidle.c
> >>>>> +++ b/arch/arm/mach-exynos/cpuidle.c
> >>>>> @@ -210,7 +210,7 @@ static int __init exynos4_init_cpuidle(void)
> >>>>>
> >>>>>            device->cpu = cpu_id;
> >>>>>
> >>>>>            /* Support IDLE only */
> >>>>>
> >>>>> -          if (cpu_id != 0)
> >>>>> +          if (soc_is_exynos5440() || cpu_id != 0)
> >>>>>
> >>>>>                    device->state_count = 1;
> >>>>>
> >>>>>            ret = cpuidle_register_device(device);
> >>>>>
> >>>>> --
> >>>>> 1.7.1
> >>>>
> >>>> Applied, thanks.
> >>>
> >>> You shouldn't have. This patch means exynos5540 has no cpuidle driver
> at
> >>> all. It should be fixed in the Kconfig to unselect CONFIG_CPU_IDLE for
> >>> an exynos5540.
> >>
> >> To shed more light on this, let me add that you need to register a
> cpuidle
> >> driver only if you have more states than a simple WFI or you need some
> >> crazy steps to enter WFI. Default setup falls back to generic ARM WFI.
> >> (Daniel, do we get the nice idle stats as provided by cpuidle core
> then?)
> >
> > Nope, but with one state, idle vs busy stats do the trick.
> >
> > BTW, I am writing a tool to do some stats based on the idle events [1].
> > It is still at a very early development stage but we can get some
> > interesting informations.
> >
> >
> >> Anyway, Exynos cpuidle is using an initcall to initialize and we
> support
> >> multiple Exynos SoCs in single zImage, so deselecting CONFIG_CPU_IDLE
> is
> >> not an option.
> >
> > Good point.
> >
> >> Considering multiplatform requirements, the driver has to
> >> be modified to initialize only on supported platforms, either by:
> >>
> >>  a) dropping the initcall and calling the init function directly from
> >> arch/arm/mach-exynos
> >>
> >>  or
> >>
> >>  b) checking if machine we are running on is supported, which would
> mean a
> >> long list of all Exynos SoCs that needs to be checked.
> >>
> >> An evolution of option a) is registering a platform device somewhere in
> >> arch/arm/mach-exynos and making exynos-cpuidle a platform driver.
> >
> > Yes, I am favorable to this solution [2].
> >
> >> The
> >> problem is that you must register a static platform device from arch
> code,
> >> because cpuidle is not a real hardware block that can be put into
> device
> >> tree.

Re: [PATCH] ARM: EXYNOS: cpuidle: Skip C1 cpuidle state for exynos5440

[Tomasz Figa]

On Monday 29 of July 2013 10:16:14 amit daniel kachhap wrote:
> Hi Daniel/Tomasz,
> 
> From the discussion I can conclude that SOC check is needed in the
> cpuidle driver for deeper C states.

A check is needed for the whole cpuidle driver, so it registers only on 
Exynos SoCs which support deeper C states.

> Only the question is where to
> insert this.

Exynos doesn't support multiplatform yet, but we must make sure that any 
code being added is multiplatform-aware. So initcall is not a good idea. I 
would put this somewhere on Exynos-specific initialization path, i.e. 
something that would not called for all platforms compiled in (in case of 
multiplatform).

As I discussed with Daniel, this should be using a traditional 
platform_driver model, with the difference that it can't be registered 
from device tree, but rather statically in mach code.

For example, you can add an exynos_register_cpuidle() function in 
arch/arm/mach-exynos/common.c, which registers such platform device and 
always call it from exynos4_dt_machine_init() in mach-exynos4-dt.c. In 
mach-exynos5-dt.c you could make this conditional and check if 
!soc_is_exynos5440().

Best regards,
Tomasz

> Also to perform the SOC there can be 2 ways like
> 1) each SOC check 4120, 4412, 5250 etc (long list)
> 2) negate the nonsupporting SOC's like 5440 (small list like current
> patch) Any opinion?
>  As Bartlomiej suggested that this patch conflicts with Daniel's
> earlier patch http://marc.info/?l=linux-arm-kernel&m=137467935712513&w=2
> So I can re-base my patch on top of this one if needed.
> 
> Thanks,
> Amit Daniel
> 
> On Sun, Jul 28, 2013 at 4:01 PM, Daniel Lezcano
> 
> <daniel.lezcano@linaro.org> wrote:
> > On 07/28/2013 11:22 AM, Tomasz Figa wrote:
> >> On Sunday 28 of July 2013 09:10:09 Daniel Lezcano wrote:
> >>> On 07/24/2013 01:47 PM, Kukjin Kim wrote:
> >>>> Amit Daniel Kachhap wrote:
> >>>>> This patch skips the deep C1(AFTR -Arm off top running) state for
> >>>>> exynos5440
> >>>>> soc as this soc does not support this state. All the cpu's only
> >>>>> allows the basic
> >>>>> C0 state.
> >>>>> 
> >>>>> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
> >>>>> ---
> >>>>> 
> >>>>>  arch/arm/mach-exynos/cpuidle.c |    2 +-
> >>>>>  1 files changed, 1 insertions(+), 1 deletions(-)
> >>>>> 
> >>>>> diff --git a/arch/arm/mach-exynos/cpuidle.c b/arch/arm/mach-
> >>>>> exynos/cpuidle.c
> >>>>> index 17a18ff..9a776a1 100644
> >>>>> --- a/arch/arm/mach-exynos/cpuidle.c
> >>>>> +++ b/arch/arm/mach-exynos/cpuidle.c
> >>>>> @@ -210,7 +210,7 @@ static int __init exynos4_init_cpuidle(void)
> >>>>> 
> >>>>>            device->cpu = cpu_id;
> >>>>>            
> >>>>>            /* Support IDLE only */
> >>>>> 
> >>>>> -          if (cpu_id != 0)
> >>>>> +          if (soc_is_exynos5440() || cpu_id != 0)
> >>>>> 
> >>>>>                    device->state_count = 1;
> >>>>>            
> >>>>>            ret = cpuidle_register_device(device);
> >>>>> 
> >>>>> --
> >>>>> 1.7.1
> >>>> 
> >>>> Applied, thanks.
> >>> 
> >>> You shouldn't have. This patch means exynos5540 has no cpuidle
> >>> driver at all. It should be fixed in the Kconfig to unselect
> >>> CONFIG_CPU_IDLE for an exynos5540.
> >> 
> >> To shed more light on this, let me add that you need to register a
> >> cpuidle driver only if you have more states than a simple WFI or you
> >> need some crazy steps to enter WFI. Default setup falls back to
> >> generic ARM WFI. (Daniel, do we get the nice idle stats as provided
> >> by cpuidle core then?)> 
> > Nope, but with one state, idle vs busy stats do the trick.
> > 
> > BTW, I am writing a tool to do some stats based on the idle events
> > [1].
> > It is still at a very early development stage but we can get some
> > interesting informations.
> > 
> >> Anyway, Exynos cpuidle is using an initcall to initialize and we
> >> support multiple Exynos SoCs in single zImage, so deselecting
> >> CONFIG_CPU_IDLE is not an option.
> > 
> > Good point.
> > 
> >> Considering multiplatform requirements, the driver has to
> >> 
> >> be modified to initialize only on supported platforms, either by:
> >>  a) dropping the initcall and calling the init function directly from
> >> 
> >> arch/arm/mach-exynos
> >> 
> >>  or
> >>  
> >>  b) checking if machine we are running on is supported, which would
> >>  mean a>> 
> >> long list of all Exynos SoCs that needs to be checked.
> >> 
> >> An evolution of option a) is registering a platform device somewhere
> >> in
> >> arch/arm/mach-exynos and making exynos-cpuidle a platform driver.
> > 
> > Yes, I am favorable to this solution [2].
> > 
> >> The
> >> problem is that you must register a static platform device from arch
> >> code, because cpuidle is not a real hardware block that can be put
> >> into device tree.
> > 
> > Thanks
> > 
> >   -- Daniel
> > 
> > [1]
> > https://git.linaro.org/gitweb?p=people/dlezcano/idlestat.git;a=summar
> > y [2] http://patches.linaro.org/18368/
> > 
> > --
> > 
> >  <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
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe
> > linux-samsung-soc" in the body of a message to
> > majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] ARM: EXYNOS: cpuidle: Skip C1 cpuidle state for exynos5440

[Tomasz Figa]

On Monday 29 of July 2013 10:16:14 amit daniel kachhap wrote:
> Hi Daniel/Tomasz,
> 
> From the discussion I can conclude that SOC check is needed in the
> cpuidle driver for deeper C states.

A check is needed for the whole cpuidle driver, so it registers only on 
Exynos SoCs which support deeper C states.

> Only the question is where to
> insert this.

Exynos doesn't support multiplatform yet, but we must make sure that any 
code being added is multiplatform-aware. So initcall is not a good idea. I 
would put this somewhere on Exynos-specific initialization path, i.e. 
something that would not called for all platforms compiled in (in case of 
multiplatform).

As I discussed with Daniel, this should be using a traditional 
platform_driver model, with the difference that it can't be registered 
from device tree, but rather statically in mach code.

For example, you can add an exynos_register_cpuidle() function in 
arch/arm/mach-exynos/common.c, which registers such platform device and 
always call it from exynos4_dt_machine_init() in mach-exynos4-dt.c. In 
mach-exynos5-dt.c you could make this conditional and check if 
!soc_is_exynos5440().

Best regards,
Tomasz

> Also to perform the SOC there can be 2 ways like
> 1) each SOC check 4120, 4412, 5250 etc (long list)
> 2) negate the nonsupporting SOC's like 5440 (small list like current
> patch) Any opinion?
>  As Bartlomiej suggested that this patch conflicts with Daniel's
> earlier patch http://marc.info/?l=linux-arm-kernel&m=137467935712513&w=2
> So I can re-base my patch on top of this one if needed.
> 
> Thanks,
> Amit Daniel
> 
> On Sun, Jul 28, 2013 at 4:01 PM, Daniel Lezcano
> 
> <daniel.lezcano@linaro.org> wrote:
> > On 07/28/2013 11:22 AM, Tomasz Figa wrote:
> >> On Sunday 28 of July 2013 09:10:09 Daniel Lezcano wrote:
> >>> On 07/24/2013 01:47 PM, Kukjin Kim wrote:
> >>>> Amit Daniel Kachhap wrote:
> >>>>> This patch skips the deep C1(AFTR -Arm off top running) state for
> >>>>> exynos5440
> >>>>> soc as this soc does not support this state. All the cpu's only
> >>>>> allows the basic
> >>>>> C0 state.
> >>>>> 
> >>>>> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
> >>>>> ---
> >>>>> 
> >>>>>  arch/arm/mach-exynos/cpuidle.c |    2 +-
> >>>>>  1 files changed, 1 insertions(+), 1 deletions(-)
> >>>>> 
> >>>>> diff --git a/arch/arm/mach-exynos/cpuidle.c b/arch/arm/mach-
> >>>>> exynos/cpuidle.c
> >>>>> index 17a18ff..9a776a1 100644
> >>>>> --- a/arch/arm/mach-exynos/cpuidle.c
> >>>>> +++ b/arch/arm/mach-exynos/cpuidle.c
> >>>>> @@ -210,7 +210,7 @@ static int __init exynos4_init_cpuidle(void)
> >>>>> 
> >>>>>            device->cpu = cpu_id;
> >>>>>            
> >>>>>            /* Support IDLE only */
> >>>>> 
> >>>>> -          if (cpu_id != 0)
> >>>>> +          if (soc_is_exynos5440() || cpu_id != 0)
> >>>>> 
> >>>>>                    device->state_count = 1;
> >>>>>            
> >>>>>            ret = cpuidle_register_device(device);
> >>>>> 
> >>>>> --
> >>>>> 1.7.1
> >>>> 
> >>>> Applied, thanks.
> >>> 
> >>> You shouldn't have. This patch means exynos5540 has no cpuidle
> >>> driver at all. It should be fixed in the Kconfig to unselect
> >>> CONFIG_CPU_IDLE for an exynos5540.
> >> 
> >> To shed more light on this, let me add that you need to register a
> >> cpuidle driver only if you have more states than a simple WFI or you
> >> need some crazy steps to enter WFI. Default setup falls back to
> >> generic ARM WFI. (Daniel, do we get the nice idle stats as provided
> >> by cpuidle core then?)> 
> > Nope, but with one state, idle vs busy stats do the trick.
> > 
> > BTW, I am writing a tool to do some stats based on the idle events
> > [1].
> > It is still at a very early development stage but we can get some
> > interesting informations.
> > 
> >> Anyway, Exynos cpuidle is using an initcall to initialize and we
> >> support multiple Exynos SoCs in single zImage, so deselecting
> >> CONFIG_CPU_IDLE is not an option.
> > 
> > Good point.
> > 
> >> Considering multiplatform requirements, the driver has to
> >> 
> >> be modified to initialize only on supported platforms, either by:
> >>  a) dropping the initcall and calling the init function directly from
> >> 
> >> arch/arm/mach-exynos
> >> 
> >>  or
> >>  
> >>  b) checking if machine we are running on is supported, which would
> >>  mean a>> 
> >> long list of all Exynos SoCs that needs to be checked.
> >> 
> >> An evolution of option a) is registering a platform device somewhere
> >> in
> >> arch/arm/mach-exynos and making exynos-cpuidle a platform driver.
> > 
> > Yes, I am favorable to this solution [2].
> > 
> >> The
> >> problem is that you must register a static platform device from arch
> >> code, because cpuidle is not a real hardware block that can be put
> >> into device tree.
> > 
> > Thanks
> > 
> >   -- Daniel
> > 
> > [1]
> > https://git.linaro.org/gitweb?p=people/dlezcano/idlestat.git;a=summar
> > y [2] http://patches.linaro.org/18368/
> > 
> > --
> > 
> >  <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
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe
> > linux-samsung-soc" in the body of a message to
> > majordomo at vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ARM: EXYNOS: cpuidle: Skip C1 cpuidle state for exynos5440

[amit daniel kachhap]

Hi Tomasz,

On Mon, Jul 29, 2013 at 2:03 PM, Tomasz Figa <tomasz.figa@gmail.com> wrote:
> On Monday 29 of July 2013 10:16:14 amit daniel kachhap wrote:
>> Hi Daniel/Tomasz,
>>
>> From the discussion I can conclude that SOC check is needed in the
>> cpuidle driver for deeper C states.
>
> A check is needed for the whole cpuidle driver, so it registers only on
> Exynos SoCs which support deeper C states.

>
>> Only the question is where to
>> insert this.
>
> Exynos doesn't support multiplatform yet, but we must make sure that any
> code being added is multiplatform-aware. So initcall is not a good idea. I
> would put this somewhere on Exynos-specific initialization path, i.e.
> something that would not called for all platforms compiled in (in case of
> multiplatform).
>
> As I discussed with Daniel, this should be using a traditional
> platform_driver model, with the difference that it can't be registered
> from device tree, but rather statically in mach code.
>
> For example, you can add an exynos_register_cpuidle() function in
> arch/arm/mach-exynos/common.c, which registers such platform device and
> always call it from exynos4_dt_machine_init() in mach-exynos4-dt.c. In
> mach-exynos5-dt.c you could make this conditional and check if
> !soc_is_exynos5440().
thanks for the detailed explanation. Certainly initcall is not good
for multikernel. Still I have few concerns.
1) Each C state might be SOC specific. Say there are 3 C states
C0(basic wfi), c1(single CPU power down) and C2 (System wide power
down like AFTR). In this case some exynos soc may support few of these
states and cpuidle driver should bind C-state to correct SOC.
2) Even for default C0 ( simple wfi) currently there is no sysfs
information if the cpuidle driver is not registered(Not quite sure).
I will try to post the new driver by dynamically creating the
cpuidle_state table based on SOC capability and removing the initcall.

Regards,
Amit D
>
> Best regards,
> Tomasz
>
>> Also to perform the SOC there can be 2 ways like
>> 1) each SOC check 4120, 4412, 5250 etc (long list)
>> 2) negate the nonsupporting SOC's like 5440 (small list like current
>> patch) Any opinion?
>>  As Bartlomiej suggested that this patch conflicts with Daniel's
>> earlier patch http://marc.info/?l=linux-arm-kernel&m=137467935712513&w=2
>> So I can re-base my patch on top of this one if needed.
>>
>> Thanks,
>> Amit Daniel
>>
>> On Sun, Jul 28, 2013 at 4:01 PM, Daniel Lezcano
>>
>> <daniel.lezcano@linaro.org> wrote:
>> > On 07/28/2013 11:22 AM, Tomasz Figa wrote:
>> >> On Sunday 28 of July 2013 09:10:09 Daniel Lezcano wrote:
>> >>> On 07/24/2013 01:47 PM, Kukjin Kim wrote:
>> >>>> Amit Daniel Kachhap wrote:
>> >>>>> This patch skips the deep C1(AFTR -Arm off top running) state for
>> >>>>> exynos5440
>> >>>>> soc as this soc does not support this state. All the cpu's only
>> >>>>> allows the basic
>> >>>>> C0 state.
>> >>>>>
>> >>>>> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
>> >>>>> ---
>> >>>>>
>> >>>>>  arch/arm/mach-exynos/cpuidle.c |    2 +-
>> >>>>>  1 files changed, 1 insertions(+), 1 deletions(-)
>> >>>>>
>> >>>>> diff --git a/arch/arm/mach-exynos/cpuidle.c b/arch/arm/mach-
>> >>>>> exynos/cpuidle.c
>> >>>>> index 17a18ff..9a776a1 100644
>> >>>>> --- a/arch/arm/mach-exynos/cpuidle.c
>> >>>>> +++ b/arch/arm/mach-exynos/cpuidle.c
>> >>>>> @@ -210,7 +210,7 @@ static int __init exynos4_init_cpuidle(void)
>> >>>>>
>> >>>>>            device->cpu = cpu_id;
>> >>>>>
>> >>>>>            /* Support IDLE only */
>> >>>>>
>> >>>>> -          if (cpu_id != 0)
>> >>>>> +          if (soc_is_exynos5440() || cpu_id != 0)
>> >>>>>
>> >>>>>                    device->state_count = 1;
>> >>>>>
>> >>>>>            ret = cpuidle_register_device(device);
>> >>>>>
>> >>>>> --
>> >>>>> 1.7.1
>> >>>>
>> >>>> Applied, thanks.
>> >>>
>> >>> You shouldn't have. This patch means exynos5540 has no cpuidle
>> >>> driver at all. It should be fixed in the Kconfig to unselect
>> >>> CONFIG_CPU_IDLE for an exynos5540.
>> >>
>> >> To shed more light on this, let me add that you need to register a
>> >> cpuidle driver only if you have more states than a simple WFI or you
>> >> need some crazy steps to enter WFI. Default setup falls back to
>> >> generic ARM WFI. (Daniel, do we get the nice idle stats as provided
>> >> by cpuidle core then?)>
>> > Nope, but with one state, idle vs busy stats do the trick.
>> >
>> > BTW, I am writing a tool to do some stats based on the idle events
>> > [1].
>> > It is still at a very early development stage but we can get some
>> > interesting informations.
>> >
>> >> Anyway, Exynos cpuidle is using an initcall to initialize and we
>> >> support multiple Exynos SoCs in single zImage, so deselecting
>> >> CONFIG_CPU_IDLE is not an option.
>> >
>> > Good point.
>> >
>> >> Considering multiplatform requirements, the driver has to
>> >>
>> >> be modified to initialize only on supported platforms, either by:
>> >>  a) dropping the initcall and calling the init function directly from
>> >>
>> >> arch/arm/mach-exynos
>> >>
>> >>  or
>> >>
>> >>  b) checking if machine we are running on is supported, which would
>> >>  mean a>>
>> >> long list of all Exynos SoCs that needs to be checked.
>> >>
>> >> An evolution of option a) is registering a platform device somewhere
>> >> in
>> >> arch/arm/mach-exynos and making exynos-cpuidle a platform driver.
>> >
>> > Yes, I am favorable to this solution [2].
>> >
>> >> The
>> >> problem is that you must register a static platform device from arch
>> >> code, because cpuidle is not a real hardware block that can be put
>> >> into device tree.
>> >
>> > Thanks
>> >
>> >   -- Daniel
>> >
>> > [1]
>> > https://git.linaro.org/gitweb?p=people/dlezcano/idlestat.git;a=summar
>> > y [2] http://patches.linaro.org/18368/
>> >
>> > --
>> >
>> >  <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
>> >
>> > --
>> > To unsubscribe from this list: send the line "unsubscribe
>> > linux-samsung-soc" in the body of a message to
>> > majordomo@vger.kernel.org
>> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH] ARM: EXYNOS: cpuidle: Skip C1 cpuidle state for exynos5440

[amit daniel kachhap]

Hi Tomasz,

On Mon, Jul 29, 2013 at 2:03 PM, Tomasz Figa <tomasz.figa@gmail.com> wrote:
> On Monday 29 of July 2013 10:16:14 amit daniel kachhap wrote:
>> Hi Daniel/Tomasz,
>>
>> From the discussion I can conclude that SOC check is needed in the
>> cpuidle driver for deeper C states.
>
> A check is needed for the whole cpuidle driver, so it registers only on
> Exynos SoCs which support deeper C states.

>
>> Only the question is where to
>> insert this.
>
> Exynos doesn't support multiplatform yet, but we must make sure that any
> code being added is multiplatform-aware. So initcall is not a good idea. I
> would put this somewhere on Exynos-specific initialization path, i.e.
> something that would not called for all platforms compiled in (in case of
> multiplatform).
>
> As I discussed with Daniel, this should be using a traditional
> platform_driver model, with the difference that it can't be registered
> from device tree, but rather statically in mach code.
>
> For example, you can add an exynos_register_cpuidle() function in
> arch/arm/mach-exynos/common.c, which registers such platform device and
> always call it from exynos4_dt_machine_init() in mach-exynos4-dt.c. In
> mach-exynos5-dt.c you could make this conditional and check if
> !soc_is_exynos5440().
thanks for the detailed explanation. Certainly initcall is not good
for multikernel. Still I have few concerns.
1) Each C state might be SOC specific. Say there are 3 C states
C0(basic wfi), c1(single CPU power down) and C2 (System wide power
down like AFTR). In this case some exynos soc may support few of these
states and cpuidle driver should bind C-state to correct SOC.
2) Even for default C0 ( simple wfi) currently there is no sysfs
information if the cpuidle driver is not registered(Not quite sure).
I will try to post the new driver by dynamically creating the
cpuidle_state table based on SOC capability and removing the initcall.

Regards,
Amit D
>
> Best regards,
> Tomasz
>
>> Also to perform the SOC there can be 2 ways like
>> 1) each SOC check 4120, 4412, 5250 etc (long list)
>> 2) negate the nonsupporting SOC's like 5440 (small list like current
>> patch) Any opinion?
>>  As Bartlomiej suggested that this patch conflicts with Daniel's
>> earlier patch http://marc.info/?l=linux-arm-kernel&m=137467935712513&w=2
>> So I can re-base my patch on top of this one if needed.
>>
>> Thanks,
>> Amit Daniel
>>
>> On Sun, Jul 28, 2013 at 4:01 PM, Daniel Lezcano
>>
>> <daniel.lezcano@linaro.org> wrote:
>> > On 07/28/2013 11:22 AM, Tomasz Figa wrote:
>> >> On Sunday 28 of July 2013 09:10:09 Daniel Lezcano wrote:
>> >>> On 07/24/2013 01:47 PM, Kukjin Kim wrote:
>> >>>> Amit Daniel Kachhap wrote:
>> >>>>> This patch skips the deep C1(AFTR -Arm off top running) state for
>> >>>>> exynos5440
>> >>>>> soc as this soc does not support this state. All the cpu's only
>> >>>>> allows the basic
>> >>>>> C0 state.
>> >>>>>
>> >>>>> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
>> >>>>> ---
>> >>>>>
>> >>>>>  arch/arm/mach-exynos/cpuidle.c |    2 +-
>> >>>>>  1 files changed, 1 insertions(+), 1 deletions(-)
>> >>>>>
>> >>>>> diff --git a/arch/arm/mach-exynos/cpuidle.c b/arch/arm/mach-
>> >>>>> exynos/cpuidle.c
>> >>>>> index 17a18ff..9a776a1 100644
>> >>>>> --- a/arch/arm/mach-exynos/cpuidle.c
>> >>>>> +++ b/arch/arm/mach-exynos/cpuidle.c
>> >>>>> @@ -210,7 +210,7 @@ static int __init exynos4_init_cpuidle(void)
>> >>>>>
>> >>>>>            device->cpu = cpu_id;
>> >>>>>
>> >>>>>            /* Support IDLE only */
>> >>>>>
>> >>>>> -          if (cpu_id != 0)
>> >>>>> +          if (soc_is_exynos5440() || cpu_id != 0)
>> >>>>>
>> >>>>>                    device->state_count = 1;
>> >>>>>
>> >>>>>            ret = cpuidle_register_device(device);
>> >>>>>
>> >>>>> --
>> >>>>> 1.7.1
>> >>>>
>> >>>> Applied, thanks.
>> >>>
>> >>> You shouldn't have. This patch means exynos5540 has no cpuidle
>> >>> driver at all. It should be fixed in the Kconfig to unselect
>> >>> CONFIG_CPU_IDLE for an exynos5540.
>> >>
>> >> To shed more light on this, let me add that you need to register a
>> >> cpuidle driver only if you have more states than a simple WFI or you
>> >> need some crazy steps to enter WFI. Default setup falls back to
>> >> generic ARM WFI. (Daniel, do we get the nice idle stats as provided
>> >> by cpuidle core then?)>
>> > Nope, but with one state, idle vs busy stats do the trick.
>> >
>> > BTW, I am writing a tool to do some stats based on the idle events
>> > [1].
>> > It is still at a very early development stage but we can get some
>> > interesting informations.
>> >
>> >> Anyway, Exynos cpuidle is using an initcall to initialize and we
>> >> support multiple Exynos SoCs in single zImage, so deselecting
>> >> CONFIG_CPU_IDLE is not an option.
>> >
>> > Good point.
>> >
>> >> Considering multiplatform requirements, the driver has to
>> >>
>> >> be modified to initialize only on supported platforms, either by:
>> >>  a) dropping the initcall and calling the init function directly from
>> >>
>> >> arch/arm/mach-exynos
>> >>
>> >>  or
>> >>
>> >>  b) checking if machine we are running on is supported, which would
>> >>  mean a>>
>> >> long list of all Exynos SoCs that needs to be checked.
>> >>
>> >> An evolution of option a) is registering a platform device somewhere
>> >> in
>> >> arch/arm/mach-exynos and making exynos-cpuidle a platform driver.
>> >
>> > Yes, I am favorable to this solution [2].
>> >
>> >> The
>> >> problem is that you must register a static platform device from arch
>> >> code, because cpuidle is not a real hardware block that can be put
>> >> into device tree.
>> >
>> > Thanks
>> >
>> >   -- Daniel
>> >
>> > [1]
>> > https://git.linaro.org/gitweb?p=people/dlezcano/idlestat.git;a=summar
>> > y [2] http://patches.linaro.org/18368/
>> >
>> > --
>> >
>> >  <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
>> >
>> > --
>> > To unsubscribe from this list: send the line "unsubscribe
>> > linux-samsung-soc" in the body of a message to
>> > majordomo at vger.kernel.org
>> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] ARM: EXYNOS: cpuidle: Skip C1 cpuidle state for exynos5440

[Daniel Lezcano]

On 07/29/2013 11:33 AM, amit daniel kachhap wrote:
> Hi Tomasz,
> 
> On Mon, Jul 29, 2013 at 2:03 PM, Tomasz Figa <tomasz.figa@gmail.com> wrote:
>> On Monday 29 of July 2013 10:16:14 amit daniel kachhap wrote:
>>> Hi Daniel/Tomasz,
>>>
>>> From the discussion I can conclude that SOC check is needed in the
>>> cpuidle driver for deeper C states.
>>
>> A check is needed for the whole cpuidle driver, so it registers only on
>> Exynos SoCs which support deeper C states.
> 
>>
>>> Only the question is where to
>>> insert this.
>>
>> Exynos doesn't support multiplatform yet, but we must make sure that any
>> code being added is multiplatform-aware. So initcall is not a good idea. I
>> would put this somewhere on Exynos-specific initialization path, i.e.
>> something that would not called for all platforms compiled in (in case of
>> multiplatform).
>>
>> As I discussed with Daniel, this should be using a traditional
>> platform_driver model, with the difference that it can't be registered
>> from device tree, but rather statically in mach code.
>>
>> For example, you can add an exynos_register_cpuidle() function in
>> arch/arm/mach-exynos/common.c, which registers such platform device and
>> always call it from exynos4_dt_machine_init() in mach-exynos4-dt.c. In
>> mach-exynos5-dt.c you could make this conditional and check if
>> !soc_is_exynos5440().
> thanks for the detailed explanation. Certainly initcall is not good
> for multikernel. Still I have few concerns.
> 1) Each C state might be SOC specific. Say there are 3 C states
> C0(basic wfi), c1(single CPU power down) and C2 (System wide power
> down like AFTR). In this case some exynos soc may support few of these
> states and cpuidle driver should bind C-state to correct SOC.
> 2) Even for default C0 ( simple wfi) currently there is no sysfs
> information if the cpuidle driver is not registered(Not quite sure).
> I will try to post the new driver by dynamically creating the
> cpuidle_state table based on SOC capability and removing the initcall.

Why don't you create a cpuidle driver for each SoC family ?

I am cleaning up all the drivers to unify the code, hoping we can factor
all the drivers through a single ARM driver and assign the ops for the
different states.

As Thomasz said, the platform driver model should be used, no need of
initcall, the code path will initialize the driver or not depending of
the SoC. You should not have any "soc_is_*" checks. This is the
direction all the ARM drivers are going to.

Look at the example I sent with the ux500, it illustrates the idea.
There are other examples also: cpuidle-kirkwood.c, cpuidle for davinci.

Concerning the "single cpu power down" state, is the exynos able to
handle that ? I mean, is it possible to power down a core and have it
woken up by an interrupt ?

Concerning the AFTR state : Is there a way to use this state with *all*
cpus online ? If not, shall we use the same approach than omap4, that is
CPU0 power down CPU1 and goes to AFTR (all interrupts migrated to CPU0)
and then wake up CPU1 when CPU0 is woken up ?

Concerning WFI and sysfs, if there are no cpuidle driver registered, no
sysfs. I would not focus on and spent too much effort for the statistics
because a lot of changes will certainly happen with the power aware
scheduler and the statistics will likely appear at this level.

Regards
  -- Daniel



>> Best regards,
>> Tomasz
>>
>>> Also to perform the SOC there can be 2 ways like
>>> 1) each SOC check 4120, 4412, 5250 etc (long list)
>>> 2) negate the nonsupporting SOC's like 5440 (small list like current
>>> patch) Any opinion?
>>>  As Bartlomiej suggested that this patch conflicts with Daniel's
>>> earlier patch http://marc.info/?l=linux-arm-kernel&m=137467935712513&w=2
>>> So I can re-base my patch on top of this one if needed.
>>>
>>> Thanks,
>>> Amit Daniel
>>>
>>> On Sun, Jul 28, 2013 at 4:01 PM, Daniel Lezcano
>>>
>>> <daniel.lezcano@linaro.org> wrote:
>>>> On 07/28/2013 11:22 AM, Tomasz Figa wrote:
>>>>> On Sunday 28 of July 2013 09:10:09 Daniel Lezcano wrote:
>>>>>> On 07/24/2013 01:47 PM, Kukjin Kim wrote:
>>>>>>> Amit Daniel Kachhap wrote:
>>>>>>>> This patch skips the deep C1(AFTR -Arm off top running) state for
>>>>>>>> exynos5440
>>>>>>>> soc as this soc does not support this state. All the cpu's only
>>>>>>>> allows the basic
>>>>>>>> C0 state.
>>>>>>>>
>>>>>>>> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
>>>>>>>> ---
>>>>>>>>
>>>>>>>>  arch/arm/mach-exynos/cpuidle.c |    2 +-
>>>>>>>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/arch/arm/mach-exynos/cpuidle.c b/arch/arm/mach-
>>>>>>>> exynos/cpuidle.c
>>>>>>>> index 17a18ff..9a776a1 100644
>>>>>>>> --- a/arch/arm/mach-exynos/cpuidle.c
>>>>>>>> +++ b/arch/arm/mach-exynos/cpuidle.c
>>>>>>>> @@ -210,7 +210,7 @@ static int __init exynos4_init_cpuidle(void)
>>>>>>>>
>>>>>>>>            device->cpu = cpu_id;
>>>>>>>>
>>>>>>>>            /* Support IDLE only */
>>>>>>>>
>>>>>>>> -          if (cpu_id != 0)
>>>>>>>> +          if (soc_is_exynos5440() || cpu_id != 0)
>>>>>>>>
>>>>>>>>                    device->state_count = 1;
>>>>>>>>
>>>>>>>>            ret = cpuidle_register_device(device);
>>>>>>>>
>>>>>>>> --
>>>>>>>> 1.7.1
>>>>>>>
>>>>>>> Applied, thanks.
>>>>>>
>>>>>> You shouldn't have. This patch means exynos5540 has no cpuidle
>>>>>> driver at all. It should be fixed in the Kconfig to unselect
>>>>>> CONFIG_CPU_IDLE for an exynos5540.
>>>>>
>>>>> To shed more light on this, let me add that you need to register a
>>>>> cpuidle driver only if you have more states than a simple WFI or you
>>>>> need some crazy steps to enter WFI. Default setup falls back to
>>>>> generic ARM WFI. (Daniel, do we get the nice idle stats as provided
>>>>> by cpuidle core then?)>
>>>> Nope, but with one state, idle vs busy stats do the trick.
>>>>
>>>> BTW, I am writing a tool to do some stats based on the idle events
>>>> [1].
>>>> It is still at a very early development stage but we can get some
>>>> interesting informations.
>>>>
>>>>> Anyway, Exynos cpuidle is using an initcall to initialize and we
>>>>> support multiple Exynos SoCs in single zImage, so deselecting
>>>>> CONFIG_CPU_IDLE is not an option.
>>>>
>>>> Good point.
>>>>
>>>>> Considering multiplatform requirements, the driver has to
>>>>>
>>>>> be modified to initialize only on supported platforms, either by:
>>>>>  a) dropping the initcall and calling the init function directly from
>>>>>
>>>>> arch/arm/mach-exynos
>>>>>
>>>>>  or
>>>>>
>>>>>  b) checking if machine we are running on is supported, which would
>>>>>  mean a>>
>>>>> long list of all Exynos SoCs that needs to be checked.
>>>>>
>>>>> An evolution of option a) is registering a platform device somewhere
>>>>> in
>>>>> arch/arm/mach-exynos and making exynos-cpuidle a platform driver.
>>>>
>>>> Yes, I am favorable to this solution [2].
>>>>
>>>>> The
>>>>> problem is that you must register a static platform device from arch
>>>>> code, because cpuidle is not a real hardware block that can be put
>>>>> into device tree.
>>>>
>>>> Thanks
>>>>
>>>>   -- Daniel
>>>>
>>>> [1]
>>>> https://git.linaro.org/gitweb?p=people/dlezcano/idlestat.git;a=summar
>>>> y [2] http://patches.linaro.org/18368/
>>>>
>>>> --
>>>>
>>>>  <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
>>>>
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe
>>>> linux-samsung-soc" in the body of a message to
>>>> majordomo@vger.kernel.org
>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel


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

[PATCH] ARM: EXYNOS: cpuidle: Skip C1 cpuidle state for exynos5440

[Daniel Lezcano]

On 07/29/2013 11:33 AM, amit daniel kachhap wrote:
> Hi Tomasz,
> 
> On Mon, Jul 29, 2013 at 2:03 PM, Tomasz Figa <tomasz.figa@gmail.com> wrote:
>> On Monday 29 of July 2013 10:16:14 amit daniel kachhap wrote:
>>> Hi Daniel/Tomasz,
>>>
>>> From the discussion I can conclude that SOC check is needed in the
>>> cpuidle driver for deeper C states.
>>
>> A check is needed for the whole cpuidle driver, so it registers only on
>> Exynos SoCs which support deeper C states.
> 
>>
>>> Only the question is where to
>>> insert this.
>>
>> Exynos doesn't support multiplatform yet, but we must make sure that any
>> code being added is multiplatform-aware. So initcall is not a good idea. I
>> would put this somewhere on Exynos-specific initialization path, i.e.
>> something that would not called for all platforms compiled in (in case of
>> multiplatform).
>>
>> As I discussed with Daniel, this should be using a traditional
>> platform_driver model, with the difference that it can't be registered
>> from device tree, but rather statically in mach code.
>>
>> For example, you can add an exynos_register_cpuidle() function in
>> arch/arm/mach-exynos/common.c, which registers such platform device and
>> always call it from exynos4_dt_machine_init() in mach-exynos4-dt.c. In
>> mach-exynos5-dt.c you could make this conditional and check if
>> !soc_is_exynos5440().
> thanks for the detailed explanation. Certainly initcall is not good
> for multikernel. Still I have few concerns.
> 1) Each C state might be SOC specific. Say there are 3 C states
> C0(basic wfi), c1(single CPU power down) and C2 (System wide power
> down like AFTR). In this case some exynos soc may support few of these
> states and cpuidle driver should bind C-state to correct SOC.
> 2) Even for default C0 ( simple wfi) currently there is no sysfs
> information if the cpuidle driver is not registered(Not quite sure).
> I will try to post the new driver by dynamically creating the
> cpuidle_state table based on SOC capability and removing the initcall.

Why don't you create a cpuidle driver for each SoC family ?

I am cleaning up all the drivers to unify the code, hoping we can factor
all the drivers through a single ARM driver and assign the ops for the
different states.

As Thomasz said, the platform driver model should be used, no need of
initcall, the code path will initialize the driver or not depending of
the SoC. You should not have any "soc_is_*" checks. This is the
direction all the ARM drivers are going to.

Look at the example I sent with the ux500, it illustrates the idea.
There are other examples also: cpuidle-kirkwood.c, cpuidle for davinci.

Concerning the "single cpu power down" state, is the exynos able to
handle that ? I mean, is it possible to power down a core and have it
woken up by an interrupt ?

Concerning the AFTR state : Is there a way to use this state with *all*
cpus online ? If not, shall we use the same approach than omap4, that is
CPU0 power down CPU1 and goes to AFTR (all interrupts migrated to CPU0)
and then wake up CPU1 when CPU0 is woken up ?

Concerning WFI and sysfs, if there are no cpuidle driver registered, no
sysfs. I would not focus on and spent too much effort for the statistics
because a lot of changes will certainly happen with the power aware
scheduler and the statistics will likely appear at this level.

Regards
  -- Daniel



>> Best regards,
>> Tomasz
>>
>>> Also to perform the SOC there can be 2 ways like
>>> 1) each SOC check 4120, 4412, 5250 etc (long list)
>>> 2) negate the nonsupporting SOC's like 5440 (small list like current
>>> patch) Any opinion?
>>>  As Bartlomiej suggested that this patch conflicts with Daniel's
>>> earlier patch http://marc.info/?l=linux-arm-kernel&m=137467935712513&w=2
>>> So I can re-base my patch on top of this one if needed.
>>>
>>> Thanks,
>>> Amit Daniel
>>>
>>> On Sun, Jul 28, 2013 at 4:01 PM, Daniel Lezcano
>>>
>>> <daniel.lezcano@linaro.org> wrote:
>>>> On 07/28/2013 11:22 AM, Tomasz Figa wrote:
>>>>> On Sunday 28 of July 2013 09:10:09 Daniel Lezcano wrote:
>>>>>> On 07/24/2013 01:47 PM, Kukjin Kim wrote:
>>>>>>> Amit Daniel Kachhap wrote:
>>>>>>>> This patch skips the deep C1(AFTR -Arm off top running) state for
>>>>>>>> exynos5440
>>>>>>>> soc as this soc does not support this state. All the cpu's only
>>>>>>>> allows the basic
>>>>>>>> C0 state.
>>>>>>>>
>>>>>>>> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
>>>>>>>> ---
>>>>>>>>
>>>>>>>>  arch/arm/mach-exynos/cpuidle.c |    2 +-
>>>>>>>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/arch/arm/mach-exynos/cpuidle.c b/arch/arm/mach-
>>>>>>>> exynos/cpuidle.c
>>>>>>>> index 17a18ff..9a776a1 100644
>>>>>>>> --- a/arch/arm/mach-exynos/cpuidle.c
>>>>>>>> +++ b/arch/arm/mach-exynos/cpuidle.c
>>>>>>>> @@ -210,7 +210,7 @@ static int __init exynos4_init_cpuidle(void)
>>>>>>>>
>>>>>>>>            device->cpu = cpu_id;
>>>>>>>>
>>>>>>>>            /* Support IDLE only */
>>>>>>>>
>>>>>>>> -          if (cpu_id != 0)
>>>>>>>> +          if (soc_is_exynos5440() || cpu_id != 0)
>>>>>>>>
>>>>>>>>                    device->state_count = 1;
>>>>>>>>
>>>>>>>>            ret = cpuidle_register_device(device);
>>>>>>>>
>>>>>>>> --
>>>>>>>> 1.7.1
>>>>>>>
>>>>>>> Applied, thanks.
>>>>>>
>>>>>> You shouldn't have. This patch means exynos5540 has no cpuidle
>>>>>> driver at all. It should be fixed in the Kconfig to unselect
>>>>>> CONFIG_CPU_IDLE for an exynos5540.
>>>>>
>>>>> To shed more light on this, let me add that you need to register a
>>>>> cpuidle driver only if you have more states than a simple WFI or you
>>>>> need some crazy steps to enter WFI. Default setup falls back to
>>>>> generic ARM WFI. (Daniel, do we get the nice idle stats as provided
>>>>> by cpuidle core then?)>
>>>> Nope, but with one state, idle vs busy stats do the trick.
>>>>
>>>> BTW, I am writing a tool to do some stats based on the idle events
>>>> [1].
>>>> It is still at a very early development stage but we can get some
>>>> interesting informations.
>>>>
>>>>> Anyway, Exynos cpuidle is using an initcall to initialize and we
>>>>> support multiple Exynos SoCs in single zImage, so deselecting
>>>>> CONFIG_CPU_IDLE is not an option.
>>>>
>>>> Good point.
>>>>
>>>>> Considering multiplatform requirements, the driver has to
>>>>>
>>>>> be modified to initialize only on supported platforms, either by:
>>>>>  a) dropping the initcall and calling the init function directly from
>>>>>
>>>>> arch/arm/mach-exynos
>>>>>
>>>>>  or
>>>>>
>>>>>  b) checking if machine we are running on is supported, which would
>>>>>  mean a>>
>>>>> long list of all Exynos SoCs that needs to be checked.
>>>>>
>>>>> An evolution of option a) is registering a platform device somewhere
>>>>> in
>>>>> arch/arm/mach-exynos and making exynos-cpuidle a platform driver.
>>>>
>>>> Yes, I am favorable to this solution [2].
>>>>
>>>>> The
>>>>> problem is that you must register a static platform device from arch
>>>>> code, because cpuidle is not a real hardware block that can be put
>>>>> into device tree.
>>>>
>>>> Thanks
>>>>
>>>>   -- Daniel
>>>>
>>>> [1]
>>>> https://git.linaro.org/gitweb?p=people/dlezcano/idlestat.git;a=summar
>>>> y [2] http://patches.linaro.org/18368/
>>>>
>>>> --
>>>>
>>>>  <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
>>>>
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe
>>>> linux-samsung-soc" in the body of a message to
>>>> majordomo at vger.kernel.org
>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel at lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel


-- 
 <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] ARM: EXYNOS: cpuidle: Skip C1 cpuidle state for exynos5440

[Bartlomiej Zolnierkiewicz]

[ added Daniel and linux-pm to cc: ]

Hi,

On Wednesday, July 24, 2013 03:23:35 PM Amit Daniel Kachhap wrote:
> This patch skips the deep C1(AFTR -Arm off top running) state for exynos5440
> soc as this soc does not support this state. All the cpu's only allows the basic
> C0 state.
> 
> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
> ---
>  arch/arm/mach-exynos/cpuidle.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm/mach-exynos/cpuidle.c b/arch/arm/mach-exynos/cpuidle.c
> index 17a18ff..9a776a1 100644
> --- a/arch/arm/mach-exynos/cpuidle.c
> +++ b/arch/arm/mach-exynos/cpuidle.c
> @@ -210,7 +210,7 @@ static int __init exynos4_init_cpuidle(void)
>  		device->cpu = cpu_id;
>  
>  		/* Support IDLE only */
> -		if (cpu_id != 0)
> +		if (soc_is_exynos5440() || cpu_id != 0)
>  			device->state_count = 1;
>  
>  		ret = cpuidle_register_device(device);

This may not achieve what you are intending to do and conflicts with
Daniel's patch (which removes code that you're modifying). Please see:

	http://marc.info/?l=linux-arm-kernel&m=137467935712513&w=2

How's about changing exynos4_idle_driver.state_count for EXYNOS5540
at the beginning of exynos4_init_cpuidle() instead?

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

[PATCH] ARM: EXYNOS: cpuidle: Skip C1 cpuidle state for exynos5440

[Bartlomiej Zolnierkiewicz]

[ added Daniel and linux-pm to cc: ]

Hi,

On Wednesday, July 24, 2013 03:23:35 PM Amit Daniel Kachhap wrote:
> This patch skips the deep C1(AFTR -Arm off top running) state for exynos5440
> soc as this soc does not support this state. All the cpu's only allows the basic
> C0 state.
> 
> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
> ---
>  arch/arm/mach-exynos/cpuidle.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm/mach-exynos/cpuidle.c b/arch/arm/mach-exynos/cpuidle.c
> index 17a18ff..9a776a1 100644
> --- a/arch/arm/mach-exynos/cpuidle.c
> +++ b/arch/arm/mach-exynos/cpuidle.c
> @@ -210,7 +210,7 @@ static int __init exynos4_init_cpuidle(void)
>  		device->cpu = cpu_id;
>  
>  		/* Support IDLE only */
> -		if (cpu_id != 0)
> +		if (soc_is_exynos5440() || cpu_id != 0)
>  			device->state_count = 1;
>  
>  		ret = cpuidle_register_device(device);

This may not achieve what you are intending to do and conflicts with
Daniel's patch (which removes code that you're modifying). Please see:

	http://marc.info/?l=linux-arm-kernel&m=137467935712513&w=2

How's about changing exynos4_idle_driver.state_count for EXYNOS5540
at the beginning of exynos4_init_cpuidle() instead?

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

Re: [PATCH V2 2/2] ARM: EXYNOS: cpuidle: Allow C1 state only in supported SOC's.

[amit daniel kachhap]

On Wed, Jul 24, 2013 at 1:06 PM, Kukjin Kim <kgene@kernel.org> wrote:
> amit daniel kachhap wrote:
>
> [...]
>
>> >>               /* Support IDLE only */
>> >> -             if (cpu_id != 0)
>> >> +             if (!(soc_is_exynos4210() || soc_is_exynos4212() ||
>> >> +                     soc_is_exynos4412() || soc_is_exynos5250()) ||
>> >> +                     cpu_id != 0)
>> >
>> > How about exynos5420?
>> >
>> > So...
>> >
>> > +               if (soc_is_exynos5440() || cpu_id !=0) ?
>> This is fine.
>>
> So, you will re-send?
Just submitted the patch with this change.

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

[PATCH V2 2/2] ARM: EXYNOS: cpuidle: Allow C1 state only in supported SOC's.

[amit daniel kachhap]

On Wed, Jul 24, 2013 at 1:06 PM, Kukjin Kim <kgene@kernel.org> wrote:
> amit daniel kachhap wrote:
>
> [...]
>
>> >>               /* Support IDLE only */
>> >> -             if (cpu_id != 0)
>> >> +             if (!(soc_is_exynos4210() || soc_is_exynos4212() ||
>> >> +                     soc_is_exynos4412() || soc_is_exynos5250()) ||
>> >> +                     cpu_id != 0)
>> >
>> > How about exynos5420?
>> >
>> > So...
>> >
>> > +               if (soc_is_exynos5440() || cpu_id !=0) ?
>> This is fine.
>>
> So, you will re-send?
Just submitted the patch with this change.

Thanks,
Amit
>
> - Kukjin
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html