[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20220614022448.pqld3zhqucsfofss@vireshk-i7>
Date: Tue, 14 Jun 2022 07:54:48 +0530
From: Viresh Kumar <viresh.kumar@...aro.org>
To: Lukasz Luba <lukasz.luba@....com>
Cc: linux-kernel@...r.kernel.org, linux-pm@...r.kernel.org,
daniel.lezcano@...aro.org, amitk@...nel.org, rui.zhang@...el.com,
rafael@...nel.org, dietmar.eggemann@....com, rostedt@...dmis.org,
mingo@...hat.com
Subject: Re: [PATCH v2 1/4] thermal: cpufreq_cooling: Use private callback
ops for each cooling device
On 13-06-22, 13:43, Lukasz Luba wrote:
> It is very unlikely that one CPU cluster would have the EM and some other
> won't have it (because EM registration failed or DT lacks needed entry).
> Although, we should avoid modifying global variable with callbacks anyway.
> Redesign this and add safety for such situation.
>
> Signed-off-by: Lukasz Luba <lukasz.luba@....com>
> ---
> drivers/thermal/cpufreq_cooling.c | 15 ++++++---------
> 1 file changed, 6 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/thermal/cpufreq_cooling.c b/drivers/thermal/cpufreq_cooling.c
> index b8151d95a806..ad8b86f5281b 100644
> --- a/drivers/thermal/cpufreq_cooling.c
> +++ b/drivers/thermal/cpufreq_cooling.c
> @@ -59,6 +59,7 @@ struct time_in_idle {
> * @cdev: thermal_cooling_device pointer to keep track of the
> * registered cooling device.
> * @policy: cpufreq policy.
> + * @cooling_ops: cpufreq callbacks to thermal cooling device ops
> * @idle_time: idle time stats
> * @qos_req: PM QoS contraint to apply
> *
> @@ -71,6 +72,7 @@ struct cpufreq_cooling_device {
> unsigned int max_level;
> struct em_perf_domain *em;
> struct cpufreq_policy *policy;
> + struct thermal_cooling_device_ops cooling_ops;
> #ifndef CONFIG_SMP
> struct time_in_idle *idle_time;
> #endif
> @@ -485,14 +487,6 @@ static int cpufreq_set_cur_state(struct thermal_cooling_device *cdev,
> return ret;
> }
>
> -/* Bind cpufreq callbacks to thermal cooling device ops */
> -
> -static struct thermal_cooling_device_ops cpufreq_cooling_ops = {
> - .get_max_state = cpufreq_get_max_state,
> - .get_cur_state = cpufreq_get_cur_state,
> - .set_cur_state = cpufreq_set_cur_state,
> -};
> -
> /**
> * __cpufreq_cooling_register - helper function to create cpufreq cooling device
> * @np: a valid struct device_node to the cooling device device tree node
> @@ -554,7 +548,10 @@ __cpufreq_cooling_register(struct device_node *np,
> /* max_level is an index, not a counter */
> cpufreq_cdev->max_level = i - 1;
>
> - cooling_ops = &cpufreq_cooling_ops;
> + cooling_ops = &cpufreq_cdev->cooling_ops;
> + cooling_ops->get_max_state = cpufreq_get_max_state;
> + cooling_ops->get_cur_state = cpufreq_get_cur_state;
> + cooling_ops->set_cur_state = cpufreq_set_cur_state;
>
> #ifdef CONFIG_THERMAL_GOV_POWER_ALLOCATOR
> if (em_is_sane(cpufreq_cdev, em)) {
Acked-by: Viresh Kumar <viresh.kumar@...aro.org>
--
viresh
Powered by blists - more mailing lists