[V4,1/3] cpufreq: Move the IS_ENABLED(CPU_THERMAL) macro in a stub

Message ID 20190627210209.32600-1-daniel.lezcano@linaro.org
State Accepted
Commit bcc61569997b2188ba89db43b5b991da01ea2d18
Headers show
Series
  • [V4,1/3] cpufreq: Move the IS_ENABLED(CPU_THERMAL) macro in a stub
Related show

Commit Message

Daniel Lezcano June 27, 2019, 9:02 p.m.
The cpufreq_online and the cpufreq_offline [un]register the driver as
a cooling device. This is done if the driver is flagged as a cooling
device in addition with a IS_ENABLED macro to compile out the branching
code.

Group this test in a stub function added in the cpufreq header instead
of having the IS_ENABLED in the code path.

Suggested-by: Rafael J. Wysocki <rafael@kernel.org>
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

---
 drivers/cpufreq/cpufreq.c | 6 ++----
 include/linux/cpufreq.h   | 6 ++++++
 2 files changed, 8 insertions(+), 4 deletions(-)

-- 
2.17.1

Comments

Rafael J. Wysocki June 27, 2019, 9:31 p.m. | #1
On Thu, Jun 27, 2019 at 11:02 PM Daniel Lezcano
<daniel.lezcano@linaro.org> wrote:
>

> The cpufreq_online and the cpufreq_offline [un]register the driver as

> a cooling device. This is done if the driver is flagged as a cooling

> device in addition with a IS_ENABLED macro to compile out the branching

> code.

>

> Group this test in a stub function added in the cpufreq header instead

> of having the IS_ENABLED in the code path.

>

> Suggested-by: Rafael J. Wysocki <rafael@kernel.org>

> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>

> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>


This one has been queued up for 5.3 already, no need to resend.

Thanks!

> ---

>  drivers/cpufreq/cpufreq.c | 6 ++----

>  include/linux/cpufreq.h   | 6 ++++++

>  2 files changed, 8 insertions(+), 4 deletions(-)

>

> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c

> index 85ff958e01f1..aee024e42618 100644

> --- a/drivers/cpufreq/cpufreq.c

> +++ b/drivers/cpufreq/cpufreq.c

> @@ -1378,8 +1378,7 @@ static int cpufreq_online(unsigned int cpu)

>         if (cpufreq_driver->ready)

>                 cpufreq_driver->ready(policy);

>

> -       if (IS_ENABLED(CONFIG_CPU_THERMAL) &&

> -           cpufreq_driver->flags & CPUFREQ_IS_COOLING_DEV)

> +       if (cpufreq_thermal_control_enabled(cpufreq_driver))

>                 policy->cdev = of_cpufreq_cooling_register(policy);

>

>         pr_debug("initialization complete\n");

> @@ -1469,8 +1468,7 @@ static int cpufreq_offline(unsigned int cpu)

>                 goto unlock;

>         }

>

> -       if (IS_ENABLED(CONFIG_CPU_THERMAL) &&

> -           cpufreq_driver->flags & CPUFREQ_IS_COOLING_DEV) {

> +       if (cpufreq_thermal_control_enabled(cpufreq_driver)) {

>                 cpufreq_cooling_unregister(policy->cdev);

>                 policy->cdev = NULL;

>         }

> diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h

> index d01a74fbc4db..a1467aa7f58b 100644

> --- a/include/linux/cpufreq.h

> +++ b/include/linux/cpufreq.h

> @@ -409,6 +409,12 @@ int cpufreq_unregister_driver(struct cpufreq_driver *driver_data);

>  const char *cpufreq_get_current_driver(void);

>  void *cpufreq_get_driver_data(void);

>

> +static inline int cpufreq_thermal_control_enabled(struct cpufreq_driver *drv)

> +{

> +       return IS_ENABLED(CONFIG_CPU_THERMAL) &&

> +               (drv->flags & CPUFREQ_IS_COOLING_DEV);

> +}

> +

>  static inline void cpufreq_verify_within_limits(struct cpufreq_policy *policy,

>                 unsigned int min, unsigned int max)

>  {

> --

> 2.17.1

>
Rafael J. Wysocki June 28, 2019, 10:03 a.m. | #2
On Fri, Jun 28, 2019 at 11:58 AM Daniel Lezcano
<daniel.lezcano@linaro.org> wrote:
>

>

> On 28/06/2019 11:12, Rafael J. Wysocki wrote:

> > On Thu, Jun 27, 2019 at 11:02 PM Daniel Lezcano

> > <daniel.lezcano@linaro.org> wrote:

> >>

> >> Currently the function cpufreq_cooling_register() returns a cooling

> >> device pointer which is used back as a pointer to call the function

> >> cpufreq_cooling_unregister(). Even if it is correct, it would make

> >> sense to not leak the structure inside a cpufreq driver and keep the

> >> code thermal code self-encapsulate. Moreover, that forces to add an

> >> extra variable in each driver using this function.

> >>

> >> Instead of passing the cooling device to unregister, pass the policy.

> >>

> >> Because the cpufreq_cooling_unregister() function uses the policy to

> >> unregister itself. The only purpose of the cooling device pointer is

> >> to unregister the cpu cooling device.

> >>

> >> As there is no more need of this pointer, remove it.

> >>

> >> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>

> >> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

> >

> > This doesn't apply for me.

> >

> > Care to rebase it on top of the Linus' tree?

>

> Sure but the patch depends on 1/3 which is in bleeding edge. Shall I

> rebase the 3 patches on v5.2-rc6 and resend ?


You can do that.

Alternatively, you can rebase on top of my linux-next branch.

Patch

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 85ff958e01f1..aee024e42618 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1378,8 +1378,7 @@  static int cpufreq_online(unsigned int cpu)
 	if (cpufreq_driver->ready)
 		cpufreq_driver->ready(policy);
 
-	if (IS_ENABLED(CONFIG_CPU_THERMAL) &&
-	    cpufreq_driver->flags & CPUFREQ_IS_COOLING_DEV)
+	if (cpufreq_thermal_control_enabled(cpufreq_driver))
 		policy->cdev = of_cpufreq_cooling_register(policy);
 
 	pr_debug("initialization complete\n");
@@ -1469,8 +1468,7 @@  static int cpufreq_offline(unsigned int cpu)
 		goto unlock;
 	}
 
-	if (IS_ENABLED(CONFIG_CPU_THERMAL) &&
-	    cpufreq_driver->flags & CPUFREQ_IS_COOLING_DEV) {
+	if (cpufreq_thermal_control_enabled(cpufreq_driver)) {
 		cpufreq_cooling_unregister(policy->cdev);
 		policy->cdev = NULL;
 	}
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index d01a74fbc4db..a1467aa7f58b 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -409,6 +409,12 @@  int cpufreq_unregister_driver(struct cpufreq_driver *driver_data);
 const char *cpufreq_get_current_driver(void);
 void *cpufreq_get_driver_data(void);
 
+static inline int cpufreq_thermal_control_enabled(struct cpufreq_driver *drv)
+{
+	return IS_ENABLED(CONFIG_CPU_THERMAL) &&
+		(drv->flags & CPUFREQ_IS_COOLING_DEV);
+}
+
 static inline void cpufreq_verify_within_limits(struct cpufreq_policy *policy,
 		unsigned int min, unsigned int max)
 {