| Message ID | 9cccfabf22ccae92dfd31175b57baab3206eac0d.1547078153.git.amit.kucheria@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [v1,1/7] drivers: thermal: of-thermal: Print name of device node with error | expand |
On 10-01-19, 05:30, Amit Kucheria wrote: > All cpufreq drivers do similar things to register as a cooling device. > Provide a generic call back so we can get rid of duplicated code in the > drivers. > > Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org> > Suggested-by: Stephen Boyd <swboyd@chromium.org> > --- > drivers/thermal/cpu_cooling.c | 18 ++++++++++++++++++ > include/linux/cpu_cooling.h | 9 +++++++++ > 2 files changed, 27 insertions(+) We may be doing this differently, but I found some other issues with the patch. > diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c > index dfd23245f778..5154ffc12332 100644 > --- a/drivers/thermal/cpu_cooling.c > +++ b/drivers/thermal/cpu_cooling.c > @@ -694,6 +694,7 @@ __cpufreq_cooling_register(struct device_node *np, > > cpufreq_cdev->clipped_freq = cpufreq_cdev->freq_table[0].frequency; > cpufreq_cdev->cdev = cdev; > + policy->cooldev = cdev; Why was this required ? The below routine was already doing it... > > mutex_lock(&cooling_list_lock); > /* Register the notifier for first cpufreq cooling device */ > @@ -785,6 +786,23 @@ of_cpufreq_cooling_register(struct cpufreq_policy *policy) > } > EXPORT_SYMBOL_GPL(of_cpufreq_cooling_register); > > +/** > + * generic_cpufreq_ready - generic driver callback to register a thermal_cooling_device > + * @policy: cpufreq policy > + */ > +void generic_cpufreq_ready(struct cpufreq_policy *policy) > +{ > + struct thermal_cooling_device **cdev = &policy->cooldev; > + > + *cdev = of_cpufreq_cooling_register(policy); ... here > + if (IS_ERR(*cdev)) { We never get an error here, only NULL. > + pr_err("Failed to register cpufreq cooling device for CPU%d: %ld\n", > + policy->cpu, PTR_ERR(cdev)); The of_cpufreq_cooling_register() routine already prints enough error messages on failures. > + cdev = NULL; This is a meaningless statement, perhaps you wanted to do *cdev = NULL :) -- viresh
diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c index dfd23245f778..5154ffc12332 100644 --- a/drivers/thermal/cpu_cooling.c +++ b/drivers/thermal/cpu_cooling.c @@ -694,6 +694,7 @@ __cpufreq_cooling_register(struct device_node *np, cpufreq_cdev->clipped_freq = cpufreq_cdev->freq_table[0].frequency; cpufreq_cdev->cdev = cdev; + policy->cooldev = cdev; mutex_lock(&cooling_list_lock); /* Register the notifier for first cpufreq cooling device */ @@ -785,6 +786,23 @@ of_cpufreq_cooling_register(struct cpufreq_policy *policy) } EXPORT_SYMBOL_GPL(of_cpufreq_cooling_register); +/** + * generic_cpufreq_ready - generic driver callback to register a thermal_cooling_device + * @policy: cpufreq policy + */ +void generic_cpufreq_ready(struct cpufreq_policy *policy) +{ + struct thermal_cooling_device **cdev = &policy->cooldev; + + *cdev = of_cpufreq_cooling_register(policy); + if (IS_ERR(*cdev)) { + pr_err("Failed to register cpufreq cooling device for CPU%d: %ld\n", + policy->cpu, PTR_ERR(cdev)); + cdev = NULL; + } +} +EXPORT_SYMBOL_GPL(generic_cpufreq_ready); + /** * cpufreq_cooling_unregister - function to remove cpufreq cooling device. * @cdev: thermal cooling device pointer. diff --git a/include/linux/cpu_cooling.h b/include/linux/cpu_cooling.h index de0dafb9399d..d7815bb2967a 100644 --- a/include/linux/cpu_cooling.h +++ b/include/linux/cpu_cooling.h @@ -65,12 +65,21 @@ void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev) */ struct thermal_cooling_device * of_cpufreq_cooling_register(struct cpufreq_policy *policy); + +/** + * generic_cpufreq_ready - generic driver callback to register a thermal_cooling_device + * @policy: cpufreq policy + */ +void generic_cpufreq_ready(struct cpufreq_policy *policy); #else static inline struct thermal_cooling_device * of_cpufreq_cooling_register(struct cpufreq_policy *policy) { return NULL; } + +void generic_cpufreq_ready(struct cpufreq_policy *policy) {} + #endif /* defined(CONFIG_THERMAL_OF) && defined(CONFIG_CPU_THERMAL) */ #endif /* __CPU_COOLING_H__ */
All cpufreq drivers do similar things to register as a cooling device. Provide a generic call back so we can get rid of duplicated code in the drivers. Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org> Suggested-by: Stephen Boyd <swboyd@chromium.org> --- drivers/thermal/cpu_cooling.c | 18 ++++++++++++++++++ include/linux/cpu_cooling.h | 9 +++++++++ 2 files changed, 27 insertions(+) -- 2.17.1