[PATCH v2 0/9] cpuidle: rework device state count handling

[PATCH v2 0/9] cpuidle: rework device state count handling

[Bartlomiej Zolnierkiewicz]

Hi,

Some cpuidle drivers assume that cpuidle core will handle cases where
device->state_count is smaller than driver->state_count, unfortunately
currently this is untrue (device->state_count is used only for handling
cpuidle state sysfs entries and driver->state_count is used for all
other cases) and will not be fixed in the future as device->state_count
is planned to be removed [1].

This patchset fixes such drivers (ARM EXYNOS cpuidle driver and ACPI
cpuidle driver), removes superflous device->state_count initialization
from drivers for which device->state_count equals driver->state_count
(POWERPC pseries cpuidle driver and intel_idle driver) and finally
removes state_count field from struct cpuidle_device.

Additionaly (while at it) this patchset fixes C1E promotion disable
quirk handling (in intel_idle driver) and converts cpuidle drivers code
to use the common cpuidle_[un]register() routines (in POWERPC pseries
cpuidle driver and intel_idle driver).

[1] http://permalink.gmane.org/gmane.linux.power-management.general/36908

Reference to v1:
	http://comments.gmane.org/gmane.linux.power-management.general/37390

Changes since v1:
- synced patch series with next-20131220
- added ACKs from Daniel Lezcano

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


Bartlomiej Zolnierkiewicz (9):
  ARM: EXYNOS: cpuidle: fix AFTR mode check
  POWERPC: pseries: cpuidle: remove superfluous dev->state_count
    initialization
  POWERPC: pseries: cpuidle: use the common cpuidle_[un]register()
    routines
  ACPI / cpuidle: fix max idle state handling with hotplug CPU support
  ACPI / cpuidle: remove dev->state_count setting
  intel_idle: do C1E promotion disable quirk for hotplugged CPUs
  intel_idle: remove superfluous dev->state_count initialization
  intel_idle: use the common cpuidle_[un]register() routines
  cpuidle: remove state_count field from struct cpuidle_device

 arch/arm/mach-exynos/cpuidle.c                  |   8 +-
 arch/powerpc/platforms/pseries/processor_idle.c |  59 +---------
 drivers/acpi/processor_idle.c                   |  29 +++--
 drivers/cpuidle/cpuidle.c                       |   3 -
 drivers/cpuidle/sysfs.c                         |   5 +-
 drivers/idle/intel_idle.c                       | 140 +++++-------------------
 include/linux/cpuidle.h                         |   1 -
 7 files changed, 51 insertions(+), 194 deletions(-)

-- 
1.8.2.3

[PATCH v2 1/9] ARM: EXYNOS: cpuidle: fix AFTR mode check

[Bartlomiej Zolnierkiewicz]

The EXYNOS cpuidle driver code assumes that cpuidle core will handle
dev->state_count smaller than drv->state_count but currently this is
untrue (dev->state_count is used only for handling cpuidle state sysfs
entries and drv->state_count is used for all other cases) and will not
be fixed in the future as dev->state_count is planned to be removed.

Fix the issue by checking for the max supported idle state in AFTR
state's ->enter handler (exynos4_enter_lowpower()) and entering AFTR
mode only when cores other than CPU0 are offline.

Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Kukjin Kim <kgene.kim@samsung.com>
---
 arch/arm/mach-exynos/cpuidle.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/arch/arm/mach-exynos/cpuidle.c b/arch/arm/mach-exynos/cpuidle.c
