LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH 1/3] thermal/drivers/cpufreq_cooling: Use device name instead of auto-numbering
@ 2021-03-10 11:45 Daniel Lezcano
  2021-03-10 11:45 ` [PATCH 2/3] thermal/drivers/devfreq_cooling: " Daniel Lezcano
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Daniel Lezcano @ 2021-03-10 11:45 UTC (permalink / raw)
  To: daniel.lezcano, rui.zhang
  Cc: amitk, linux-pm, Amit Daniel Kachhap, Viresh Kumar, Javi Merino,
	open list

Currently the naming of a cooling device is just a cooling technique
followed by a number. When there are multiple cooling devices using
the same technique, it is impossible to clearly identify the related
device as this one is just a number.

For instance:

 thermal-cpufreq-0
 thermal-cpufreq-1
 etc ...

The 'thermal' prefix is redundant with the subsystem namespace. This
patch removes the 'thermal prefix and changes the number by the device
name. So the naming above becomes:

 cpufreq-cpu0
 cpufreq-cpu4
 etc ...

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 drivers/thermal/cpufreq_cooling.c | 28 +++++++---------------------
 1 file changed, 7 insertions(+), 21 deletions(-)

diff --git a/drivers/thermal/cpufreq_cooling.c b/drivers/thermal/cpufreq_cooling.c
index 10af3341e5ea..cf0332bbdcd3 100644
--- a/drivers/thermal/cpufreq_cooling.c
+++ b/drivers/thermal/cpufreq_cooling.c
@@ -13,6 +13,7 @@
 #include <linux/cpu.h>
 #include <linux/cpufreq.h>
 #include <linux/cpu_cooling.h>
+#include <linux/device.h>
 #include <linux/energy_model.h>
 #include <linux/err.h>
 #include <linux/export.h>
@@ -50,8 +51,6 @@ struct time_in_idle {
 
 /**
  * struct cpufreq_cooling_device - data for cooling device with cpufreq
- * @id: unique integer value corresponding to each cpufreq_cooling_device
- *	registered.
  * @last_load: load measured by the latest call to cpufreq_get_requested_power()
  * @cpufreq_state: integer value representing the current state of cpufreq
  *	cooling	devices.
@@ -69,7 +68,6 @@ struct time_in_idle {
  * cpufreq_cooling_device.
  */
 struct cpufreq_cooling_device {
-	int id;
 	u32 last_load;
 	unsigned int cpufreq_state;
 	unsigned int max_level;
@@ -82,7 +80,6 @@ struct cpufreq_cooling_device {
 	struct freq_qos_request qos_req;
 };
 
-static DEFINE_IDA(cpufreq_ida);
 static DEFINE_MUTEX(cooling_list_lock);
 static LIST_HEAD(cpufreq_cdev_list);
 
@@ -528,11 +525,11 @@ __cpufreq_cooling_register(struct device_node *np,
 {
 	struct thermal_cooling_device *cdev;
 	struct cpufreq_cooling_device *cpufreq_cdev;
-	char dev_name[THERMAL_NAME_LENGTH];
 	unsigned int i;
 	struct device *dev;
 	int ret;
 	struct thermal_cooling_device_ops *cooling_ops;
+	char name[THERMAL_NAME_LENGTH];
 
 	dev = get_cpu_device(policy->cpu);
 	if (unlikely(!dev)) {
@@ -567,16 +564,6 @@ __cpufreq_cooling_register(struct device_node *np,
 	/* max_level is an index, not a counter */
 	cpufreq_cdev->max_level = i - 1;
 
-	ret = ida_simple_get(&cpufreq_ida, 0, 0, GFP_KERNEL);
-	if (ret < 0) {
-		cdev = ERR_PTR(ret);
-		goto free_idle_time;
-	}
-	cpufreq_cdev->id = ret;
-
-	snprintf(dev_name, sizeof(dev_name), "thermal-cpufreq-%d",
-		 cpufreq_cdev->id);
-
 	cooling_ops = &cpufreq_cooling_ops;
 
 #ifdef CONFIG_THERMAL_GOV_POWER_ALLOCATOR
@@ -591,7 +578,7 @@ __cpufreq_cooling_register(struct device_node *np,
 		pr_err("%s: unsorted frequency tables are not supported\n",
 		       __func__);
 		cdev = ERR_PTR(-EINVAL);
-		goto remove_ida;
+		goto free_idle_time;
 	}
 
 	ret = freq_qos_add_request(&policy->constraints,
@@ -601,10 +588,12 @@ __cpufreq_cooling_register(struct device_node *np,
 		pr_err("%s: Failed to add freq constraint (%d)\n", __func__,
 		       ret);
 		cdev = ERR_PTR(ret);
-		goto remove_ida;
+		goto free_idle_time;
 	}
 
-	cdev = thermal_of_cooling_device_register(np, dev_name, cpufreq_cdev,
+	snprintf(name, sizeof(name), "cpufreq-%s", dev_name(dev));
+
+	cdev = thermal_of_cooling_device_register(np, name, cpufreq_cdev,
 						  cooling_ops);
 	if (IS_ERR(cdev))
 		goto remove_qos_req;
@@ -617,8 +606,6 @@ __cpufreq_cooling_register(struct device_node *np,
 
 remove_qos_req:
 	freq_qos_remove_request(&cpufreq_cdev->qos_req);
-remove_ida:
-	ida_simple_remove(&cpufreq_ida, cpufreq_cdev->id);
 free_idle_time:
 	free_idle_time(cpufreq_cdev);
 free_cdev:
@@ -712,7 +699,6 @@ void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev)
 
 	thermal_cooling_device_unregister(cdev);
 	freq_qos_remove_request(&cpufreq_cdev->qos_req);
-	ida_simple_remove(&cpufreq_ida, cpufreq_cdev->id);
 	free_idle_time(cpufreq_cdev);
 	kfree(cpufreq_cdev);
 }
-- 
2.17.1


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

* [PATCH 2/3] thermal/drivers/devfreq_cooling: Use device name instead of auto-numbering
  2021-03-10 11:45 [PATCH 1/3] thermal/drivers/cpufreq_cooling: Use device name instead of auto-numbering Daniel Lezcano
@ 2021-03-10 11:45 ` Daniel Lezcano
  2021-03-12 11:15   ` Lukasz Luba
  2021-03-10 11:46 ` [PATCH 3/3] thermal/drivers/cpuidle_cooling: " Daniel Lezcano
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 8+ messages in thread
From: Daniel Lezcano @ 2021-03-10 11:45 UTC (permalink / raw)
  To: daniel.lezcano, rui.zhang; +Cc: amitk, linux-pm, open list

