On Fri, Oct 26, 2018 at 04:49:15PM +0200, Ulf Hansson wrote:
> The only reason that remains, to why the ARM cpuidle driver calls
> cpuidle_register_driver(), is to avoid printing an error message in case
> another driver already have been registered for the CPU. This seems a bit
> silly, but more importantly, if that is a common scenario, perhaps we
> should change cpuidle_register() accordingly instead.
I can't remember if there was a specific reason why we wanted to avoid
the kernel log on -EBUSY but I agree that's questionable.
> In either case, let's consolidate the code, by converting to use
> cpuidle_register|unregsiter(), which also avoids the unnecessary allocation
s/unregsiter/unregister/
> of the struct cpuidle_device.
>
> Signed-off-by: Ulf Hansson <ulf.hansson@xxxxxxxxxx>
> ---
> drivers/cpuidle/cpuidle-arm.c | 30 +++---------------------------
> 1 file changed, 3 insertions(+), 27 deletions(-)
It seems correct to me:
Reviewed-by: Lorenzo Pieralisi <lorenzo.pieralisi@xxxxxxx>
> diff --git a/drivers/cpuidle/cpuidle-arm.c b/drivers/cpuidle/cpuidle-arm.c
> index df564d783216..3a407a3ef22b 100644
> --- a/drivers/cpuidle/cpuidle-arm.c
> +++ b/drivers/cpuidle/cpuidle-arm.c
> @@ -82,7 +82,6 @@ static int __init arm_idle_init_cpu(int cpu)
> {
> int ret;
> struct cpuidle_driver *drv;
> - struct cpuidle_device *dev;
>
> drv = kmemdup(&arm_idle_driver, sizeof(*drv), GFP_KERNEL);
> if (!drv)
> @@ -119,33 +118,12 @@ static int __init arm_idle_init_cpu(int cpu)
> goto out_kfree_drv;
> }
>
> - ret = cpuidle_register_driver(drv);
> - if (ret) {
> - if (ret != -EBUSY)
> - pr_err("Failed to register cpuidle driver\n");
> + ret = cpuidle_register(drv, NULL);
> + if (ret)
> goto out_kfree_drv;
> - }
> -
> - dev = kzalloc(sizeof(*dev), GFP_KERNEL);
> - if (!dev) {
> - ret = -ENOMEM;
> - goto out_unregister_drv;
> - }
> - dev->cpu = cpu;
> -
> - ret = cpuidle_register_device(dev);
> - if (ret) {
> - pr_err("Failed to register cpuidle device for CPU %d\n",
> - cpu);
> - goto out_kfree_dev;
> - }
>
> return 0;
>
> -out_kfree_dev:
> - kfree(dev);
> -out_unregister_drv:
> - cpuidle_unregister_driver(drv);
> out_kfree_drv:
> kfree(drv);
> return ret;
> @@ -176,9 +154,7 @@ static int __init arm_idle_init(void)
> while (--cpu >= 0) {
> dev = per_cpu(cpuidle_devices, cpu);
> drv = cpuidle_get_cpu_driver(dev);
> - cpuidle_unregister_device(dev);
> - cpuidle_unregister_driver(drv);
> - kfree(dev);
> + cpuidle_unregister(drv);
> kfree(drv);
> }
>
> --
> 2.17.1
>
