[V4,2/3] thermal/drivers/cpu_cooling: Unregister with the policy

Commit Message

Daniel Lezcano June 27, 2019, 9:02 p.m. UTC

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 <[e-mail]>
Acked-by: Viresh Kumar <[e-mail]>
---
 drivers/cpufreq/arm_big_little.c              |  9 ++--
 drivers/cpufreq/cpufreq.c                     |  8 ++--
 drivers/thermal/cpu_cooling.c                 | 42 +++++++++++--------
 drivers/thermal/imx_thermal.c                 | 12 +++---
 .../ti-soc-thermal/ti-thermal-common.c        | 10 ++---
 include/linux/cpu_cooling.h                   |  6 +--
 include/linux/cpufreq.h                       |  3 --
 7 files changed, 45 insertions(+), 45 deletions(-)

Comments

Rafael J. Wysocki June 28, 2019, 9:12 a.m. UTC | #1

On Thu, Jun 27, 2019 at 11:02 PM Daniel Lezcano
<[e-mail]> 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 <[e-mail]>
> Acked-by: Viresh Kumar <[e-mail]>

This doesn't apply for me.

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

Also see below.

> ---
>  drivers/cpufreq/arm_big_little.c              |  9 ++--
>  drivers/cpufreq/cpufreq.c                     |  8 ++--
>  drivers/thermal/cpu_cooling.c                 | 42 +++++++++++--------
>  drivers/thermal/imx_thermal.c                 | 12 +++---
>  .../ti-soc-thermal/ti-thermal-common.c        | 10 ++---
>  include/linux/cpu_cooling.h                   |  6 +--
>  include/linux/cpufreq.h                       |  3 --
>  7 files changed, 45 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/cpufreq/arm_big_little.c b/drivers/cpufreq/arm_big_little.c
> index 7fe52fcddcf1..718c63231e66 100644
> --- a/drivers/cpufreq/arm_big_little.c
> +++ b/drivers/cpufreq/arm_big_little.c
> @@ -56,7 +56,6 @@ static bool bL_switching_enabled;
>  #define ACTUAL_FREQ(cluster, freq)  ((cluster == A7_CLUSTER) ? freq << 1 : freq)
>  #define VIRT_FREQ(cluster, freq)    ((cluster == A7_CLUSTER) ? freq >> 1 : freq)
>
> -static struct thermal_cooling_device *cdev[MAX_CLUSTERS];
>  static const struct cpufreq_arm_bL_ops *arm_bL_ops;
>  static struct clk *clk[MAX_CLUSTERS];
>  static struct cpufreq_frequency_table *freq_table[MAX_CLUSTERS + 1];
> @@ -501,10 +500,8 @@ static int bL_cpufreq_exit(struct cpufreq_policy *policy)
>         struct device *cpu_dev;
>         int cur_cluster = cpu_to_cluster(policy->cpu);
>
> -       if (cur_cluster < MAX_CLUSTERS) {
> -               cpufreq_cooling_unregister(cdev[cur_cluster]);
> -               cdev[cur_cluster] = NULL;
> -       }
> +       if (cur_cluster < MAX_CLUSTERS)
> +               cpufreq_cooling_unregister(policy);
>
>         cpu_dev = get_cpu_device(policy->cpu);
>         if (!cpu_dev) {
> @@ -527,7 +524,7 @@ static void bL_cpufreq_ready(struct cpufreq_policy *policy)
>         if (cur_cluster >= MAX_CLUSTERS)
>                 return;
>
> -       cdev[cur_cluster] = of_cpufreq_cooling_register(policy);
> +       of_cpufreq_cooling_register(policy);
>  }
>
>  static struct cpufreq_driver bL_cpufreq_driver = {
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index aee024e42618..1663a5601811 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -1379,7 +1379,7 @@ static int cpufreq_online(unsigned int cpu)
>                 cpufreq_driver->ready(policy);
>
>         if (cpufreq_thermal_control_enabled(cpufreq_driver))
> -               policy->cdev = of_cpufreq_cooling_register(policy);
> +               of_cpufreq_cooling_register(policy);
>
>         pr_debug("initialization complete\n");
>
> @@ -1468,10 +1468,8 @@ static int cpufreq_offline(unsigned int cpu)
>                 goto unlock;
>         }
>
> -       if (cpufreq_thermal_control_enabled(cpufreq_driver)) {
> -               cpufreq_cooling_unregister(policy->cdev);
> -               policy->cdev = NULL;
> -       }
> +       if (cpufreq_thermal_control_enabled(cpufreq_driver))
> +               cpufreq_cooling_unregister(policy);
>
>         if (cpufreq_driver->stop_cpu)
>                 cpufreq_driver->stop_cpu(policy);
> diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
> index 83486775e593..be01546a656f 100644
> --- a/drivers/thermal/cpu_cooling.c
> +++ b/drivers/thermal/cpu_cooling.c
> @@ -78,6 +78,7 @@ struct cpufreq_cooling_device {
>         struct cpufreq_policy *policy;
>         struct list_head node;
>         struct time_in_idle *idle_time;
> +       struct thermal_cooling_device *cdev;
>  };
>
>  static DEFINE_IDA(cpufreq_ida);
> @@ -606,6 +607,7 @@ __cpufreq_cooling_register(struct device_node *np,
>                 goto remove_ida;
>
>         cpufreq_cdev->clipped_freq = get_state_freq(cpufreq_cdev, 0);
> +       cpufreq_cdev->cdev = cdev;
>
>         mutex_lock(&cooling_list_lock);
>         /* Register the notifier for first cpufreq cooling device */
> @@ -693,35 +695,41 @@ of_cpufreq_cooling_register(struct cpufreq_policy *policy)
>  }
>  EXPORT_SYMBOL_GPL(of_cpufreq_cooling_register);
>
> +void __cpufreq_cooling_unregister(struct cpufreq_cooling_device *cpufreq_cdev, int last)
> +{
> +       /* Unregister the notifier for the last cpufreq cooling device */
> +       if (last)
> +               cpufreq_unregister_notifier(&thermal_cpufreq_notifier_block,
> +                                           CPUFREQ_POLICY_NOTIFIER);
> +
> +       thermal_cooling_device_unregister(cpufreq_cdev->cdev);
> +       ida_simple_remove(&cpufreq_ida, cpufreq_cdev->id);
> +       kfree(cpufreq_cdev->idle_time);
> +       kfree(cpufreq_cdev);
> +}
> +
>  /**
>   * cpufreq_cooling_unregister - function to remove cpufreq cooling device.
>   * @cdev: thermal cooling device pointer.
>   *
>   * This interface function unregisters the "thermal-cpufreq-%x" cooling device.
>   */
> -void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev)
> +void cpufreq_cooling_unregister(struct cpufreq_policy *policy)
>  {
>         struct cpufreq_cooling_device *cpufreq_cdev;

I would do

        struct cpufreq_cooling_device *ccd, *cpufreq_cdev = NULL;

and then ->

>         bool last;
>
> -       if (!cdev)
> -               return;
> -
> -       cpufreq_cdev = cdev->devdata;
> -
>         mutex_lock(&cooling_list_lock);
> -       list_del(&cpufreq_cdev->node);
> -       /* Unregister the notifier for the last cpufreq cooling device */
> -       last = list_empty(&cpufreq_cdev_list);
> +       list_for_each_entry(cpufreq_cdev, &cpufreq_cdev_list, node) {

-> list_for_each_entry(ccd, &cpufreq_cdev_list, node) {
                if (ccd->policy == policy) {

> +               if (cpufreq_cdev->policy == policy) {

                           cpufreq_cdev = ccd;

> +                       list_del(&cpufreq_cdev->node);
> +                       last = list_empty(&cpufreq_cdev_list);
> +                       break;
> +               }
> +       }
>         mutex_unlock(&cooling_list_lock);

And here

if (!cpufreq_cdev)
        return;

And that's it.  No new functions needed.

> -       if (last)
> -               cpufreq_unregister_notifier(&thermal_cpufreq_notifier_block,
> -                                           CPUFREQ_POLICY_NOTIFIER);
> -

And I don't that the above needs to be changed at all in any case.


> -       thermal_cooling_device_unregister(cdev);
> -       ida_simple_remove(&cpufreq_ida, cpufreq_cdev->id);
> -       kfree(cpufreq_cdev->idle_time);
> -       kfree(cpufreq_cdev);
> +       if (cpufreq_cdev->policy == policy)
> +               __cpufreq_cooling_unregister(cpufreq_cdev, last);
>  }
>  EXPORT_SYMBOL_GPL(cpufreq_cooling_unregister);
> diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
> index bb6754a5342c..021c0948b740 100644
> --- a/drivers/thermal/imx_thermal.c
> +++ b/drivers/thermal/imx_thermal.c
> @@ -203,7 +203,6 @@ static struct thermal_soc_data thermal_imx7d_data = {
>  struct imx_thermal_data {
>         struct cpufreq_policy *policy;
>         struct thermal_zone_device *tz;
> -       struct thermal_cooling_device *cdev;
>         enum thermal_device_mode mode;
>         struct regmap *tempmon;
>         u32 c1, c2; /* See formula in imx_init_calib() */
> @@ -656,6 +655,7 @@ MODULE_DEVICE_TABLE(of, of_imx_thermal_match);
>  static int imx_thermal_register_legacy_cooling(struct imx_thermal_data *data)
>  {
>         struct device_node *np;
> +       struct thermal_cooling_device *cdev;
>         int ret;
>
>         data->policy = cpufreq_cpu_get(0);
> @@ -667,9 +667,9 @@ static int imx_thermal_register_legacy_cooling(struct imx_thermal_data *data)
>         np = of_get_cpu_node(data->policy->cpu, NULL);
>
>         if (!np || !of_find_property(np, "#cooling-cells", NULL)) {
> -               data->cdev = cpufreq_cooling_register(data->policy);
> -               if (IS_ERR(data->cdev)) {
> -                       ret = PTR_ERR(data->cdev);
> +               cdev = cpufreq_cooling_register(data->policy);
> +               if (IS_ERR(cdev)) {
> +                       ret = PTR_ERR(cdev);
>                         cpufreq_cpu_put(data->policy);
>                         return ret;
>                 }
> @@ -680,7 +680,7 @@ static int imx_thermal_register_legacy_cooling(struct imx_thermal_data *data)
>
>  static void imx_thermal_unregister_legacy_cooling(struct imx_thermal_data *data)
>  {
> -       cpufreq_cooling_unregister(data->cdev);
> +       cpufreq_cooling_unregister(data->policy);
>         cpufreq_cpu_put(data->policy);
>  }
>
> @@ -872,7 +872,7 @@ static int imx_thermal_remove(struct platform_device *pdev)
>                 clk_disable_unprepare(data->thermal_clk);
>
>         thermal_zone_device_unregister(data->tz);
> -       cpufreq_cooling_unregister(data->cdev);
> +       cpufreq_cooling_unregister(data->policy);
>         cpufreq_cpu_put(data->policy);
>
>         return 0;
> diff --git a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c
> index b4f981daeaf2..170b70b6ec61 100644
> --- a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c
> +++ b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c
> @@ -41,7 +41,6 @@ struct ti_thermal_data {
>         struct cpufreq_policy *policy;
>         struct thermal_zone_device *ti_thermal;
>         struct thermal_zone_device *pcb_tz;
> -       struct thermal_cooling_device *cool_dev;
>         struct ti_bandgap *bgp;
>         enum thermal_device_mode mode;
>         struct work_struct thermal_wq;
> @@ -233,6 +232,7 @@ int ti_thermal_register_cpu_cooling(struct ti_bandgap *bgp, int id)
>  {
>         struct ti_thermal_data *data;
>         struct device_node *np = bgp->dev->of_node;
> +       struct thermal_cooling_device *cdev;
>
>         /*
>          * We are assuming here that if one deploys the zone
> @@ -256,9 +256,9 @@ int ti_thermal_register_cpu_cooling(struct ti_bandgap *bgp, int id)
>         }
>
>         /* Register cooling device */
> -       data->cool_dev = cpufreq_cooling_register(data->policy);
> -       if (IS_ERR(data->cool_dev)) {
> -               int ret = PTR_ERR(data->cool_dev);
> +       cdev = cpufreq_cooling_register(data->policy);
> +       if (IS_ERR(cdev)) {
> +               int ret = PTR_ERR(cdev);
>                 dev_err(bgp->dev, "Failed to register cpu cooling device %d\n",
>                         ret);
>                 cpufreq_cpu_put(data->policy);
> @@ -277,7 +277,7 @@ int ti_thermal_unregister_cpu_cooling(struct ti_bandgap *bgp, int id)
>         data = ti_bandgap_get_sensor_data(bgp, id);
>
>         if (data) {
> -               cpufreq_cooling_unregister(data->cool_dev);
> +               cpufreq_cooling_unregister(data->policy);
>                 if (data->policy)
>                         cpufreq_cpu_put(data->policy);
>         }
> diff --git a/include/linux/cpu_cooling.h b/include/linux/cpu_cooling.h
> index bae54bb7c048..89f469ee4be4 100644
> --- a/include/linux/cpu_cooling.h
> +++ b/include/linux/cpu_cooling.h
> @@ -29,9 +29,9 @@ cpufreq_cooling_register(struct cpufreq_policy *policy);
>
>  /**
>   * cpufreq_cooling_unregister - function to remove cpufreq cooling device.
> - * @cdev: thermal cooling device pointer.
> + * @policy: cpufreq policy
>   */
> -void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev);
> +void cpufreq_cooling_unregister(struct cpufreq_policy *policy);
>
>  #else /* !CONFIG_CPU_THERMAL */
>  static inline struct thermal_cooling_device *
> @@ -41,7 +41,7 @@ cpufreq_cooling_register(struct cpufreq_policy *policy)
>  }
>
>  static inline
> -void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev)
> +void cpufreq_cooling_unregister(struct cpufreq_policy *policy)
>  {
>         return;
>  }
> diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
> index a1467aa7f58b..ce13204df972 100644
> --- a/include/linux/cpufreq.h
> +++ b/include/linux/cpufreq.h
> @@ -144,9 +144,6 @@ struct cpufreq_policy {
>
>         /* For cpufreq driver's internal use */
>         void                    *driver_data;
> -
> -       /* Pointer to the cooling device if used for thermal mitigation */
> -       struct thermal_cooling_device *cdev;
>  };
>
>  struct cpufreq_freqs {
> --
> 2.17.1
>

Daniel Lezcano June 28, 2019, 9:57 a.m. UTC | #2

On 28/06/2019 11:12, Rafael J. Wysocki wrote:
> On Thu, Jun 27, 2019 at 11:02 PM Daniel Lezcano
> <[e-mail]> 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 <[e-mail]>
>> Acked-by: Viresh Kumar <[e-mail]>
> 
> 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 ?

Rafael J. Wysocki June 28, 2019, 10:03 a.m. UTC | #3

On Fri, Jun 28, 2019 at 11:58 AM Daniel Lezcano
<[e-mail]> wrote:
>
>
> On 28/06/2019 11:12, Rafael J. Wysocki wrote:
> > On Thu, Jun 27, 2019 at 11:02 PM Daniel Lezcano
> > <[e-mail]> 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 <[e-mail]>
> >> Acked-by: Viresh Kumar <[e-mail]>
> >
> > 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.

Rafael J. Wysocki June 28, 2019, 10:07 a.m. UTC | #4

On Fri, Jun 28, 2019 at 11:12 AM Rafael J. Wysocki <[e-mail]> wrote:
>
> On Thu, Jun 27, 2019 at 11:02 PM Daniel Lezcano
> <[e-mail]> 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 <[e-mail]>
> > Acked-by: Viresh Kumar <[e-mail]>

[cut]

> > -void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev)
> > +void cpufreq_cooling_unregister(struct cpufreq_policy *policy)
> >  {
> >         struct cpufreq_cooling_device *cpufreq_cdev;
>
> I would do
>
>         struct cpufreq_cooling_device *ccd, *cpufreq_cdev = NULL;
>
> and then ->

Not even that. ->

>
> >         bool last;
> >
> > -       if (!cdev)
> > -               return;
> > -
> > -       cpufreq_cdev = cdev->devdata;
> > -
> >         mutex_lock(&cooling_list_lock);
> > -       list_del(&cpufreq_cdev->node);
> > -       /* Unregister the notifier for the last cpufreq cooling device */
> > -       last = list_empty(&cpufreq_cdev_list);
> > +       list_for_each_entry(cpufreq_cdev, &cpufreq_cdev_list, node) {
>
> -> list_for_each_entry(ccd, &cpufreq_cdev_list, node) {
>                 if (ccd->policy == policy) {
>
> > +               if (cpufreq_cdev->policy == policy) {
>
>                            cpufreq_cdev = ccd;
>
> > +                       list_del(&cpufreq_cdev->node);
> > +                       last = list_empty(&cpufreq_cdev_list);
> > +                       break;
> > +               }
> > +       }
> >         mutex_unlock(&cooling_list_lock);
>
> And here
>
> if (!cpufreq_cdev)
>         return;

-> It would be sufficient to simply do:

if (cpufreq_cdev->policy != policy)
        return;

here AFAICS.

>
> And that's it.  No new functions needed.
>
> > -       if (last)
> > -               cpufreq_unregister_notifier(&thermal_cpufreq_notifier_block,
> > -                                           CPUFREQ_POLICY_NOTIFIER);
> > -
>
> And I don't that the above needs to be changed at all in any case.
>
>
> > -       thermal_cooling_device_unregister(cdev);
> > -       ida_simple_remove(&cpufreq_ida, cpufreq_cdev->id);
> > -       kfree(cpufreq_cdev->idle_time);
> > -       kfree(cpufreq_cdev);
> > +       if (cpufreq_cdev->policy == policy)
> > +               __cpufreq_cooling_unregister(cpufreq_cdev, last);
> >  }

Daniel Lezcano June 28, 2019, 11:57 a.m. UTC | #5

On 28/06/2019 12:03, Rafael J. Wysocki wrote:
> On Fri, Jun 28, 2019 at 11:58 AM Daniel Lezcano
> <[e-mail]> wrote:
>>
>>
>> On 28/06/2019 11:12, Rafael J. Wysocki wrote:
>>> On Thu, Jun 27, 2019 at 11:02 PM Daniel Lezcano
>>> <[e-mail]> 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 <[e-mail]>
>>>> Acked-by: Viresh Kumar <[e-mail]>
>>>
>>> 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.

Ok, it is rebased on top of linux-next, however the conflict is coming
from the energy model patchset sent by Quentin [1][2] I used to based my
series which is not yet applied in the thermal tree.

I'm wondering if it wouldn't make sense to take Quentin's series also,
it is a long time around in the mailing list, reviewed and acked. So I
can send the two remaining patches on top of it without conflict,
otherwise we will have a conflict in the merge window.

[1] https://lkml.org/lkml/2019/5/30/794
[2] https://lkml.org/lkml/2019/6/19/190

Patch

diff --git a/drivers/cpufreq/arm_big_little.c b/drivers/cpufreq/arm_big_little.c
index 7fe52fcddcf1..718c63231e66 100644
--- a/drivers/cpufreq/arm_big_little.c
+++ b/drivers/cpufreq/arm_big_little.c
@@ -56,7 +56,6 @@  static bool bL_switching_enabled;
 #define ACTUAL_FREQ(cluster, freq)  ((cluster == A7_CLUSTER) ? freq << 1 : freq)
 #define VIRT_FREQ(cluster, freq)    ((cluster == A7_CLUSTER) ? freq >> 1 : freq)
 
-static struct thermal_cooling_device *cdev[MAX_CLUSTERS];
 static const struct cpufreq_arm_bL_ops *arm_bL_ops;
 static struct clk *clk[MAX_CLUSTERS];
 static struct cpufreq_frequency_table *freq_table[MAX_CLUSTERS + 1];
@@ -501,10 +500,8 @@  static int bL_cpufreq_exit(struct cpufreq_policy *policy)
 	struct device *cpu_dev;
 	int cur_cluster = cpu_to_cluster(policy->cpu);
 
-	if (cur_cluster < MAX_CLUSTERS) {
-		cpufreq_cooling_unregister(cdev[cur_cluster]);
-		cdev[cur_cluster] = NULL;
-	}
+	if (cur_cluster < MAX_CLUSTERS)
+		cpufreq_cooling_unregister(policy);
 
 	cpu_dev = get_cpu_device(policy->cpu);
 	if (!cpu_dev) {
@@ -527,7 +524,7 @@  static void bL_cpufreq_ready(struct cpufreq_policy *policy)
 	if (cur_cluster >= MAX_CLUSTERS)
 		return;
 
-	cdev[cur_cluster] = of_cpufreq_cooling_register(policy);
+	of_cpufreq_cooling_register(policy);
 }
 
 static struct cpufreq_driver bL_cpufreq_driver = {
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index aee024e42618..1663a5601811 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1379,7 +1379,7 @@  static int cpufreq_online(unsigned int cpu)
 		cpufreq_driver->ready(policy);
 
 	if (cpufreq_thermal_control_enabled(cpufreq_driver))
-		policy->cdev = of_cpufreq_cooling_register(policy);
+		of_cpufreq_cooling_register(policy);
 
 	pr_debug("initialization complete\n");
 
@@ -1468,10 +1468,8 @@  static int cpufreq_offline(unsigned int cpu)
 		goto unlock;
 	}
 
-	if (cpufreq_thermal_control_enabled(cpufreq_driver)) {
-		cpufreq_cooling_unregister(policy->cdev);
-		policy->cdev = NULL;
-	}
+	if (cpufreq_thermal_control_enabled(cpufreq_driver))
+		cpufreq_cooling_unregister(policy);
 
 	if (cpufreq_driver->stop_cpu)
 		cpufreq_driver->stop_cpu(policy);
diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
index 83486775e593..be01546a656f 100644
--- a/drivers/thermal/cpu_cooling.c
+++ b/drivers/thermal/cpu_cooling.c
@@ -78,6 +78,7 @@  struct cpufreq_cooling_device {
 	struct cpufreq_policy *policy;
 	struct list_head node;
 	struct time_in_idle *idle_time;
+	struct thermal_cooling_device *cdev;
 };
 
 static DEFINE_IDA(cpufreq_ida);
@@ -606,6 +607,7 @@  __cpufreq_cooling_register(struct device_node *np,
 		goto remove_ida;
 
 	cpufreq_cdev->clipped_freq = get_state_freq(cpufreq_cdev, 0);
+	cpufreq_cdev->cdev = cdev;
 
 	mutex_lock(&cooling_list_lock);
 	/* Register the notifier for first cpufreq cooling device */
@@ -693,35 +695,41 @@  of_cpufreq_cooling_register(struct cpufreq_policy *policy)
 }
 EXPORT_SYMBOL_GPL(of_cpufreq_cooling_register);
 
+void __cpufreq_cooling_unregister(struct cpufreq_cooling_device *cpufreq_cdev, int last)
+{
+	/* Unregister the notifier for the last cpufreq cooling device */
+	if (last)
+		cpufreq_unregister_notifier(&thermal_cpufreq_notifier_block,
+					    CPUFREQ_POLICY_NOTIFIER);
+
+	thermal_cooling_device_unregister(cpufreq_cdev->cdev);
+	ida_simple_remove(&cpufreq_ida, cpufreq_cdev->id);
+	kfree(cpufreq_cdev->idle_time);
+	kfree(cpufreq_cdev);
+}
+
 /**
  * cpufreq_cooling_unregister - function to remove cpufreq cooling device.
  * @cdev: thermal cooling device pointer.
  *
  * This interface function unregisters the "thermal-cpufreq-%x" cooling device.
  */
-void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev)
+void cpufreq_cooling_unregister(struct cpufreq_policy *policy)
 {
 	struct cpufreq_cooling_device *cpufreq_cdev;
 	bool last;
 
-	if (!cdev)
-		return;
-
-	cpufreq_cdev = cdev->devdata;
-
 	mutex_lock(&cooling_list_lock);
-	list_del(&cpufreq_cdev->node);
-	/* Unregister the notifier for the last cpufreq cooling device */
-	last = list_empty(&cpufreq_cdev_list);
+	list_for_each_entry(cpufreq_cdev, &cpufreq_cdev_list, node) {
+		if (cpufreq_cdev->policy == policy) {
+			list_del(&cpufreq_cdev->node);
+			last = list_empty(&cpufreq_cdev_list);
+			break;
+		}
+	}
 	mutex_unlock(&cooling_list_lock);
 
-	if (last)
-		cpufreq_unregister_notifier(&thermal_cpufreq_notifier_block,
-					    CPUFREQ_POLICY_NOTIFIER);
-
-	thermal_cooling_device_unregister(cdev);
-	ida_simple_remove(&cpufreq_ida, cpufreq_cdev->id);
-	kfree(cpufreq_cdev->idle_time);
-	kfree(cpufreq_cdev);
+	if (cpufreq_cdev->policy == policy)
+		__cpufreq_cooling_unregister(cpufreq_cdev, last);
 }
 EXPORT_SYMBOL_GPL(cpufreq_cooling_unregister);
diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
index bb6754a5342c..021c0948b740 100644
--- a/drivers/thermal/imx_thermal.c
+++ b/drivers/thermal/imx_thermal.c
@@ -203,7 +203,6 @@  static struct thermal_soc_data thermal_imx7d_data = {
 struct imx_thermal_data {
 	struct cpufreq_policy *policy;
 	struct thermal_zone_device *tz;
-	struct thermal_cooling_device *cdev;
 	enum thermal_device_mode mode;
 	struct regmap *tempmon;
 	u32 c1, c2; /* See formula in imx_init_calib() */
@@ -656,6 +655,7 @@  MODULE_DEVICE_TABLE(of, of_imx_thermal_match);
 static int imx_thermal_register_legacy_cooling(struct imx_thermal_data *data)
 {
 	struct device_node *np;
+	struct thermal_cooling_device *cdev;
 	int ret;
 
 	data->policy = cpufreq_cpu_get(0);
@@ -667,9 +667,9 @@  static int imx_thermal_register_legacy_cooling(struct imx_thermal_data *data)
 	np = of_get_cpu_node(data->policy->cpu, NULL);
 
 	if (!np || !of_find_property(np, "#cooling-cells", NULL)) {
-		data->cdev = cpufreq_cooling_register(data->policy);
-		if (IS_ERR(data->cdev)) {
-			ret = PTR_ERR(data->cdev);
+		cdev = cpufreq_cooling_register(data->policy);
+		if (IS_ERR(cdev)) {
+			ret = PTR_ERR(cdev);
 			cpufreq_cpu_put(data->policy);
 			return ret;
 		}
@@ -680,7 +680,7 @@  static int imx_thermal_register_legacy_cooling(struct imx_thermal_data *data)
 
 static void imx_thermal_unregister_legacy_cooling(struct imx_thermal_data *data)
 {
-	cpufreq_cooling_unregister(data->cdev);
+	cpufreq_cooling_unregister(data->policy);
 	cpufreq_cpu_put(data->policy);
 }
 
@@ -872,7 +872,7 @@  static int imx_thermal_remove(struct platform_device *pdev)
 		clk_disable_unprepare(data->thermal_clk);
 
 	thermal_zone_device_unregister(data->tz);
-	cpufreq_cooling_unregister(data->cdev);
+	cpufreq_cooling_unregister(data->policy);
 	cpufreq_cpu_put(data->policy);
 
 	return 0;
diff --git a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c
index b4f981daeaf2..170b70b6ec61 100644
--- a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c
+++ b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c
@@ -41,7 +41,6 @@  struct ti_thermal_data {
 	struct cpufreq_policy *policy;
 	struct thermal_zone_device *ti_thermal;
 	struct thermal_zone_device *pcb_tz;
-	struct thermal_cooling_device *cool_dev;
 	struct ti_bandgap *bgp;
 	enum thermal_device_mode mode;
 	struct work_struct thermal_wq;
@@ -233,6 +232,7 @@  int ti_thermal_register_cpu_cooling(struct ti_bandgap *bgp, int id)
 {
 	struct ti_thermal_data *data;
 	struct device_node *np = bgp->dev->of_node;
+	struct thermal_cooling_device *cdev;
 
 	/*
 	 * We are assuming here that if one deploys the zone
@@ -256,9 +256,9 @@  int ti_thermal_register_cpu_cooling(struct ti_bandgap *bgp, int id)
 	}
 
 	/* Register cooling device */
-	data->cool_dev = cpufreq_cooling_register(data->policy);
-	if (IS_ERR(data->cool_dev)) {
-		int ret = PTR_ERR(data->cool_dev);
+	cdev = cpufreq_cooling_register(data->policy);
+	if (IS_ERR(cdev)) {
+		int ret = PTR_ERR(cdev);
 		dev_err(bgp->dev, "Failed to register cpu cooling device %d\n",
 			ret);
 		cpufreq_cpu_put(data->policy);
@@ -277,7 +277,7 @@  int ti_thermal_unregister_cpu_cooling(struct ti_bandgap *bgp, int id)
 	data = ti_bandgap_get_sensor_data(bgp, id);
 
 	if (data) {
-		cpufreq_cooling_unregister(data->cool_dev);
+		cpufreq_cooling_unregister(data->policy);
 		if (data->policy)
 			cpufreq_cpu_put(data->policy);
 	}
diff --git a/include/linux/cpu_cooling.h b/include/linux/cpu_cooling.h
index bae54bb7c048..89f469ee4be4 100644
--- a/include/linux/cpu_cooling.h
+++ b/include/linux/cpu_cooling.h
@@ -29,9 +29,9 @@  cpufreq_cooling_register(struct cpufreq_policy *policy);
 
 /**
  * cpufreq_cooling_unregister - function to remove cpufreq cooling device.
- * @cdev: thermal cooling device pointer.
+ * @policy: cpufreq policy
  */
-void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev);
+void cpufreq_cooling_unregister(struct cpufreq_policy *policy);
 
 #else /* !CONFIG_CPU_THERMAL */
 static inline struct thermal_cooling_device *
@@ -41,7 +41,7 @@  cpufreq_cooling_register(struct cpufreq_policy *policy)
 }
 
 static inline
-void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev)
+void cpufreq_cooling_unregister(struct cpufreq_policy *policy)
 {
 	return;
 }
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index a1467aa7f58b..ce13204df972 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -144,9 +144,6 @@  struct cpufreq_policy {
 
 	/* For cpufreq driver's internal use */
 	void			*driver_data;
-
-	/* Pointer to the cooling device if used for thermal mitigation */
-	struct thermal_cooling_device *cdev;
 };
 
 struct cpufreq_freqs {