On Monday, June 12, 2017 05:55:10 PM Daniel Lezcano wrote:
> Some hardware have clusters with different idle states. The current code does
> not support this and fails as it expects all the idle states to be identical.
>
> Because of this, the Mediatek mtk8173 had to create the same idle state for a
> big.Little system and now the Hisilicon 960 is facing the same situation.
>
> Solve this by simply assuming the multiple driver will be needed for all the
> platforms using the ARM generic cpuidle driver which makes sense because of
> the
> different topologies we can support with a single kernel for ARM32 or ARM64.
>
> Every CPU has its own driver, so every single CPU can specify in the DT the
> idle states.
>
> This simple approach allows to support the future dynamIQ system, current SMP
> and HMP.
>
> Tested on:
> - 96boards: Hikey 620
> - 96boards: Hikey 960
> - 96boards: dragonboard410c
> - Mediatek 8173
>
> Cc: Sudeep Holla <[email protected]>
> Cc: Lorenzo Pieralisi <[email protected]>
> Tested-by: Leo Yan <[email protected]>
> Signed-off-by: Daniel Lezcano <[email protected]>
There seems to have been quite some discussion regarding this one and I'm not
sure about the resolution of it.
I'd feel more comfortable with an ACK or Reviewed-by from Sudeep or Lorenzo
here.
> ---
> drivers/cpuidle/Kconfig.arm | 1 +
> drivers/cpuidle/cpuidle-arm.c | 62
> ++++++++++++++++++++++++++-----------------
> 2 files changed, 39 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/cpuidle/Kconfig.arm b/drivers/cpuidle/Kconfig.arm
> index 21340e0..f521448 100644
> --- a/drivers/cpuidle/Kconfig.arm
> +++ b/drivers/cpuidle/Kconfig.arm
> @@ -4,6 +4,7 @@
> config ARM_CPUIDLE
> bool "Generic ARM/ARM64 CPU idle Driver"
> select DT_IDLE_STATES
> + select CPU_IDLE_MULTIPLE_DRIVERS
> help
> Select this to enable generic cpuidle driver for ARM.
> It provides a generic idle driver whose idle states are configured
> diff --git a/drivers/cpuidle/cpuidle-arm.c b/drivers/cpuidle/cpuidle-arm.c
> index f440d38..7080c38 100644
> --- a/drivers/cpuidle/cpuidle-arm.c
> +++ b/drivers/cpuidle/cpuidle-arm.c
> @@ -18,6 +18,7 @@
> #include <linux/module.h>
> #include <linux/of.h>
> #include <linux/slab.h>
> +#include <linux/topology.h>
>
> #include <asm/cpuidle.h>
>
> @@ -44,7 +45,7 @@ static int arm_enter_idle_state(struct cpuidle_device *dev,
> return CPU_PM_CPU_IDLE_ENTER(arm_cpuidle_suspend, idx);
> }
>
> -static struct cpuidle_driver arm_idle_driver = {
> +static struct cpuidle_driver arm_idle_driver __initdata = {
> .name = "arm_idle",
> .owner = THIS_MODULE,
> /*
> @@ -80,30 +81,42 @@ static const struct of_device_id arm_idle_state_match[]
> __initconst = {
> static int __init arm_idle_init(void)
> {
> int cpu, ret;
> - struct cpuidle_driver *drv = &arm_idle_driver;
> + struct cpuidle_driver *drv;
> struct cpuidle_device *dev;
>
> - /*
> - * Initialize idle states data, starting at index 1.
> - * This driver is DT only, if no DT idle states are detected (ret == 0)
> - * let the driver initialization fail accordingly since there is no
> - * reason to initialize the idle driver if only wfi is supported.
> - */
> - ret = dt_init_idle_driver(drv, arm_idle_state_match, 1);
> - if (ret <= 0)
> - return ret ? : -ENODEV;
> -
> - ret = cpuidle_register_driver(drv);
> - if (ret) {
> - pr_err("Failed to register cpuidle driver\n");
> - return ret;
> - }
> -
> - /*
> - * Call arch CPU operations in order to initialize
> - * idle states suspend back-end specific data
> - */
> for_each_possible_cpu(cpu) {
> +
> + drv = kmemdup(&arm_idle_driver, sizeof(*drv), GFP_KERNEL);
> + if (!drv) {
> + ret = -ENOMEM;
> + goto out_fail;
> + }
> +
> + drv->cpumask = (struct cpumask *)cpumask_of(cpu);
> +
> + /*
> + * Initialize idle states data, starting at index 1. This
> + * driver is DT only, if no DT idle states are detected (ret
> + * == 0) let the driver initialization fail accordingly since
> + * there is no reason to initialize the idle driver if only
> + * wfi is supported.
> + */
> + ret = dt_init_idle_driver(drv, arm_idle_state_match, 1);
> + if (ret <= 0) {
> + ret = ret ? : -ENODEV;
> + goto out_fail;
> + }
> +
> + ret = cpuidle_register_driver(drv);
> + if (ret) {
> + pr_err("Failed to register cpuidle driver\n");
> + goto out_fail;
> + }
> +
> + /*
> + * Call arch CPU operations in order to initialize
> + * idle states suspend back-end specific data
> + */
> ret = arm_cpuidle_init(cpu);
>
> /*
> @@ -141,10 +154,11 @@ static int __init arm_idle_init(void)
> dev = per_cpu(cpuidle_devices, cpu);
> cpuidle_unregister_device(dev);
> kfree(dev);
> + drv = cpuidle_get_driver();
> + cpuidle_unregister_driver(drv);
> + kfree(drv);
> }
>
> - cpuidle_unregister_driver(drv);
> -
> return ret;
> }
> device_initcall(arm_idle_init);
>
Thanks,
Rafael
