Re: [PATCH] cpufreq: sparc: Don't allocate cpufreq_driver dynamically

[Rafael J. Wysocki]

On Thu, May 11, 2023 at 11:01 AM Viresh Kumar <viresh.kumar@xxxxxxxxxx> wrote:
>
> There is no point allocating the cpufreq driver dynamically and add so
> much complexity in the driver.
>
> Do what is done for other cpufreq drivers and statically allocate the
> cpufreq driver.
>
> Reported-by: Markus Elfring <Markus.Elfring@xxxxxx>
> Signed-off-by: Viresh Kumar <viresh.kumar@xxxxxxxxxx>

Acked-by: Rafael J. Wysocki <rafael@xxxxxxxxxx>

> ---
> Looks like I wrote this back in April and forgot to send it :(

Oh well.

>  drivers/cpufreq/sparc-us2e-cpufreq.c | 58 ++++++++--------------------
>  drivers/cpufreq/sparc-us3-cpufreq.c  | 58 ++++++++--------------------
>  2 files changed, 34 insertions(+), 82 deletions(-)
>
> diff --git a/drivers/cpufreq/sparc-us2e-cpufreq.c b/drivers/cpufreq/sparc-us2e-cpufreq.c
> index 92acbb25abb3..d3510cfdb3eb 100644
> --- a/drivers/cpufreq/sparc-us2e-cpufreq.c
> +++ b/drivers/cpufreq/sparc-us2e-cpufreq.c
> @@ -20,8 +20,6 @@
>  #include <asm/asi.h>
>  #include <asm/timer.h>
>
> -static struct cpufreq_driver *cpufreq_us2e_driver;
> -
>  struct us2e_freq_percpu_info {
>         struct cpufreq_frequency_table table[6];
>  };
> @@ -300,12 +298,19 @@ static int __init us2e_freq_cpu_init(struct cpufreq_policy *policy)
>
>  static int us2e_freq_cpu_exit(struct cpufreq_policy *policy)
>  {
> -       if (cpufreq_us2e_driver)
> -               us2e_freq_target(policy, 0);
> -
> +       us2e_freq_target(policy, 0);
>         return 0;
>  }
>
> +static struct cpufreq_driver cpufreq_us2e_driver = {
> +       .name = "UltraSPARC-IIe",
> +       .init = us2e_freq_cpu_init,
> +       .verify = cpufreq_generic_frequency_table_verify,
> +       .target_index = us2e_freq_target,
> +       .get = us2e_freq_get,
> +       .exit = us2e_freq_cpu_exit,
> +};
> +
>  static int __init us2e_freq_init(void)
>  {
>         unsigned long manuf, impl, ver;
> @@ -319,39 +324,15 @@ static int __init us2e_freq_init(void)
>         impl  = ((ver >> 32) & 0xffff);
>
>         if (manuf == 0x17 && impl == 0x13) {
> -               struct cpufreq_driver *driver;
> -
> -               ret = -ENOMEM;
> -               driver = kzalloc(sizeof(*driver), GFP_KERNEL);
> -               if (!driver)
> -                       goto err_out;
> -
> -               us2e_freq_table = kzalloc((NR_CPUS * sizeof(*us2e_freq_table)),
> -                       GFP_KERNEL);
> +               us2e_freq_table = kzalloc(NR_CPUS * sizeof(*us2e_freq_table),
> +                                         GFP_KERNEL);
>                 if (!us2e_freq_table)
> -                       goto err_out;
> -
> -               driver->init = us2e_freq_cpu_init;
> -               driver->verify = cpufreq_generic_frequency_table_verify;
> -               driver->target_index = us2e_freq_target;
> -               driver->get = us2e_freq_get;
> -               driver->exit = us2e_freq_cpu_exit;
> -               strcpy(driver->name, "UltraSPARC-IIe");
> +                       return -ENOMEM;
>
> -               cpufreq_us2e_driver = driver;
> -               ret = cpufreq_register_driver(driver);
> +               ret = cpufreq_register_driver(&cpufreq_us2e_driver);
>                 if (ret)
> -                       goto err_out;
> +                       kfree(us2e_freq_table);
>
> -               return 0;
> -
> -err_out:
> -               if (driver) {
> -                       kfree(driver);
> -                       cpufreq_us2e_driver = NULL;
> -               }
> -               kfree(us2e_freq_table);
> -               us2e_freq_table = NULL;
>                 return ret;
>         }
>
> @@ -360,13 +341,8 @@ static int __init us2e_freq_init(void)
>
>  static void __exit us2e_freq_exit(void)
>  {
> -       if (cpufreq_us2e_driver) {
> -               cpufreq_unregister_driver(cpufreq_us2e_driver);
> -               kfree(cpufreq_us2e_driver);
> -               cpufreq_us2e_driver = NULL;
> -               kfree(us2e_freq_table);
> -               us2e_freq_table = NULL;
> -       }
> +       cpufreq_unregister_driver(&cpufreq_us2e_driver);
> +       kfree(us2e_freq_table);
>  }
>
>  MODULE_AUTHOR("David S. Miller <davem@xxxxxxxxxx>");
> diff --git a/drivers/cpufreq/sparc-us3-cpufreq.c b/drivers/cpufreq/sparc-us3-cpufreq.c
> index e41b35b16afd..91d1ed558136 100644
> --- a/drivers/cpufreq/sparc-us3-cpufreq.c
> +++ b/drivers/cpufreq/sparc-us3-cpufreq.c
> @@ -19,8 +19,6 @@
>  #include <asm/head.h>
>  #include <asm/timer.h>
>
> -static struct cpufreq_driver *cpufreq_us3_driver;
> -
>  struct us3_freq_percpu_info {
>         struct cpufreq_frequency_table table[4];
>  };
> @@ -144,12 +142,19 @@ static int __init us3_freq_cpu_init(struct cpufreq_policy *policy)
>
>  static int us3_freq_cpu_exit(struct cpufreq_policy *policy)
>  {
> -       if (cpufreq_us3_driver)
> -               us3_freq_target(policy, 0);
> -
> +       us3_freq_target(policy, 0);
>         return 0;
>  }
>
> +static struct cpufreq_driver cpufreq_us3_driver = {
> +       .name = "UltraSPARC-III",
> +       .init = us3_freq_cpu_init,
> +       .verify = cpufreq_generic_frequency_table_verify,
> +       .target_index = us3_freq_target,
> +       .get = us3_freq_get,
> +       .exit = us3_freq_cpu_exit,
> +};
> +
>  static int __init us3_freq_init(void)
>  {
>         unsigned long manuf, impl, ver;
> @@ -167,39 +172,15 @@ static int __init us3_freq_init(void)
>              impl == CHEETAH_PLUS_IMPL ||
>              impl == JAGUAR_IMPL ||
>              impl == PANTHER_IMPL)) {
> -               struct cpufreq_driver *driver;
> -
> -               ret = -ENOMEM;
> -               driver = kzalloc(sizeof(*driver), GFP_KERNEL);
> -               if (!driver)
> -                       goto err_out;
> -
> -               us3_freq_table = kzalloc((NR_CPUS * sizeof(*us3_freq_table)),
> -                       GFP_KERNEL);
> +               us3_freq_table = kzalloc(NR_CPUS * sizeof(*us3_freq_table),
> +                                        GFP_KERNEL);
>                 if (!us3_freq_table)
> -                       goto err_out;
> -
> -               driver->init = us3_freq_cpu_init;
> -               driver->verify = cpufreq_generic_frequency_table_verify;
> -               driver->target_index = us3_freq_target;
> -               driver->get = us3_freq_get;
> -               driver->exit = us3_freq_cpu_exit;
> -               strcpy(driver->name, "UltraSPARC-III");
> +                       return -ENOMEM;
>
> -               cpufreq_us3_driver = driver;
> -               ret = cpufreq_register_driver(driver);
> +               ret = cpufreq_register_driver(&cpufreq_us3_driver);
>                 if (ret)
> -                       goto err_out;
> +                       kfree(us3_freq_table);
>
> -               return 0;
> -
> -err_out:
> -               if (driver) {
> -                       kfree(driver);
> -                       cpufreq_us3_driver = NULL;
> -               }
> -               kfree(us3_freq_table);
> -               us3_freq_table = NULL;
>                 return ret;
>         }
>
> @@ -208,13 +189,8 @@ static int __init us3_freq_init(void)
>
>  static void __exit us3_freq_exit(void)
>  {
> -       if (cpufreq_us3_driver) {
> -               cpufreq_unregister_driver(cpufreq_us3_driver);
> -               kfree(cpufreq_us3_driver);
> -               cpufreq_us3_driver = NULL;
> -               kfree(us3_freq_table);
> -               us3_freq_table = NULL;
> -       }
> +       cpufreq_unregister_driver(&cpufreq_us3_driver);
> +       kfree(us3_freq_table);
>  }
>
>  MODULE_AUTHOR("David S. Miller <davem@xxxxxxxxxx>");
> --
> 2.31.1.272.g89b43f80a514
>