Re: [v2] cpufreq: sparc: Fix exception handling in two functions

[Viresh Kumar <viresh.kumar@xxxxxxxxxx>]

On 10-04-23, 15:08, Markus Elfring wrote:
> >> @@ -337,21 +337,17 @@ static int __init us2e_freq_init(void)
> >>  		driver->get = us2e_freq_get;
> >>  		driver->exit = us2e_freq_cpu_exit;
> >>  		strcpy(driver->name, "UltraSPARC-IIe");
> >> -
> >> -		cpufreq_us2e_driver = driver;
> >
> > This changes the behavior of the code here as "cpufreq_us2e_driver"
> > is used in us2e_freq_cpu_exit(). If some failure occurs after a
> > policy is initialized, and driver doesn't register successfully, then
> > we won't set the frequency to the lowest index of the table anymore.
> 
> The setting of the variables “cpufreq_us…_driver” influences the need
> to reset them to null pointers for the desired exception handling,
> doesn't it?

This is what all should be done for these drivers I guess. There is no
points doing the dance of {de}allocating resources unnecessarily.

diff --git a/drivers/cpufreq/sparc-us2e-cpufreq.c b/drivers/cpufreq/sparc-us2e-cpufreq.c
index 92acbb25abb3..b31fb07f3f39 100644
--- a/drivers/cpufreq/sparc-us2e-cpufreq.c
+++ b/drivers/cpufreq/sparc-us2e-cpufreq.c
@@ -20,7 +20,14 @@
 #include <asm/asi.h>
 #include <asm/timer.h>

-static struct cpufreq_driver *cpufreq_us2e_driver;
+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,
+};

 struct us2e_freq_percpu_info {
        struct cpufreq_frequency_table table[6];
@@ -300,9 +307,7 @@ 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;
 }

@@ -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);
                if (!us2e_freq_table)
-                       goto err_out;
+                       return -ENOMEM;

-               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");
-
-               cpufreq_us2e_driver = driver;
                ret = cpufreq_register_driver(driver);
                if (ret)
-                       goto err_out;
-
-               return 0;
+                       kfree(us2e_freq_table);

-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>");

-- 
viresh