From 0f1be51c358f740fe5183bd0bcd60076fdfb53d0 Mon Sep 17 00:00:00 2001 From: Eduardo Valentin Date: Thu, 4 Dec 2014 09:41:43 +0530 Subject: thermal: cpu_cooling: check for the readiness of cpufreq layer In this patch, the cpu_cooling code checks for the usability of cpufreq layer before proceeding with the CPU cooling device registration. The main reason is: CPU cooling device is not usable if cpufreq cannot switch frequencies. Similar checks are spread in thermal drivers. Thus, the advantage now is to have the check in a single place: cpu cooling device registration. For this reason, this patch also updates the existing drivers that depend on CPU cooling to simply propagate the error code of the cpu cooling registration call. Therefore, in case cpufreq is not ready, the thermal drivers will still return -EPROBE_DEFER, in an attempt to try again when cpufreq layer gets ready. Cc: devicetree@vger.kernel.org Cc: Grant Likely Cc: Kukjin Kim Cc: linux-arm-kernel@lists.infradead.org Cc: linux-kernel@vger.kernel.org Cc: linux-pm@vger.kernel.org Cc: linux-samsung-soc@vger.kernel.org Cc: Naveen Krishna Chatradhi Cc: Rob Herring Cc: Zhang Rui Acked-by: Viresh Kumar Signed-off-by: Viresh Kumar Signed-off-by: Eduardo Valentin --- drivers/thermal/cpu_cooling.c | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'drivers/thermal/cpu_cooling.c') diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c index ad09e51ffae4..f98a763af2f5 100644 --- a/drivers/thermal/cpu_cooling.c +++ b/drivers/thermal/cpu_cooling.c @@ -443,6 +443,11 @@ __cpufreq_cooling_register(struct device_node *np, int ret = 0, i; struct cpufreq_policy policy; + if (!cpufreq_frequency_get_table(cpumask_first(clip_cpus))) { + pr_debug("%s: CPUFreq table not found\n", __func__); + return ERR_PTR(-EPROBE_DEFER); + } + /* Verify that all the clip cpus have same freq_min, freq_max limit */ for_each_cpu(i, clip_cpus) { /* continue if cpufreq policy not found and not return error */ -- cgit v1.2.3 From 728c03c9592198717fed3e9fbae7260cff300175 Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Thu, 4 Dec 2014 09:41:47 +0530 Subject: thermal: cpu_cooling: random comment fixups s/give/given Acked-by: Eduardo Valentin Signed-off-by: Viresh Kumar Signed-off-by: Eduardo Valentin --- drivers/thermal/cpu_cooling.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'drivers/thermal/cpu_cooling.c') diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c index f98a763af2f5..6f2d41e7a9e7 100644 --- a/drivers/thermal/cpu_cooling.c +++ b/drivers/thermal/cpu_cooling.c @@ -121,7 +121,7 @@ enum cpufreq_cooling_property { }; /** - * get_property - fetch a property of interest for a give cpu. + * get_property - fetch a property of interest for a given cpu. * @cpu: cpu for which the property is required * @input: query parameter * @output: query return @@ -131,6 +131,7 @@ enum cpufreq_cooling_property { * 1. get maximum cpu cooling states * 2. translate frequency to cooling state * 3. translate cooling state to frequency + * * Note that the code may be not in good shape * but it is written in this way in order to: * a) reduce duplicate code as most of the code can be shared. @@ -211,7 +212,7 @@ static int get_property(unsigned int cpu, unsigned long input, } /** - * cpufreq_cooling_get_level - for a give cpu, return the cooling level. + * cpufreq_cooling_get_level - for a given cpu, return the cooling level. * @cpu: cpu for which the level is required * @freq: the frequency of interest * -- cgit v1.2.3 From beca6053fc21bbe0ed0242a3f79c0cca5749a90f Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Thu, 4 Dec 2014 09:41:48 +0530 Subject: thermal: cpu_cooling: fix doc comment over struct cpufreq_cooling_device cooling_cpufreq_lock isn't used to protect this structure and so the comment over it is outdated. Fix it. Signed-off-by: Viresh Kumar Signed-off-by: Eduardo Valentin --- drivers/thermal/cpu_cooling.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'drivers/thermal/cpu_cooling.c') diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c index 6f2d41e7a9e7..cc10641be111 100644 --- a/drivers/thermal/cpu_cooling.c +++ b/drivers/thermal/cpu_cooling.c @@ -40,9 +40,8 @@ * frequency. * @allowed_cpus: all the cpus involved for this cpufreq_cooling_device. * - * This structure is required for keeping information of each - * cpufreq_cooling_device registered. In order to prevent corruption of this a - * mutex lock cooling_cpufreq_lock is used. + * This structure is required for keeping information of each registered + * cpufreq_cooling_device. */ struct cpufreq_cooling_device { int id; -- cgit v1.2.3 From 07d888d831b038c01c5415f8945f41c743f49fb2 Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Thu, 4 Dec 2014 09:41:49 +0530 Subject: thermal: cpu_cooling: Add comment to clarify relation between cooling state and frequency This wasn't explained well anywhere and should be clearly specified. Lets add a top level comment for this. Signed-off-by: Viresh Kumar Signed-off-by: Eduardo Valentin --- drivers/thermal/cpu_cooling.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) (limited to 'drivers/thermal/cpu_cooling.c') diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c index cc10641be111..a5a931726aed 100644 --- a/drivers/thermal/cpu_cooling.c +++ b/drivers/thermal/cpu_cooling.c @@ -28,6 +28,20 @@ #include #include +/* + * Cooling state <-> CPUFreq frequency + * + * Cooling states are translated to frequencies throughout this driver and this + * is the relation between them. + * + * Highest cooling state corresponds to lowest possible frequency. + * + * i.e. + * level 0 --> 1st Max Freq + * level 1 --> 2nd Max Freq + * ... + */ + /** * struct cpufreq_cooling_device - data for cooling device with cpufreq * @id: unique integer value corresponding to each cpufreq_cooling_device -- cgit v1.2.3 From 98d522f056568007557867d53833770dee9d8fe8 Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Thu, 4 Dec 2014 09:41:50 +0530 Subject: thermal: cpu_cooling: Pass variable instead of its type to sizeof() Just following coding guidelines here. Signed-off-by: Viresh Kumar Signed-off-by: Eduardo Valentin --- drivers/thermal/cpu_cooling.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'drivers/thermal/cpu_cooling.c') diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c index a5a931726aed..537856127f78 100644 --- a/drivers/thermal/cpu_cooling.c +++ b/drivers/thermal/cpu_cooling.c @@ -476,8 +476,7 @@ __cpufreq_cooling_register(struct device_node *np, return ERR_PTR(-EINVAL); } } - cpufreq_dev = kzalloc(sizeof(struct cpufreq_cooling_device), - GFP_KERNEL); + cpufreq_dev = kzalloc(sizeof(*cpufreq_dev), GFP_KERNEL); if (!cpufreq_dev) return ERR_PTR(-ENOMEM); -- cgit v1.2.3 From 92e615ec82c0314fb480eeb19396f4ac15bf97ef Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Thu, 4 Dec 2014 09:41:51 +0530 Subject: thermal: cpu_cooling: no need to set cpufreq_state to zero Its already zero, we allocated cpufreq_dev with kzalloc. Signed-off-by: Viresh Kumar Signed-off-by: Eduardo Valentin --- drivers/thermal/cpu_cooling.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/thermal/cpu_cooling.c') diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c index 537856127f78..30e2ecbb75a7 100644 --- a/drivers/thermal/cpu_cooling.c +++ b/drivers/thermal/cpu_cooling.c @@ -499,7 +499,7 @@ __cpufreq_cooling_register(struct device_node *np, return cool_dev; } cpufreq_dev->cool_dev = cool_dev; - cpufreq_dev->cpufreq_state = 0; + mutex_lock(&cooling_cpufreq_lock); /* Register the notifier for first cpufreq cooling device */ -- cgit v1.2.3 From 5d3bdb8998e066fe270f2f71db7163d5ac40d989 Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Thu, 4 Dec 2014 09:41:52 +0530 Subject: thermal: cpu_cooling: no need to set cpufreq_dev to NULL It will be overwritten soon with return value of kzalloc(). Signed-off-by: Viresh Kumar Signed-off-by: Eduardo Valentin --- drivers/thermal/cpu_cooling.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/thermal/cpu_cooling.c') diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c index 30e2ecbb75a7..c144493e940b 100644 --- a/drivers/thermal/cpu_cooling.c +++ b/drivers/thermal/cpu_cooling.c @@ -451,7 +451,7 @@ __cpufreq_cooling_register(struct device_node *np, const struct cpumask *clip_cpus) { struct thermal_cooling_device *cool_dev; - struct cpufreq_cooling_device *cpufreq_dev = NULL; + struct cpufreq_cooling_device *cpufreq_dev; unsigned int min = 0, max = 0; char dev_name[THERMAL_NAME_LENGTH]; int ret = 0, i; -- cgit v1.2.3 From 8e54d442fe3cdd1ffe5f563ee843b4d48e14ef6e Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Thu, 4 Dec 2014 09:41:53 +0530 Subject: thermal: cpu_cooling: no need to initialze 'ret' ret is initialized before it is used, so no need to set it to 0 in its declaration. Signed-off-by: Viresh Kumar Signed-off-by: Eduardo Valentin --- drivers/thermal/cpu_cooling.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/thermal/cpu_cooling.c') diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c index c144493e940b..d57b8bb7d423 100644 --- a/drivers/thermal/cpu_cooling.c +++ b/drivers/thermal/cpu_cooling.c @@ -454,7 +454,7 @@ __cpufreq_cooling_register(struct device_node *np, struct cpufreq_cooling_device *cpufreq_dev; unsigned int min = 0, max = 0; char dev_name[THERMAL_NAME_LENGTH]; - int ret = 0, i; + int ret, i; struct cpufreq_policy policy; if (!cpufreq_frequency_get_table(cpumask_first(clip_cpus))) { -- cgit v1.2.3 From 268ac445ee1b2b7c2806e7a21076e6d94aca1ca3 Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Thu, 4 Dec 2014 09:41:54 +0530 Subject: thermal: cpu_cooling: propagate error returned by idr_alloc() We aren't supposed to return our own error type here. Return what we got. Signed-off-by: Viresh Kumar Signed-off-by: Eduardo Valentin --- drivers/thermal/cpu_cooling.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/thermal/cpu_cooling.c') diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c index d57b8bb7d423..5c9a2efeec22 100644 --- a/drivers/thermal/cpu_cooling.c +++ b/drivers/thermal/cpu_cooling.c @@ -485,7 +485,7 @@ __cpufreq_cooling_register(struct device_node *np, ret = get_idr(&cpufreq_idr, &cpufreq_dev->id); if (ret) { kfree(cpufreq_dev); - return ERR_PTR(-EINVAL); + return ERR_PTR(ret); } snprintf(dev_name, sizeof(dev_name), "thermal-cpufreq-%d", -- cgit v1.2.3 From 405fb8256226ad68cf6ba5172d289a70cb447c81 Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Thu, 4 Dec 2014 09:41:55 +0530 Subject: thermal: cpu_cooling: Don't match min/max frequencies for all CPUs on cooling register In __cpufreq_cooling_register() we try to match min/max frequencies for all CPUs passed in 'clip_cpus' mask. This mask is the cpumask of cpus where the frequency constraints will be applied. Same frequency constraint can be applied only to the CPUs belonging to the same cluster (i.e. CPUs sharing clock line). For all such CPUs we have a single 'struct cpufreq_policy' structure managing them and so getting policies for all CPUs wouldn't make any sense as they will all return the same pointer. So, remove this useless check of checking min/max for all CPUs. Also update doc comment to make this more obvious that clip_cpus should be same as policy->related_cpus. Signed-off-by: Viresh Kumar Signed-off-by: Eduardo Valentin --- drivers/thermal/cpu_cooling.c | 19 ++----------------- 1 file changed, 2 insertions(+), 17 deletions(-) (limited to 'drivers/thermal/cpu_cooling.c') diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c index 5c9a2efeec22..f32573818db9 100644 --- a/drivers/thermal/cpu_cooling.c +++ b/drivers/thermal/cpu_cooling.c @@ -437,6 +437,7 @@ static struct notifier_block thermal_cpufreq_notifier_block = { * __cpufreq_cooling_register - helper function to create cpufreq cooling device * @np: a valid struct device_node to the cooling device device tree node * @clip_cpus: cpumask of cpus where the frequency constraints will happen. + * Normally this should be same as cpufreq policy->related_cpus. * * This interface function registers the cpufreq cooling device with the name * "thermal-cpufreq-%x". This api can support multiple instances of cpufreq @@ -452,30 +453,14 @@ __cpufreq_cooling_register(struct device_node *np, { struct thermal_cooling_device *cool_dev; struct cpufreq_cooling_device *cpufreq_dev; - unsigned int min = 0, max = 0; char dev_name[THERMAL_NAME_LENGTH]; - int ret, i; - struct cpufreq_policy policy; + int ret; if (!cpufreq_frequency_get_table(cpumask_first(clip_cpus))) { pr_debug("%s: CPUFreq table not found\n", __func__); return ERR_PTR(-EPROBE_DEFER); } - /* Verify that all the clip cpus have same freq_min, freq_max limit */ - for_each_cpu(i, clip_cpus) { - /* continue if cpufreq policy not found and not return error */ - if (!cpufreq_get_policy(&policy, i)) - continue; - if (min == 0 && max == 0) { - min = policy.cpuinfo.min_freq; - max = policy.cpuinfo.max_freq; - } else { - if (min != policy.cpuinfo.min_freq || - max != policy.cpuinfo.max_freq) - return ERR_PTR(-EINVAL); - } - } cpufreq_dev = kzalloc(sizeof(*cpufreq_dev), GFP_KERNEL); if (!cpufreq_dev) return ERR_PTR(-ENOMEM); -- cgit v1.2.3 From e1fae554fb69b8869acbea9397d15758a93d1204 Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Thu, 4 Dec 2014 09:41:56 +0530 Subject: thermal: cpu_cooling: don't iterate over all allowed_cpus to update cpufreq policy All CPUs present in 'allowed_cpus' share the same 'struct cpufreq_policy' structure and so calling cpufreq_update_policy() for each of them doesn't make sense. Signed-off-by: Viresh Kumar Signed-off-by: Eduardo Valentin --- drivers/thermal/cpu_cooling.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) (limited to 'drivers/thermal/cpu_cooling.c') diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c index f32573818db9..7f27f1b44776 100644 --- a/drivers/thermal/cpu_cooling.c +++ b/drivers/thermal/cpu_cooling.c @@ -285,11 +285,10 @@ static unsigned int get_cpu_frequency(unsigned int cpu, unsigned long level) static int cpufreq_apply_cooling(struct cpufreq_cooling_device *cpufreq_device, unsigned long cooling_state) { - unsigned int cpuid, clip_freq; + unsigned int clip_freq; struct cpumask *mask = &cpufreq_device->allowed_cpus; unsigned int cpu = cpumask_any(mask); - /* Check if the old cooling action is same as new cooling action */ if (cpufreq_device->cpufreq_state == cooling_state) return 0; @@ -301,10 +300,8 @@ static int cpufreq_apply_cooling(struct cpufreq_cooling_device *cpufreq_device, cpufreq_device->cpufreq_state = cooling_state; cpufreq_device->cpufreq_val = clip_freq; - for_each_cpu(cpuid, mask) { - if (is_cpufreq_valid(cpuid)) - cpufreq_update_policy(cpuid); - } + if (is_cpufreq_valid(cpu)) + cpufreq_update_policy(cpu); return 0; } -- cgit v1.2.3 From c9ca319f0579cd51b07a666683157233c2cf720d Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Thu, 4 Dec 2014 09:41:57 +0530 Subject: thermal: cpu_cooling: Don't check is_cpufreq_valid() Because get_cpu_frequency() has returned a valid frequency, it means that the cpufreq policy is surely valid and so no point checking that again with is_cpufreq_valid(). Get rid of the routine as well as there are no more users. Signed-off-by: Viresh Kumar Signed-off-by: Eduardo Valentin --- drivers/thermal/cpu_cooling.c | 20 +------------------- 1 file changed, 1 insertion(+), 19 deletions(-) (limited to 'drivers/thermal/cpu_cooling.c') diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c index 7f27f1b44776..1dd4cc403a2a 100644 --- a/drivers/thermal/cpu_cooling.c +++ b/drivers/thermal/cpu_cooling.c @@ -110,23 +110,6 @@ static void release_idr(struct idr *idr, int id) /* Below code defines functions to be used for cpufreq as cooling device */ -/** - * is_cpufreq_valid - function to check frequency transitioning capability. - * @cpu: cpu for which check is needed. - * - * This function will check the current state of the system if - * it is capable of changing the frequency for a given @cpu. - * - * Return: 0 if the system is not currently capable of changing - * the frequency of given cpu. !0 in case the frequency is changeable. - */ -static int is_cpufreq_valid(int cpu) -{ - struct cpufreq_policy policy; - - return !cpufreq_get_policy(&policy, cpu); -} - enum cpufreq_cooling_property { GET_LEVEL, GET_FREQ, @@ -300,8 +283,7 @@ static int cpufreq_apply_cooling(struct cpufreq_cooling_device *cpufreq_device, cpufreq_device->cpufreq_state = cooling_state; cpufreq_device->cpufreq_val = clip_freq; - if (is_cpufreq_valid(cpu)) - cpufreq_update_policy(cpu); + cpufreq_update_policy(cpu); return 0; } -- cgit v1.2.3 From 730abe064b6f8860302b75a689ceed059c08e0b1 Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Thu, 4 Dec 2014 09:41:58 +0530 Subject: thermal: cpu_cooling: do error handling at the bottom in __cpufreq_cooling_register() This makes life easy and bug free. And is scalable for future resource allocations. Acked-by: Javi Merino Signed-off-by: Viresh Kumar Signed-off-by: Eduardo Valentin --- drivers/thermal/cpu_cooling.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) (limited to 'drivers/thermal/cpu_cooling.c') diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c index 1dd4cc403a2a..491d90aeeebe 100644 --- a/drivers/thermal/cpu_cooling.c +++ b/drivers/thermal/cpu_cooling.c @@ -448,8 +448,8 @@ __cpufreq_cooling_register(struct device_node *np, ret = get_idr(&cpufreq_idr, &cpufreq_dev->id); if (ret) { - kfree(cpufreq_dev); - return ERR_PTR(ret); + cool_dev = ERR_PTR(ret); + goto free_cdev; } snprintf(dev_name, sizeof(dev_name), "thermal-cpufreq-%d", @@ -457,11 +457,9 @@ __cpufreq_cooling_register(struct device_node *np, cool_dev = thermal_of_cooling_device_register(np, dev_name, cpufreq_dev, &cpufreq_cooling_ops); - if (IS_ERR(cool_dev)) { - release_idr(&cpufreq_idr, cpufreq_dev->id); - kfree(cpufreq_dev); - return cool_dev; - } + if (IS_ERR(cool_dev)) + goto remove_idr; + cpufreq_dev->cool_dev = cool_dev; mutex_lock(&cooling_cpufreq_lock); @@ -475,6 +473,13 @@ __cpufreq_cooling_register(struct device_node *np, mutex_unlock(&cooling_cpufreq_lock); + return cool_dev; + +remove_idr: + release_idr(&cpufreq_idr, cpufreq_dev->id); +free_cdev: + kfree(cpufreq_dev); + return cool_dev; } -- cgit v1.2.3 From 7adb635b3cd790e4e0d7e9d0b3dd30574ae36596 Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Thu, 4 Dec 2014 09:41:59 +0530 Subject: thermal: cpu_cooling: initialize 'cpufreq_val' on registration There is no point checking for validity of 'cpufreq_val' from cpufreq_thermal_notifier() every time the routine is called. Its guaranteed to be 0 on the first call but will be valid otherwise. Lets update it once while the device registers. Signed-off-by: Viresh Kumar Signed-off-by: Eduardo Valentin --- drivers/thermal/cpu_cooling.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) (limited to 'drivers/thermal/cpu_cooling.c') diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c index 491d90aeeebe..86bcf8dc14d3 100644 --- a/drivers/thermal/cpu_cooling.c +++ b/drivers/thermal/cpu_cooling.c @@ -316,11 +316,6 @@ static int cpufreq_thermal_notifier(struct notifier_block *nb, &cpufreq_dev->allowed_cpus)) continue; - if (!cpufreq_dev->cpufreq_val) - cpufreq_dev->cpufreq_val = get_cpu_frequency( - cpumask_any(&cpufreq_dev->allowed_cpus), - cpufreq_dev->cpufreq_state); - max_freq = cpufreq_dev->cpufreq_val; if (policy->max != max_freq) @@ -444,6 +439,13 @@ __cpufreq_cooling_register(struct device_node *np, if (!cpufreq_dev) return ERR_PTR(-ENOMEM); + cpufreq_dev->cpufreq_val = get_cpu_frequency(cpumask_any(clip_cpus), 0); + if (!cpufreq_dev->cpufreq_val) { + pr_err("%s: Failed to get frequency", __func__); + cool_dev = ERR_PTR(-EINVAL); + goto free_cdev; + } + cpumask_copy(&cpufreq_dev->allowed_cpus, clip_cpus); ret = get_idr(&cpufreq_idr, &cpufreq_dev->id); -- cgit v1.2.3 From 5194fe469927e50367b35e556812c7fc6ce130d1 Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Thu, 4 Dec 2014 09:42:00 +0530 Subject: thermal: cpu_cooling: Merge cpufreq_apply_cooling() into cpufreq_set_cur_state() cpufreq_apply_cooling() has a single caller, cpufreq_set_cur_state() and cpufreq_set_cur_state() is an unnecessary wrapper over cpufreq_apply_cooling(). Get rid of it by merging both routines. Signed-off-by: Viresh Kumar Signed-off-by: Eduardo Valentin --- drivers/thermal/cpu_cooling.c | 52 +++++++++++++------------------------------ 1 file changed, 16 insertions(+), 36 deletions(-) (limited to 'drivers/thermal/cpu_cooling.c') diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c index 86bcf8dc14d3..a3dd74f57540 100644 --- a/drivers/thermal/cpu_cooling.c +++ b/drivers/thermal/cpu_cooling.c @@ -253,41 +253,6 @@ static unsigned int get_cpu_frequency(unsigned int cpu, unsigned long level) return freq; } -/** - * cpufreq_apply_cooling - function to apply frequency clipping. - * @cpufreq_device: cpufreq_cooling_device pointer containing frequency - * clipping data. - * @cooling_state: value of the cooling state. - * - * Function used to make sure the cpufreq layer is aware of current thermal - * limits. The limits are applied by updating the cpufreq policy. - * - * Return: 0 on success, an error code otherwise (-EINVAL in case wrong - * cooling state). - */ -static int cpufreq_apply_cooling(struct cpufreq_cooling_device *cpufreq_device, - unsigned long cooling_state) -{ - unsigned int clip_freq; - struct cpumask *mask = &cpufreq_device->allowed_cpus; - unsigned int cpu = cpumask_any(mask); - - /* Check if the old cooling action is same as new cooling action */ - if (cpufreq_device->cpufreq_state == cooling_state) - return 0; - - clip_freq = get_cpu_frequency(cpu, cooling_state); - if (!clip_freq) - return -EINVAL; - - cpufreq_device->cpufreq_state = cooling_state; - cpufreq_device->cpufreq_val = clip_freq; - - cpufreq_update_policy(cpu); - - return 0; -} - /** * cpufreq_thermal_notifier - notifier callback for cpufreq policy change. * @nb: struct notifier_block * with callback info. @@ -391,8 +356,23 @@ static int cpufreq_set_cur_state(struct thermal_cooling_device *cdev, unsigned long state) { struct cpufreq_cooling_device *cpufreq_device = cdev->devdata; + unsigned int cpu = cpumask_any(&cpufreq_device->allowed_cpus); + unsigned int clip_freq; + + /* Check if the old cooling action is same as new cooling action */ + if (cpufreq_device->cpufreq_state == state) + return 0; - return cpufreq_apply_cooling(cpufreq_device, state); + clip_freq = get_cpu_frequency(cpu, state); + if (!clip_freq) + return -EINVAL; + + cpufreq_device->cpufreq_state = state; + cpufreq_device->cpufreq_val = clip_freq; + + cpufreq_update_policy(cpu); + + return 0; } /* Bind cpufreq callbacks to thermal cooling device ops */ -- cgit v1.2.3 From 521a2e5831704efef8aa826d6b22abef55650d59 Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Thu, 4 Dec 2014 09:42:01 +0530 Subject: thermal: cpu_cooling: remove unnecessary wrapper get_cpu_frequency() get_cpu_frequency() isn't doing much by itself, just calling get_property(). And so this wrapper isn't required at all. Get rid of it. Signed-off-by: Viresh Kumar Signed-off-by: Eduardo Valentin --- drivers/thermal/cpu_cooling.c | 40 +++++++++------------------------------- 1 file changed, 9 insertions(+), 31 deletions(-) (limited to 'drivers/thermal/cpu_cooling.c') diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c index a3dd74f57540..2c4c4853cd9f 100644 --- a/drivers/thermal/cpu_cooling.c +++ b/drivers/thermal/cpu_cooling.c @@ -229,30 +229,6 @@ unsigned long cpufreq_cooling_get_level(unsigned int cpu, unsigned int freq) } EXPORT_SYMBOL_GPL(cpufreq_cooling_get_level); -/** - * get_cpu_frequency - get the absolute value of frequency from level. - * @cpu: cpu for which frequency is fetched. - * @level: cooling level - * - * This function matches cooling level with frequency. Based on a cooling level - * of frequency, equals cooling state of cpu cooling device, it will return - * the corresponding frequency. - * e.g level=0 --> 1st MAX FREQ, level=1 ---> 2nd MAX FREQ, .... etc - * - * Return: 0 on error, the corresponding frequency otherwise. - */ -static unsigned int get_cpu_frequency(unsigned int cpu, unsigned long level) -{ - int ret = 0; - unsigned int freq; - - ret = get_property(cpu, level, &freq, GET_FREQ); - if (ret) - return 0; - - return freq; -} - /** * cpufreq_thermal_notifier - notifier callback for cpufreq policy change. * @nb: struct notifier_block * with callback info. @@ -358,14 +334,15 @@ static int cpufreq_set_cur_state(struct thermal_cooling_device *cdev, struct cpufreq_cooling_device *cpufreq_device = cdev->devdata; unsigned int cpu = cpumask_any(&cpufreq_device->allowed_cpus); unsigned int clip_freq; + int ret; /* Check if the old cooling action is same as new cooling action */ if (cpufreq_device->cpufreq_state == state) return 0; - clip_freq = get_cpu_frequency(cpu, state); - if (!clip_freq) - return -EINVAL; + ret = get_property(cpu, state, &clip_freq, GET_FREQ); + if (ret) + return ret; cpufreq_device->cpufreq_state = state; cpufreq_device->cpufreq_val = clip_freq; @@ -419,10 +396,11 @@ __cpufreq_cooling_register(struct device_node *np, if (!cpufreq_dev) return ERR_PTR(-ENOMEM); - cpufreq_dev->cpufreq_val = get_cpu_frequency(cpumask_any(clip_cpus), 0); - if (!cpufreq_dev->cpufreq_val) { - pr_err("%s: Failed to get frequency", __func__); - cool_dev = ERR_PTR(-EINVAL); + ret = get_property(cpumask_any(clip_cpus), 0, &cpufreq_dev->cpufreq_val, + GET_FREQ); + if (ret) { + pr_err("%s: Failed to get frequency: %d", __func__, ret); + cool_dev = ERR_PTR(ret); goto free_cdev; } -- cgit v1.2.3 From dcc6c7fdef9e705b1300be22213fb23e3fd1994d Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Thu, 4 Dec 2014 09:42:02 +0530 Subject: thermal: cpu_cooling: find max level during device registration CPU frequency tables don't update after the driver is registered and so we don't need to iterate over them to find total number of states every time cpufreq_get_max_state() is called. Do it once at boot time. Signed-off-by: Viresh Kumar Signed-off-by: Eduardo Valentin --- drivers/thermal/cpu_cooling.c | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) (limited to 'drivers/thermal/cpu_cooling.c') diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c index 2c4c4853cd9f..d34cc5b27021 100644 --- a/drivers/thermal/cpu_cooling.c +++ b/drivers/thermal/cpu_cooling.c @@ -52,6 +52,8 @@ * cooling devices. * @cpufreq_val: integer value representing the absolute value of the clipped * frequency. + * @max_level: maximum cooling level. One less than total number of valid + * cpufreq frequencies. * @allowed_cpus: all the cpus involved for this cpufreq_cooling_device. * * This structure is required for keeping information of each registered @@ -62,6 +64,7 @@ struct cpufreq_cooling_device { struct thermal_cooling_device *cool_dev; unsigned int cpufreq_state; unsigned int cpufreq_val; + unsigned int max_level; struct cpumask allowed_cpus; struct list_head node; }; @@ -283,19 +286,9 @@ static int cpufreq_get_max_state(struct thermal_cooling_device *cdev, unsigned long *state) { struct cpufreq_cooling_device *cpufreq_device = cdev->devdata; - struct cpumask *mask = &cpufreq_device->allowed_cpus; - unsigned int cpu; - unsigned int count = 0; - int ret; - - cpu = cpumask_any(mask); - - ret = get_property(cpu, 0, &count, GET_MAXL); - - if (count > 0) - *state = count; - return ret; + *state = cpufreq_device->max_level; + return 0; } /** @@ -385,9 +378,11 @@ __cpufreq_cooling_register(struct device_node *np, struct thermal_cooling_device *cool_dev; struct cpufreq_cooling_device *cpufreq_dev; char dev_name[THERMAL_NAME_LENGTH]; + struct cpufreq_frequency_table *pos, *table; int ret; - if (!cpufreq_frequency_get_table(cpumask_first(clip_cpus))) { + table = cpufreq_frequency_get_table(cpumask_first(clip_cpus)); + if (!table) { pr_debug("%s: CPUFreq table not found\n", __func__); return ERR_PTR(-EPROBE_DEFER); } @@ -404,6 +399,13 @@ __cpufreq_cooling_register(struct device_node *np, goto free_cdev; } + /* Find max levels */ + cpufreq_for_each_valid_entry(pos, table) + cpufreq_dev->max_level++; + + /* max_level is an index, not a counter */ + cpufreq_dev->max_level--; + cpumask_copy(&cpufreq_dev->allowed_cpus, clip_cpus); ret = get_idr(&cpufreq_idr, &cpufreq_dev->id); -- cgit v1.2.3 From 97afa4aafb821eca197f678b6552488c46f8c48e Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Thu, 4 Dec 2014 09:42:03 +0530 Subject: thermal: cpu_cooling: get_property() doesn't need to support GET_MAXL anymore We don't use get_property() to find max levels anymore as it is done at boot now. So, don't support GET_MAXL in get_property(). Signed-off-by: Viresh Kumar Signed-off-by: Eduardo Valentin --- drivers/thermal/cpu_cooling.c | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) (limited to 'drivers/thermal/cpu_cooling.c') diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c index d34cc5b27021..d2e6f845fe7d 100644 --- a/drivers/thermal/cpu_cooling.c +++ b/drivers/thermal/cpu_cooling.c @@ -116,7 +116,6 @@ static void release_idr(struct idr *idr, int id) enum cpufreq_cooling_property { GET_LEVEL, GET_FREQ, - GET_MAXL, }; /** @@ -124,12 +123,11 @@ enum cpufreq_cooling_property { * @cpu: cpu for which the property is required * @input: query parameter * @output: query return - * @property: type of query (frequency, level, max level) + * @property: type of query (frequency, level) * * This is the common function to - * 1. get maximum cpu cooling states - * 2. translate frequency to cooling state - * 3. translate cooling state to frequency + * 1. translate frequency to cooling state + * 2. translate cooling state to frequency * * Note that the code may be not in good shape * but it is written in this way in order to: @@ -176,12 +174,6 @@ static int get_property(unsigned int cpu, unsigned long input, /* max_level is an index, not a counter */ max_level--; - /* get max level */ - if (property == GET_MAXL) { - *output = (unsigned int)max_level; - return 0; - } - if (property == GET_FREQ) level = descend ? input : (max_level - input); -- cgit v1.2.3 From 2479bb6443d6a793f896219a34bfab0cc410f0b4 Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Thu, 4 Dec 2014 09:42:04 +0530 Subject: thermal: cpu_cooling: use cpufreq_dev_list instead of cpufreq_dev_count As we already have a list of cpufreq_cooling_devices now, lets use it instead of a local counter. Signed-off-by: Viresh Kumar Signed-off-by: Eduardo Valentin --- drivers/thermal/cpu_cooling.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) (limited to 'drivers/thermal/cpu_cooling.c') diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c index d2e6f845fe7d..32ff6dc5efee 100644 --- a/drivers/thermal/cpu_cooling.c +++ b/drivers/thermal/cpu_cooling.c @@ -71,8 +71,6 @@ struct cpufreq_cooling_device { static DEFINE_IDR(cpufreq_idr); static DEFINE_MUTEX(cooling_cpufreq_lock); -static unsigned int cpufreq_dev_count; - static LIST_HEAD(cpufreq_dev_list); /** @@ -419,10 +417,9 @@ __cpufreq_cooling_register(struct device_node *np, mutex_lock(&cooling_cpufreq_lock); /* Register the notifier for first cpufreq cooling device */ - if (cpufreq_dev_count == 0) + if (list_empty(&cpufreq_dev_list)) cpufreq_register_notifier(&thermal_cpufreq_notifier_block, CPUFREQ_POLICY_NOTIFIER); - cpufreq_dev_count++; list_add(&cpufreq_dev->node, &cpufreq_dev_list); mutex_unlock(&cooling_cpufreq_lock); @@ -495,10 +492,9 @@ void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev) cpufreq_dev = cdev->devdata; mutex_lock(&cooling_cpufreq_lock); list_del(&cpufreq_dev->node); - cpufreq_dev_count--; /* Unregister the notifier for the last cpufreq cooling device */ - if (cpufreq_dev_count == 0) + if (list_empty(&cpufreq_dev_list)) cpufreq_unregister_notifier(&thermal_cpufreq_notifier_block, CPUFREQ_POLICY_NOTIFIER); mutex_unlock(&cooling_cpufreq_lock); -- cgit v1.2.3 From b9f8b4160310e4459c08b54b918cd83da141f7f0 Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Thu, 4 Dec 2014 09:42:05 +0530 Subject: thermal: cpu_cooling: Pass 'cpufreq_dev' to get_property() We already know the value of 'cpufreq_dev->max_level' and so there is no need calculating that once again. For this, we need to send 'cpufreq_dev' to get_property(). Make all necessary changes for this change. Because cpufreq_cooling_get_level() doesn't have access to 'cpufreq_dev', it is updated to iterate over the list of cpufreq_cooling_devices to get cooling device for the cpu number passed to it. This also makes it robust to return levels only for the CPU registered via a cooling device. We don't have to support anything that isn't registered yet. Signed-off-by: Viresh Kumar Signed-off-by: Eduardo Valentin --- drivers/thermal/cpu_cooling.c | 50 +++++++++++++++++++++++-------------------- 1 file changed, 27 insertions(+), 23 deletions(-) (limited to 'drivers/thermal/cpu_cooling.c') diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c index 32ff6dc5efee..7687922cb02e 100644 --- a/drivers/thermal/cpu_cooling.c +++ b/drivers/thermal/cpu_cooling.c @@ -118,7 +118,7 @@ enum cpufreq_cooling_property { /** * get_property - fetch a property of interest for a given cpu. - * @cpu: cpu for which the property is required + * @cpufreq_dev: cpufreq_dev for which the property is required * @input: query parameter * @output: query return * @property: type of query (frequency, level) @@ -135,20 +135,20 @@ enum cpufreq_cooling_property { * * Return: 0 on success, -EINVAL when invalid parameters are passed. */ -static int get_property(unsigned int cpu, unsigned long input, - unsigned int *output, +static int get_property(struct cpufreq_cooling_device *cpufreq_dev, + unsigned long input, unsigned int *output, enum cpufreq_cooling_property property) { int i; - unsigned long max_level = 0, level = 0; + unsigned long level = 0; unsigned int freq = CPUFREQ_ENTRY_INVALID; int descend = -1; - struct cpufreq_frequency_table *pos, *table = - cpufreq_frequency_get_table(cpu); + struct cpufreq_frequency_table *pos, *table; if (!output) return -EINVAL; + table = cpufreq_frequency_get_table(cpumask_first(&cpufreq_dev->allowed_cpus)); if (!table) return -EINVAL; @@ -162,18 +162,10 @@ static int get_property(unsigned int cpu, unsigned long input, descend = freq > pos->frequency; freq = pos->frequency; - max_level++; } - /* No valid cpu frequency entry */ - if (max_level == 0) - return -EINVAL; - - /* max_level is an index, not a counter */ - max_level--; - if (property == GET_FREQ) - level = descend ? input : (max_level - input); + level = descend ? input : (cpufreq_dev->max_level - input); i = 0; cpufreq_for_each_valid_entry(pos, table) { @@ -186,7 +178,7 @@ static int get_property(unsigned int cpu, unsigned long input, if (property == GET_LEVEL && (unsigned int)input == freq) { /* get level by frequency */ - *output = descend ? i : (max_level - i); + *output = descend ? i : (cpufreq_dev->max_level - i); return 0; } if (property == GET_FREQ && level == i) { @@ -213,12 +205,25 @@ static int get_property(unsigned int cpu, unsigned long input, */ unsigned long cpufreq_cooling_get_level(unsigned int cpu, unsigned int freq) { - unsigned int val; + struct cpufreq_cooling_device *cpufreq_dev; - if (get_property(cpu, (unsigned long)freq, &val, GET_LEVEL)) - return THERMAL_CSTATE_INVALID; + mutex_lock(&cooling_cpufreq_lock); + list_for_each_entry(cpufreq_dev, &cpufreq_dev_list, node) { + if (cpumask_test_cpu(cpu, &cpufreq_dev->allowed_cpus)) { + unsigned int val; + + mutex_unlock(&cooling_cpufreq_lock); + if (get_property(cpufreq_dev, (unsigned long)freq, &val, + GET_LEVEL)) + return THERMAL_CSTATE_INVALID; + + return (unsigned long)val; + } + } + mutex_unlock(&cooling_cpufreq_lock); - return (unsigned long)val; + pr_err("%s: cpu:%d not part of any cooling device\n", __func__, cpu); + return THERMAL_CSTATE_INVALID; } EXPORT_SYMBOL_GPL(cpufreq_cooling_get_level); @@ -323,7 +328,7 @@ static int cpufreq_set_cur_state(struct thermal_cooling_device *cdev, if (cpufreq_device->cpufreq_state == state) return 0; - ret = get_property(cpu, state, &clip_freq, GET_FREQ); + ret = get_property(cpufreq_device, state, &clip_freq, GET_FREQ); if (ret) return ret; @@ -381,8 +386,7 @@ __cpufreq_cooling_register(struct device_node *np, if (!cpufreq_dev) return ERR_PTR(-ENOMEM); - ret = get_property(cpumask_any(clip_cpus), 0, &cpufreq_dev->cpufreq_val, - GET_FREQ); + ret = get_property(cpufreq_dev, 0, &cpufreq_dev->cpufreq_val, GET_FREQ); if (ret) { pr_err("%s: Failed to get frequency: %d", __func__, ret); cool_dev = ERR_PTR(ret); -- cgit v1.2.3 From f6859014c7e7cc0e7688525741fc3a0e7aee63be Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Thu, 4 Dec 2014 09:42:06 +0530 Subject: thermal: cpu_cooling: Store frequencies in descending order CPUFreq framework *doesn't* guarantee that frequencies present in cpufreq table will be in ascending or descending order. But cpu_cooling somehow assumes that. Probably because most of current users are creating this list from DT, which is done with the help of OPP layer. And OPP layer creates the list in ascending order of frequencies. But cpu_cooling can be used for other platforms too, which don't have frequencies arranged in any order. This patch tries to fix this issue by creating another list of valid frequencies in descending order. Care is also taken to throw warnings for duplicate entries. Later patches would use it to simplify code. Signed-off-by: Viresh Kumar Signed-off-by: Eduardo Valentin --- drivers/thermal/cpu_cooling.c | 41 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 40 insertions(+), 1 deletion(-) (limited to 'drivers/thermal/cpu_cooling.c') diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c index 7687922cb02e..cb5a4b914187 100644 --- a/drivers/thermal/cpu_cooling.c +++ b/drivers/thermal/cpu_cooling.c @@ -65,6 +65,7 @@ struct cpufreq_cooling_device { unsigned int cpufreq_state; unsigned int cpufreq_val; unsigned int max_level; + unsigned int *freq_table; /* In descending order */ struct cpumask allowed_cpus; struct list_head node; }; @@ -352,6 +353,20 @@ static struct notifier_block thermal_cpufreq_notifier_block = { .notifier_call = cpufreq_thermal_notifier, }; +static unsigned int find_next_max(struct cpufreq_frequency_table *table, + unsigned int prev_max) +{ + struct cpufreq_frequency_table *pos; + unsigned int max = 0; + + cpufreq_for_each_valid_entry(pos, table) { + if (pos->frequency > max && pos->frequency < prev_max) + max = pos->frequency; + } + + return max; +} + /** * __cpufreq_cooling_register - helper function to create cpufreq cooling device * @np: a valid struct device_node to the cooling device device tree node @@ -374,6 +389,7 @@ __cpufreq_cooling_register(struct device_node *np, struct cpufreq_cooling_device *cpufreq_dev; char dev_name[THERMAL_NAME_LENGTH]; struct cpufreq_frequency_table *pos, *table; + unsigned int freq, i; int ret; table = cpufreq_frequency_get_table(cpumask_first(clip_cpus)); @@ -397,6 +413,14 @@ __cpufreq_cooling_register(struct device_node *np, cpufreq_for_each_valid_entry(pos, table) cpufreq_dev->max_level++; + cpufreq_dev->freq_table = kmalloc(sizeof(*cpufreq_dev->freq_table) * + cpufreq_dev->max_level, GFP_KERNEL); + if (!cpufreq_dev->freq_table) { + return ERR_PTR(-ENOMEM); + cool_dev = ERR_PTR(-ENOMEM); + goto free_cdev; + } + /* max_level is an index, not a counter */ cpufreq_dev->max_level--; @@ -405,7 +429,7 @@ __cpufreq_cooling_register(struct device_node *np, ret = get_idr(&cpufreq_idr, &cpufreq_dev->id); if (ret) { cool_dev = ERR_PTR(ret); - goto free_cdev; + goto free_table; } snprintf(dev_name, sizeof(dev_name), "thermal-cpufreq-%d", @@ -416,6 +440,18 @@ __cpufreq_cooling_register(struct device_node *np, if (IS_ERR(cool_dev)) goto remove_idr; + /* Fill freq-table in descending order of frequencies */ + for (i = 0, freq = -1; i <= cpufreq_dev->max_level; i++) { + freq = find_next_max(table, freq); + cpufreq_dev->freq_table[i] = freq; + + /* Warn for duplicate entries */ + if (!freq) + pr_warn("%s: table has duplicate entries\n", __func__); + else + pr_debug("%s: freq:%u KHz\n", __func__, freq); + } + cpufreq_dev->cool_dev = cool_dev; mutex_lock(&cooling_cpufreq_lock); @@ -432,6 +468,8 @@ __cpufreq_cooling_register(struct device_node *np, remove_idr: release_idr(&cpufreq_idr, cpufreq_dev->id); +free_table: + kfree(cpufreq_dev->freq_table); free_cdev: kfree(cpufreq_dev); @@ -505,6 +543,7 @@ void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev) thermal_cooling_device_unregister(cpufreq_dev->cool_dev); release_idr(&cpufreq_idr, cpufreq_dev->id); + kfree(cpufreq_dev->freq_table); kfree(cpufreq_dev); } EXPORT_SYMBOL_GPL(cpufreq_cooling_unregister); -- cgit v1.2.3 From 4843c4a190495aec41c8a87365697e933dc88bc9 Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Thu, 4 Dec 2014 09:42:07 +0530 Subject: thermal: cpu_cooling: Use cpufreq_dev->freq_table for finding level/freq get_property() was an over complicated beast with BUGs. It used to believe that cpufreq table is present in ascending or descending order, which might not always be true. Previous patch has created another freq table in descending order for us and we better use it now. With that get_property() simply goes away and another helper get_level() comes in. Signed-off-by: Viresh Kumar Signed-off-by: Eduardo Valentin --- drivers/thermal/cpu_cooling.c | 108 ++++++++---------------------------------- 1 file changed, 19 insertions(+), 89 deletions(-) (limited to 'drivers/thermal/cpu_cooling.c') diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c index cb5a4b914187..cd6f64291076 100644 --- a/drivers/thermal/cpu_cooling.c +++ b/drivers/thermal/cpu_cooling.c @@ -112,85 +112,27 @@ static void release_idr(struct idr *idr, int id) /* Below code defines functions to be used for cpufreq as cooling device */ -enum cpufreq_cooling_property { - GET_LEVEL, - GET_FREQ, -}; - /** - * get_property - fetch a property of interest for a given cpu. + * get_level: Find the level for a particular frequency * @cpufreq_dev: cpufreq_dev for which the property is required - * @input: query parameter - * @output: query return - * @property: type of query (frequency, level) - * - * This is the common function to - * 1. translate frequency to cooling state - * 2. translate cooling state to frequency + * @freq: Frequency * - * Note that the code may be not in good shape - * but it is written in this way in order to: - * a) reduce duplicate code as most of the code can be shared. - * b) make sure the logic is consistent when translating between - * cooling states and frequencies. - * - * Return: 0 on success, -EINVAL when invalid parameters are passed. + * Return: level on success, THERMAL_CSTATE_INVALID on error. */ -static int get_property(struct cpufreq_cooling_device *cpufreq_dev, - unsigned long input, unsigned int *output, - enum cpufreq_cooling_property property) +static unsigned long get_level(struct cpufreq_cooling_device *cpufreq_dev, + unsigned int freq) { - int i; - unsigned long level = 0; - unsigned int freq = CPUFREQ_ENTRY_INVALID; - int descend = -1; - struct cpufreq_frequency_table *pos, *table; - - if (!output) - return -EINVAL; - - table = cpufreq_frequency_get_table(cpumask_first(&cpufreq_dev->allowed_cpus)); - if (!table) - return -EINVAL; - - cpufreq_for_each_valid_entry(pos, table) { - /* ignore duplicate entry */ - if (freq == pos->frequency) - continue; - - /* get the frequency order */ - if (freq != CPUFREQ_ENTRY_INVALID && descend == -1) - descend = freq > pos->frequency; - - freq = pos->frequency; - } - - if (property == GET_FREQ) - level = descend ? input : (cpufreq_dev->max_level - input); - - i = 0; - cpufreq_for_each_valid_entry(pos, table) { - /* ignore duplicate entry */ - if (freq == pos->frequency) - continue; + unsigned long level; - /* now we have a valid frequency entry */ - freq = pos->frequency; + for (level = 0; level <= cpufreq_dev->max_level; level++) { + if (freq == cpufreq_dev->freq_table[level]) + return level; - if (property == GET_LEVEL && (unsigned int)input == freq) { - /* get level by frequency */ - *output = descend ? i : (cpufreq_dev->max_level - i); - return 0; - } - if (property == GET_FREQ && level == i) { - /* get frequency by level */ - *output = freq; - return 0; - } - i++; + if (freq > cpufreq_dev->freq_table[level]) + break; } - return -EINVAL; + return THERMAL_CSTATE_INVALID; } /** @@ -211,14 +153,8 @@ unsigned long cpufreq_cooling_get_level(unsigned int cpu, unsigned int freq) mutex_lock(&cooling_cpufreq_lock); list_for_each_entry(cpufreq_dev, &cpufreq_dev_list, node) { if (cpumask_test_cpu(cpu, &cpufreq_dev->allowed_cpus)) { - unsigned int val; - mutex_unlock(&cooling_cpufreq_lock); - if (get_property(cpufreq_dev, (unsigned long)freq, &val, - GET_LEVEL)) - return THERMAL_CSTATE_INVALID; - - return (unsigned long)val; + return get_level(cpufreq_dev, freq); } } mutex_unlock(&cooling_cpufreq_lock); @@ -323,16 +259,16 @@ static int cpufreq_set_cur_state(struct thermal_cooling_device *cdev, struct cpufreq_cooling_device *cpufreq_device = cdev->devdata; unsigned int cpu = cpumask_any(&cpufreq_device->allowed_cpus); unsigned int clip_freq; - int ret; + + /* Request state should be less than max_level */ + if (WARN_ON(state > cpufreq_device->max_level)) + return -EINVAL; /* Check if the old cooling action is same as new cooling action */ if (cpufreq_device->cpufreq_state == state) return 0; - ret = get_property(cpufreq_device, state, &clip_freq, GET_FREQ); - if (ret) - return ret; - + clip_freq = cpufreq_device->freq_table[state]; cpufreq_device->cpufreq_state = state; cpufreq_device->cpufreq_val = clip_freq; @@ -402,13 +338,6 @@ __cpufreq_cooling_register(struct device_node *np, if (!cpufreq_dev) return ERR_PTR(-ENOMEM); - ret = get_property(cpufreq_dev, 0, &cpufreq_dev->cpufreq_val, GET_FREQ); - if (ret) { - pr_err("%s: Failed to get frequency: %d", __func__, ret); - cool_dev = ERR_PTR(ret); - goto free_cdev; - } - /* Find max levels */ cpufreq_for_each_valid_entry(pos, table) cpufreq_dev->max_level++; @@ -452,6 +381,7 @@ __cpufreq_cooling_register(struct device_node *np, pr_debug("%s: freq:%u KHz\n", __func__, freq); } + cpufreq_dev->cpufreq_val = cpufreq_dev->freq_table[0]; cpufreq_dev->cool_dev = cool_dev; mutex_lock(&cooling_cpufreq_lock); -- cgit v1.2.3 From 73904cbc1a5a5143323743209257d4668fadc7f3 Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Thu, 4 Dec 2014 09:42:08 +0530 Subject: thermal: cpu_cooling: update copyright tags Adding my copyright information for two purposes: - To get cc'd for future patches to review (Only if people read this header while sending mail) - Have done enough changes to earn a place here? Cc: Amit Daniel Kachhap Signed-off-by: Viresh Kumar Signed-off-by: Eduardo Valentin --- drivers/thermal/cpu_cooling.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'drivers/thermal/cpu_cooling.c') diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c index cd6f64291076..051eb4821bc7 100644 --- a/drivers/thermal/cpu_cooling.c +++ b/drivers/thermal/cpu_cooling.c @@ -4,6 +4,8 @@ * Copyright (C) 2012 Samsung Electronics Co., Ltd(http://www.samsung.com) * Copyright (C) 2012 Amit Daniel * + * Copyright (C) 2014 Viresh Kumar + * * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by -- cgit v1.2.3 From 2d2e95ea8f124869b96ad929d1701bd64844a06a Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Wed, 17 Dec 2014 02:55:53 +0300 Subject: thermal: cpu_cooling: small memory leak on error There was a left over return here so the error handling isn't run. It leads to a small memory leak and a static checker warning. drivers/thermal/cpu_cooling.c:351 __cpufreq_cooling_register() info: ignoring unreachable code. Fixes: f6859014c7e7 ("thermal: cpu_cooling: Store frequencies in descending order") Acked-by: Viresh Kumar Signed-off-by: Dan Carpenter Signed-off-by: Eduardo Valentin --- drivers/thermal/cpu_cooling.c | 1 - 1 file changed, 1 deletion(-) (limited to 'drivers/thermal/cpu_cooling.c') diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c index 051eb4821bc7..9b45f6426f44 100644 --- a/drivers/thermal/cpu_cooling.c +++ b/drivers/thermal/cpu_cooling.c @@ -347,7 +347,6 @@ __cpufreq_cooling_register(struct device_node *np, cpufreq_dev->freq_table = kmalloc(sizeof(*cpufreq_dev->freq_table) * cpufreq_dev->max_level, GFP_KERNEL); if (!cpufreq_dev->freq_table) { - return ERR_PTR(-ENOMEM); cool_dev = ERR_PTR(-ENOMEM); goto free_cdev; } -- cgit v1.2.3