Currently the naming of a cooling device is just a cooling technique
followed by a number. When there are multiple cooling devices using
the same technique, it is impossible to clearly identify the related
device as this one is just a number.

For instance:

 thermal-devfreq-0
 thermal-devfreq-1
 etc ...

The 'thermal' prefix is redundant with the subsystem namespace. This
patch removes the 'thermal prefix and changes the number by the device
name. So the naming above becomes:

 devfreq-5000000.gpu
 devfreq-1d84000.ufshc
 etc ...

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 drivers/thermal/devfreq_cooling.c | 21 ++++-----------------
 1 file changed, 4 insertions(+), 17 deletions(-)

diff --git a/drivers/thermal/devfreq_cooling.c b/drivers/thermal/devfreq_cooling.c
index fed3121ff2a1..62abcffeb422 100644
--- a/drivers/thermal/devfreq_cooling.c
+++ b/drivers/thermal/devfreq_cooling.c
@@ -25,11 +25,8 @@
 #define HZ_PER_KHZ		1000
 #define SCALE_ERROR_MITIGATION	100
 
-static DEFINE_IDA(devfreq_ida);
-
 /**
  * struct devfreq_cooling_device - Devfreq cooling device
- * @id:		unique integer value corresponding to each
  *		devfreq_cooling_device registered.
  * @cdev:	Pointer to associated thermal cooling device.
  * @devfreq:	Pointer to associated devfreq device.
@@ -51,7 +48,6 @@ static DEFINE_IDA(devfreq_ida);
  * @em_pd:		Energy Model for the associated Devfreq device
  */
 struct devfreq_cooling_device {
-	int id;
 	struct thermal_cooling_device *cdev;
 	struct devfreq *devfreq;
 	unsigned long cooling_state;
@@ -363,7 +359,7 @@ of_devfreq_cooling_register_power(struct device_node *np, struct devfreq *df,
 	struct thermal_cooling_device *cdev;
 	struct device *dev = df->dev.parent;
 	struct devfreq_cooling_device *dfc;
-	char dev_name[THERMAL_NAME_LENGTH];
+	char name[THERMAL_NAME_LENGTH];
 	int err, num_opps;
 
 	dfc = kzalloc(sizeof(*dfc), GFP_KERNEL);
@@ -407,30 +403,22 @@ of_devfreq_cooling_register_power(struct device_node *np, struct devfreq *df,
 	if (err < 0)
 		goto free_table;
 
-	err = ida_simple_get(&devfreq_ida, 0, 0, GFP_KERNEL);
-	if (err < 0)
-		goto remove_qos_req;
-
-	dfc->id = err;
-
-	snprintf(dev_name, sizeof(dev_name), "thermal-devfreq-%d", dfc->id);
+	snprintf(name, sizeof(name), "devfreq-%s", dev_name(dev));
 
-	cdev = thermal_of_cooling_device_register(np, dev_name, dfc,
+	cdev = thermal_of_cooling_device_register(np, name, dfc,
 						  &devfreq_cooling_ops);
 	if (IS_ERR(cdev)) {
 		err = PTR_ERR(cdev);
 		dev_err(dev,
 			"Failed to register devfreq cooling device (%d)\n",
 			err);
-		goto release_ida;
+		goto remove_qos_req;
 	}
 
 	dfc->cdev = cdev;
 
 	return cdev;
 
-release_ida:
-	ida_simple_remove(&devfreq_ida, dfc->id);
 remove_qos_req:
 	dev_pm_qos_remove_request(&dfc->req_max_freq);
 free_table:
@@ -527,7 +515,6 @@ void devfreq_cooling_unregister(struct thermal_cooling_device *cdev)
 	dev = dfc->devfreq->dev.parent;
 
 	thermal_cooling_device_unregister(dfc->cdev);
-	ida_simple_remove(&devfreq_ida, dfc->id);
 	dev_pm_qos_remove_request(&dfc->req_max_freq);
 
 	em_dev_unregister_perf_domain(dev);
-- 
2.17.1


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

* [PATCH 3/3] thermal/drivers/cpuidle_cooling: Use device name instead of auto-numbering
  2021-03-10 11:45 [PATCH 1/3] thermal/drivers/cpufreq_cooling: Use device name instead of auto-numbering Daniel Lezcano
  2021-03-10 11:45 ` [PATCH 2/3] thermal/drivers/devfreq_cooling: " Daniel Lezcano
@ 2021-03-10 11:46 ` Daniel Lezcano
  2021-03-12 11:34   ` Lukasz Luba
  2021-03-12  5:10 ` [PATCH 1/3] thermal/drivers/cpufreq_cooling: " Viresh Kumar
  2021-03-12 10:58 ` Lukasz Luba
  3 siblings, 1 reply; 8+ messages in thread
From: Daniel Lezcano @ 2021-03-10 11:46 UTC (permalink / raw)
  To: daniel.lezcano, rui.zhang
  Cc: amitk, linux-pm, Amit Daniel Kachhap, Viresh Kumar, Javi Merino,
	open list

Currently the naming of a cooling device is just a cooling technique
followed by a number. When there are multiple cooling devices using
the same technique, it is impossible to clearly identify the related
device as this one is just a number.

For instance:

 thermal-idle-0
 thermal-idle-1
 thermal-idle-2
 thermal-idle-3
 etc ...

The 'thermal' prefix is redundant with the subsystem namespace. This
patch removes the 'thermal prefix and changes the number by the device
name. So the naming above becomes:

 idle-cpu0
 idle-cpu1
 idle-cpu2
 idle-cpu3
 etc ...

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 drivers/thermal/cpuidle_cooling.c | 26 ++++++++++----------------
 1 file changed, 10 insertions(+), 16 deletions(-)

diff --git a/drivers/thermal/cpuidle_cooling.c b/drivers/thermal/cpuidle_cooling.c
index 7ecab4b16b29..8bc0a9b46358 100644
--- a/drivers/thermal/cpuidle_cooling.c
+++ b/drivers/thermal/cpuidle_cooling.c
@@ -9,6 +9,7 @@
 
 #include <linux/cpu_cooling.h>
 #include <linux/cpuidle.h>
+#include <linux/device.h>
 #include <linux/err.h>
 #include <linux/idle_inject.h>
 #include <linux/idr.h>
@@ -26,8 +27,6 @@ struct cpuidle_cooling_device {
 	unsigned long state;
 };
 
-static DEFINE_IDA(cpuidle_ida);
-
 /**
  * cpuidle_cooling_runtime - Running time computation
  * @idle_duration_us: CPU idle time to inject in microseconds
@@ -174,10 +173,11 @@ static int __cpuidle_cooling_register(struct device_node *np,
 	struct idle_inject_device *ii_dev;
 	struct cpuidle_cooling_device *idle_cdev;
 	struct thermal_cooling_device *cdev;
+	struct device *dev;
 	unsigned int idle_duration_us = TICK_USEC;
 	unsigned int latency_us = UINT_MAX;
-	char dev_name[THERMAL_NAME_LENGTH];
-	int id, ret;
+	char name[THERMAL_NAME_LENGTH];
+	int ret;
 
 	idle_cdev = kzalloc(sizeof(*idle_cdev), GFP_KERNEL);
 	if (!idle_cdev) {
@@ -185,16 +185,10 @@ static int __cpuidle_cooling_register(struct device_node *np,
 		goto out;
 	}
 
-	id = ida_simple_get(&cpuidle_ida, 0, 0, GFP_KERNEL);
-	if (id < 0) {
-		ret = id;
-		goto out_kfree;
-	}
-
 	ii_dev = idle_inject_register(drv->cpumask);
 	if (!ii_dev) {
 		ret = -EINVAL;
-		goto out_id;
+		goto out_kfree;
 	}
 
 	of_property_read_u32(np, "duration-us", &idle_duration_us);
@@ -205,9 +199,11 @@ static int __cpuidle_cooling_register(struct device_node *np,
 
 	idle_cdev->ii_dev = ii_dev;
 
-	snprintf(dev_name, sizeof(dev_name), "thermal-idle-%d", id);
+	dev = get_cpu_device(cpumask_first(drv->cpumask));
+
+	snprintf(name, sizeof(name), "idle-%s", dev_name(dev));
 
-	cdev = thermal_of_cooling_device_register(np, dev_name, idle_cdev,
+	cdev = thermal_of_cooling_device_register(np, name, idle_cdev,
 						  &cpuidle_cooling_ops);
 	if (IS_ERR(cdev)) {
 		ret = PTR_ERR(cdev);
@@ -215,14 +211,12 @@ static int __cpuidle_cooling_register(struct device_node *np,
 	}
 
 	pr_debug("%s: Idle injection set with idle duration=%u, latency=%u\n",
-		 dev_name, idle_duration_us, latency_us);
+		 name, idle_duration_us, latency_us);
 
 	return 0;
 
 out_unregister:
 	idle_inject_unregister(ii_dev);
-out_id:
-	ida_simple_remove(&cpuidle_ida, id);
 out_kfree:
 	kfree(idle_cdev);
 out:
-- 
2.17.1


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

* Re: [PATCH 1/3] thermal/drivers/cpufreq_cooling: Use device name instead of auto-numbering
  2021-03-10 11:45 [PATCH 1/3] thermal/drivers/cpufreq_cooling: Use device name instead of auto-numbering Daniel Lezcano
  2021-03-10 11:45 ` [PATCH 2/3] thermal/drivers/devfreq_cooling: " Daniel Lezcano
  2021-03-10 11:46 ` [PATCH 3/3] thermal/drivers/cpuidle_cooling: " Daniel Lezcano
@ 2021-03-12  5:10 ` Viresh Kumar
  2021-03-12 10:58 ` Lukasz Luba
  3 siblings, 0 replies; 8+ messages in thread
From: Viresh Kumar @ 2021-03-12  5:10 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: rui.zhang, amitk, linux-pm, Amit Daniel Kachhap, Javi Merino, open list

On 10-03-21, 12:45, Daniel Lezcano wrote:
> Currently the naming of a cooling device is just a cooling technique
> followed by a number. When there are multiple cooling devices using
> the same technique, it is impossible to clearly identify the related
> device as this one is just a number.
> 
> For instance:
> 
>  thermal-cpufreq-0
>  thermal-cpufreq-1
>  etc ...
> 
> The 'thermal' prefix is redundant with the subsystem namespace. This
> patch removes the 'thermal prefix and changes the number by the device
> name. So the naming above becomes:
> 
>  cpufreq-cpu0
>  cpufreq-cpu4
>  etc ...
> 
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> ---
>  drivers/thermal/cpufreq_cooling.c | 28 +++++++---------------------
>  1 file changed, 7 insertions(+), 21 deletions(-)

For 1,3/3

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

-- 
viresh

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

* Re: [PATCH 1/3] thermal/drivers/cpufreq_cooling: Use device name instead of auto-numbering
  2021-03-10 11:45 [PATCH 1/3] thermal/drivers/cpufreq_cooling: Use device name instead of auto-numbering Daniel Lezcano
                   ` (2 preceding siblings ...)
  2021-03-12  5:10 ` [PATCH 1/3] thermal/drivers/cpufreq_cooling: " Viresh Kumar
@ 2021-03-12 10:58 ` Lukasz Luba
  3 siblings, 0 replies; 8+ messages in thread
From: Lukasz Luba @ 2021-03-12 10:58 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: rui.zhang, amitk, linux-pm, Amit Daniel Kachhap, Viresh Kumar,
	Javi Merino, open list

Hi Daniel,

Only minor comments.

On 3/10/21 11:45 AM, Daniel Lezcano wrote:
> Currently the naming of a cooling device is just a cooling technique
> followed by a number. When there are multiple cooling devices using
> the same technique, it is impossible to clearly identify the related
> device as this one is just a number.
> 
> For instance:
> 
>   thermal-cpufreq-0
>   thermal-cpufreq-1
>   etc ...
> 
> The 'thermal' prefix is redundant with the subsystem namespace. This
> patch removes the 'thermal prefix and changes the number by the device

missing ', after 'thermal

> name. So the naming above becomes:
> 
>   cpufreq-cpu0
>   cpufreq-cpu4
>   etc ...
> 
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> ---
>   drivers/thermal/cpufreq_cooling.c | 28 +++++++---------------------
>   1 file changed, 7 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/thermal/cpufreq_cooling.c b/drivers/thermal/cpufreq_cooling.c
> index 10af3341e5ea..cf0332bbdcd3 100644
> --- a/drivers/thermal/cpufreq_cooling.c
> +++ b/drivers/thermal/cpufreq_cooling.c
> @@ -13,6 +13,7 @@
>   #include <linux/cpu.h>
>   #include <linux/cpufreq.h>
>   #include <linux/cpu_cooling.h>
> +#include <linux/device.h>
>   #include <linux/energy_model.h>
>   #include <linux/err.h>
>   #include <linux/export.h>

You can now also remove the header:
#include <linux/idr.h>


other than that, LGTM

Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>

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

* Re: [PATCH 2/3] thermal/drivers/devfreq_cooling: Use device name instead of auto-numbering
  2021-03-10 11:45 ` [PATCH 2/3] thermal/drivers/devfreq_cooling: " Daniel Lezcano
@ 2021-03-12 11:15   ` Lukasz Luba
  2021-03-12 15:53     ` Daniel Lezcano
  0 siblings, 1 reply; 8+ messages in thread
From: Lukasz Luba @ 2021-03-12 11:15 UTC (permalink / raw)
  To: Daniel Lezcano; +Cc: rui.zhang, amitk, linux-pm, open list



On 3/10/21 11:45 AM, Daniel Lezcano wrote:
> Currently the naming of a cooling device is just a cooling technique
> followed by a number. When there are multiple cooling devices using
> the same technique, it is impossible to clearly identify the related
> device as this one is just a number.
> 
> For instance:
> 
>   thermal-devfreq-0
>   thermal-devfreq-1
>   etc ...
> 
> The 'thermal' prefix is redundant with the subsystem namespace. This
> patch removes the 'thermal prefix and changes the number by the device

missing ' after 'thermal

> name. So the naming above becomes:
> 
>   devfreq-5000000.gpu
>   devfreq-1d84000.ufshc
>   etc ...
> 
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> ---
>   drivers/thermal/devfreq_cooling.c | 21 ++++-----------------
>   1 file changed, 4 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/thermal/devfreq_cooling.c b/drivers/thermal/devfreq_cooling.c
> index fed3121ff2a1..62abcffeb422 100644
> --- a/drivers/thermal/devfreq_cooling.c
> +++ b/drivers/thermal/devfreq_cooling.c

same here, you can now remove the idr.h header

> @@ -25,11 +25,8 @@
>   #define HZ_PER_KHZ		1000
>   #define SCALE_ERROR_MITIGATION	100
>   
> -static DEFINE_IDA(devfreq_ida);
> -
>   /**
>    * struct devfreq_cooling_device - Devfreq cooling device
> - * @id:		unique integer value corresponding to each
>    *		devfreq_cooling_device registered.
>    * @cdev:	Pointer to associated thermal cooling device.
>    * @devfreq:	Pointer to associated devfreq device.
> @@ -51,7 +48,6 @@ static DEFINE_IDA(devfreq_ida);
>    * @em_pd:		Energy Model for the associated Devfreq device
>    */
>   struct devfreq_cooling_device {
> -	int id;
>   	struct thermal_cooling_device *cdev;
>   	struct devfreq *devfreq;
>   	unsigned long cooling_state;
> @@ -363,7 +359,7 @@ of_devfreq_cooling_register_power(struct device_node *np, struct devfreq *df,
>   	struct thermal_cooling_device *cdev;
>   	struct device *dev = df->dev.parent;
>   	struct devfreq_cooling_device *dfc;
> -	char dev_name[THERMAL_NAME_LENGTH];
> +	char name[THERMAL_NAME_LENGTH];

This is probably too short (20 char) array. For example in my phone's
devfreq dir, there are really long names:

---------------------------------------------------------
redfin:/sys/class/devfreq # for f in `ls ./` ; do echo $f; cat $f/name | 
wc -m ; done 

18321000.qcom,devfreq-l3:qcom,cdsp-cdsp-l3-lat
47
18321000.qcom,devfreq-l3:qcom,cpu0-cpu-l3-lat
46
18321000.qcom,devfreq-l3:qcom,cpu6-cpu-l3-lat
46
18321000.qcom,devfreq-l3:qcom,cpu7-cpu-l3-lat
46
3d00000.qcom,kgsl-3d0
22
soc:qcom,cpu-cpu-llcc-bw
25
soc:qcom,cpu-llcc-ddr-bw
25
soc:qcom,cpu0-cpu-ddr-latfloor
31
soc:qcom,cpu0-cpu-llcc-lat
27
soc:qcom,cpu0-llcc-ddr-lat
27
soc:qcom,cpu6-cpu-ddr-latfloor
31
soc:qcom,cpu6-cpu-llcc-lat
27
soc:qcom,cpu6-llcc-ddr-lat
27
soc:qcom,cpu7-cpu-ddr-latfloor
31
soc:qcom,gpubw
15
soc:qcom,kgsl-busmon
21
soc:qcom,npu-llcc-ddr-bw
25
soc:qcom,npu-npu-llcc-bw
25
soc:qcom,npudsp-npu-ddr-bw
27
soc:qcom,snoc_cnoc_keepalive
29
---------------------------------------------------------

We should allocate tmp buffer for it, to not loose the meaningful part
of that string name or end up with only the same prefix, like for the
first 3 from top:

devfreq-18321000.qco

or for the GPU:
devfreq-3d00000.qcom

This is tricky area and vendors might put any non-meaningful prefix.

The rest of the code looks OK, only this name construction part.

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

* Re: [PATCH 3/3] thermal/drivers/cpuidle_cooling: Use device name instead of auto-numbering
  2021-03-10 11:46 ` [PATCH 3/3] thermal/drivers/cpuidle_cooling: " Daniel Lezcano
@ 2021-03-12 11:34   ` Lukasz Luba
  0 siblings, 0 replies; 8+ messages in thread
From: Lukasz Luba @ 2021-03-12 11:34 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: rui.zhang, amitk, linux-pm, Amit Daniel Kachhap, Viresh Kumar,
	Javi Merino, open list



On 3/10/21 11:46 AM, Daniel Lezcano wrote:
> Currently the naming of a cooling device is just a cooling technique
> followed by a number. When there are multiple cooling devices using
> the same technique, it is impossible to clearly identify the related
> device as this one is just a number.
> 
> For instance:
> 
>   thermal-idle-0
>   thermal-idle-1
>   thermal-idle-2
>   thermal-idle-3
>   etc ...
> 
> The 'thermal' prefix is redundant with the subsystem namespace. This
> patch removes the 'thermal prefix and changes the number by the device

missing ' after 'thermal

> name. So the naming above becomes:
> 
>   idle-cpu0
>   idle-cpu1
>   idle-cpu2
>   idle-cpu3
>   etc ...
> 
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> ---
>   drivers/thermal/cpuidle_cooling.c | 26 ++++++++++----------------
>   1 file changed, 10 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/thermal/cpuidle_cooling.c b/drivers/thermal/cpuidle_cooling.c
> index 7ecab4b16b29..8bc0a9b46358 100644
> --- a/drivers/thermal/cpuidle_cooling.c
> +++ b/drivers/thermal/cpuidle_cooling.c
> @@ -9,6 +9,7 @@
>   
>   #include <linux/cpu_cooling.h>
>   #include <linux/cpuidle.h>
> +#include <linux/device.h>
>   #include <linux/err.h>
>   #include <linux/idle_inject.h>
>   #include <linux/idr.h>

idr.h not needed any more


other than that LGTM

Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>


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

* Re: [PATCH 2/3] thermal/drivers/devfreq_cooling: Use device name instead of auto-numbering
  2021-03-12 11:15   ` Lukasz Luba
@ 2021-03-12 15:53     ` Daniel Lezcano
  0 siblings, 0 replies; 8+ messages in thread
From: Daniel Lezcano @ 2021-03-12 15:53 UTC (permalink / raw)
  To: Lukasz Luba; +Cc: rui.zhang, amitk, linux-pm, open list

On 12/03/2021 12:15, Lukasz Luba wrote:
> 
> 
> On 3/10/21 11:45 AM, Daniel Lezcano wrote:
>> Currently the naming of a cooling device is just a cooling technique
>> followed by a number. When there are multiple cooling devices using
>> the same technique, it is impossible to clearly identify the related
>> device as this one is just a number.
>>
>> For instance:
>>
>>   thermal-devfreq-0
>>   thermal-devfreq-1
>>   etc ...
>>
>> The 'thermal' prefix is redundant with the subsystem namespace. This
>> patch removes the 'thermal prefix and changes the number by the device
> 
> missing ' after 'thermal
> 
>> name. So the naming above becomes:
>>
>>   devfreq-5000000.gpu
>>   devfreq-1d84000.ufshc
>>   etc ...
>>
>> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
>> ---

[ ... ]

> ---------------------------------------------------------
> 
> We should allocate tmp buffer for it, to not loose the meaningful part
> of that string name or end up with only the same prefix, like for the
> first 3 from top:
> 
> devfreq-18321000.qco
> 
> or for the GPU:
> devfreq-3d00000.qcom
> 
> This is tricky area and vendors might put any non-meaningful prefix.
> 
> The rest of the code looks OK, only this name construction part.

That requires a change in the thermal_core code to replace the strlcpy
into the cdev->type by a kstrdup.

Otherwise the name will be truncated in any case by the underlying
thermal_cooling_device_register() function.


-- 
<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] 8+ messages in thread

end of thread, other threads:[~2021-03-12 15:54 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-10 11:45 [PATCH 1/3] thermal/drivers/cpufreq_cooling: Use device name instead of auto-numbering Daniel Lezcano
2021-03-10 11:45 ` [PATCH 2/3] thermal/drivers/devfreq_cooling: " Daniel Lezcano
2021-03-12 11:15   ` Lukasz Luba
2021-03-12 15:53     ` Daniel Lezcano
2021-03-10 11:46 ` [PATCH 3/3] thermal/drivers/cpuidle_cooling: " Daniel Lezcano
2021-03-12 11:34   ` Lukasz Luba
2021-03-12  5:10 ` [PATCH 1/3] thermal/drivers/cpufreq_cooling: " Viresh Kumar
2021-03-12 10:58 ` 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).