[PATCH v4 05/11] x86/intel_pstate: relocate the driver register function

[PATCH v4 05/11] x86/intel_pstate: relocate the driver register function

[Wei Wang]

Register the CPU hotplug notifier when the driver is
registered, and move the driver register function to
the cpufreq.c.

v4 changes:
1) Coding style change (the position of "||").

Signed-off-by: Wei Wang <wei.w.wang@intel.com>
---
 xen/drivers/cpufreq/cpufreq.c      | 14 +++++++++++---
 xen/include/acpi/cpufreq/cpufreq.h | 27 +--------------------------
 2 files changed, 12 insertions(+), 29 deletions(-)

diff --git a/xen/drivers/cpufreq/cpufreq.c b/xen/drivers/cpufreq/cpufreq.c
index 91b6c25..acc4bb5 100644
--- a/xen/drivers/cpufreq/cpufreq.c
+++ b/xen/drivers/cpufreq/cpufreq.c
@@ -630,10 +630,18 @@ static struct notifier_block cpu_nfb = {
     .notifier_call = cpu_callback
 };
 
-static int __init cpufreq_presmp_init(void)
+int cpufreq_register_driver(struct cpufreq_driver *driver_data)
 {
+    if (!driver_data || !driver_data->init ||
+        !driver_data->verify || !driver_data->exit ||
+        (!driver_data->target == !driver_data->setpolicy))
+        return -EINVAL;
+
+    if (cpufreq_driver)
+        return -EBUSY;
+
+    cpufreq_driver = driver_data;
+
     register_cpu_notifier(&cpu_nfb);
     return 0;
 }
-presmp_initcall(cpufreq_presmp_init);
-
diff --git a/xen/include/acpi/cpufreq/cpufreq.h b/xen/include/acpi/cpufreq/cpufreq.h
index af37e90..502774f 100644
--- a/xen/include/acpi/cpufreq/cpufreq.h
+++ b/xen/include/acpi/cpufreq/cpufreq.h
@@ -183,32 +183,7 @@ struct cpufreq_driver {
 
 extern struct cpufreq_driver *cpufreq_driver;
 
-static __inline__ 
-int cpufreq_register_driver(struct cpufreq_driver *driver_data)
-{
-    if (!driver_data         || 
-        !driver_data->init   || 
-        !driver_data->exit   || 
-        !driver_data->verify || 
-        !driver_data->target)
-        return -EINVAL;
-
-    if (cpufreq_driver)
-        return -EBUSY;
-
-    cpufreq_driver = driver_data;
-    return 0;
-}
-
-static __inline__ 
-int cpufreq_unregister_driver(struct cpufreq_driver *driver)
-{
-    if (!cpufreq_driver || (driver != cpufreq_driver))
-        return -EINVAL;
-
-    cpufreq_driver = NULL;
-    return 0;
-}
+extern int cpufreq_register_driver(struct cpufreq_driver *driver_data);
 
 static __inline__
 void cpufreq_verify_within_limits(struct cpufreq_policy *policy,
-- 
1.9.1

Re: [PATCH v4 05/11] x86/intel_pstate: relocate the driver register function

[Jan Beulich]

>>> On 25.06.15 at 13:16, <wei.w.wang@intel.com> wrote:
> Register the CPU hotplug notifier when the driver is
> registered, and move the driver register function to
> the cpufreq.c.

At the very least this ought to be merged with the previous patch.

> --- a/xen/drivers/cpufreq/cpufreq.c
> +++ b/xen/drivers/cpufreq/cpufreq.c
> @@ -630,10 +630,18 @@ static struct notifier_block cpu_nfb = {
>      .notifier_call = cpu_callback
>  };
>  
> -static int __init cpufreq_presmp_init(void)
> +int cpufreq_register_driver(struct cpufreq_driver *driver_data)
>  {
> +    if (!driver_data || !driver_data->init ||
> +        !driver_data->verify || !driver_data->exit ||
> +        (!driver_data->target == !driver_data->setpolicy))
> +        return -EINVAL;
> +
> +    if (cpufreq_driver)
> +        return -EBUSY;
> +
> +    cpufreq_driver = driver_data;
> +
>      register_cpu_notifier(&cpu_nfb);
>      return 0;
>  }
> -presmp_initcall(cpufreq_presmp_init);

But then the code is left inconsistent: When will the notifier be
called for all the CPUs that are already up?

Jan

Re: [PATCH v4 05/11] x86/intel_pstate: relocate the driver register function

[Wang, Wei W]

On 24/07/2015 21:36,  Jan Beulich wrote:
> >>> On 25.06.15 at 13:16, <wei.w.wang@intel.com> wrote:
> > Register the CPU hotplug notifier when the driver is registered, and
> > move the driver register function to the cpufreq.c.
> 
> At the very least this ought to be merged with the previous patch.
> 
> > --- a/xen/drivers/cpufreq/cpufreq.c
> > +++ b/xen/drivers/cpufreq/cpufreq.c
> > @@ -630,10 +630,18 @@ static struct notifier_block cpu_nfb = {
> >      .notifier_call = cpu_callback
> >  };
> >
> > -static int __init cpufreq_presmp_init(void)
> > +int cpufreq_register_driver(struct cpufreq_driver *driver_data)
> >  {
> > +    if (!driver_data || !driver_data->init ||
> > +        !driver_data->verify || !driver_data->exit ||
> > +        (!driver_data->target == !driver_data->setpolicy))
> > +        return -EINVAL;
> > +
> > +    if (cpufreq_driver)
> > +        return -EBUSY;
> > +
> > +    cpufreq_driver = driver_data;
> > +
> >      register_cpu_notifier(&cpu_nfb);
> >      return 0;
> >  }
> > -presmp_initcall(cpufreq_presmp_init);
> 
> But then the code is left inconsistent: When will the notifier be called for all
> the CPUs that are already up?

Since I will keep the cpufreq_presmp_init() there, register_cpu_notifier(&cpu_nfb) will be called in the presmp function there.

Best,
Wei