linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/4] Thermal cpufreq & devfreq cooling minor clean-ups
@ 2022-06-13 12:43 Lukasz Luba
  2022-06-13 12:43 ` [PATCH v2 1/4] thermal: cpufreq_cooling: Use private callback ops for each cooling device Lukasz Luba
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: Lukasz Luba @ 2022-06-13 12:43 UTC (permalink / raw)
  To: linux-kernel, linux-pm
  Cc: daniel.lezcano, amitk, rui.zhang, lukasz.luba, viresh.kumar,
	rafael, dietmar.eggemann, rostedt, mingo

Hi all,

This is v2 of some minor clean-ups for the thermal cpufreq and devfreq
cooling code.

Changes:
v2:
- extened the cpufreq_cooling_device with private ops field, to not waste
  memory and simplify allocation/free code (Viresh)
- added devfreq_cooling clean-up to align with cpufreq cooling code
- added ACKs from Viresh for patch 2/4 and path 3/4
- added missing maintainers of tracing to CC list

Regards,
Lukasz

Lukasz Luba (4):
  thermal: cpufreq_cooling: Use private callback ops for each cooling
    device
  thermal: cpufreq_cooling : Refactor thermal_power_cpu_get_power
    tracing
  thermal: cpufreq_cooling: Update outdated comments
  thermal: devfreq_cooling: Extend the devfreq_cooling_device with ops

 drivers/thermal/cpufreq_cooling.c | 77 ++++++++++---------------------
 drivers/thermal/devfreq_cooling.c | 27 ++++-------
 include/trace/events/thermal.h    | 28 ++++-------
 3 files changed, 42 insertions(+), 90 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2 1/4] thermal: cpufreq_cooling: Use private callback ops for each cooling device
  2022-06-13 12:43 [PATCH v2 0/4] Thermal cpufreq & devfreq cooling minor clean-ups Lukasz Luba
@ 2022-06-13 12:43 ` Lukasz Luba
  2022-06-14  2:24   ` Viresh Kumar
  2022-06-13 12:43 ` [PATCH v2 2/4] thermal: cpufreq_cooling : Refactor thermal_power_cpu_get_power tracing Lukasz Luba
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 10+ messages in thread
From: Lukasz Luba @ 2022-06-13 12:43 UTC (permalink / raw)
  To: linux-kernel, linux-pm
  Cc: daniel.lezcano, amitk, rui.zhang, lukasz.luba, viresh.kumar,
	rafael, dietmar.eggemann, rostedt, mingo

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@arm.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)) {
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 2/4] thermal: cpufreq_cooling : Refactor thermal_power_cpu_get_power tracing
  2022-06-13 12:43 [PATCH v2 0/4] Thermal cpufreq & devfreq cooling minor clean-ups Lukasz Luba
  2022-06-13 12:43 ` [PATCH v2 1/4] thermal: cpufreq_cooling: Use private callback ops for each cooling device Lukasz Luba
@ 2022-06-13 12:43 ` Lukasz Luba
  2022-06-13 12:43 ` [PATCH v2 3/4] thermal: cpufreq_cooling: Update outdated comments Lukasz Luba
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Lukasz Luba @ 2022-06-13 12:43 UTC (permalink / raw)
  To: linux-kernel, linux-pm
  Cc: daniel.lezcano, amitk, rui.zhang, lukasz.luba, viresh.kumar,
	rafael, dietmar.eggemann, rostedt, mingo

Simplify the thermal_power_cpu_get_power trace event by removing
complicated cpumask and variable length array. Now the tools parsing trace
output don't have to hassle to get this power data. The simplified format
version uses 'policy->cpu'. Remove also the 'load' information completely
since there is very little value of it in this trace event. To get the
CPUs' load (or utilization) there are other dedicated trace hooks in the
kernel. This patch also simplifies and speeds-up the main cooling code
when that trace event is enabled.

Rename the trace event to avoid confusion of tools which parse the trace
file.

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Lukasz Luba <lukasz.luba@arm.com>
---
 drivers/thermal/cpufreq_cooling.c | 18 +-----------------
 include/trace/events/thermal.h    | 28 ++++++++--------------------
 2 files changed, 9 insertions(+), 37 deletions(-)

diff --git a/drivers/thermal/cpufreq_cooling.c b/drivers/thermal/cpufreq_cooling.c
index ad8b86f5281b..492a67e267e8 100644
--- a/drivers/thermal/cpufreq_cooling.c
+++ b/drivers/thermal/cpufreq_cooling.c
@@ -216,16 +216,9 @@ static int cpufreq_get_requested_power(struct thermal_cooling_device *cdev,
 	u32 total_load = 0;
 	struct cpufreq_cooling_device *cpufreq_cdev = cdev->devdata;
 	struct cpufreq_policy *policy = cpufreq_cdev->policy;
-	u32 *load_cpu = NULL;
 
 	freq = cpufreq_quick_get(policy->cpu);
 
-	if (trace_thermal_power_cpu_get_power_enabled()) {
-		u32 ncpus = cpumask_weight(policy->related_cpus);
-
-		load_cpu = kcalloc(ncpus, sizeof(*load_cpu), GFP_KERNEL);
-	}
-
 	for_each_cpu(cpu, policy->related_cpus) {
 		u32 load;
 
@@ -235,22 +228,13 @@ static int cpufreq_get_requested_power(struct thermal_cooling_device *cdev,
 			load = 0;
 
 		total_load += load;
-		if (load_cpu)
-			load_cpu[i] = load;
-
-		i++;
 	}
 
 	cpufreq_cdev->last_load = total_load;
 
 	*power = get_dynamic_power(cpufreq_cdev, freq);
 
-	if (load_cpu) {
-		trace_thermal_power_cpu_get_power(policy->related_cpus, freq,
-						  load_cpu, i, *power);
-
-		kfree(load_cpu);
-	}
+	trace_thermal_power_cpu_get_power_simple(policy->cpu, *power);
 
 	return 0;
 }
diff --git a/include/trace/events/thermal.h b/include/trace/events/thermal.h
index 8a5f04888abd..e58bf3072f32 100644
--- a/include/trace/events/thermal.h
+++ b/include/trace/events/thermal.h
@@ -92,34 +92,22 @@ TRACE_EVENT(thermal_zone_trip,
 );
 
 #ifdef CONFIG_CPU_THERMAL
-TRACE_EVENT(thermal_power_cpu_get_power,
-	TP_PROTO(const struct cpumask *cpus, unsigned long freq, u32 *load,
-		size_t load_len, u32 dynamic_power),
+TRACE_EVENT(thermal_power_cpu_get_power_simple,
+	TP_PROTO(int cpu, u32 power),
 
-	TP_ARGS(cpus, freq, load, load_len, dynamic_power),
+	TP_ARGS(cpu, power),
 
 	TP_STRUCT__entry(
-		__bitmask(cpumask, num_possible_cpus())
-		__field(unsigned long, freq          )
-		__dynamic_array(u32,   load, load_len)
-		__field(size_t,        load_len      )
-		__field(u32,           dynamic_power )
+		__field(int, cpu)
+		__field(u32, power)
 	),
 
 	TP_fast_assign(
-		__assign_bitmask(cpumask, cpumask_bits(cpus),
-				num_possible_cpus());
-		__entry->freq = freq;
-		memcpy(__get_dynamic_array(load), load,
-			load_len * sizeof(*load));
-		__entry->load_len = load_len;
-		__entry->dynamic_power = dynamic_power;
+		__entry->cpu = cpu;
+		__entry->power = power;
 	),
 
-	TP_printk("cpus=%s freq=%lu load={%s} dynamic_power=%d",
-		__get_bitmask(cpumask), __entry->freq,
-		__print_array(__get_dynamic_array(load), __entry->load_len, 4),
-		__entry->dynamic_power)
+	TP_printk("cpu=%d power=%u", __entry->cpu, __entry->power)
 );
 
 TRACE_EVENT(thermal_power_cpu_limit,
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 3/4] thermal: cpufreq_cooling: Update outdated comments
  2022-06-13 12:43 [PATCH v2 0/4] Thermal cpufreq & devfreq cooling minor clean-ups Lukasz Luba
  2022-06-13 12:43 ` [PATCH v2 1/4] thermal: cpufreq_cooling: Use private callback ops for each cooling device Lukasz Luba
  2022-06-13 12:43 ` [PATCH v2 2/4] thermal: cpufreq_cooling : Refactor thermal_power_cpu_get_power tracing Lukasz Luba
@ 2022-06-13 12:43 ` Lukasz Luba
  2022-06-13 12:43 ` [PATCH v2 4/4] thermal: devfreq_cooling: Extend the devfreq_cooling_device with ops Lukasz Luba
  2022-06-14 12:51 ` [PATCH v2 0/4] Thermal cpufreq & devfreq cooling minor clean-ups Lukasz Luba
  4 siblings, 0 replies; 10+ messages in thread
