Message ID | 0036de796308b8919eac74a9a910f0d7b9c0cbf8.1548084260.git.amit.kucheria@linaro.org |
---|---|
State | New |
Headers | show |
Series | [v2,1/9] thermal: cpu_cooling: Require thermal core to be compiled in | expand |
On Wed, Jan 23, 2019 at 11:36 AM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > On 21-01-19, 21:10, Amit Kucheria wrote: > > @@ -151,6 +152,11 @@ struct cpufreq_policy { > > > > /* For cpufreq driver's internal use */ > > void *driver_data; > > + > > +#ifdef CONFIG_CPU_THERMAL > > + /* Pointer to the cooling device if used for thermal mitigation */ > > + struct thermal_cooling_device *cdev; > > +#endif > > }; > > > > /* Only for ACPI */ > > @@ -386,6 +392,12 @@ struct cpufreq_driver { > > */ > > #define CPUFREQ_NO_AUTO_DYNAMIC_SWITCHING BIT(6) > > > > +/* > > + * Set by drivers that want the core to automatically register the cpufreq > > + * driver as a thermal cooling device. > > + */ > > +#define CPUFREQ_AUTO_REGISTER_COOLING_DEV BIT(7) > > + > > int cpufreq_register_driver(struct cpufreq_driver *driver_data); > > int cpufreq_unregister_driver(struct cpufreq_driver *driver_data); > > > > @@ -415,6 +427,19 @@ cpufreq_verify_within_cpu_limits(struct cpufreq_policy *policy) > > policy->cpuinfo.max_freq); > > } > > > > +#ifdef CONFIG_CPU_THERMAL > > +static inline void register_cooling_device(struct cpufreq_policy *policy) { > > + policy->cdev = of_cpufreq_cooling_register(policy); > > +} > > + > > +static inline void unregister_cooling_device(struct cpufreq_policy *policy) { > > + cpufreq_cooling_unregister(policy->cdev); > > + policy->cdev = NULL; > > +} > > +#else > > +static inline void register_cooling_device(struct cpufreq_policy *policy) {} > > +static inline void unregister_cooling_device(struct cpufreq_policy *policy) {} > > +#endif > > The whole ifdef hackery here saves space for a pointer per policy. > Just get rid of it, it isn't worth it. Is struct thermal_cooling_device defined if CONFIG_THERMAL is unset?
On 23-01-19, 11:39, Rafael J. Wysocki wrote: > On Wed, Jan 23, 2019 at 11:36 AM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > > > On 21-01-19, 21:10, Amit Kucheria wrote: > > > @@ -151,6 +152,11 @@ struct cpufreq_policy { > > > > > > /* For cpufreq driver's internal use */ > > > void *driver_data; > > > + > > > +#ifdef CONFIG_CPU_THERMAL > > > + /* Pointer to the cooling device if used for thermal mitigation */ > > > + struct thermal_cooling_device *cdev; > > > +#endif > > > }; > > > > > > /* Only for ACPI */ > > > @@ -386,6 +392,12 @@ struct cpufreq_driver { > > > */ > > > #define CPUFREQ_NO_AUTO_DYNAMIC_SWITCHING BIT(6) > > > > > > +/* > > > + * Set by drivers that want the core to automatically register the cpufreq > > > + * driver as a thermal cooling device. > > > + */ > > > +#define CPUFREQ_AUTO_REGISTER_COOLING_DEV BIT(7) > > > + > > > int cpufreq_register_driver(struct cpufreq_driver *driver_data); > > > int cpufreq_unregister_driver(struct cpufreq_driver *driver_data); > > > > > > @@ -415,6 +427,19 @@ cpufreq_verify_within_cpu_limits(struct cpufreq_policy *policy) > > > policy->cpuinfo.max_freq); > > > } > > > > > > +#ifdef CONFIG_CPU_THERMAL > > > +static inline void register_cooling_device(struct cpufreq_policy *policy) { > > > + policy->cdev = of_cpufreq_cooling_register(policy); > > > +} > > > + > > > +static inline void unregister_cooling_device(struct cpufreq_policy *policy) { > > > + cpufreq_cooling_unregister(policy->cdev); > > > + policy->cdev = NULL; > > > +} > > > +#else > > > +static inline void register_cooling_device(struct cpufreq_policy *policy) {} > > > +static inline void unregister_cooling_device(struct cpufreq_policy *policy) {} > > > +#endif > > > > The whole ifdef hackery here saves space for a pointer per policy. > > Just get rid of it, it isn't worth it. > > Is struct thermal_cooling_device defined if CONFIG_THERMAL is unset? No and it is defined in thermal.h without any ifdef stuff. -- viresh
On 23-01-19, 16:13, Viresh Kumar wrote: > On 23-01-19, 11:39, Rafael J. Wysocki wrote: > > On Wed, Jan 23, 2019 at 11:36 AM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > > > > > On 21-01-19, 21:10, Amit Kucheria wrote: > > > > @@ -151,6 +152,11 @@ struct cpufreq_policy { > > > > > > > > /* For cpufreq driver's internal use */ > > > > void *driver_data; > > > > + > > > > +#ifdef CONFIG_CPU_THERMAL > > > > + /* Pointer to the cooling device if used for thermal mitigation */ > > > > + struct thermal_cooling_device *cdev; > > > > +#endif > > > > }; > > > > > > > > /* Only for ACPI */ > > > > @@ -386,6 +392,12 @@ struct cpufreq_driver { > > > > */ > > > > #define CPUFREQ_NO_AUTO_DYNAMIC_SWITCHING BIT(6) > > > > > > > > +/* > > > > + * Set by drivers that want the core to automatically register the cpufreq > > > > + * driver as a thermal cooling device. > > > > + */ > > > > +#define CPUFREQ_AUTO_REGISTER_COOLING_DEV BIT(7) > > > > + > > > > int cpufreq_register_driver(struct cpufreq_driver *driver_data); > > > > int cpufreq_unregister_driver(struct cpufreq_driver *driver_data); > > > > > > > > @@ -415,6 +427,19 @@ cpufreq_verify_within_cpu_limits(struct cpufreq_policy *policy) > > > > policy->cpuinfo.max_freq); > > > > } > > > > > > > > +#ifdef CONFIG_CPU_THERMAL > > > > +static inline void register_cooling_device(struct cpufreq_policy *policy) { > > > > + policy->cdev = of_cpufreq_cooling_register(policy); > > > > +} > > > > + > > > > +static inline void unregister_cooling_device(struct cpufreq_policy *policy) { > > > > + cpufreq_cooling_unregister(policy->cdev); > > > > + policy->cdev = NULL; > > > > +} > > > > +#else > > > > +static inline void register_cooling_device(struct cpufreq_policy *policy) {} > > > > +static inline void unregister_cooling_device(struct cpufreq_policy *policy) {} > > > > +#endif > > > > > > The whole ifdef hackery here saves space for a pointer per policy. > > > Just get rid of it, it isn't worth it. > > > > Is struct thermal_cooling_device defined if CONFIG_THERMAL is unset? > > No and it is defined in thermal.h without any ifdef stuff. I meant it is always available and doesn't depend on CONFIG_THERMAL. -- viresh
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index e35a886e00bc..cf1be057caf4 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -1318,6 +1318,9 @@ static int cpufreq_online(unsigned int cpu) if (cpufreq_driver->ready) cpufreq_driver->ready(policy); + if (cpufreq_driver->flags & CPUFREQ_AUTO_REGISTER_COOLING_DEV) + register_cooling_device(policy); + pr_debug("initialization complete\n"); return 0; @@ -1405,6 +1408,9 @@ static int cpufreq_offline(unsigned int cpu) goto unlock; } + if (cpufreq_driver->flags & CPUFREQ_AUTO_REGISTER_COOLING_DEV) + unregister_cooling_device(policy); + if (cpufreq_driver->stop_cpu) cpufreq_driver->stop_cpu(policy); diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h index bd7fbd6a4478..02fc9f210aee 100644 --- a/include/linux/cpufreq.h +++ b/include/linux/cpufreq.h @@ -13,6 +13,7 @@ #include <linux/clk.h> #include <linux/cpumask.h> +#include <linux/cpu_cooling.h> #include <linux/completion.h> #include <linux/kobject.h> #include <linux/notifier.h> @@ -151,6 +152,11 @@ struct cpufreq_policy { /* For cpufreq driver's internal use */ void *driver_data; + +#ifdef CONFIG_CPU_THERMAL + /* Pointer to the cooling device if used for thermal mitigation */ + struct thermal_cooling_device *cdev; +#endif }; /* Only for ACPI */ @@ -386,6 +392,12 @@ struct cpufreq_driver { */ #define CPUFREQ_NO_AUTO_DYNAMIC_SWITCHING BIT(6) +/* + * Set by drivers that want the core to automatically register the cpufreq + * driver as a thermal cooling device. + */ +#define CPUFREQ_AUTO_REGISTER_COOLING_DEV BIT(7) + int cpufreq_register_driver(struct cpufreq_driver *driver_data); int cpufreq_unregister_driver(struct cpufreq_driver *driver_data); @@ -415,6 +427,19 @@ cpufreq_verify_within_cpu_limits(struct cpufreq_policy *policy) policy->cpuinfo.max_freq); } +#ifdef CONFIG_CPU_THERMAL +static inline void register_cooling_device(struct cpufreq_policy *policy) { + policy->cdev = of_cpufreq_cooling_register(policy); +} + +static inline void unregister_cooling_device(struct cpufreq_policy *policy) { + cpufreq_cooling_unregister(policy->cdev); + policy->cdev = NULL; +} +#else +static inline void register_cooling_device(struct cpufreq_policy *policy) {} +static inline void unregister_cooling_device(struct cpufreq_policy *policy) {} +#endif #ifdef CONFIG_CPU_FREQ void cpufreq_suspend(void); void cpufreq_resume(void);