index da65b03..f57cb91 100644
--- a/arch/arm/mach-exynos/cpuidle.c
+++ b/arch/arm/mach-exynos/cpuidle.c
@@ -172,8 +172,8 @@ static int exynos4_enter_lowpower(struct cpuidle_device *dev,
 {
 	int new_index = index;
 
-	/* This mode only can be entered when other core's are offline */
-	if (num_online_cpus() > 1)
+	/* AFTR can only be entered when cores other than CPU0 are offline */
+	if (num_online_cpus() > 1 || dev->cpu != 0)
 		new_index = drv->safe_state_index;
 
 	if (new_index == 0)
@@ -235,10 +235,6 @@ static int exynos_cpuidle_probe(struct platform_device *pdev)
 		device = &per_cpu(exynos4_cpuidle_device, cpu_id);
 		device->cpu = cpu_id;
 
-		/* Support IDLE only */
-		if (cpu_id != 0)
-			device->state_count = 1;
-
 		ret = cpuidle_register_device(device);
 		if (ret) {
 			dev_err(&pdev->dev, "failed to register cpuidle device\n");
-- 
1.8.2.3

Re: [PATCH v2 1/9] ARM: EXYNOS: cpuidle: fix AFTR mode check

[Kukjin Kim]

On 12/21/13 03:47, Bartlomiej Zolnierkiewicz wrote:
> The EXYNOS cpuidle driver code assumes that cpuidle core will handle
> dev->state_count smaller than drv->state_count but currently this is
> untrue (dev->state_count is used only for handling cpuidle state sysfs
> entries and drv->state_count is used for all other cases) and will not
> be fixed in the future as dev->state_count is planned to be removed.
>
> Fix the issue by checking for the max supported idle state in AFTR
> state's ->enter handler (exynos4_enter_lowpower()) and entering AFTR
> mode only when cores other than CPU0 are offline.
>
> Signed-off-by: Bartlomiej Zolnierkiewicz<b.zolnierkie@samsung.com>
> Signed-off-by: Kyungmin Park<kyungmin.park@samsung.com>
> Acked-by: Daniel Lezcano<daniel.lezcano@linaro.org>
> Cc: Kukjin Kim<kgene.kim@samsung.com>

Acked-by: Kukjin Kim <kgene.kim@samsung.com>

Thanks,
Kukjin

> ---
>   arch/arm/mach-exynos/cpuidle.c | 8 ++------
>   1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm/mach-exynos/cpuidle.c b/arch/arm/mach-exynos/cpuidle.c
> index da65b03..f57cb91 100644
> --- a/arch/arm/mach-exynos/cpuidle.c
> +++ b/arch/arm/mach-exynos/cpuidle.c
> @@ -172,8 +172,8 @@ static int exynos4_enter_lowpower(struct cpuidle_device *dev,
>   {
>   	int new_index = index;
>
> -	/* This mode only can be entered when other core's are offline */
> -	if (num_online_cpus()>  1)
> +	/* AFTR can only be entered when cores other than CPU0 are offline */
> +	if (num_online_cpus()>  1 || dev->cpu != 0)
>   		new_index = drv->safe_state_index;
>
>   	if (new_index == 0)
> @@ -235,10 +235,6 @@ static int exynos_cpuidle_probe(struct platform_device *pdev)
>   		device =&per_cpu(exynos4_cpuidle_device, cpu_id);
>   		device->cpu = cpu_id;
>
> -		/* Support IDLE only */
> -		if (cpu_id != 0)
> -			device->state_count = 1;
> -
>   		ret = cpuidle_register_device(device);
>   		if (ret) {
>   			dev_err(&pdev->dev, "failed to register cpuidle device\n");

Re: [PATCH v2 1/9] ARM: EXYNOS: cpuidle: fix AFTR mode check

[Daniel Lezcano]

On 12/20/2013 07:47 PM, Bartlomiej Zolnierkiewicz wrote:
> The EXYNOS cpuidle driver code assumes that cpuidle core will handle
> dev->state_count smaller than drv->state_count but currently this is
> untrue (dev->state_count is used only for handling cpuidle state sysfs
> entries and drv->state_count is used for all other cases) and will not
> be fixed in the future as dev->state_count is planned to be removed.
>
> Fix the issue by checking for the max supported idle state in AFTR
> state's ->enter handler (exynos4_enter_lowpower()) and entering AFTR
> mode only when cores other than CPU0 are offline.
>
> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> Cc: Kukjin Kim <kgene.kim@samsung.com>
> ---
>   arch/arm/mach-exynos/cpuidle.c | 8 ++------
>   1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm/mach-exynos/cpuidle.c b/arch/arm/mach-exynos/cpuidle.c
> index da65b03..f57cb91 100644
> --- a/arch/arm/mach-exynos/cpuidle.c
> +++ b/arch/arm/mach-exynos/cpuidle.c
> @@ -172,8 +172,8 @@ static int exynos4_enter_lowpower(struct cpuidle_device *dev,
>   {
>   	int new_index = index;
>
> -	/* This mode only can be entered when other core's are offline */
> -	if (num_online_cpus() > 1)
> +	/* AFTR can only be entered when cores other than CPU0 are offline */
> +	if (num_online_cpus() > 1 || dev->cpu != 0)
>   		new_index = drv->safe_state_index;
>
>   	if (new_index == 0)
> @@ -235,10 +235,6 @@ static int exynos_cpuidle_probe(struct platform_device *pdev)
>   		device = &per_cpu(exynos4_cpuidle_device, cpu_id);
>   		device->cpu = cpu_id;
>
> -		/* Support IDLE only */
> -		if (cpu_id != 0)
> -			device->state_count = 1;
> -
>   		ret = cpuidle_register_device(device);
>   		if (ret) {
>   			dev_err(&pdev->dev, "failed to register cpuidle device\n");
>

Hi Bartlomiej,

thanks for this cleanup. May be you can also add another patch to switch 
to the generic cpuidle_register function ?

Thanks
   -- Daniel

-- 
  <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 v2 2/9] POWERPC: pseries: cpuidle: remove superfluous dev->state_count initialization

[Bartlomiej Zolnierkiewicz]

pseries cpuidle driver sets dev->state_count to drv->state_count so
the default dev->state_count initialization in cpuidle_enable_device()
(called from cpuidle_register_device()) can be used instead.

Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Deepthi Dharwar <deepthi@linux.vnet.ibm.com>
---
 arch/powerpc/platforms/pseries/processor_idle.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/processor_idle.c b/arch/powerpc/platforms/pseries/processor_idle.c
index a166e38..8aa8c40 100644
--- a/arch/powerpc/platforms/pseries/processor_idle.c
+++ b/arch/powerpc/platforms/pseries/processor_idle.c
@@ -271,7 +271,6 @@ static void pseries_idle_devices_uninit(void)
 static int pseries_idle_devices_init(void)
 {
 	int i;
-	struct cpuidle_driver *drv = &pseries_idle_driver;
 	struct cpuidle_device *dev;
 
 	pseries_cpuidle_devices = alloc_percpu(struct cpuidle_device);
@@ -280,7 +279,6 @@ static int pseries_idle_devices_init(void)
 
 	for_each_possible_cpu(i) {
 		dev = per_cpu_ptr(pseries_cpuidle_devices, i);
-		dev->state_count = drv->state_count;
 		dev->cpu = i;
 		if (cpuidle_register_device(dev)) {
 			printk(KERN_DEBUG \
-- 
1.8.2.3

Re: [PATCH v2 2/9] POWERPC: pseries: cpuidle: remove superfluous dev->state_count initialization

[Deepthi Dharwar]

On 12/21/2013 12:17 AM, Bartlomiej Zolnierkiewicz wrote:
> pseries cpuidle driver sets dev->state_count to drv->state_count so
> the default dev->state_count initialization in cpuidle_enable_device()
> (called from cpuidle_register_device()) can be used instead.
> 
> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>

Acked-by: Deepthi Dharwar <deepthi@linux.vnet.ibm.com>

> Cc: Deepthi Dharwar <deepthi@linux.vnet.ibm.com>
> ---
>  arch/powerpc/platforms/pseries/processor_idle.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/pseries/processor_idle.c b/arch/powerpc/platforms/pseries/processor_idle.c
> index a166e38..8aa8c40 100644
> --- a/arch/powerpc/platforms/pseries/processor_idle.c
> +++ b/arch/powerpc/platforms/pseries/processor_idle.c
> @@ -271,7 +271,6 @@ static void pseries_idle_devices_uninit(void)
>  static int pseries_idle_devices_init(void)
>  {
>  	int i;
> -	struct cpuidle_driver *drv = &pseries_idle_driver;
>  	struct cpuidle_device *dev;
> 
>  	pseries_cpuidle_devices = alloc_percpu(struct cpuidle_device);
> @@ -280,7 +279,6 @@ static int pseries_idle_devices_init(void)
> 
>  	for_each_possible_cpu(i) {
>  		dev = per_cpu_ptr(pseries_cpuidle_devices, i);
> -		dev->state_count = drv->state_count;
>  		dev->cpu = i;
>  		if (cpuidle_register_device(dev)) {
>  			printk(KERN_DEBUG \
> 

[PATCH v2 3/9] POWERPC: pseries: cpuidle: use the common cpuidle_[un]register() routines

[Bartlomiej Zolnierkiewicz]

It is now possible to use the common cpuidle_[un]register() routines
(instead of open-coding them) so do it.

Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Deepthi Dharwar <deepthi@linux.vnet.ibm.com>
---
 arch/powerpc/platforms/pseries/processor_idle.c | 57 ++-----------------------
 1 file changed, 3 insertions(+), 54 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/processor_idle.c b/arch/powerpc/platforms/pseries/processor_idle.c
index 8aa8c40..94134a5 100644
--- a/arch/powerpc/platforms/pseries/processor_idle.c
+++ b/arch/powerpc/platforms/pseries/processor_idle.c
@@ -28,7 +28,6 @@ struct cpuidle_driver pseries_idle_driver = {
 #define MAX_IDLE_STATE_COUNT	2
 
 static int max_idle_state = MAX_IDLE_STATE_COUNT - 1;
-static struct cpuidle_device __percpu *pseries_cpuidle_devices;
 static struct cpuidle_state *cpuidle_state_table;
 
 static inline void idle_loop_prolog(unsigned long *in_purr)
@@ -191,7 +190,7 @@ static int pseries_cpuidle_add_cpu_notifier(struct notifier_block *n,
 {
 	int hotcpu = (unsigned long)hcpu;
 	struct cpuidle_device *dev =
-			per_cpu_ptr(pseries_cpuidle_devices, hotcpu);
+			per_cpu_ptr(cpuidle_devices, hotcpu);
 
 	if (dev && cpuidle_get_driver()) {
 		switch (action) {
@@ -248,48 +247,6 @@ static int pseries_cpuidle_driver_init(void)
 	return 0;
 }
 
-/* pseries_idle_devices_uninit(void)
- * unregister cpuidle devices and de-allocate memory
- */
-static void pseries_idle_devices_uninit(void)
-{
-	int i;
-	struct cpuidle_device *dev;
-
-	for_each_possible_cpu(i) {
-		dev = per_cpu_ptr(pseries_cpuidle_devices, i);
-		cpuidle_unregister_device(dev);
-	}
-
-	free_percpu(pseries_cpuidle_devices);
-	return;
-}
-
-/* pseries_idle_devices_init()
- * allocate, initialize and register cpuidle device
- */
-static int pseries_idle_devices_init(void)
-{
-	int i;
-	struct cpuidle_device *dev;
-
-	pseries_cpuidle_devices = alloc_percpu(struct cpuidle_device);
-	if (pseries_cpuidle_devices == NULL)
-		return -ENOMEM;
-
-	for_each_possible_cpu(i) {
-		dev = per_cpu_ptr(pseries_cpuidle_devices, i);
-		dev->cpu = i;
-		if (cpuidle_register_device(dev)) {
-			printk(KERN_DEBUG \
-				"cpuidle_register_device %d failed!\n", i);
-			return -EIO;
-		}
-	}
-
-	return 0;
-}
-
 /*
  * pseries_idle_probe()
  * Choose state table for shared versus dedicated partition
@@ -325,19 +282,12 @@ static int __init pseries_processor_idle_init(void)
 		return retval;
 
 	pseries_cpuidle_driver_init();
-	retval = cpuidle_register_driver(&pseries_idle_driver);
+	retval = cpuidle_register(&pseries_idle_driver, NULL);
 	if (retval) {
 		printk(KERN_DEBUG "Registration of pseries driver failed.\n");
 		return retval;
 	}
 
-	retval = pseries_idle_devices_init();
-	if (retval) {
-		pseries_idle_devices_uninit();
-		cpuidle_unregister_driver(&pseries_idle_driver);
-		return retval;
-	}
-
 	register_cpu_notifier(&setup_hotplug_notifier);
 	printk(KERN_DEBUG "pseries_idle_driver registered\n");
 
@@ -348,8 +298,7 @@ static void __exit pseries_processor_idle_exit(void)
 {
 
 	unregister_cpu_notifier(&setup_hotplug_notifier);
-	pseries_idle_devices_uninit();
-	cpuidle_unregister_driver(&pseries_idle_driver);
+	cpuidle_unregister(&pseries_idle_driver);
 
 	return;
 }
-- 
1.8.2.3

Re: [PATCH v2 3/9] POWERPC: pseries: cpuidle: use the common cpuidle_[un]register() routines

[Deepthi Dharwar]

On 12/21/2013 12:17 AM, Bartlomiej Zolnierkiewicz wrote:
> It is now possible to use the common cpuidle_[un]register() routines
> (instead of open-coding them) so do it.
> 
> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>

Acked-by: Deepthi Dharwar <deepthi@linux.vnet.ibm.com>

> Cc: Deepthi Dharwar <deepthi@linux.vnet.ibm.com>
> ---
>  arch/powerpc/platforms/pseries/processor_idle.c | 57 ++-----------------------
>  1 file changed, 3 insertions(+), 54 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/pseries/processor_idle.c b/arch/powerpc/platforms/pseries/processor_idle.c
> index 8aa8c40..94134a5 100644
> --- a/arch/powerpc/platforms/pseries/processor_idle.c
> +++ b/arch/powerpc/platforms/pseries/processor_idle.c
> @@ -28,7 +28,6 @@ struct cpuidle_driver pseries_idle_driver = {
>  #define MAX_IDLE_STATE_COUNT	2
> 
>  static int max_idle_state = MAX_IDLE_STATE_COUNT - 1;
> -static struct cpuidle_device __percpu *pseries_cpuidle_devices;
>  static struct cpuidle_state *cpuidle_state_table;
> 
>  static inline void idle_loop_prolog(unsigned long *in_purr)
> @@ -191,7 +190,7 @@ static int pseries_cpuidle_add_cpu_notifier(struct notifier_block *n,
>  {
>  	int hotcpu = (unsigned long)hcpu;
>  	struct cpuidle_device *dev =
> -			per_cpu_ptr(pseries_cpuidle_devices, hotcpu);
> +			per_cpu_ptr(cpuidle_devices, hotcpu);
> 
>  	if (dev && cpuidle_get_driver()) {
>  		switch (action) {
> @@ -248,48 +247,6 @@ static int pseries_cpuidle_driver_init(void)
>  	return 0;
>  }
> 
> -/* pseries_idle_devices_uninit(void)
> - * unregister cpuidle devices and de-allocate memory
> - */
> -static void pseries_idle_devices_uninit(void)
> -{
> -	int i;
> -	struct cpuidle_device *dev;
> -
> -	for_each_possible_cpu(i) {
> -		dev = per_cpu_ptr(pseries_cpuidle_devices, i);
> -		cpuidle_unregister_device(dev);
> -	}
> -
> -	free_percpu(pseries_cpuidle_devices);
> -	return;
> -}
> -
> -/* pseries_idle_devices_init()
> - * allocate, initialize and register cpuidle device
> - */
> -static int pseries_idle_devices_init(void)
> -{
> -	int i;
> -	struct cpuidle_device *dev;
> -
> -	pseries_cpuidle_devices = alloc_percpu(struct cpuidle_device);
> -	if (pseries_cpuidle_devices == NULL)
> -		return -ENOMEM;
> -
> -	for_each_possible_cpu(i) {
> -		dev = per_cpu_ptr(pseries_cpuidle_devices, i);
> -		dev->cpu = i;
> -		if (cpuidle_register_device(dev)) {
> -			printk(KERN_DEBUG \
> -				"cpuidle_register_device %d failed!\n", i);
> -			return -EIO;
> -		}
> -	}
> -
> -	return 0;
> -}
> -
>  /*
>   * pseries_idle_probe()
>   * Choose state table for shared versus dedicated partition
> @@ -325,19 +282,12 @@ static int __init pseries_processor_idle_init(void)
>  		return retval;
> 
>  	pseries_cpuidle_driver_init();
> -	retval = cpuidle_register_driver(&pseries_idle_driver);
> +	retval = cpuidle_register(&pseries_idle_driver, NULL);
>  	if (retval) {
>  		printk(KERN_DEBUG "Registration of pseries driver failed.\n");
>  		return retval;
>  	}
> 
> -	retval = pseries_idle_devices_init();
> -	if (retval) {
> -		pseries_idle_devices_uninit();
> -		cpuidle_unregister_driver(&pseries_idle_driver);
> -		return retval;
> -	}
> -
>  	register_cpu_notifier(&setup_hotplug_notifier);
>  	printk(KERN_DEBUG "pseries_idle_driver registered\n");
> 
> @@ -348,8 +298,7 @@ static void __exit pseries_processor_idle_exit(void)
>  {
> 
>  	unregister_cpu_notifier(&setup_hotplug_notifier);
> -	pseries_idle_devices_uninit();
> -	cpuidle_unregister_driver(&pseries_idle_driver);
> +	cpuidle_unregister(&pseries_idle_driver);
> 
>  	return;
>  }
> 

[PATCH v2 4/9] ACPI / cpuidle: fix max idle state handling with hotplug CPU support

[Bartlomiej Zolnierkiewicz]

acpi_processor_hotplug() calls acpi_processor_setup_cpuidle_cx()
without calling acpi_processor_setup_cpuidle_states() first so it
is possible that dev->state_count becomes different from
drv->state_count (in case of SMP system with unsupported C2/C3
states + enabled CPU hotplug and num_online_cpus() becoming > 1).

The driver code assumes that cpuidle core will handle such cases
but currently this is untrue (dev->state_count is used only for
handling cpuidle state sysfs entries and drv->state_count is used
for all other cases) and will not be fixed in the future as
dev->state_count is planned to be removed.

Fix the issue by checking for the max supported idle state in
C2/C3 state's ->enter handler (acpi_idle_enter_simple() for C2/C3
and acpi_idle_enter_bm() for C3 + bm_check flag set) and setting
the C1 state (instead of higher states) when needed.

Also remove no longer needed max idle state checks from
acpi_processor_setup_cpuidle_[states,cx]().

Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Len Brown <lenb@kernel.org>
---
 drivers/acpi/processor_idle.c | 27 ++++++++++++++-------------
 1 file changed, 14 insertions(+), 13 deletions(-)

diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
index a0cc5ef4f..c080c99 100644
--- a/drivers/acpi/processor_idle.c
+++ b/drivers/acpi/processor_idle.c
@@ -783,6 +783,13 @@ static int acpi_idle_enter_simple(struct cpuidle_device *dev,
 	if (unlikely(!pr))
 		return -EINVAL;
 
+#ifdef CONFIG_HOTPLUG_CPU
+	if ((cx->type != ACPI_STATE_C1) && (num_online_cpus() > 1) &&
+	    !pr->flags.has_cst &&
+	    !(acpi_gbl_FADT.flags & ACPI_FADT_C2_MP_SUPPORTED))
+		return acpi_idle_enter_c1(dev, drv, CPUIDLE_DRIVER_STATE_START);
+#endif
+
 	if (cx->entry_method == ACPI_CSTATE_FFH) {
 		if (current_set_polling_and_test())
 			return -EINVAL;
@@ -829,6 +836,13 @@ static int acpi_idle_enter_bm(struct cpuidle_device *dev,
 	if (unlikely(!pr))
 		return -EINVAL;
 
+#ifdef CONFIG_HOTPLUG_CPU
+	if ((cx->type != ACPI_STATE_C1) && (num_online_cpus() > 1) &&
+	    !pr->flags.has_cst &&
+	    !(acpi_gbl_FADT.flags & ACPI_FADT_C2_MP_SUPPORTED))
+		return acpi_idle_enter_c1(dev, drv, CPUIDLE_DRIVER_STATE_START);
+#endif
+
 	if (!cx->bm_sts_skip && acpi_idle_bm_check()) {
 		if (drv->safe_state_index >= 0) {
 			return drv->states[drv->safe_state_index].enter(dev,
@@ -930,12 +944,6 @@ static int acpi_processor_setup_cpuidle_cx(struct acpi_processor *pr,
 		if (!cx->valid)
 			continue;
 
-#ifdef CONFIG_HOTPLUG_CPU
-		if ((cx->type != ACPI_STATE_C1) && (num_online_cpus() > 1) &&
-		    !pr->flags.has_cst &&
-		    !(acpi_gbl_FADT.flags & ACPI_FADT_C2_MP_SUPPORTED))
-			continue;
-#endif
 		per_cpu(acpi_cstate[count], dev->cpu) = cx;
 
 		count++;
@@ -985,13 +993,6 @@ static int acpi_processor_setup_cpuidle_states(struct acpi_processor *pr)
 		if (!cx->valid)
 			continue;
 
-#ifdef CONFIG_HOTPLUG_CPU
-		if ((cx->type != ACPI_STATE_C1) && (num_online_cpus() > 1) &&
-		    !pr->flags.has_cst &&
-		    !(acpi_gbl_FADT.flags & ACPI_FADT_C2_MP_SUPPORTED))
-			continue;
-#endif
-
 		state = &drv->states[count];
 		snprintf(state->name, CPUIDLE_NAME_LEN, "C%d", i);
 		strncpy(state->desc, cx->desc, CPUIDLE_DESC_LEN);
-- 
1.8.2.3

[PATCH v2 5/9] ACPI / cpuidle: remove dev->state_count setting

[Bartlomiej Zolnierkiewicz]

dev->state_count is now always equal to drv->state_count and
drv->state_count no longer can change during driver's lifetime so
the default dev->state_count initialization in cpuidle_enable_device()
(called from cpuidle_register_device()) can be used instead.

Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Len Brown <lenb@kernel.org>
---
 drivers/acpi/processor_idle.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
index c080c99..da8ce91 100644
--- a/drivers/acpi/processor_idle.c
+++ b/drivers/acpi/processor_idle.c
@@ -951,8 +951,6 @@ static int acpi_processor_setup_cpuidle_cx(struct acpi_processor *pr,
 			break;
 	}
 
-	dev->state_count = count;
-
 	if (!count)
 		return -EINVAL;
 
-- 
1.8.2.3

[PATCH v2 6/9] intel_idle: do C1E promotion disable quirk for hotplugged CPUs

[Bartlomiej Zolnierkiewicz]

If the system is booted with some CPUs offline C1E promotion disable quirk
won't be applied because on_each_cpu() in intel_idle_cpuidle_driver_init()
operates only on online CPUs. Fix it by adding the C1E promotion disable
handling to intel_idle_cpu_init() (which is also called during CPU_ONLINE
operation).

Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Len Brown <lenb@kernel.org>
---
 drivers/idle/intel_idle.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
index 92d1206..38da3fb 100644
--- a/drivers/idle/intel_idle.c
+++ b/drivers/idle/intel_idle.c
@@ -683,6 +683,9 @@ static int intel_idle_cpu_init(int cpu)
 	if (icpu->auto_demotion_disable_flags)
 		smp_call_function_single(cpu, auto_demotion_disable, NULL, 1);
 
+	if (icpu->disable_promotion_to_c1e)
+		smp_call_function_single(cpu, c1e_promotion_disable, NULL, 1);
+
 	return 0;
 }
 
-- 
1.8.2.3

Re: [PATCH v2 6/9] intel_idle: do C1E promotion disable quirk for hotplugged CPUs

[Daniel Lezcano]

On 12/20/2013 07:47 PM, Bartlomiej Zolnierkiewicz wrote:
> If the system is booted with some CPUs offline C1E promotion disable quirk
> won't be applied because on_each_cpu() in intel_idle_cpuidle_driver_init()
> operates only on online CPUs. Fix it by adding the C1E promotion disable
> handling to intel_idle_cpu_init() (which is also called during CPU_ONLINE
> operation).
>
> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> Cc: Len Brown <lenb@kernel.org>

Reviewed-by: Daniel Lezcano <daniel.lezcano@linaro.org>

> ---
>   drivers/idle/intel_idle.c | 3 +++
>   1 file changed, 3 insertions(+)
>
> diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
> index 92d1206..38da3fb 100644
> --- a/drivers/idle/intel_idle.c
> +++ b/drivers/idle/intel_idle.c
> @@ -683,6 +683,9 @@ static int intel_idle_cpu_init(int cpu)
>   	if (icpu->auto_demotion_disable_flags)
>   		smp_call_function_single(cpu, auto_demotion_disable, NULL, 1);
>
> +	if (icpu->disable_promotion_to_c1e)
> +		smp_call_function_single(cpu, c1e_promotion_disable, NULL, 1);
> +
>   	return 0;
>   }

-- 
  <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 v2 7/9] intel_idle: remove superfluous dev->state_count initialization

[Bartlomiej Zolnierkiewicz]

intel_idle driver sets dev->state_count to drv->state_count so
the default dev->state_count initialization in cpuidle_enable_device()
(called from cpuidle_register_device()) can be used instead.

Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Len Brown <lenb@kernel.org>
---
 drivers/idle/intel_idle.c | 29 -----------------------------
 1 file changed, 29 deletions(-)

diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
index 38da3fb..524d07b 100644
--- a/drivers/idle/intel_idle.c
+++ b/drivers/idle/intel_idle.c
@@ -639,39 +639,10 @@ static int __init intel_idle_cpuidle_driver_init(void)
  */
 static int intel_idle_cpu_init(int cpu)
 {
-	int cstate;
 	struct cpuidle_device *dev;
 
 	dev = per_cpu_ptr(intel_idle_cpuidle_devices, cpu);
 
-	dev->state_count = 1;
-
-	for (cstate = 0; cstate < CPUIDLE_STATE_MAX; ++cstate) {
-		int num_substates, mwait_hint, mwait_cstate, mwait_substate;
-
-		if (cpuidle_state_table[cstate].enter == NULL)
-			break;
-
-		if (cstate + 1 > max_cstate) {
-			printk(PREFIX "max_cstate %d reached\n", max_cstate);
-			break;
-		}
-
-		mwait_hint = flg2MWAIT(cpuidle_state_table[cstate].flags);
-		mwait_cstate = MWAIT_HINT2CSTATE(mwait_hint);
-		mwait_substate = MWAIT_HINT2SUBSTATE(mwait_hint);
-
-		/* does the state exist in CPUID.MWAIT? */
-		num_substates = (mwait_substates >> ((mwait_cstate + 1) * 4))
-					& MWAIT_SUBSTATE_MASK;
-
-		/* if sub-state in table is not enumerated by CPUID */
-		if ((mwait_substate + 1) > num_substates)
-			continue;
-
-		dev->state_count += 1;
-	}
-
 	dev->cpu = cpu;
 
 	if (cpuidle_register_device(dev)) {
-- 
1.8.2.3

Re: [PATCH v2 7/9] intel_idle: remove superfluous dev->state_count initialization

[Daniel Lezcano]

On 12/20/2013 07:47 PM, Bartlomiej Zolnierkiewicz wrote:
> intel_idle driver sets dev->state_count to drv->state_count so
> the default dev->state_count initialization in cpuidle_enable_device()
> (called from cpuidle_register_device()) can be used instead.
>
> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> Cc: Len Brown <lenb@kernel.org>

Reviewed-by: Daniel Lezcano <daniel.lezcano@linaro.org>

> ---
>   drivers/idle/intel_idle.c | 29 -----------------------------
>   1 file changed, 29 deletions(-)
>
> diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
> index 38da3fb..524d07b 100644
> --- a/drivers/idle/intel_idle.c
> +++ b/drivers/idle/intel_idle.c
> @@ -639,39 +639,10 @@ static int __init intel_idle_cpuidle_driver_init(void)
>    */
>   static int intel_idle_cpu_init(int cpu)
>   {
> -	int cstate;
>   	struct cpuidle_device *dev;
>
>   	dev = per_cpu_ptr(intel_idle_cpuidle_devices, cpu);
>
> -	dev->state_count = 1;
> -
> -	for (cstate = 0; cstate < CPUIDLE_STATE_MAX; ++cstate) {
> -		int num_substates, mwait_hint, mwait_cstate, mwait_substate;
> -
> -		if (cpuidle_state_table[cstate].enter == NULL)
> -			break;
> -
> -		if (cstate + 1 > max_cstate) {
> -			printk(PREFIX "max_cstate %d reached\n", max_cstate);
> -			break;
> -		}
> -
> -		mwait_hint = flg2MWAIT(cpuidle_state_table[cstate].flags);
> -		mwait_cstate = MWAIT_HINT2CSTATE(mwait_hint);
> -		mwait_substate = MWAIT_HINT2SUBSTATE(mwait_hint);
> -
> -		/* does the state exist in CPUID.MWAIT? */
> -		num_substates = (mwait_substates >> ((mwait_cstate + 1) * 4))
> -					& MWAIT_SUBSTATE_MASK;
> -
> -		/* if sub-state in table is not enumerated by CPUID */
> -		if ((mwait_substate + 1) > num_substates)
> -			continue;
> -
> -		dev->state_count += 1;
> -	}
> -
>   	dev->cpu = cpu;
>
>   	if (cpuidle_register_device(dev)) {
>


-- 
  <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 v2 8/9] intel_idle: use the common cpuidle_[un]register() routines

[Bartlomiej Zolnierkiewicz]

It is now possible to use the common cpuidle_[un]register() routines
(instead of open-coding them) so do it.

Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Len Brown <lenb@kernel.org>
---
 drivers/idle/intel_idle.c | 114 ++++++++++++----------------------------------
 1 file changed, 29 insertions(+), 85 deletions(-)

diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
index 524d07b..a1a4dbd 100644
--- a/drivers/idle/intel_idle.c
+++ b/drivers/idle/intel_idle.c
@@ -93,10 +93,8 @@ struct idle_cpu {
 };
 
 static const struct idle_cpu *icpu;
-static struct cpuidle_device __percpu *intel_idle_cpuidle_devices;
 static int intel_idle(struct cpuidle_device *dev,
 			struct cpuidle_driver *drv, int index);
-static int intel_idle_cpu_init(int cpu);
 
 static struct cpuidle_state *cpuidle_state_table;
 
@@ -400,11 +398,27 @@ static void __setup_broadcast_timer(void *arg)
 	clockevents_notify(reason, &cpu);
 }
 
+static void auto_demotion_disable(void *dummy)
+{
+	unsigned long long msr_bits;
+
+	rdmsrl(MSR_NHM_SNB_PKG_CST_CFG_CTL, msr_bits);
+	msr_bits &= ~(icpu->auto_demotion_disable_flags);
+	wrmsrl(MSR_NHM_SNB_PKG_CST_CFG_CTL, msr_bits);
+}
+static void c1e_promotion_disable(void *dummy)
+{
+	unsigned long long msr_bits;
+
+	rdmsrl(MSR_IA32_POWER_CTL, msr_bits);
+	msr_bits &= ~0x2;
+	wrmsrl(MSR_IA32_POWER_CTL, msr_bits);
+}
+
 static int cpu_hotplug_notify(struct notifier_block *n,
 			      unsigned long action, void *hcpu)
 {
 	int hotcpu = (unsigned long)hcpu;
-	struct cpuidle_device *dev;
 
 	switch (action & ~CPU_TASKS_FROZEN) {
 	case CPU_ONLINE:
@@ -416,11 +430,15 @@ static int cpu_hotplug_notify(struct notifier_block *n,
 		/*
 		 * Some systems can hotplug a cpu at runtime after
 		 * the kernel has booted, we have to initialize the
-		 * driver in this case
+		 * hardware in this case.
 		 */
-		dev = per_cpu_ptr(intel_idle_cpuidle_devices, hotcpu);
-		if (!dev->registered)
-			intel_idle_cpu_init(hotcpu);
+		if (icpu->auto_demotion_disable_flags)
+			smp_call_function_single(hotcpu, auto_demotion_disable,
+						NULL, 1);
+
+		if (icpu->disable_promotion_to_c1e)
+			smp_call_function_single(hotcpu, c1e_promotion_disable,
+						NULL, 1);
 
 		break;
 	}
@@ -431,23 +449,6 @@ static struct notifier_block cpu_hotplug_notifier = {
 	.notifier_call = cpu_hotplug_notify,
 };
 
-static void auto_demotion_disable(void *dummy)
-{
-	unsigned long long msr_bits;
-
-	rdmsrl(MSR_NHM_SNB_PKG_CST_CFG_CTL, msr_bits);
-	msr_bits &= ~(icpu->auto_demotion_disable_flags);
-	wrmsrl(MSR_NHM_SNB_PKG_CST_CFG_CTL, msr_bits);
-}
-static void c1e_promotion_disable(void *dummy)
-{
-	unsigned long long msr_bits;
-
-	rdmsrl(MSR_IA32_POWER_CTL, msr_bits);
-	msr_bits &= ~0x2;
-	wrmsrl(MSR_IA32_POWER_CTL, msr_bits);
-}
-
 static const struct idle_cpu idle_cpu_nehalem = {
 	.state_table = nehalem_cstates,
 	.auto_demotion_disable_flags = NHM_C1_AUTO_DEMOTE | NHM_C3_AUTO_DEMOTE,
@@ -560,23 +561,6 @@ static int __init intel_idle_probe(void)
 }
 
 /*
- * intel_idle_cpuidle_devices_uninit()
- * unregister, free cpuidle_devices
- */
-static void intel_idle_cpuidle_devices_uninit(void)
-{
-	int i;
-	struct cpuidle_device *dev;
-
-	for_each_online_cpu(i) {
-		dev = per_cpu_ptr(intel_idle_cpuidle_devices, i);
-		cpuidle_unregister_device(dev);
-	}
-
-	free_percpu(intel_idle_cpuidle_devices);
-	return;
-}
-/*
  * intel_idle_cpuidle_driver_init()
  * allocate, initialize cpuidle_states
  */
@@ -632,37 +616,9 @@ static int __init intel_idle_cpuidle_driver_init(void)
 }
 
 
-/*
- * intel_idle_cpu_init()
- * allocate, initialize, register cpuidle_devices
- * @cpu: cpu/core to initialize
- */
-static int intel_idle_cpu_init(int cpu)
-{
-	struct cpuidle_device *dev;
-
-	dev = per_cpu_ptr(intel_idle_cpuidle_devices, cpu);
-
-	dev->cpu = cpu;
-
-	if (cpuidle_register_device(dev)) {
-		pr_debug(PREFIX "cpuidle_register_device %d failed!\n", cpu);
-		intel_idle_cpuidle_devices_uninit();
-		return -EIO;
-	}
-
-	if (icpu->auto_demotion_disable_flags)
-		smp_call_function_single(cpu, auto_demotion_disable, NULL, 1);
-
-	if (icpu->disable_promotion_to_c1e)
-		smp_call_function_single(cpu, c1e_promotion_disable, NULL, 1);
-
-	return 0;
-}
-
 static int __init intel_idle_init(void)
 {
-	int retval, i;
+	int retval;
 
 	/* Do not load intel_idle at all for now if idle= is passed */
 	if (boot_option_idle_override != IDLE_NO_OVERRIDE)
@@ -673,7 +629,8 @@ static int __init intel_idle_init(void)
 		return retval;
 
 	intel_idle_cpuidle_driver_init();
-	retval = cpuidle_register_driver(&intel_idle_driver);
+
+	retval = cpuidle_register(&intel_idle_driver, NULL);
 	if (retval) {
 		struct cpuidle_driver *drv = cpuidle_get_driver();
 		printk(KERN_DEBUG PREFIX "intel_idle yielding to %s",
@@ -681,17 +638,6 @@ static int __init intel_idle_init(void)
 		return retval;
 	}
 
-	intel_idle_cpuidle_devices = alloc_percpu(struct cpuidle_device);
-	if (intel_idle_cpuidle_devices == NULL)
-		return -ENOMEM;
-
-	for_each_online_cpu(i) {
-		retval = intel_idle_cpu_init(i);
-		if (retval) {
-			cpuidle_unregister_driver(&intel_idle_driver);
-			return retval;
-		}
-	}
 	register_cpu_notifier(&cpu_hotplug_notifier);
 
 	return 0;
@@ -699,9 +645,7 @@ static int __init intel_idle_init(void)
 
 static void __exit intel_idle_exit(void)
 {
-	intel_idle_cpuidle_devices_uninit();
-	cpuidle_unregister_driver(&intel_idle_driver);
-
+	cpuidle_unregister(&intel_idle_driver);
 
 	if (lapic_timer_reliable_states != LAPIC_TIMER_ALWAYS_RELIABLE)
 		on_each_cpu(__setup_broadcast_timer, (void *)false, 1);
-- 
1.8.2.3

Re: [PATCH v2 8/9] intel_idle: use the common cpuidle_[un]register() routines

[Daniel Lezcano]

On 12/20/2013 07:47 PM, Bartlomiej Zolnierkiewicz wrote:
> It is now possible to use the common cpuidle_[un]register() routines
> (instead of open-coding them) so do it.

Just an addition:

The cpuidle_register common routine calls cpuidle_register_driver which 
initialize the driver's cpumask to cpu_possible_mask if not set (which 
is the default on most platform) and right after it uses this mask to 
register the cpuidle devices. That's why the cpu hotplug does not need 
to register the device unlike before this patch where the cpumask was 
cpu_online_mask. So we can't fall in the "Some systems can hotplug a cpu 
at runtime after the kernel has booted" case.

Reviewed-by: Daniel Lezcano <daniel.lezcano@linaro.org>

> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> Cc: Len Brown <lenb@kernel.org>
> ---
>   drivers/idle/intel_idle.c | 114 ++++++++++++----------------------------------
>   1 file changed, 29 insertions(+), 85 deletions(-)
>
> diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
> index 524d07b..a1a4dbd 100644
> --- a/drivers/idle/intel_idle.c
> +++ b/drivers/idle/intel_idle.c
> @@ -93,10 +93,8 @@ struct idle_cpu {
>   };
>
>   static const struct idle_cpu *icpu;
> -static struct cpuidle_device __percpu *intel_idle_cpuidle_devices;
>   static int intel_idle(struct cpuidle_device *dev,
>   			struct cpuidle_driver *drv, int index);
> -static int intel_idle_cpu_init(int cpu);
>
>   static struct cpuidle_state *cpuidle_state_table;
>
> @@ -400,11 +398,27 @@ static void __setup_broadcast_timer(void *arg)
>   	clockevents_notify(reason, &cpu);
>   }
>
> +static void auto_demotion_disable(void *dummy)
> +{
> +	unsigned long long msr_bits;
> +
> +	rdmsrl(MSR_NHM_SNB_PKG_CST_CFG_CTL, msr_bits);
> +	msr_bits &= ~(icpu->auto_demotion_disable_flags);
> +	wrmsrl(MSR_NHM_SNB_PKG_CST_CFG_CTL, msr_bits);
> +}
> +static void c1e_promotion_disable(void *dummy)
> +{
> +	unsigned long long msr_bits;
> +
> +	rdmsrl(MSR_IA32_POWER_CTL, msr_bits);
> +	msr_bits &= ~0x2;
> +	wrmsrl(MSR_IA32_POWER_CTL, msr_bits);
> +}
> +
>   static int cpu_hotplug_notify(struct notifier_block *n,
>   			      unsigned long action, void *hcpu)
>   {
>   	int hotcpu = (unsigned long)hcpu;
> -	struct cpuidle_device *dev;
>
>   	switch (action & ~CPU_TASKS_FROZEN) {
>   	case CPU_ONLINE:
> @@ -416,11 +430,15 @@ static int cpu_hotplug_notify(struct notifier_block *n,
>   		/*
>   		 * Some systems can hotplug a cpu at runtime after
>   		 * the kernel has booted, we have to initialize the
> -		 * driver in this case
> +		 * hardware in this case.
>   		 */
> -		dev = per_cpu_ptr(intel_idle_cpuidle_devices, hotcpu);
> -		if (!dev->registered)
> -			intel_idle_cpu_init(hotcpu);
> +		if (icpu->auto_demotion_disable_flags)
> +			smp_call_function_single(hotcpu, auto_demotion_disable,
> +						NULL, 1);
> +
> +		if (icpu->disable_promotion_to_c1e)
> +			smp_call_function_single(hotcpu, c1e_promotion_disable,
> +						NULL, 1);
>
>   		break;
>   	}
> @@ -431,23 +449,6 @@ static struct notifier_block cpu_hotplug_notifier = {
>   	.notifier_call = cpu_hotplug_notify,
>   };
>
> -static void auto_demotion_disable(void *dummy)
> -{
> -	unsigned long long msr_bits;
> -
> -	rdmsrl(MSR_NHM_SNB_PKG_CST_CFG_CTL, msr_bits);
> -	msr_bits &= ~(icpu->auto_demotion_disable_flags);
> -	wrmsrl(MSR_NHM_SNB_PKG_CST_CFG_CTL, msr_bits);
> -}
> -static void c1e_promotion_disable(void *dummy)
> -{
> -	unsigned long long msr_bits;
> -
> -	rdmsrl(MSR_IA32_POWER_CTL, msr_bits);
> -	msr_bits &= ~0x2;
> -	wrmsrl(MSR_IA32_POWER_CTL, msr_bits);
> -}
> -
>   static const struct idle_cpu idle_cpu_nehalem = {
>   	.state_table = nehalem_cstates,
>   	.auto_demotion_disable_flags = NHM_C1_AUTO_DEMOTE | NHM_C3_AUTO_DEMOTE,
> @@ -560,23 +561,6 @@ static int __init intel_idle_probe(void)
>   }
>
>   /*
> - * intel_idle_cpuidle_devices_uninit()
> - * unregister, free cpuidle_devices
> - */
> -static void intel_idle_cpuidle_devices_uninit(void)
> -{
> -	int i;
> -	struct cpuidle_device *dev;
> -
> -	for_each_online_cpu(i) {
> -		dev = per_cpu_ptr(intel_idle_cpuidle_devices, i);
> -		cpuidle_unregister_device(dev);
> -	}
> -
> -	free_percpu(intel_idle_cpuidle_devices);
> -	return;
> -}
> -/*
>    * intel_idle_cpuidle_driver_init()
>    * allocate, initialize cpuidle_states
>    */
> @@ -632,37 +616,9 @@ static int __init intel_idle_cpuidle_driver_init(void)
>   }
>
>
> -/*
> - * intel_idle_cpu_init()
> - * allocate, initialize, register cpuidle_devices
> - * @cpu: cpu/core to initialize
> - */
> -static int intel_idle_cpu_init(int cpu)
> -{
> -	struct cpuidle_device *dev;
> -
> -	dev = per_cpu_ptr(intel_idle_cpuidle_devices, cpu);
> -
> -	dev->cpu = cpu;
> -
> -	if (cpuidle_register_device(dev)) {
> -		pr_debug(PREFIX "cpuidle_register_device %d failed!\n", cpu);
> -		intel_idle_cpuidle_devices_uninit();
> -		return -EIO;
> -	}
> -
> -	if (icpu->auto_demotion_disable_flags)
> -		smp_call_function_single(cpu, auto_demotion_disable, NULL, 1);
> -
> -	if (icpu->disable_promotion_to_c1e)
> -		smp_call_function_single(cpu, c1e_promotion_disable, NULL, 1);
> -
> -	return 0;
> -}
> -
>   static int __init intel_idle_init(void)
>   {
> -	int retval, i;
> +	int retval;
>
>   	/* Do not load intel_idle at all for now if idle= is passed */
>   	if (boot_option_idle_override != IDLE_NO_OVERRIDE)
> @@ -673,7 +629,8 @@ static int __init intel_idle_init(void)
>   		return retval;
>
>   	intel_idle_cpuidle_driver_init();
> -	retval = cpuidle_register_driver(&intel_idle_driver);
> +
> +	retval = cpuidle_register(&intel_idle_driver, NULL);
>   	if (retval) {
>   		struct cpuidle_driver *drv = cpuidle_get_driver();
>   		printk(KERN_DEBUG PREFIX "intel_idle yielding to %s",
> @@ -681,17 +638,6 @@ static int __init intel_idle_init(void)
>   		return retval;
>   	}
>
> -	intel_idle_cpuidle_devices = alloc_percpu(struct cpuidle_device);
> -	if (intel_idle_cpuidle_devices == NULL)
> -		return -ENOMEM;
> -
> -	for_each_online_cpu(i) {
> -		retval = intel_idle_cpu_init(i);
> -		if (retval) {
> -			cpuidle_unregister_driver(&intel_idle_driver);
> -			return retval;
> -		}
> -	}
>   	register_cpu_notifier(&cpu_hotplug_notifier);
>
>   	return 0;
> @@ -699,9 +645,7 @@ static int __init intel_idle_init(void)
>
>   static void __exit intel_idle_exit(void)
>   {
> -	intel_idle_cpuidle_devices_uninit();
> -	cpuidle_unregister_driver(&intel_idle_driver);
> -
> +	cpuidle_unregister(&intel_idle_driver);
>
>   	if (lapic_timer_reliable_states != LAPIC_TIMER_ALWAYS_RELIABLE)
>   		on_each_cpu(__setup_broadcast_timer, (void *)false, 1);
>


-- 
  <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 v2 9/9] cpuidle: remove state_count field from struct cpuidle_device

[Bartlomiej Zolnierkiewicz]

dev->state_count is now always equal to drv->state_count so
it can be removed.

Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/cpuidle/cpuidle.c | 3 ---
 drivers/cpuidle/sysfs.c   | 5 +++--
 include/linux/cpuidle.h   | 1 -
 3 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
index a55e68f..e3d2052 100644
--- a/drivers/cpuidle/cpuidle.c
+++ b/drivers/cpuidle/cpuidle.c
@@ -252,9 +252,6 @@ int cpuidle_enable_device(struct cpuidle_device *dev)
 	if (!dev->registered)
 		return -EINVAL;
 
-	if (!dev->state_count)
-		dev->state_count = drv->state_count;
-
 	ret = cpuidle_add_device_sysfs(dev);
 	if (ret)
 		return ret;
diff --git a/drivers/cpuidle/sysfs.c b/drivers/cpuidle/sysfs.c
index e918b6d..dcaae4c 100644
--- a/drivers/cpuidle/sysfs.c
+++ b/drivers/cpuidle/sysfs.c
@@ -398,7 +398,7 @@ static int cpuidle_add_state_sysfs(struct cpuidle_device *device)
 	struct cpuidle_driver *drv = cpuidle_get_cpu_driver(device);
 
 	/* state statistics */
-	for (i = 0; i < device->state_count; i++) {
+	for (i = 0; i < drv->state_count; i++) {
 		kobj = kzalloc(sizeof(struct cpuidle_state_kobj), GFP_KERNEL);
 		if (!kobj)
 			goto error_state;
@@ -430,9 +430,10 @@ error_state:
  */
 static void cpuidle_remove_state_sysfs(struct cpuidle_device *device)
 {
+	struct cpuidle_driver *drv = cpuidle_get_cpu_driver(device);
 	int i;
 
-	for (i = 0; i < device->state_count; i++)
+	for (i = 0; i < drv->state_count; i++)
 		cpuidle_free_state_kobj(device, i);
 }
 
diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h
index 50fcbb0..d133817 100644
--- a/include/linux/cpuidle.h
+++ b/include/linux/cpuidle.h
@@ -69,7 +69,6 @@ struct cpuidle_device {
 	unsigned int		cpu;
 
 	int			last_residency;
-	int			state_count;
 	struct cpuidle_state_usage	states_usage[CPUIDLE_STATE_MAX];
 	struct cpuidle_state_kobj *kobjs[CPUIDLE_STATE_MAX];
 	struct cpuidle_driver_kobj *kobj_driver;
-- 
1.8.2.3

Re: [PATCH v2 9/9] cpuidle: remove state_count field from struct cpuidle_device

[Daniel Lezcano]

On 12/20/2013 07:47 PM, Bartlomiej Zolnierkiewicz wrote:
> dev->state_count is now always equal to drv->state_count so
> it can be removed.
>
> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>

Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>

> ---
>   drivers/cpuidle/cpuidle.c | 3 ---
>   drivers/cpuidle/sysfs.c   | 5 +++--
>   include/linux/cpuidle.h   | 1 -
>   3 files changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
> index a55e68f..e3d2052 100644
> --- a/drivers/cpuidle/cpuidle.c
> +++ b/drivers/cpuidle/cpuidle.c
> @@ -252,9 +252,6 @@ int cpuidle_enable_device(struct cpuidle_device *dev)
>   	if (!dev->registered)
>   		return -EINVAL;
>
> -	if (!dev->state_count)
> -		dev->state_count = drv->state_count;
> -
>   	ret = cpuidle_add_device_sysfs(dev);
>   	if (ret)
>   		return ret;
> diff --git a/drivers/cpuidle/sysfs.c b/drivers/cpuidle/sysfs.c
> index e918b6d..dcaae4c 100644
> --- a/drivers/cpuidle/sysfs.c
> +++ b/drivers/cpuidle/sysfs.c
> @@ -398,7 +398,7 @@ static int cpuidle_add_state_sysfs(struct cpuidle_device *device)
>   	struct cpuidle_driver *drv = cpuidle_get_cpu_driver(device);
>
>   	/* state statistics */
> -	for (i = 0; i < device->state_count; i++) {
> +	for (i = 0; i < drv->state_count; i++) {
>   		kobj = kzalloc(sizeof(struct cpuidle_state_kobj), GFP_KERNEL);
>   		if (!kobj)
>   			goto error_state;
> @@ -430,9 +430,10 @@ error_state:
>    */
>   static void cpuidle_remove_state_sysfs(struct cpuidle_device *device)
>   {
> +	struct cpuidle_driver *drv = cpuidle_get_cpu_driver(device);
>   	int i;
>
> -	for (i = 0; i < device->state_count; i++)
> +	for (i = 0; i < drv->state_count; i++)
>   		cpuidle_free_state_kobj(device, i);
>   }
>
> diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h
> index 50fcbb0..d133817 100644
> --- a/include/linux/cpuidle.h
> +++ b/include/linux/cpuidle.h
> @@ -69,7 +69,6 @@ struct cpuidle_device {
>   	unsigned int		cpu;
>
>   	int			last_residency;
> -	int			state_count;
>   	struct cpuidle_state_usage	states_usage[CPUIDLE_STATE_MAX];
>   	struct cpuidle_state_kobj *kobjs[CPUIDLE_STATE_MAX];
>   	struct cpuidle_driver_kobj *kobj_driver;
>


-- 
  <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 v2 0/9] cpuidle: rework device state count handling

[Rafael J. Wysocki]

On Friday, December 20, 2013 07:47:22 PM Bartlomiej Zolnierkiewicz wrote:
> Hi,
> 
> Some cpuidle drivers assume that cpuidle core will handle cases where
> device->state_count is smaller than driver->state_count, unfortunately
> currently this is untrue (device->state_count is used only for handling
> cpuidle state sysfs entries and driver->state_count is used for all
> other cases) and will not be fixed in the future as device->state_count
> is planned to be removed [1].
> 
> This patchset fixes such drivers (ARM EXYNOS cpuidle driver and ACPI
> cpuidle driver), removes superflous device->state_count initialization
> from drivers for which device->state_count equals driver->state_count
> (POWERPC pseries cpuidle driver and intel_idle driver) and finally
> removes state_count field from struct cpuidle_device.
> 
> Additionaly (while at it) this patchset fixes C1E promotion disable
> quirk handling (in intel_idle driver) and converts cpuidle drivers code
> to use the common cpuidle_[un]register() routines (in POWERPC pseries
> cpuidle driver and intel_idle driver).
> 
> [1] http://permalink.gmane.org/gmane.linux.power-management.general/36908
> 
> Reference to v1:
> 	http://comments.gmane.org/gmane.linux.power-management.general/37390
> 
> Changes since v1:
> - synced patch series with next-20131220
> - added ACKs from Daniel Lezcano

I've queued up the series for 3.14, thanks!

> Best regards,
> --
> Bartlomiej Zolnierkiewicz
> Samsung R&D Institute Poland
> Samsung Electronics
> 
> 
> Bartlomiej Zolnierkiewicz (9):
>   ARM: EXYNOS: cpuidle: fix AFTR mode check
>   POWERPC: pseries: cpuidle: remove superfluous dev->state_count
>     initialization
>   POWERPC: pseries: cpuidle: use the common cpuidle_[un]register()
>     routines
>   ACPI / cpuidle: fix max idle state handling with hotplug CPU support
>   ACPI / cpuidle: remove dev->state_count setting
>   intel_idle: do C1E promotion disable quirk for hotplugged CPUs
>   intel_idle: remove superfluous dev->state_count initialization
>   intel_idle: use the common cpuidle_[un]register() routines
>   cpuidle: remove state_count field from struct cpuidle_device
> 
>  arch/arm/mach-exynos/cpuidle.c                  |   8 +-
>  arch/powerpc/platforms/pseries/processor_idle.c |  59 +---------
>  drivers/acpi/processor_idle.c                   |  29 +++--
>  drivers/cpuidle/cpuidle.c                       |   3 -
>  drivers/cpuidle/sysfs.c                         |   5 +-
>  drivers/idle/intel_idle.c                       | 140 +++++-------------------
>  include/linux/cpuidle.h                         |   1 -
>  7 files changed, 51 insertions(+), 194 deletions(-)
> 
> 

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH v2 0/9] cpuidle: rework device state count handling

[Rafael J. Wysocki]

On Friday, December 20, 2013 07:47:22 PM Bartlomiej Zolnierkiewicz wrote:
> Hi,
> 
> Some cpuidle drivers assume that cpuidle core will handle cases where
> device->state_count is smaller than driver->state_count, unfortunately
> currently this is untrue (device->state_count is used only for handling
> cpuidle state sysfs entries and driver->state_count is used for all
> other cases) and will not be fixed in the future as device->state_count
> is planned to be removed [1].
> 
> This patchset fixes such drivers (ARM EXYNOS cpuidle driver and ACPI
> cpuidle driver), removes superflous device->state_count initialization
> from drivers for which device->state_count equals driver->state_count
> (POWERPC pseries cpuidle driver and intel_idle driver) and finally
> removes state_count field from struct cpuidle_device.
> 
> Additionaly (while at it) this patchset fixes C1E promotion disable
> quirk handling (in intel_idle driver) and converts cpuidle drivers code
> to use the common cpuidle_[un]register() routines (in POWERPC pseries
> cpuidle driver and intel_idle driver).
> 
> [1] http://permalink.gmane.org/gmane.linux.power-management.general/36908
> 
> Reference to v1:
> 	http://comments.gmane.org/gmane.linux.power-management.general/37390
> 
> Changes since v1:
> - synced patch series with next-20131220
> - added ACKs from Daniel Lezcano

This series breaks boot on one of my test machines with intel_idle, so I'm
not sure how well it has been tested.

I've dropped it entirely for now.  If I have the time, I will try to identify
the root cause of the failure, but that may not happen before the merge window.
Sorry about that.

Thanks!

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH v2 0/9] cpuidle: rework device state count handling

[Rafael J. Wysocki]

On Saturday, January 11, 2014 01:37:29 AM Rafael J. Wysocki wrote:
> On Friday, December 20, 2013 07:47:22 PM Bartlomiej Zolnierkiewicz wrote:
> > Hi,
> > 
> > Some cpuidle drivers assume that cpuidle core will handle cases where
> > device->state_count is smaller than driver->state_count, unfortunately
> > currently this is untrue (device->state_count is used only for handling
> > cpuidle state sysfs entries and driver->state_count is used for all
> > other cases) and will not be fixed in the future as device->state_count
> > is planned to be removed [1].
> > 
> > This patchset fixes such drivers (ARM EXYNOS cpuidle driver and ACPI
> > cpuidle driver), removes superflous device->state_count initialization
> > from drivers for which device->state_count equals driver->state_count
> > (POWERPC pseries cpuidle driver and intel_idle driver) and finally
> > removes state_count field from struct cpuidle_device.
> > 
> > Additionaly (while at it) this patchset fixes C1E promotion disable
> > quirk handling (in intel_idle driver) and converts cpuidle drivers code
> > to use the common cpuidle_[un]register() routines (in POWERPC pseries
> > cpuidle driver and intel_idle driver).
> > 
> > [1] http://permalink.gmane.org/gmane.linux.power-management.general/36908
> > 
> > Reference to v1:
> > 	http://comments.gmane.org/gmane.linux.power-management.general/37390
> > 
> > Changes since v1:
> > - synced patch series with next-20131220
> > - added ACKs from Daniel Lezcano
> 
> This series breaks boot on one of my test machines with intel_idle, so I'm
> not sure how well it has been tested.
> 
> I've dropped it entirely for now.  If I have the time, I will try to identify
> the root cause of the failure, but that may not happen before the merge window.
> Sorry about that.

The breakage was introduced by patch [8/9], so I've re-applied patches [1-7/9]
from this series.  Please refer to Fengguang's report [1] for the breakage
details.

Thanks!

[1] http://marc.info/?l=linux-kernel&m=138964167909907&w=2

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.