From: Lukasz Luba @ 2022-06-13 12:43 UTC (permalink / raw)
  To: linux-kernel, linux-pm
  Cc: daniel.lezcano, amitk, rui.zhang, lukasz.luba, viresh.kumar,
	rafael, dietmar.eggemann, rostedt, mingo

The code has moved and left some comments stale. Update them where
there is a need.

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Lukasz Luba <lukasz.luba@arm.com>
---
 drivers/thermal/cpufreq_cooling.c | 44 +++++++++++++------------------
 1 file changed, 18 insertions(+), 26 deletions(-)

diff --git a/drivers/thermal/cpufreq_cooling.c b/drivers/thermal/cpufreq_cooling.c
index 492a67e267e8..50f8b90abba6 100644
--- a/drivers/thermal/cpufreq_cooling.c
+++ b/drivers/thermal/cpufreq_cooling.c
@@ -206,7 +206,7 @@ static u32 get_dynamic_power(struct cpufreq_cooling_device *cpufreq_cdev,
  * complex code may be needed if experiments show that it's not
  * accurate enough.
  *
- * Return: 0 on success, -E* if getting the static power failed.
+ * Return: 0 on success, this function doesn't fail.
  */
 static int cpufreq_get_requested_power(struct thermal_cooling_device *cdev,
 				       u32 *power)
@@ -249,9 +249,8 @@ static int cpufreq_get_requested_power(struct thermal_cooling_device *cdev,
  * milliwatts assuming 100% load.  Store the calculated power in
  * @power.
  *
- * Return: 0 on success, -EINVAL if the cooling device state could not
- * be converted into a frequency or other -E* if there was an error
- * when calculating the static power.
+ * Return: 0 on success, -EINVAL if the cooling device state is bigger
+ * than maximum allowed.
  */
 static int cpufreq_state2power(struct thermal_cooling_device *cdev,
 			       unsigned long state, u32 *power)
@@ -281,15 +280,11 @@ static int cpufreq_state2power(struct thermal_cooling_device *cdev,
  * Calculate a cooling device state for the cpus described by @cdev
  * that would allow them to consume at most @power mW and store it in
  * @state.  Note that this calculation depends on external factors
- * such as the cpu load or the current static power.  Calling this
- * function with the same power as input can yield different cooling
- * device states depending on those external factors.
- *
- * Return: 0 on success, -ENODEV if no cpus are online or -EINVAL if
- * the calculated frequency could not be converted to a valid state.
- * The latter should not happen unless the frequencies available to
- * cpufreq have changed since the initialization of the cpu cooling
- * device.
+ * such as the CPUs load.  Calling this function with the same power
+ * as input can yield different cooling device states depending on those
+ * external factors.
+ *
+ * Return: 0 on success, this function doesn't fail.
  */
 static int cpufreq_power2state(struct thermal_cooling_device *cdev,
 			       u32 power, unsigned long *state)
@@ -401,7 +396,7 @@ static unsigned int get_state_freq(struct cpufreq_cooling_device *cpufreq_cdev,
  * Callback for the thermal cooling device to return the cpufreq
  * max cooling state.
  *
- * Return: 0 on success, an error code otherwise.
+ * Return: 0 on success, this function doesn't fail.
  */
 static int cpufreq_get_max_state(struct thermal_cooling_device *cdev,
 				 unsigned long *state)
@@ -420,7 +415,7 @@ static int cpufreq_get_max_state(struct thermal_cooling_device *cdev,
  * Callback for the thermal cooling device to return the cpufreq
  * current cooling state.
  *
- * Return: 0 on success, an error code otherwise.
+ * Return: 0 on success, this function doesn't fail.
  */
 static int cpufreq_get_cur_state(struct thermal_cooling_device *cdev,
 				 unsigned long *state)
@@ -479,7 +474,7 @@ static int cpufreq_set_cur_state(struct thermal_cooling_device *cdev,
  * @em: Energy Model of the cpufreq policy
  *
  * This interface function registers the cpufreq cooling device with the name
- * "thermal-cpufreq-%x". This api can support multiple instances of cpufreq
+ * "cpufreq-%s". This API can support multiple instances of cpufreq
  * cooling devices. It also gives the opportunity to link the cooling device
  * with a device tree node, in order to bind it via the thermal DT code.
  *
@@ -590,8 +585,8 @@ __cpufreq_cooling_register(struct device_node *np,
  * @policy: cpufreq policy
  *
  * This interface function registers the cpufreq cooling device with the name
- * "thermal-cpufreq-%x". This api can support multiple instances of cpufreq
- * cooling devices.
+ * "cpufreq-%s". This API can support multiple instances of cpufreq cooling
+ * devices.
  *
  * Return: a valid struct thermal_cooling_device pointer on success,
  * on failure, it returns a corresponding ERR_PTR().
@@ -608,17 +603,14 @@ EXPORT_SYMBOL_GPL(cpufreq_cooling_register);
  * @policy: cpufreq policy
  *
  * This interface function registers the cpufreq cooling device with the name
- * "thermal-cpufreq-%x". This api can support multiple instances of cpufreq
- * cooling devices. Using this API, the cpufreq cooling device will be
- * linked to the device tree node provided.
+ * "cpufreq-%s". This API can support multiple instances of cpufreq cooling
+ * devices. Using this API, the cpufreq cooling device will be linked to the
+ * device tree node provided.
  *
  * Using this function, the cooling device will implement the power
- * extensions by using a simple cpu power model.  The cpus must have
+ * extensions by using the Energy Model (if present).  The cpus must have
  * registered their OPPs using the OPP library.
  *
- * It also takes into account, if property present in policy CPU node, the
- * static power consumed by the cpu.
- *
  * Return: a valid struct thermal_cooling_device pointer on success,
  * and NULL on failure.
  */
@@ -654,7 +646,7 @@ EXPORT_SYMBOL_GPL(of_cpufreq_cooling_register);
  * cpufreq_cooling_unregister - function to remove cpufreq cooling device.
  * @cdev: thermal cooling device pointer.
  *
- * This interface function unregisters the "thermal-cpufreq-%x" cooling device.
+ * This interface function unregisters the "cpufreq-%x" cooling device.
  */
 void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev)
 {
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 4/4] thermal: devfreq_cooling: Extend the devfreq_cooling_device with ops
  2022-06-13 12:43 [PATCH v2 0/4] Thermal cpufreq & devfreq cooling minor clean-ups Lukasz Luba
                   ` (2 preceding siblings ...)
  2022-06-13 12:43 ` [PATCH v2 3/4] thermal: cpufreq_cooling: Update outdated comments Lukasz Luba
@ 2022-06-13 12:43 ` Lukasz Luba
  2022-06-14 12:51 ` [PATCH v2 0/4] Thermal cpufreq & devfreq cooling minor clean-ups Lukasz Luba
  4 siblings, 0 replies; 10+ messages in thread
From: Lukasz Luba @ 2022-06-13 12:43 UTC (permalink / raw)
  To: linux-kernel, linux-pm
  Cc: daniel.lezcano, amitk, rui.zhang, lukasz.luba, viresh.kumar,
	rafael, dietmar.eggemann, rostedt, mingo

Remove unneeded global variable devfreq_cooling_ops which is used only
as a copy pattern. Instead, extend the struct devfreq_cooling_device with
the needed ops structure. This also simplifies the allocation/free code
during the setup/cleanup.

Signed-off-by: Lukasz Luba <lukasz.luba@arm.com>
---
 drivers/thermal/devfreq_cooling.c | 27 +++++++++------------------
 1 file changed, 9 insertions(+), 18 deletions(-)

diff --git a/drivers/thermal/devfreq_cooling.c b/drivers/thermal/devfreq_cooling.c
index 8c76f9655e57..67b618b1afc8 100644
--- a/drivers/thermal/devfreq_cooling.c
+++ b/drivers/thermal/devfreq_cooling.c
@@ -28,6 +28,7 @@
  * struct devfreq_cooling_device - Devfreq cooling device
  *		devfreq_cooling_device registered.
  * @cdev:	Pointer to associated thermal cooling device.
+ * @cooling_ops: devfreq callbacks to thermal cooling device ops
  * @devfreq:	Pointer to associated devfreq device.
  * @cooling_state:	Current cooling state.
  * @freq_table:	Pointer to a table with the frequencies sorted in descending
@@ -48,6 +49,7 @@
  */
 struct devfreq_cooling_device {
 	struct thermal_cooling_device *cdev;
+	struct thermal_cooling_device_ops cooling_ops;
 	struct devfreq *devfreq;
 	unsigned long cooling_state;
 	u32 *freq_table;
@@ -290,12 +292,6 @@ static int devfreq_cooling_power2state(struct thermal_cooling_device *cdev,
 	return 0;
 }
 
-static struct thermal_cooling_device_ops devfreq_cooling_ops = {
-	.get_max_state = devfreq_cooling_get_max_state,
-	.get_cur_state = devfreq_cooling_get_cur_state,
-	.set_cur_state = devfreq_cooling_set_cur_state,
-};
-
 /**
  * devfreq_cooling_gen_tables() - Generate frequency table.
  * @dfc:	Pointer to devfreq cooling device.
@@ -363,18 +359,18 @@ of_devfreq_cooling_register_power(struct device_node *np, struct devfreq *df,
 	char *name;
 	int err, num_opps;
 
-	ops = kmemdup(&devfreq_cooling_ops, sizeof(*ops), GFP_KERNEL);
-	if (!ops)
-		return ERR_PTR(-ENOMEM);
 
 	dfc = kzalloc(sizeof(*dfc), GFP_KERNEL);
-	if (!dfc) {
-		err = -ENOMEM;
-		goto free_ops;
-	}
+	if (!dfc)
+		return ERR_PTR(-ENOMEM);
 
 	dfc->devfreq = df;
 
+	ops = &dfc->cooling_ops;
+	ops->get_max_state = devfreq_cooling_get_max_state;
+	ops->get_cur_state = devfreq_cooling_get_cur_state;
+	ops->set_cur_state = devfreq_cooling_set_cur_state;
+
 	em = em_pd_get(dev);
 	if (em && !em_is_artificial(em)) {
 		dfc->em_pd = em;
@@ -437,8 +433,6 @@ of_devfreq_cooling_register_power(struct device_node *np, struct devfreq *df,
 	kfree(dfc->freq_table);
 free_dfc:
 	kfree(dfc);
-free_ops:
-	kfree(ops);
 
 	return ERR_PTR(err);
 }
@@ -520,13 +514,11 @@ EXPORT_SYMBOL_GPL(devfreq_cooling_em_register);
 void devfreq_cooling_unregister(struct thermal_cooling_device *cdev)
 {
 	struct devfreq_cooling_device *dfc;
-	const struct thermal_cooling_device_ops *ops;
 	struct device *dev;
 
 	if (IS_ERR_OR_NULL(cdev))
 		return;
 
-	ops = cdev->ops;
 	dfc = cdev->devdata;
 	dev = dfc->devfreq->dev.parent;
 
@@ -537,6 +529,5 @@ void devfreq_cooling_unregister(struct thermal_cooling_device *cdev)
 
 	kfree(dfc->freq_table);
 	kfree(dfc);
-	kfree(ops);
 }
 EXPORT_SYMBOL_GPL(devfreq_cooling_unregister);
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/4] thermal: cpufreq_cooling: Use private callback ops for each cooling device
  2022-06-13 12:43 ` [PATCH v2 1/4] thermal: cpufreq_cooling: Use private callback ops for each cooling device Lukasz Luba
@ 2022-06-14  2:24   ` Viresh Kumar
  2022-06-14  9:50     ` Lukasz Luba
  0 siblings, 1 reply; 10+ messages in thread
From: Viresh Kumar @ 2022-06-14  2:24 UTC (permalink / raw)
  To: Lukasz Luba
  Cc: linux-kernel, linux-pm, daniel.lezcano, amitk, rui.zhang, rafael,
	dietmar.eggemann, rostedt, mingo

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@arm.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@linaro.org>

-- 
viresh

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/4] thermal: cpufreq_cooling: Use private callback ops for each cooling device
  2022-06-14  2:24   ` Viresh Kumar
@ 2022-06-14  9:50     ` Lukasz Luba
  0 siblings, 0 replies; 10+ messages in thread
From: Lukasz Luba @ 2022-06-14  9:50 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: linux-kernel, linux-pm, daniel.lezcano, amitk, rui.zhang, rafael,
	dietmar.eggemann, rostedt, mingo



On 6/14/22 03:24, Viresh Kumar wrote:
> 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@arm.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@linaro.org>
> 


Thank you Viresh for the ACK!

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 0/4] Thermal cpufreq & devfreq cooling minor clean-ups
  2022-06-13 12:43 [PATCH v2 0/4] Thermal cpufreq & devfreq cooling minor clean-ups Lukasz Luba
                   ` (3 preceding siblings ...)
  2022-06-13 12:43 ` [PATCH v2 4/4] thermal: devfreq_cooling: Extend the devfreq_cooling_device with ops Lukasz Luba
@ 2022-06-14 12:51 ` Lukasz Luba
  2022-06-14 18:26   ` Daniel Lezcano
  4 siblings, 1 reply; 10+ messages in thread
From: Lukasz Luba @ 2022-06-14 12:51 UTC (permalink / raw)
  To: daniel.lezcano
  Cc: amitk, rui.zhang, viresh.kumar, rafael, dietmar.eggemann,
	rostedt, mingo, linux-pm, linux-kernel

Hi Daniel,


On 6/13/22 13:43, Lukasz Luba wrote:
> Hi all,
> 
> This is v2 of some minor clean-ups for the thermal cpufreq and devfreq
> cooling code.
> 
> Changes:
> v2:
> - extened the cpufreq_cooling_device with private ops field, to not waste
>    memory and simplify allocation/free code (Viresh)
> - added devfreq_cooling clean-up to align with cpufreq cooling code
> - added ACKs from Viresh for patch 2/4 and path 3/4
> - added missing maintainers of tracing to CC list
> 
> Regards,
> Lukasz
> 
> Lukasz Luba (4):
>    thermal: cpufreq_cooling: Use private callback ops for each cooling
>      device
>    thermal: cpufreq_cooling : Refactor thermal_power_cpu_get_power
>      tracing
>    thermal: cpufreq_cooling: Update outdated comments
>    thermal: devfreq_cooling: Extend the devfreq_cooling_device with ops
> 
>   drivers/thermal/cpufreq_cooling.c | 77 ++++++++++---------------------
>   drivers/thermal/devfreq_cooling.c | 27 ++++-------
>   include/trace/events/thermal.h    | 28 ++++-------
>   3 files changed, 42 insertions(+), 90 deletions(-)
> 

Could you have a look and take the patches into your tree, please?
The 3 of 4 patches got ACKs, the last one is devfreq cooling, which
is pretty minor change.

Regards,
Lukasz

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 0/4] Thermal cpufreq & devfreq cooling minor clean-ups
  2022-06-14 12:51 ` [PATCH v2 0/4] Thermal cpufreq & devfreq cooling minor clean-ups Lukasz Luba
@ 2022-06-14 18:26   ` Daniel Lezcano
  2022-06-15  7:47     ` Lukasz Luba
  0 siblings, 1 reply; 10+ messages in thread
From: Daniel Lezcano @ 2022-06-14 18:26 UTC (permalink / raw)
  To: Lukasz Luba
  Cc: amitk, rui.zhang, viresh.kumar, rafael, dietmar.eggemann,
	rostedt, mingo, linux-pm, linux-kernel

On 14/06/2022 14:51, Lukasz Luba wrote:
> Hi Daniel,
> 
> 
> On 6/13/22 13:43, Lukasz Luba wrote:
>> Hi all,
>>
>> This is v2 of some minor clean-ups for the thermal cpufreq and devfreq
>> cooling code.
>>
>> Changes:
>> v2:
>> - extened the cpufreq_cooling_device with private ops field, to not waste
>>    memory and simplify allocation/free code (Viresh)
>> - added devfreq_cooling clean-up to align with cpufreq cooling code
>> - added ACKs from Viresh for patch 2/4 and path 3/4
>> - added missing maintainers of tracing to CC list
>>
>> Regards,
>> Lukasz
>>
>> Lukasz Luba (4):
>>    thermal: cpufreq_cooling: Use private callback ops for each cooling
>>      device
>>    thermal: cpufreq_cooling : Refactor thermal_power_cpu_get_power
>>      tracing
>>    thermal: cpufreq_cooling: Update outdated comments
>>    thermal: devfreq_cooling: Extend the devfreq_cooling_device with ops
>>
>>   drivers/thermal/cpufreq_cooling.c | 77 ++++++++++---------------------
>>   drivers/thermal/devfreq_cooling.c | 27 ++++-------
>>   include/trace/events/thermal.h    | 28 ++++-------
>>   3 files changed, 42 insertions(+), 90 deletions(-)
>>
> 
> Could you have a look and take the patches into your tree, please?
> The 3 of 4 patches got ACKs, the last one is devfreq cooling, which
> is pretty minor change.

Applied, thanks


-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 0/4] Thermal cpufreq & devfreq cooling minor clean-ups
  2022-06-14 18:26   ` Daniel Lezcano
@ 2022-06-15  7:47     ` Lukasz Luba
  0 siblings, 0 replies; 10+ messages in thread
From: Lukasz Luba @ 2022-06-15  7:47 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: amitk, rui.zhang, viresh.kumar, rafael, dietmar.eggemann,
	rostedt, mingo, linux-pm, linux-kernel



On 6/14/22 19:26, Daniel Lezcano wrote:
> On 14/06/2022 14:51, Lukasz Luba wrote:
>> Hi Daniel,
>>
>>
>> On 6/13/22 13:43, Lukasz Luba wrote:
>>> Hi all,
>>>
>>> This is v2 of some minor clean-ups for the thermal cpufreq and devfreq
>>> cooling code.
>>>
>>> Changes:
>>> v2:
>>> - extened the cpufreq_cooling_device with private ops field, to not 
>>> waste
>>>    memory and simplify allocation/free code (Viresh)
>>> - added devfreq_cooling clean-up to align with cpufreq cooling code
>>> - added ACKs from Viresh for patch 2/4 and path 3/4
>>> - added missing maintainers of tracing to CC list
>>>
>>> Regards,
>>> Lukasz
>>>
>>> Lukasz Luba (4):
>>>    thermal: cpufreq_cooling: Use private callback ops for each cooling
>>>      device
>>>    thermal: cpufreq_cooling : Refactor thermal_power_cpu_get_power
>>>      tracing
>>>    thermal: cpufreq_cooling: Update outdated comments
>>>    thermal: devfreq_cooling: Extend the devfreq_cooling_device with ops
>>>
>>>   drivers/thermal/cpufreq_cooling.c | 77 ++++++++++---------------------
>>>   drivers/thermal/devfreq_cooling.c | 27 ++++-------
>>>   include/trace/events/thermal.h    | 28 ++++-------
>>>   3 files changed, 42 insertions(+), 90 deletions(-)
>>>
>>
>> Could you have a look and take the patches into your tree, please?
>> The 3 of 4 patches got ACKs, the last one is devfreq cooling, which
>> is pretty minor change.
> 
> Applied, thanks
> 
> 

Thanks Daniel!

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2022-06-15  7:48 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-13 12:43 [PATCH v2 0/4] Thermal cpufreq & devfreq cooling minor clean-ups Lukasz Luba
2022-06-13 12:43 ` [PATCH v2 1/4] thermal: cpufreq_cooling: Use private callback ops for each cooling device Lukasz Luba
2022-06-14  2:24   ` Viresh Kumar
2022-06-14  9:50     ` Lukasz Luba
2022-06-13 12:43 ` [PATCH v2 2/4] thermal: cpufreq_cooling : Refactor thermal_power_cpu_get_power tracing Lukasz Luba
2022-06-13 12:43 ` [PATCH v2 3/4] thermal: cpufreq_cooling: Update outdated comments Lukasz Luba
2022-06-13 12:43 ` [PATCH v2 4/4] thermal: devfreq_cooling: Extend the devfreq_cooling_device with ops Lukasz Luba
2022-06-14 12:51 ` [PATCH v2 0/4] Thermal cpufreq & devfreq cooling minor clean-ups Lukasz Luba
2022-06-14 18:26   ` Daniel Lezcano
2022-06-15  7:47     ` Lukasz Luba

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).