Add a flag to be used by cpufreq drivers to tell cpufreq core to
auto-register themselves as a thermal cooling device.
There series converts over all the drivers except arm_big_little.c.
Tested on SDM845 with the qcom-cpufreq-hw driver. Only compile-tested the
others.
Things needing fixing:
- Look at how to detect that we're not in IKS mode in arm_big_little's
.ready callback.
- The other pending issue is to fix allmodconfig that leaves us with
CPU_FREQ=y and THERMAL=m (CPU_THERMAL=y). That leads to undefined
references for functions defined in cpu_cooling.c
Amit Kucheria (10):
cpufreq: Add thermal_cooling_device pointer to struct cpufreq_policy
cpufreq: Add a flag to auto-register a cooling device
cpufreq: Replace open-coded << with BIT()
cpufreq: qcom-hw: Register as a cpufreq cooling device
cpufreq: imx6q: Use auto-registration of thermal cooling device
cpufreq: cpufreq-dt: Use auto-registration of thermal cooling device
cpufreq: mediatek: Use auto-registration of thermal cooling device
cpufreq: qoriq: Use auto-registration of thermal cooling device
cpufreq: scmi: Use auto-registration of thermal cooling device
cpufreq: scpi: Use auto-registration of thermal cooling device
drivers/cpufreq/cpufreq-dt.c | 14 ++----------
drivers/cpufreq/cpufreq.c | 17 ++++++++++++++
drivers/cpufreq/imx6q-cpufreq.c | 18 ++-------------
drivers/cpufreq/mediatek-cpufreq.c | 14 ++----------
drivers/cpufreq/qcom-cpufreq-hw.c | 3 ++-
drivers/cpufreq/qoriq-cpufreq.c | 15 ++-----------
drivers/cpufreq/scmi-cpufreq.c | 14 ++----------
drivers/cpufreq/scpi-cpufreq.c | 14 ++----------
include/linux/cpufreq.h | 36 ++++++++++++++++++++----------
9 files changed, 55 insertions(+), 90 deletions(-)
--
2.17.1
Several cpufreq drivers register themselves as thermal cooling devices.
Adding a pointer to struct cpufreq_policy removes the need for them to
store this pointer in a private data structure.
We can then auto-register the cpufreq driver as a thermal cooling device
from cpufreq core code.
Signed-off-by: Amit Kucheria <[email protected]>
---
include/linux/cpufreq.h | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index c86d6d8bdfed..7d0cf54125fa 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -95,6 +95,11 @@ struct cpufreq_policy {
struct cpufreq_frequency_table *freq_table;
enum cpufreq_table_sorting freq_table_sorted;
+#ifdef CONFIG_CPU_THERMAL
+ /* Pointer to the cooling device if used for thermal mitigation */
+ struct thermal_cooling_device *cooldev;
+#endif
+
struct list_head policy_list;
struct kobject kobj;
struct completion kobj_unregister;
--
2.17.1
Minor clean-up to use BIT() and keep checkpatch happy. Clean up the
comment formatting while we're at it to make it easier to read.
Signed-off-by: Amit Kucheria <[email protected]>
---
include/linux/cpufreq.h | 27 ++++++++++++++-------------
1 file changed, 14 insertions(+), 13 deletions(-)
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index 70ad02088825..ea303dfd5f05 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -351,14 +351,15 @@ struct cpufreq_driver {
};
/* flags */
-#define CPUFREQ_STICKY (1 << 0) /* driver isn't removed even if
- all ->init() calls failed */
-#define CPUFREQ_CONST_LOOPS (1 << 1) /* loops_per_jiffy or other
- kernel "constants" aren't
- affected by frequency
- transitions */
-#define CPUFREQ_PM_NO_WARN (1 << 2) /* don't warn on suspend/resume
- speed mismatches */
+
+/* driver isn't removed even if all ->init() calls failed */
+#define CPUFREQ_STICKY BIT(0)
+
+/* loops_per_jiffy or other kernel "constants" aren't affected by frequency transitions */
+#define CPUFREQ_CONST_LOOPS BIT(1)
+
+/* don't warn on suspend/resume speed mismatches */
+#define CPUFREQ_PM_NO_WARN BIT(2)
/*
* This should be set by platforms having multiple clock-domains, i.e.
@@ -366,14 +367,14 @@ struct cpufreq_driver {
* be created in cpu/cpu<num>/cpufreq/ directory and so they can use the same
* governor with different tunables for different clusters.
*/
-#define CPUFREQ_HAVE_GOVERNOR_PER_POLICY (1 << 3)
+#define CPUFREQ_HAVE_GOVERNOR_PER_POLICY BIT(3)
/*
* Driver will do POSTCHANGE notifications from outside of their ->target()
* routine and so must set cpufreq_driver->flags with this flag, so that core
* can handle them specially.
*/
-#define CPUFREQ_ASYNC_NOTIFICATION (1 << 4)
+#define CPUFREQ_ASYNC_NOTIFICATION BIT(4)
/*
* Set by drivers which want cpufreq core to check if CPU is running at a
@@ -382,19 +383,19 @@ struct cpufreq_driver {
* from the table. And if that fails, we will stop further boot process by
* issuing a BUG_ON().
*/
-#define CPUFREQ_NEED_INITIAL_FREQ_CHECK (1 << 5)
+#define CPUFREQ_NEED_INITIAL_FREQ_CHECK BIT(5)
/*
* Set by drivers to disallow use of governors with "dynamic_switching" flag
* set.
*/
-#define CPUFREQ_NO_AUTO_DYNAMIC_SWITCHING (1 << 6)
+#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 (1 << 7)
+#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);
--
2.17.1
All cpufreq drivers do similar things to register as a cooling device.
Provide a cpufreq driver flag so drivers can just ask the cpufreq core
to register the cooling device on their behalf. This allows us to get
rid of duplicated code in the drivers.
Suggested-by: Stephen Boyd <[email protected]>
Suggested-by: Viresh Kumar <[email protected]>
Signed-off-by: Amit Kucheria <[email protected]>
---
drivers/cpufreq/cpufreq.c | 17 +++++++++++++++++
include/linux/cpufreq.h | 6 ++++++
2 files changed, 23 insertions(+)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 6f23ebb395f1..cd6e750d3d82 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -30,6 +30,7 @@
#include <linux/syscore_ops.h>
#include <linux/tick.h>
#include <trace/events/power.h>
+#include <linux/cpu_cooling.h>
static LIST_HEAD(cpufreq_policy_list);
@@ -1318,6 +1319,14 @@ static int cpufreq_online(unsigned int cpu)
if (cpufreq_driver->ready)
cpufreq_driver->ready(policy);
+#ifdef CONFIG_CPU_THERMAL
+ if (cpufreq_driver->flags & CPUFREQ_AUTO_REGISTER_COOLING_DEV) {
+ struct thermal_cooling_device **cdev = &policy->cooldev;
+
+ *cdev = of_cpufreq_cooling_register(policy);
+ }
+#endif
+
pr_debug("initialization complete\n");
return 0;
@@ -1411,6 +1420,14 @@ static int cpufreq_offline(unsigned int cpu)
if (has_target())
cpufreq_exit_governor(policy);
+#ifdef CONFIG_CPU_THERMAL
+ if (cpufreq_driver->flags & CPUFREQ_AUTO_REGISTER_COOLING_DEV) {
+ struct thermal_cooling_device **cdev = &policy->cooldev;
+
+ cpufreq_cooling_unregister(*cdev);
+ }
+#endif
+
/*
* Perform the ->exit() even during light-weight tear-down,
* since this is a core component, and is essential for the
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index 7d0cf54125fa..70ad02088825 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -390,6 +390,12 @@ struct cpufreq_driver {
*/
#define CPUFREQ_NO_AUTO_DYNAMIC_SWITCHING (1 << 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 (1 << 7)
+
int cpufreq_register_driver(struct cpufreq_driver *driver_data);
int cpufreq_unregister_driver(struct cpufreq_driver *driver_data);
--
2.17.1
Add the CPUFREQ_AUTO_REGISTER_COOLING_DEV flag to allow the cpufreq core
to auto-register the driver as a cooling device.
Signed-off-by: Amit Kucheria <[email protected]>
---
drivers/cpufreq/qcom-cpufreq-hw.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c
index d83939a1b3d4..ed32849a3d40 100644
--- a/drivers/cpufreq/qcom-cpufreq-hw.c
+++ b/drivers/cpufreq/qcom-cpufreq-hw.c
@@ -231,7 +231,8 @@ static struct freq_attr *qcom_cpufreq_hw_attr[] = {
static struct cpufreq_driver cpufreq_qcom_hw_driver = {
.flags = CPUFREQ_STICKY | CPUFREQ_NEED_INITIAL_FREQ_CHECK |
- CPUFREQ_HAVE_GOVERNOR_PER_POLICY,
+ CPUFREQ_HAVE_GOVERNOR_PER_POLICY |
+ CPUFREQ_AUTO_REGISTER_COOLING_DEV,
.verify = cpufreq_generic_frequency_table_verify,
.target_index = qcom_cpufreq_hw_target_index,
.get = qcom_cpufreq_hw_get,
--
2.17.1
Use the CPUFREQ_AUTO_REGISTER_COOLING_DEV flag to allow cpufreq core to
automatically register as a thermal cooling device.
This allows removal of boiler plate code from the driver.
Signed-off-by: Amit Kucheria <[email protected]>
---
drivers/cpufreq/imx6q-cpufreq.c | 18 ++----------------
1 file changed, 2 insertions(+), 16 deletions(-)
diff --git a/drivers/cpufreq/imx6q-cpufreq.c b/drivers/cpufreq/imx6q-cpufreq.c
index 9fedf627e000..ffb00291f48e 100644
--- a/drivers/cpufreq/imx6q-cpufreq.c
+++ b/drivers/cpufreq/imx6q-cpufreq.c
@@ -9,7 +9,6 @@
#include <linux/clk.h>
#include <linux/cpu.h>
#include <linux/cpufreq.h>
-#include <linux/cpu_cooling.h>
#include <linux/err.h>
#include <linux/module.h>
#include <linux/nvmem-consumer.h>
@@ -52,7 +51,6 @@ static struct clk_bulk_data clks[] = {
};
static struct device *cpu_dev;
-static struct thermal_cooling_device *cdev;
static bool free_opp;
static struct cpufreq_frequency_table *freq_table;
static unsigned int max_freq;
@@ -193,16 +191,6 @@ static int imx6q_set_target(struct cpufreq_policy *policy, unsigned int index)
return 0;
}
-static void imx6q_cpufreq_ready(struct cpufreq_policy *policy)
-{
- cdev = of_cpufreq_cooling_register(policy);
-
- if (!cdev)
- dev_err(cpu_dev,
- "running cpufreq without cooling device: %ld\n",
- PTR_ERR(cdev));
-}
-
static int imx6q_cpufreq_init(struct cpufreq_policy *policy)
{
int ret;
@@ -216,20 +204,18 @@ static int imx6q_cpufreq_init(struct cpufreq_policy *policy)
static int imx6q_cpufreq_exit(struct cpufreq_policy *policy)
{
- cpufreq_cooling_unregister(cdev);
-
return 0;
}
static struct cpufreq_driver imx6q_cpufreq_driver = {
- .flags = CPUFREQ_NEED_INITIAL_FREQ_CHECK,
+ .flags = CPUFREQ_NEED_INITIAL_FREQ_CHECK |
+ CPUFREQ_AUTO_REGISTER_COOLING_DEV,
.verify = cpufreq_generic_frequency_table_verify,
.target_index = imx6q_set_target,
.get = cpufreq_generic_get,
.init = imx6q_cpufreq_init,
.exit = imx6q_cpufreq_exit,
.name = "imx6q-cpufreq",
- .ready = imx6q_cpufreq_ready,
.attr = cpufreq_generic_attr,
.suspend = cpufreq_generic_suspend,
};
--
2.17.1
Use the CPUFREQ_AUTO_REGISTER_COOLING_DEV flag to allow cpufreq core to
automatically register as a thermal cooling device.
This allows removal of boiler plate code from the driver.
Signed-off-by: Amit Kucheria <[email protected]>
---
drivers/cpufreq/cpufreq-dt.c | 14 ++------------
1 file changed, 2 insertions(+), 12 deletions(-)
diff --git a/drivers/cpufreq/cpufreq-dt.c b/drivers/cpufreq/cpufreq-dt.c
index e58bfcb1169e..2a4c4ea7980b 100644
--- a/drivers/cpufreq/cpufreq-dt.c
+++ b/drivers/cpufreq/cpufreq-dt.c
@@ -13,7 +13,6 @@
#include <linux/clk.h>
#include <linux/cpu.h>
-#include <linux/cpu_cooling.h>
#include <linux/cpufreq.h>
#include <linux/cpumask.h>
#include <linux/err.h>
@@ -30,7 +29,6 @@
struct private_data {
struct opp_table *opp_table;
struct device *cpu_dev;
- struct thermal_cooling_device *cdev;
const char *reg_name;
bool have_static_opps;
};
@@ -301,7 +299,6 @@ static int cpufreq_exit(struct cpufreq_policy *policy)
{
struct private_data *priv = policy->driver_data;
- cpufreq_cooling_unregister(priv->cdev);
dev_pm_opp_free_cpufreq_table(priv->cpu_dev, &policy->freq_table);
if (priv->have_static_opps)
dev_pm_opp_of_cpumask_remove_table(policy->related_cpus);
@@ -314,21 +311,14 @@ static int cpufreq_exit(struct cpufreq_policy *policy)
return 0;
}
-static void cpufreq_ready(struct cpufreq_policy *policy)
-{
- struct private_data *priv = policy->driver_data;
-
- priv->cdev = of_cpufreq_cooling_register(policy);
-}
-
static struct cpufreq_driver dt_cpufreq_driver = {
- .flags = CPUFREQ_STICKY | CPUFREQ_NEED_INITIAL_FREQ_CHECK,
+ .flags = CPUFREQ_STICKY | CPUFREQ_NEED_INITIAL_FREQ_CHECK |
+ CPUFREQ_AUTO_REGISTER_COOLING_DEV,
.verify = cpufreq_generic_frequency_table_verify,
.target_index = set_target,
.get = cpufreq_generic_get,
.init = cpufreq_init,
.exit = cpufreq_exit,
- .ready = cpufreq_ready,
.name = "cpufreq-dt",
.attr = cpufreq_dt_attr,
.suspend = cpufreq_generic_suspend,
--
2.17.1
Use the CPUFREQ_AUTO_REGISTER_COOLING_DEV flag to allow cpufreq core to
automatically register as a thermal cooling device.
This allows removal of boiler plate code from the driver.
Signed-off-by: Amit Kucheria <[email protected]>
---
drivers/cpufreq/mediatek-cpufreq.c | 14 ++------------
1 file changed, 2 insertions(+), 12 deletions(-)
diff --git a/drivers/cpufreq/mediatek-cpufreq.c b/drivers/cpufreq/mediatek-cpufreq.c
index eb8920d39818..9a937f4c63e7 100644
--- a/drivers/cpufreq/mediatek-cpufreq.c
+++ b/drivers/cpufreq/mediatek-cpufreq.c
@@ -14,7 +14,6 @@
#include <linux/clk.h>
#include <linux/cpu.h>
-#include <linux/cpu_cooling.h>
#include <linux/cpufreq.h>
#include <linux/cpumask.h>
#include <linux/module.h>
@@ -48,7 +47,6 @@ struct mtk_cpu_dvfs_info {
struct regulator *sram_reg;
struct clk *cpu_clk;
struct clk *inter_clk;
- struct thermal_cooling_device *cdev;
struct list_head list_head;
int intermediate_voltage;
bool need_voltage_tracking;
@@ -307,13 +305,6 @@ static int mtk_cpufreq_set_target(struct cpufreq_policy *policy,
#define DYNAMIC_POWER "dynamic-power-coefficient"
-static void mtk_cpufreq_ready(struct cpufreq_policy *policy)
-{
- struct mtk_cpu_dvfs_info *info = policy->driver_data;
-
- info->cdev = of_cpufreq_cooling_register(policy);
-}
-
static int mtk_cpu_dvfs_info_init(struct mtk_cpu_dvfs_info *info, int cpu)
{
struct device *cpu_dev;
@@ -472,7 +463,6 @@ static int mtk_cpufreq_exit(struct cpufreq_policy *policy)
{
struct mtk_cpu_dvfs_info *info = policy->driver_data;
- cpufreq_cooling_unregister(info->cdev);
dev_pm_opp_free_cpufreq_table(info->cpu_dev, &policy->freq_table);
return 0;
@@ -480,13 +470,13 @@ static int mtk_cpufreq_exit(struct cpufreq_policy *policy)
static struct cpufreq_driver mtk_cpufreq_driver = {
.flags = CPUFREQ_STICKY | CPUFREQ_NEED_INITIAL_FREQ_CHECK |
- CPUFREQ_HAVE_GOVERNOR_PER_POLICY,
+ CPUFREQ_HAVE_GOVERNOR_PER_POLICY |
+ CPUFREQ_AUTO_REGISTER_COOLING_DEV,
.verify = cpufreq_generic_frequency_table_verify,
.target_index = mtk_cpufreq_set_target,
.get = cpufreq_generic_get,
.init = mtk_cpufreq_init,
.exit = mtk_cpufreq_exit,
- .ready = mtk_cpufreq_ready,
.name = "mtk-cpufreq",
.attr = cpufreq_generic_attr,
};
--
2.17.1
Use the CPUFREQ_AUTO_REGISTER_COOLING_DEV flag to allow cpufreq core to
automatically register as a thermal cooling device.
This allows removal of boiler plate code from the driver.
Signed-off-by: Amit Kucheria <[email protected]>
---
drivers/cpufreq/qoriq-cpufreq.c | 15 ++-------------
1 file changed, 2 insertions(+), 13 deletions(-)
diff --git a/drivers/cpufreq/qoriq-cpufreq.c b/drivers/cpufreq/qoriq-cpufreq.c
index 3d773f64b4df..b206e6cb55f0 100644
--- a/drivers/cpufreq/qoriq-cpufreq.c
+++ b/drivers/cpufreq/qoriq-cpufreq.c
@@ -13,7 +13,6 @@
#include <linux/clk.h>
#include <linux/clk-provider.h>
#include <linux/cpufreq.h>
-#include <linux/cpu_cooling.h>
#include <linux/errno.h>
#include <linux/init.h>
#include <linux/kernel.h>
@@ -31,7 +30,6 @@
struct cpu_data {
struct clk **pclk;
struct cpufreq_frequency_table *table;
- struct thermal_cooling_device *cdev;
};
/*
@@ -239,7 +237,6 @@ static int qoriq_cpufreq_cpu_exit(struct cpufreq_policy *policy)
{
struct cpu_data *data = policy->driver_data;
- cpufreq_cooling_unregister(data->cdev);
kfree(data->pclk);
kfree(data->table);
kfree(data);
@@ -258,23 +255,15 @@ static int qoriq_cpufreq_target(struct cpufreq_policy *policy,
return clk_set_parent(policy->clk, parent);
}
-
-static void qoriq_cpufreq_ready(struct cpufreq_policy *policy)
-{
- struct cpu_data *cpud = policy->driver_data;
-
- cpud->cdev = of_cpufreq_cooling_register(policy);
-}
-
static struct cpufreq_driver qoriq_cpufreq_driver = {
.name = "qoriq_cpufreq",
- .flags = CPUFREQ_CONST_LOOPS,
+ .flags = CPUFREQ_CONST_LOOPS |
+ CPUFREQ_AUTO_REGISTER_COOLING_DEV,
.init = qoriq_cpufreq_cpu_init,
.exit = qoriq_cpufreq_cpu_exit,
.verify = cpufreq_generic_frequency_table_verify,
.target_index = qoriq_cpufreq_target,
.get = cpufreq_generic_get,
- .ready = qoriq_cpufreq_ready,
.attr = cpufreq_generic_attr,
};
--
2.17.1
Use the CPUFREQ_AUTO_REGISTER_COOLING_DEV flag to allow cpufreq core to
automatically register as a thermal cooling device.
This allows removal of boiler plate code from the driver.
Signed-off-by: Amit Kucheria <[email protected]>
---
drivers/cpufreq/scmi-cpufreq.c | 14 ++------------
1 file changed, 2 insertions(+), 12 deletions(-)
diff --git a/drivers/cpufreq/scmi-cpufreq.c b/drivers/cpufreq/scmi-cpufreq.c
index 50b1551ba894..f062a84a1175 100644
--- a/drivers/cpufreq/scmi-cpufreq.c
+++ b/drivers/cpufreq/scmi-cpufreq.c
@@ -11,7 +11,6 @@
#include <linux/cpu.h>
#include <linux/cpufreq.h>
#include <linux/cpumask.h>
-#include <linux/cpu_cooling.h>
#include <linux/export.h>
#include <linux/module.h>
#include <linux/pm_opp.h>
@@ -22,7 +21,6 @@
struct scmi_data {
int domain_id;
struct device *cpu_dev;
- struct thermal_cooling_device *cdev;
};
static const struct scmi_handle *handle;
@@ -185,7 +183,6 @@ static int scmi_cpufreq_exit(struct cpufreq_policy *policy)
{
struct scmi_data *priv = policy->driver_data;
- cpufreq_cooling_unregister(priv->cdev);
dev_pm_opp_free_cpufreq_table(priv->cpu_dev, &policy->freq_table);
kfree(priv);
dev_pm_opp_cpumask_remove_table(policy->related_cpus);
@@ -193,17 +190,11 @@ static int scmi_cpufreq_exit(struct cpufreq_policy *policy)
return 0;
}
-static void scmi_cpufreq_ready(struct cpufreq_policy *policy)
-{
- struct scmi_data *priv = policy->driver_data;
-
- priv->cdev = of_cpufreq_cooling_register(policy);
-}
-
static struct cpufreq_driver scmi_cpufreq_driver = {
.name = "scmi",
.flags = CPUFREQ_STICKY | CPUFREQ_HAVE_GOVERNOR_PER_POLICY |
- CPUFREQ_NEED_INITIAL_FREQ_CHECK,
+ CPUFREQ_NEED_INITIAL_FREQ_CHECK |
+ CPUFREQ_AUTO_REGISTER_COOLING_DEV,
.verify = cpufreq_generic_frequency_table_verify,
.attr = cpufreq_generic_attr,
.target_index = scmi_cpufreq_set_target,
@@ -211,7 +202,6 @@ static struct cpufreq_driver scmi_cpufreq_driver = {
.get = scmi_cpufreq_get_rate,
.init = scmi_cpufreq_init,
.exit = scmi_cpufreq_exit,
- .ready = scmi_cpufreq_ready,
};
static int scmi_cpufreq_probe(struct scmi_device *sdev)
--
2.17.1
Use the CPUFREQ_AUTO_REGISTER_COOLING_DEV flag to allow cpufreq core to
automatically register as a thermal cooling device.
This allows removal of boiler plate code from the driver.
Signed-off-by: Amit Kucheria <[email protected]>
---
drivers/cpufreq/scpi-cpufreq.c | 14 ++------------
1 file changed, 2 insertions(+), 12 deletions(-)
diff --git a/drivers/cpufreq/scpi-cpufreq.c b/drivers/cpufreq/scpi-cpufreq.c
index 87a98ec77773..c01c63a342c9 100644
--- a/drivers/cpufreq/scpi-cpufreq.c
+++ b/drivers/cpufreq/scpi-cpufreq.c
@@ -22,7 +22,6 @@
#include <linux/cpu.h>
#include <linux/cpufreq.h>
#include <linux/cpumask.h>
-#include <linux/cpu_cooling.h>
#include <linux/export.h>
#include <linux/module.h>
#include <linux/of_platform.h>
@@ -34,7 +33,6 @@
struct scpi_data {
struct clk *clk;
struct device *cpu_dev;
- struct thermal_cooling_device *cdev;
};
static struct scpi_ops *scpi_ops;
@@ -186,7 +184,6 @@ static int scpi_cpufreq_exit(struct cpufreq_policy *policy)
{
struct scpi_data *priv = policy->driver_data;
- cpufreq_cooling_unregister(priv->cdev);
clk_put(priv->clk);
dev_pm_opp_free_cpufreq_table(priv->cpu_dev, &policy->freq_table);
kfree(priv);
@@ -195,23 +192,16 @@ static int scpi_cpufreq_exit(struct cpufreq_policy *policy)
return 0;
}
-static void scpi_cpufreq_ready(struct cpufreq_policy *policy)
-{
- struct scpi_data *priv = policy->driver_data;
-
- priv->cdev = of_cpufreq_cooling_register(policy);
-}
-
static struct cpufreq_driver scpi_cpufreq_driver = {
.name = "scpi-cpufreq",
.flags = CPUFREQ_STICKY | CPUFREQ_HAVE_GOVERNOR_PER_POLICY |
- CPUFREQ_NEED_INITIAL_FREQ_CHECK,
+ CPUFREQ_NEED_INITIAL_FREQ_CHECK |
+ CPUFREQ_AUTO_REGISTER_COOLING_DEV,
.verify = cpufreq_generic_frequency_table_verify,
.attr = cpufreq_generic_attr,
.get = scpi_cpufreq_get_rate,
.init = scpi_cpufreq_init,
.exit = scpi_cpufreq_exit,
- .ready = scpi_cpufreq_ready,
.target_index = scpi_cpufreq_set_target,
};
--
2.17.1
On Mon, Jan 14, 2019 at 10:04:54PM +0530, Amit Kucheria wrote:
> All cpufreq drivers do similar things to register as a cooling device.
> Provide a cpufreq driver flag so drivers can just ask the cpufreq core
> to register the cooling device on their behalf. This allows us to get
> rid of duplicated code in the drivers.
>
> Suggested-by: Stephen Boyd <[email protected]>
> Suggested-by: Viresh Kumar <[email protected]>
> Signed-off-by: Amit Kucheria <[email protected]>
> ---
> drivers/cpufreq/cpufreq.c | 17 +++++++++++++++++
> include/linux/cpufreq.h | 6 ++++++
> 2 files changed, 23 insertions(+)
>
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 6f23ebb395f1..cd6e750d3d82 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -30,6 +30,7 @@
> #include <linux/syscore_ops.h>
> #include <linux/tick.h>
> #include <trace/events/power.h>
> +#include <linux/cpu_cooling.h>
>
> static LIST_HEAD(cpufreq_policy_list);
>
> @@ -1318,6 +1319,14 @@ static int cpufreq_online(unsigned int cpu)
> if (cpufreq_driver->ready)
> cpufreq_driver->ready(policy);
>
> +#ifdef CONFIG_CPU_THERMAL
> + if (cpufreq_driver->flags & CPUFREQ_AUTO_REGISTER_COOLING_DEV) {
> + struct thermal_cooling_device **cdev = &policy->cooldev;
> +
> + *cdev = of_cpufreq_cooling_register(policy);
> + }
> +#endif
> +
> pr_debug("initialization complete\n");
>
> return 0;
> @@ -1411,6 +1420,14 @@ static int cpufreq_offline(unsigned int cpu)
> if (has_target())
> cpufreq_exit_governor(policy);
>
> +#ifdef CONFIG_CPU_THERMAL
> + if (cpufreq_driver->flags & CPUFREQ_AUTO_REGISTER_COOLING_DEV) {
> + struct thermal_cooling_device **cdev = &policy->cooldev;
> +
> + cpufreq_cooling_unregister(*cdev);
> + }
> +#endif
> +
> /*
> * Perform the ->exit() even during light-weight tear-down,
> * since this is a core component, and is essential for the
> diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
> index 7d0cf54125fa..70ad02088825 100644
> --- a/include/linux/cpufreq.h
> +++ b/include/linux/cpufreq.h
> @@ -390,6 +390,12 @@ struct cpufreq_driver {
> */
> #define CPUFREQ_NO_AUTO_DYNAMIC_SWITCHING (1 << 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 (1 << 7)
> +
> int cpufreq_register_driver(struct cpufreq_driver *driver_data);
> int cpufreq_unregister_driver(struct cpufreq_driver *driver_data);
Reviewed-by: Matthias Kaehlcke <[email protected]>
Tested-by: Matthias Kaehlcke <[email protected]>
On Mon, Jan 14, 2019 at 10:04:56PM +0530, Amit Kucheria wrote:
> Add the CPUFREQ_AUTO_REGISTER_COOLING_DEV flag to allow the cpufreq core
> to auto-register the driver as a cooling device.
>
> Signed-off-by: Amit Kucheria <[email protected]>
> ---
> drivers/cpufreq/qcom-cpufreq-hw.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c
> index d83939a1b3d4..ed32849a3d40 100644
> --- a/drivers/cpufreq/qcom-cpufreq-hw.c
> +++ b/drivers/cpufreq/qcom-cpufreq-hw.c
> @@ -231,7 +231,8 @@ static struct freq_attr *qcom_cpufreq_hw_attr[] = {
>
> static struct cpufreq_driver cpufreq_qcom_hw_driver = {
> .flags = CPUFREQ_STICKY | CPUFREQ_NEED_INITIAL_FREQ_CHECK |
> - CPUFREQ_HAVE_GOVERNOR_PER_POLICY,
> + CPUFREQ_HAVE_GOVERNOR_PER_POLICY |
> + CPUFREQ_AUTO_REGISTER_COOLING_DEV,
> .verify = cpufreq_generic_frequency_table_verify,
> .target_index = qcom_cpufreq_hw_target_index,
> .get = qcom_cpufreq_hw_get,
Reviewed-by: Matthias Kaehlcke <[email protected]>
Tested-by: Matthias Kaehlcke <[email protected]>
On Mon, Jan 14, 2019 at 10:05:01PM +0530, Amit Kucheria wrote:
> Use the CPUFREQ_AUTO_REGISTER_COOLING_DEV flag to allow cpufreq core to
> automatically register as a thermal cooling device.
>
> This allows removal of boiler plate code from the driver.
>
For this and the next patch(scpi),
Acked-by: Sudeep Holla <[email protected]>
--
Regards,
Sudeep
Quoting Amit Kucheria (2019-01-14 08:34:55)
> Minor clean-up to use BIT() and keep checkpatch happy. Clean up the
> comment formatting while we're at it to make it easier to read.
>
> Signed-off-by: Amit Kucheria <[email protected]>
> ---
Reviewed-by: Stephen Boyd <[email protected]>
On Monday, January 14, 2019 5:34:55 PM CET Amit Kucheria wrote:
> Minor clean-up to use BIT() and keep checkpatch happy. Clean up the
> comment formatting while we're at it to make it easier to read.
>
> Signed-off-by: Amit Kucheria <[email protected]>
Can you please do this cleanup as the first patch in the series, so it
doesn't depend on the other patches in it, which isn't necessary IMO?
> ---
> include/linux/cpufreq.h | 27 ++++++++++++++-------------
> 1 file changed, 14 insertions(+), 13 deletions(-)
>
> diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
> index 70ad02088825..ea303dfd5f05 100644
> --- a/include/linux/cpufreq.h
> +++ b/include/linux/cpufreq.h
> @@ -351,14 +351,15 @@ struct cpufreq_driver {
> };
>
> /* flags */
> -#define CPUFREQ_STICKY (1 << 0) /* driver isn't removed even if
> - all ->init() calls failed */
> -#define CPUFREQ_CONST_LOOPS (1 << 1) /* loops_per_jiffy or other
> - kernel "constants" aren't
> - affected by frequency
> - transitions */
> -#define CPUFREQ_PM_NO_WARN (1 << 2) /* don't warn on suspend/resume
> - speed mismatches */
> +
> +/* driver isn't removed even if all ->init() calls failed */
> +#define CPUFREQ_STICKY BIT(0)
> +
> +/* loops_per_jiffy or other kernel "constants" aren't affected by frequency transitions */
> +#define CPUFREQ_CONST_LOOPS BIT(1)
> +
> +/* don't warn on suspend/resume speed mismatches */
> +#define CPUFREQ_PM_NO_WARN BIT(2)
>
> /*
> * This should be set by platforms having multiple clock-domains, i.e.
> @@ -366,14 +367,14 @@ struct cpufreq_driver {
> * be created in cpu/cpu<num>/cpufreq/ directory and so they can use the same
> * governor with different tunables for different clusters.
> */
> -#define CPUFREQ_HAVE_GOVERNOR_PER_POLICY (1 << 3)
> +#define CPUFREQ_HAVE_GOVERNOR_PER_POLICY BIT(3)
>
> /*
> * Driver will do POSTCHANGE notifications from outside of their ->target()
> * routine and so must set cpufreq_driver->flags with this flag, so that core
> * can handle them specially.
> */
> -#define CPUFREQ_ASYNC_NOTIFICATION (1 << 4)
> +#define CPUFREQ_ASYNC_NOTIFICATION BIT(4)
>
> /*
> * Set by drivers which want cpufreq core to check if CPU is running at a
> @@ -382,19 +383,19 @@ struct cpufreq_driver {
> * from the table. And if that fails, we will stop further boot process by
> * issuing a BUG_ON().
> */
> -#define CPUFREQ_NEED_INITIAL_FREQ_CHECK (1 << 5)
> +#define CPUFREQ_NEED_INITIAL_FREQ_CHECK BIT(5)
>
> /*
> * Set by drivers to disallow use of governors with "dynamic_switching" flag
> * set.
> */
> -#define CPUFREQ_NO_AUTO_DYNAMIC_SWITCHING (1 << 6)
> +#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 (1 << 7)
> +#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);
>
On Monday, January 14, 2019 5:34:53 PM CET Amit Kucheria wrote:
> Several cpufreq drivers register themselves as thermal cooling devices.
> Adding a pointer to struct cpufreq_policy removes the need for them to
> store this pointer in a private data structure.
>
> We can then auto-register the cpufreq driver as a thermal cooling device
> from cpufreq core code.
>
> Signed-off-by: Amit Kucheria <[email protected]>
> ---
> include/linux/cpufreq.h | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
> index c86d6d8bdfed..7d0cf54125fa 100644
> --- a/include/linux/cpufreq.h
> +++ b/include/linux/cpufreq.h
> @@ -95,6 +95,11 @@ struct cpufreq_policy {
> struct cpufreq_frequency_table *freq_table;
> enum cpufreq_table_sorting freq_table_sorted;
>
> +#ifdef CONFIG_CPU_THERMAL
> + /* Pointer to the cooling device if used for thermal mitigation */
> + struct thermal_cooling_device *cooldev;
> +#endif
> +
Why here and not at the end of the struct?
> struct list_head policy_list;
> struct kobject kobj;
> struct completion kobj_unregister;
>
Also, I would suggest combining this one with patch [02/10].
On Monday, January 14, 2019 5:34:54 PM CET Amit Kucheria wrote:
> All cpufreq drivers do similar things to register as a cooling device.
> Provide a cpufreq driver flag so drivers can just ask the cpufreq core
> to register the cooling device on their behalf. This allows us to get
> rid of duplicated code in the drivers.
>
> Suggested-by: Stephen Boyd <[email protected]>
> Suggested-by: Viresh Kumar <[email protected]>
> Signed-off-by: Amit Kucheria <[email protected]>
> ---
> drivers/cpufreq/cpufreq.c | 17 +++++++++++++++++
> include/linux/cpufreq.h | 6 ++++++
> 2 files changed, 23 insertions(+)
>
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 6f23ebb395f1..cd6e750d3d82 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -30,6 +30,7 @@
> #include <linux/syscore_ops.h>
> #include <linux/tick.h>
> #include <trace/events/power.h>
> +#include <linux/cpu_cooling.h>
>
> static LIST_HEAD(cpufreq_policy_list);
>
> @@ -1318,6 +1319,14 @@ static int cpufreq_online(unsigned int cpu)
> if (cpufreq_driver->ready)
> cpufreq_driver->ready(policy);
>
> +#ifdef CONFIG_CPU_THERMAL
> + if (cpufreq_driver->flags & CPUFREQ_AUTO_REGISTER_COOLING_DEV) {
> + struct thermal_cooling_device **cdev = &policy->cooldev;
> +
> + *cdev = of_cpufreq_cooling_register(policy);
What would be wrong with
policy->cooldev = of_cpufreq_cooling_register(policy);
> + }
> +#endif
Please remove the #ifdefs from cpufreq_online() and cpufreq_offline().
Use wrappers that would become empty stubs for CONFIG_CPU_THERMAL unset.
> +
> pr_debug("initialization complete\n");
>
> return 0;
> @@ -1411,6 +1420,14 @@ static int cpufreq_offline(unsigned int cpu)
> if (has_target())
> cpufreq_exit_governor(policy);
>
> +#ifdef CONFIG_CPU_THERMAL
> + if (cpufreq_driver->flags & CPUFREQ_AUTO_REGISTER_COOLING_DEV) {
> + struct thermal_cooling_device **cdev = &policy->cooldev;
> +
> + cpufreq_cooling_unregister(*cdev);
Again, why don't you simply pass policy->cooldev here?
Also, would it make sense to clear policy->cooldev at this point? It points
to freed memory after cpufreq_cooling_unregister().
> + }
> +#endif
> +
> /*
> * Perform the ->exit() even during light-weight tear-down,
> * since this is a core component, and is essential for the
> diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
> index 7d0cf54125fa..70ad02088825 100644
> --- a/include/linux/cpufreq.h
> +++ b/include/linux/cpufreq.h
> @@ -390,6 +390,12 @@ struct cpufreq_driver {
> */
> #define CPUFREQ_NO_AUTO_DYNAMIC_SWITCHING (1 << 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 (1 << 7)
> +
> int cpufreq_register_driver(struct cpufreq_driver *driver_data);
> int cpufreq_unregister_driver(struct cpufreq_driver *driver_data);
>
>
On 14-01-19, 22:04, Amit Kucheria wrote:
> Use the CPUFREQ_AUTO_REGISTER_COOLING_DEV flag to allow cpufreq core to
> automatically register as a thermal cooling device.
>
> This allows removal of boiler plate code from the driver.
>
> Signed-off-by: Amit Kucheria <[email protected]>
> ---
> drivers/cpufreq/imx6q-cpufreq.c | 18 ++----------------
> 1 file changed, 2 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/cpufreq/imx6q-cpufreq.c b/drivers/cpufreq/imx6q-cpufreq.c
> index 9fedf627e000..ffb00291f48e 100644
> --- a/drivers/cpufreq/imx6q-cpufreq.c
> +++ b/drivers/cpufreq/imx6q-cpufreq.c
> @@ -9,7 +9,6 @@
> #include <linux/clk.h>
> #include <linux/cpu.h>
> #include <linux/cpufreq.h>
> -#include <linux/cpu_cooling.h>
> #include <linux/err.h>
> #include <linux/module.h>
> #include <linux/nvmem-consumer.h>
> @@ -52,7 +51,6 @@ static struct clk_bulk_data clks[] = {
> };
>
> static struct device *cpu_dev;
> -static struct thermal_cooling_device *cdev;
> static bool free_opp;
> static struct cpufreq_frequency_table *freq_table;
> static unsigned int max_freq;
> @@ -193,16 +191,6 @@ static int imx6q_set_target(struct cpufreq_policy *policy, unsigned int index)
> return 0;
> }
>
> -static void imx6q_cpufreq_ready(struct cpufreq_policy *policy)
> -{
> - cdev = of_cpufreq_cooling_register(policy);
> -
> - if (!cdev)
> - dev_err(cpu_dev,
> - "running cpufreq without cooling device: %ld\n",
> - PTR_ERR(cdev));
> -}
> -
> static int imx6q_cpufreq_init(struct cpufreq_policy *policy)
> {
> int ret;
> @@ -216,20 +204,18 @@ static int imx6q_cpufreq_init(struct cpufreq_policy *policy)
>
> static int imx6q_cpufreq_exit(struct cpufreq_policy *policy)
> {
> - cpufreq_cooling_unregister(cdev);
> -
> return 0;
> }
Remove the empty routine as well.
> static struct cpufreq_driver imx6q_cpufreq_driver = {
> - .flags = CPUFREQ_NEED_INITIAL_FREQ_CHECK,
> + .flags = CPUFREQ_NEED_INITIAL_FREQ_CHECK |
> + CPUFREQ_AUTO_REGISTER_COOLING_DEV,
> .verify = cpufreq_generic_frequency_table_verify,
> .target_index = imx6q_set_target,
> .get = cpufreq_generic_get,
> .init = imx6q_cpufreq_init,
> .exit = imx6q_cpufreq_exit,
> .name = "imx6q-cpufreq",
> - .ready = imx6q_cpufreq_ready,
> .attr = cpufreq_generic_attr,
> .suspend = cpufreq_generic_suspend,
> };
> --
> 2.17.1
--
viresh
On Thu, Jan 17, 2019 at 6:49 AM Viresh Kumar <[email protected]> wrote:
>
> On 14-01-19, 22:04, Amit Kucheria wrote:
> > Add a flag to be used by cpufreq drivers to tell cpufreq core to
> > auto-register themselves as a thermal cooling device.
> >
> > There series converts over all the drivers except arm_big_little.c.
> > Tested on SDM845 with the qcom-cpufreq-hw driver. Only compile-tested the
> > others.
> >
> > Things needing fixing:
> > - Look at how to detect that we're not in IKS mode in arm_big_little's
> > .ready callback.
>
> is_bL_switching_enabled() lets you know if IKS is enabled or not.
> Set/clear flag conditionally before the cpufreq-driver is registered,
> based on the output of is_bL_switching_enabled().
>
> > - The other pending issue is to fix allmodconfig that leaves us with
> > CPU_FREQ=y and THERMAL=m (CPU_THERMAL=y). That leads to undefined
> > references for functions defined in cpu_cooling.c
>
> Okay, that's a terrible thing and the solution looks to be rather
> difficult.
>
> For others who may not be aware of the issue here, currently the
> cpufreq drivers use helpers of cpu_cooling.c (CONFIG_CPU_THERMAL),
> which uses helpers of the thermal core (CONFIG_THERMAL).
> CONFIG_THERMAL is defined as tristate and CONFIG_CPU_THERMAL as bool
> in Kconfigs.
And CONFIG_CPU_THERMAL is defined under "if THERMAL".
> The cpufreq drivers using the cpu_cooling.c file have this in their
> Kconfig entry:
>
> # if CPU_THERMAL is on and THERMAL=m, ARM_BIT_LITTLE_CPUFREQ cannot be =y
> # depends on !CPU_THERMAL || THERMAL
>
>
> This series now places the cpufreq core in place of the cpufreq driver
> and it messes up everything. It is not just about allmodconfig, but
> any configuration which makes the compilation fail.
>
> What are the solutions we have now ?
>
> 1. Have following for CONFIG_CPU_FREQ
> depends on !CPU_THERMAL || THERMAL
Sorry, but this makes my teeth hurt.
> The platforms which don't need CPU_THERMAL (like x86) should not
> enable CPU_THERMAL anymore if they want CONFIG_THERMAL=m.
>
> @amit: If this gets accepted, please update the Kconfig entries for
> all those drivers to not have above lines anymore.
>
> - Change CONFIG_THERMAL to bool instead of tristate ?
>
> - Anything else ?
The design in the thermal subsystem seems to be upside-down.
Non-modular code should never be made depend on anything only defined
in a module.
Would an explicit "select THERMAL" under CPU_THERMAL cause THERMAL to be 'y'?
On 17-01-19, 11:08, Rafael J. Wysocki wrote:
> On Thu, Jan 17, 2019 at 6:49 AM Viresh Kumar <[email protected]> wrote:
> > 1. Have following for CONFIG_CPU_FREQ
> > depends on !CPU_THERMAL || THERMAL
>
> Sorry, but this makes my teeth hurt.
I knew it :)
> > The platforms which don't need CPU_THERMAL (like x86) should not
> > enable CPU_THERMAL anymore if they want CONFIG_THERMAL=m.
> >
> > @amit: If this gets accepted, please update the Kconfig entries for
> > all those drivers to not have above lines anymore.
> >
> > - Change CONFIG_THERMAL to bool instead of tristate ?
> >
> > - Anything else ?
>
> The design in the thermal subsystem seems to be upside-down.
> Non-modular code should never be made depend on anything only defined
> in a module.
>
> Would an explicit "select THERMAL" under CPU_THERMAL cause THERMAL to be 'y'?
That causes recursive-dependency issues:
drivers/thermal/Kconfig:5:error: recursive dependency detected!
drivers/thermal/Kconfig:5: symbol THERMAL is selected by CPU_THERMAL
drivers/thermal/Kconfig:16: symbol CPU_THERMAL depends on THERMAL_OF
drivers/thermal/Kconfig:70: symbol THERMAL_OF depends on THERMAL
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
something like this works though:
diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index 30323426902e..ee9f9f2a795b 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -13,6 +13,20 @@ menuconfig THERMAL
All platforms with ACPI thermal support can use this driver.
If you want this support, you should say Y or M here.
+config CPU_THERMAL
+ bool "generic cpu cooling support"
+ depends on CPU_FREQ
+ select THERMAL_OF
+ select THERMAL
+ help
+ This implements the generic cpu cooling mechanism through frequency
+ reduction. An ACPI version of this already exists
+ (drivers/acpi/processor_thermal.c).
+ This will be useful for platforms using the generic thermal interface
+ and not the ACPI interface.
+
+ If you want this support, you should say Y here.
+
if THERMAL
config THERMAL_STATISTICS
@@ -148,19 +162,6 @@ config THERMAL_GOV_POWER_ALLOCATOR
Enable this to manage platform thermals by dynamically
allocating and limiting power to devices.
-config CPU_THERMAL
- bool "generic cpu cooling support"
- depends on CPU_FREQ
- depends on THERMAL_OF
- help
- This implements the generic cpu cooling mechanism through frequency
- reduction. An ACPI version of this already exists
- (drivers/acpi/processor_thermal.c).
- This will be useful for platforms using the generic thermal interface
- and not the ACPI interface.
-
- If you want this support, you should say Y here.
-
config CLOCK_THERMAL
bool "Generic clock cooling support"
depends on COMMON_CLK
What about make CONFIG_THERMAL bool instead ? Who wants it to be a module ?
$ git grep "CONFIG_THERMAL=m"
arch/arm/configs/mini2440_defconfig:CONFIG_THERMAL=m
arch/arm/configs/omap2plus_defconfig:CONFIG_THERMAL=m
arch/arm/configs/pxa_defconfig:CONFIG_THERMAL=m
arch/mips/configs/ip22_defconfig:CONFIG_THERMAL=m
arch/mips/configs/ip27_defconfig:CONFIG_THERMAL=m
arch/unicore32/configs/unicore32_defconfig:#CONFIG_THERMAL=m
Not sure if they really want this code out :(
--
viresh
On 17-01-19, 11:14, Rafael J. Wysocki wrote:
> Alternatively, "depends on THERMAL=y" under CPU_THERMAL should work
> too, shouldn't it? "!CPU_THERMAL || THERMAL" would be always true
> then, AFAICS.
That works and this is the easiest of the changes to make :)
--
viresh
On Thu, Jan 17, 2019 at 11:21 AM Viresh Kumar <[email protected]> wrote:
>
> On 17-01-19, 11:08, Rafael J. Wysocki wrote:
> > On Thu, Jan 17, 2019 at 6:49 AM Viresh Kumar <[email protected]> wrote:
> > > 1. Have following for CONFIG_CPU_FREQ
> > > depends on !CPU_THERMAL || THERMAL
> >
> > Sorry, but this makes my teeth hurt.
>
> I knew it :)
>
> > > The platforms which don't need CPU_THERMAL (like x86) should not
> > > enable CPU_THERMAL anymore if they want CONFIG_THERMAL=m.
> > >
> > > @amit: If this gets accepted, please update the Kconfig entries for
> > > all those drivers to not have above lines anymore.
> > >
> > > - Change CONFIG_THERMAL to bool instead of tristate ?
> > >
> > > - Anything else ?
> >
> > The design in the thermal subsystem seems to be upside-down.
> > Non-modular code should never be made depend on anything only defined
> > in a module.
> >
> > Would an explicit "select THERMAL" under CPU_THERMAL cause THERMAL to be 'y'?
>
> That causes recursive-dependency issues:
>
> drivers/thermal/Kconfig:5:error: recursive dependency detected!
> drivers/thermal/Kconfig:5: symbol THERMAL is selected by CPU_THERMAL
> drivers/thermal/Kconfig:16: symbol CPU_THERMAL depends on THERMAL_OF
> drivers/thermal/Kconfig:70: symbol THERMAL_OF depends on THERMAL
> For a resolution refer to Documentation/kbuild/kconfig-language.txt
> subsection "Kconfig recursive dependency limitations"
>
> something like this works though:
>
> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> index 30323426902e..ee9f9f2a795b 100644
> --- a/drivers/thermal/Kconfig
> +++ b/drivers/thermal/Kconfig
> @@ -13,6 +13,20 @@ menuconfig THERMAL
> All platforms with ACPI thermal support can use this driver.
> If you want this support, you should say Y or M here.
>
> +config CPU_THERMAL
> + bool "generic cpu cooling support"
> + depends on CPU_FREQ
> + select THERMAL_OF
> + select THERMAL
> + help
> + This implements the generic cpu cooling mechanism through frequency
> + reduction. An ACPI version of this already exists
> + (drivers/acpi/processor_thermal.c).
> + This will be useful for platforms using the generic thermal interface
> + and not the ACPI interface.
> +
> + If you want this support, you should say Y here.
> +
My sort of educated guess is that it is under the "if THERMAL" so that
it doesn't show up at all when THERMAL is unset.
> if THERMAL
>
> config THERMAL_STATISTICS
> @@ -148,19 +162,6 @@ config THERMAL_GOV_POWER_ALLOCATOR
> Enable this to manage platform thermals by dynamically
> allocating and limiting power to devices.
>
> -config CPU_THERMAL
> - bool "generic cpu cooling support"
> - depends on CPU_FREQ
But what about adding
depends on THERMAL=y
here as I said in the other message?
> - depends on THERMAL_OF
> - help
> - This implements the generic cpu cooling mechanism through frequency
> - reduction. An ACPI version of this already exists
> - (drivers/acpi/processor_thermal.c).
> - This will be useful for platforms using the generic thermal interface
> - and not the ACPI interface.
> -
> - If you want this support, you should say Y here.
> -
> config CLOCK_THERMAL
> bool "Generic clock cooling support"
> depends on COMMON_CLK
>
>
> What about make CONFIG_THERMAL bool instead ? Who wants it to be a module ?
>
> $ git grep "CONFIG_THERMAL=m"
> arch/arm/configs/mini2440_defconfig:CONFIG_THERMAL=m
> arch/arm/configs/omap2plus_defconfig:CONFIG_THERMAL=m
> arch/arm/configs/pxa_defconfig:CONFIG_THERMAL=m
> arch/mips/configs/ip22_defconfig:CONFIG_THERMAL=m
> arch/mips/configs/ip27_defconfig:CONFIG_THERMAL=m
> arch/unicore32/configs/unicore32_defconfig:#CONFIG_THERMAL=m
>
> Not sure if they really want this code out :(
Me neither.
On Thu, Jan 17, 2019 at 11:28 AM Viresh Kumar <[email protected]> wrote:
>
> On 17-01-19, 11:14, Rafael J. Wysocki wrote:
> > Alternatively, "depends on THERMAL=y" under CPU_THERMAL should work
> > too, shouldn't it? "!CPU_THERMAL || THERMAL" would be always true
> > then, AFAICS.
>
> That works and this is the easiest of the changes to make :)
Let's do that, then, I think.
On Thu, Jan 17, 2019 at 11:08 AM Rafael J. Wysocki <[email protected]> wrote:
>
> On Thu, Jan 17, 2019 at 6:49 AM Viresh Kumar <[email protected]> wrote:
> >
> > On 14-01-19, 22:04, Amit Kucheria wrote:
> > > Add a flag to be used by cpufreq drivers to tell cpufreq core to
> > > auto-register themselves as a thermal cooling device.
> > >
> > > There series converts over all the drivers except arm_big_little.c.
> > > Tested on SDM845 with the qcom-cpufreq-hw driver. Only compile-tested the
> > > others.
> > >
> > > Things needing fixing:
> > > - Look at how to detect that we're not in IKS mode in arm_big_little's
> > > .ready callback.
> >
> > is_bL_switching_enabled() lets you know if IKS is enabled or not.
> > Set/clear flag conditionally before the cpufreq-driver is registered,
> > based on the output of is_bL_switching_enabled().
> >
> > > - The other pending issue is to fix allmodconfig that leaves us with
> > > CPU_FREQ=y and THERMAL=m (CPU_THERMAL=y). That leads to undefined
> > > references for functions defined in cpu_cooling.c
> >
> > Okay, that's a terrible thing and the solution looks to be rather
> > difficult.
> >
> > For others who may not be aware of the issue here, currently the
> > cpufreq drivers use helpers of cpu_cooling.c (CONFIG_CPU_THERMAL),
> > which uses helpers of the thermal core (CONFIG_THERMAL).
> > CONFIG_THERMAL is defined as tristate and CONFIG_CPU_THERMAL as bool
> > in Kconfigs.
>
> And CONFIG_CPU_THERMAL is defined under "if THERMAL".
>
> > The cpufreq drivers using the cpu_cooling.c file have this in their
> > Kconfig entry:
> >
> > # if CPU_THERMAL is on and THERMAL=m, ARM_BIT_LITTLE_CPUFREQ cannot be =y
> > # depends on !CPU_THERMAL || THERMAL
> >
> >
> > This series now places the cpufreq core in place of the cpufreq driver
> > and it messes up everything. It is not just about allmodconfig, but
> > any configuration which makes the compilation fail.
> >
> > What are the solutions we have now ?
> >
> > 1. Have following for CONFIG_CPU_FREQ
> > depends on !CPU_THERMAL || THERMAL
>
> Sorry, but this makes my teeth hurt.
>
> > The platforms which don't need CPU_THERMAL (like x86) should not
> > enable CPU_THERMAL anymore if they want CONFIG_THERMAL=m.
> >
> > @amit: If this gets accepted, please update the Kconfig entries for
> > all those drivers to not have above lines anymore.
> >
> > - Change CONFIG_THERMAL to bool instead of tristate ?
> >
> > - Anything else ?
>
> The design in the thermal subsystem seems to be upside-down.
> Non-modular code should never be made depend on anything only defined
> in a module.
>
> Would an explicit "select THERMAL" under CPU_THERMAL cause THERMAL to be 'y'?
Alternatively, "depends on THERMAL=y" under CPU_THERMAL should work
too, shouldn't it? "!CPU_THERMAL || THERMAL" would be always true
then, AFAICS.
On 17-01-19, 00:03, Rafael J. Wysocki wrote:
> On Monday, January 14, 2019 5:34:54 PM CET Amit Kucheria wrote:
> > All cpufreq drivers do similar things to register as a cooling device.
> > Provide a cpufreq driver flag so drivers can just ask the cpufreq core
> > to register the cooling device on their behalf. This allows us to get
> > rid of duplicated code in the drivers.
> >
> > Suggested-by: Stephen Boyd <[email protected]>
> > Suggested-by: Viresh Kumar <[email protected]>
> > Signed-off-by: Amit Kucheria <[email protected]>
> > ---
> > drivers/cpufreq/cpufreq.c | 17 +++++++++++++++++
> > include/linux/cpufreq.h | 6 ++++++
> > 2 files changed, 23 insertions(+)
> >
> > diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> > index 6f23ebb395f1..cd6e750d3d82 100644
> > --- a/drivers/cpufreq/cpufreq.c
> > +++ b/drivers/cpufreq/cpufreq.c
> > @@ -30,6 +30,7 @@
> > #include <linux/syscore_ops.h>
> > #include <linux/tick.h>
> > #include <trace/events/power.h>
> > +#include <linux/cpu_cooling.h>
> >
> > static LIST_HEAD(cpufreq_policy_list);
> >
> > @@ -1318,6 +1319,14 @@ static int cpufreq_online(unsigned int cpu)
> > if (cpufreq_driver->ready)
> > cpufreq_driver->ready(policy);
> >
> > +#ifdef CONFIG_CPU_THERMAL
> > + if (cpufreq_driver->flags & CPUFREQ_AUTO_REGISTER_COOLING_DEV) {
> > + struct thermal_cooling_device **cdev = &policy->cooldev;
We use cdev for the cooling device everywhere in the kernel, so please
do s/cooldev/cdev/ in your patches.
> > +
> > + *cdev = of_cpufreq_cooling_register(policy);
>
> What would be wrong with
>
> policy->cooldev = of_cpufreq_cooling_register(policy);
>
> > + }
> > +#endif
>
> Please remove the #ifdefs from cpufreq_online() and cpufreq_offline().
>
> Use wrappers that would become empty stubs for CONFIG_CPU_THERMAL unset.
>
> > +
> > pr_debug("initialization complete\n");
> >
> > return 0;
> > @@ -1411,6 +1420,14 @@ static int cpufreq_offline(unsigned int cpu)
> > if (has_target())
> > cpufreq_exit_governor(policy);
> >
> > +#ifdef CONFIG_CPU_THERMAL
> > + if (cpufreq_driver->flags & CPUFREQ_AUTO_REGISTER_COOLING_DEV) {
> > + struct thermal_cooling_device **cdev = &policy->cooldev;
> > +
> > + cpufreq_cooling_unregister(*cdev);
>
> Again, why don't you simply pass policy->cooldev here?
I also had the same comments when I looked at your patch :)
I also think we must do the unregistering before calling stop_cpu()
callback.
> Also, would it make sense to clear policy->cooldev at this point? It points
> to freed memory after cpufreq_cooling_unregister().
Since the core doesn't refer to this field at all and uses it only
while registering/unregistering as a cooling device, there is no
technical issue that we will have today. If someone uses the dangling
pointer later on in future, it will be a bug. So I wouldn't care much
about resetting it here.
> > + }
> > +#endif
> > +
> > /*
> > * Perform the ->exit() even during light-weight tear-down,
> > * since this is a core component, and is essential for the
> > diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
> > index 7d0cf54125fa..70ad02088825 100644
> > --- a/include/linux/cpufreq.h
> > +++ b/include/linux/cpufreq.h
> > @@ -390,6 +390,12 @@ struct cpufreq_driver {
> > */
> > #define CPUFREQ_NO_AUTO_DYNAMIC_SWITCHING (1 << 6)
> >
> > +/*
> > + * Set by drivers that want the core to automatically register the cpufreq
> > + * driver as a thermal cooling device
Add a full-stop here please.
> > + */
> > +#define CPUFREQ_AUTO_REGISTER_COOLING_DEV (1 << 7)
> > +
> > int cpufreq_register_driver(struct cpufreq_driver *driver_data);
> > int cpufreq_unregister_driver(struct cpufreq_driver *driver_data);
> >
> >
>
--
viresh
On 14-01-19, 22:04, Amit Kucheria wrote:
> Add a flag to be used by cpufreq drivers to tell cpufreq core to
> auto-register themselves as a thermal cooling device.
>
> There series converts over all the drivers except arm_big_little.c.
> Tested on SDM845 with the qcom-cpufreq-hw driver. Only compile-tested the
> others.
>
> Things needing fixing:
> - Look at how to detect that we're not in IKS mode in arm_big_little's
> .ready callback.
is_bL_switching_enabled() lets you know if IKS is enabled or not.
Set/clear flag conditionally before the cpufreq-driver is registered,
based on the output of is_bL_switching_enabled().
> - The other pending issue is to fix allmodconfig that leaves us with
> CPU_FREQ=y and THERMAL=m (CPU_THERMAL=y). That leads to undefined
> references for functions defined in cpu_cooling.c
Okay, that's a terrible thing and the solution looks to be rather
difficult.
For others who may not be aware of the issue here, currently the
cpufreq drivers use helpers of cpu_cooling.c (CONFIG_CPU_THERMAL),
which uses helpers of the thermal core (CONFIG_THERMAL).
CONFIG_THERMAL is defined as tristate and CONFIG_CPU_THERMAL as bool
in Kconfigs.
The cpufreq drivers using the cpu_cooling.c file have this in their
Kconfig entry:
# if CPU_THERMAL is on and THERMAL=m, ARM_BIT_LITTLE_CPUFREQ cannot be =y
# depends on !CPU_THERMAL || THERMAL
This series now places the cpufreq core in place of the cpufreq driver
and it messes up everything. It is not just about allmodconfig, but
any configuration which makes the compilation fail.
What are the solutions we have now ?
1. Have following for CONFIG_CPU_FREQ
depends on !CPU_THERMAL || THERMAL
The platforms which don't need CPU_THERMAL (like x86) should not
enable CPU_THERMAL anymore if they want CONFIG_THERMAL=m.
@amit: If this gets accepted, please update the Kconfig entries for
all those drivers to not have above lines anymore.
- Change CONFIG_THERMAL to bool instead of tristate ?
- Anything else ?
--
viresh
On Thu, Jan 17, 2019 at 4:26 AM Rafael J. Wysocki <[email protected]> wrote:
>
> On Monday, January 14, 2019 5:34:55 PM CET Amit Kucheria wrote:
> > Minor clean-up to use BIT() and keep checkpatch happy. Clean up the
> > comment formatting while we're at it to make it easier to read.
> >
> > Signed-off-by: Amit Kucheria <[email protected]>
>
> Can you please do this cleanup as the first patch in the series, so it
> doesn't depend on the other patches in it, which isn't necessary IMO?
I've sent it out separately now, unconnected to the series.
Thanks.
Regards,
Amit
> > ---
> > include/linux/cpufreq.h | 27 ++++++++++++++-------------
> > 1 file changed, 14 insertions(+), 13 deletions(-)
> >
> > diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
> > index 70ad02088825..ea303dfd5f05 100644
> > --- a/include/linux/cpufreq.h
> > +++ b/include/linux/cpufreq.h
> > @@ -351,14 +351,15 @@ struct cpufreq_driver {
> > };
> >
> > /* flags */
> > -#define CPUFREQ_STICKY (1 << 0) /* driver isn't removed even if
> > - all ->init() calls failed */
> > -#define CPUFREQ_CONST_LOOPS (1 << 1) /* loops_per_jiffy or other
> > - kernel "constants" aren't
> > - affected by frequency
> > - transitions */
> > -#define CPUFREQ_PM_NO_WARN (1 << 2) /* don't warn on suspend/resume
> > - speed mismatches */
> > +
> > +/* driver isn't removed even if all ->init() calls failed */
> > +#define CPUFREQ_STICKY BIT(0)
> > +
> > +/* loops_per_jiffy or other kernel "constants" aren't affected by frequency transitions */
> > +#define CPUFREQ_CONST_LOOPS BIT(1)
> > +
> > +/* don't warn on suspend/resume speed mismatches */
> > +#define CPUFREQ_PM_NO_WARN BIT(2)
> >
> > /*
> > * This should be set by platforms having multiple clock-domains, i.e.
> > @@ -366,14 +367,14 @@ struct cpufreq_driver {
> > * be created in cpu/cpu<num>/cpufreq/ directory and so they can use the same
> > * governor with different tunables for different clusters.
> > */
> > -#define CPUFREQ_HAVE_GOVERNOR_PER_POLICY (1 << 3)
> > +#define CPUFREQ_HAVE_GOVERNOR_PER_POLICY BIT(3)
> >
> > /*
> > * Driver will do POSTCHANGE notifications from outside of their ->target()
> > * routine and so must set cpufreq_driver->flags with this flag, so that core
> > * can handle them specially.
> > */
> > -#define CPUFREQ_ASYNC_NOTIFICATION (1 << 4)
> > +#define CPUFREQ_ASYNC_NOTIFICATION BIT(4)
> >
> > /*
> > * Set by drivers which want cpufreq core to check if CPU is running at a
> > @@ -382,19 +383,19 @@ struct cpufreq_driver {
> > * from the table. And if that fails, we will stop further boot process by
> > * issuing a BUG_ON().
> > */
> > -#define CPUFREQ_NEED_INITIAL_FREQ_CHECK (1 << 5)
> > +#define CPUFREQ_NEED_INITIAL_FREQ_CHECK BIT(5)
> >
> > /*
> > * Set by drivers to disallow use of governors with "dynamic_switching" flag
> > * set.
> > */
> > -#define CPUFREQ_NO_AUTO_DYNAMIC_SWITCHING (1 << 6)
> > +#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 (1 << 7)
> > +#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);
> >
>
>
On Thu, Jan 17, 2019 at 10:12 AM Viresh Kumar <[email protected]> wrote:
>
> On 17-01-19, 00:03, Rafael J. Wysocki wrote:
> > On Monday, January 14, 2019 5:34:54 PM CET Amit Kucheria wrote:
> > > All cpufreq drivers do similar things to register as a cooling device.
> > > Provide a cpufreq driver flag so drivers can just ask the cpufreq core
> > > to register the cooling device on their behalf. This allows us to get
> > > rid of duplicated code in the drivers.
> > >
> > > Suggested-by: Stephen Boyd <[email protected]>
> > > Suggested-by: Viresh Kumar <[email protected]>
> > > Signed-off-by: Amit Kucheria <[email protected]>
> > > ---
> > > drivers/cpufreq/cpufreq.c | 17 +++++++++++++++++
> > > include/linux/cpufreq.h | 6 ++++++
> > > 2 files changed, 23 insertions(+)
> > >
> > > diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> > > index 6f23ebb395f1..cd6e750d3d82 100644
> > > --- a/drivers/cpufreq/cpufreq.c
> > > +++ b/drivers/cpufreq/cpufreq.c
> > > @@ -30,6 +30,7 @@
> > > #include <linux/syscore_ops.h>
> > > #include <linux/tick.h>
> > > #include <trace/events/power.h>
> > > +#include <linux/cpu_cooling.h>
> > >
> > > static LIST_HEAD(cpufreq_policy_list);
> > >
> > > @@ -1318,6 +1319,14 @@ static int cpufreq_online(unsigned int cpu)
> > > if (cpufreq_driver->ready)
> > > cpufreq_driver->ready(policy);
> > >
> > > +#ifdef CONFIG_CPU_THERMAL
> > > + if (cpufreq_driver->flags & CPUFREQ_AUTO_REGISTER_COOLING_DEV) {
> > > + struct thermal_cooling_device **cdev = &policy->cooldev;
>
> We use cdev for the cooling device everywhere in the kernel, so please
> do s/cooldev/cdev/ in your patches.
Fixed
> > > +
> > > + *cdev = of_cpufreq_cooling_register(policy);
> >
> > What would be wrong with
> >
> > policy->cooldev = of_cpufreq_cooling_register(policy);
> >
> > > + }
> > > +#endif
> >
> > Please remove the #ifdefs from cpufreq_online() and cpufreq_offline().
> >
> > Use wrappers that would become empty stubs for CONFIG_CPU_THERMAL unset.
> >
> > > +
> > > pr_debug("initialization complete\n");
> > >
> > > return 0;
> > > @@ -1411,6 +1420,14 @@ static int cpufreq_offline(unsigned int cpu)
> > > if (has_target())
> > > cpufreq_exit_governor(policy);
> > >
> > > +#ifdef CONFIG_CPU_THERMAL
> > > + if (cpufreq_driver->flags & CPUFREQ_AUTO_REGISTER_COOLING_DEV) {
> > > + struct thermal_cooling_device **cdev = &policy->cooldev;
> > > +
> > > + cpufreq_cooling_unregister(*cdev);
> >
> > Again, why don't you simply pass policy->cooldev here?
>
> I also had the same comments when I looked at your patch :)
>
> I also think we must do the unregistering before calling stop_cpu()
> callback.
Fixed.
> > Also, would it make sense to clear policy->cooldev at this point? It points
> > to freed memory after cpufreq_cooling_unregister().
>
> Since the core doesn't refer to this field at all and uses it only
> while registering/unregistering as a cooling device, there is no
> technical issue that we will have today. If someone uses the dangling
> pointer later on in future, it will be a bug. So I wouldn't care much
> about resetting it here.
>
> > > + }
> > > +#endif
> > > +
> > > /*
> > > * Perform the ->exit() even during light-weight tear-down,
> > > * since this is a core component, and is essential for the
> > > diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
> > > index 7d0cf54125fa..70ad02088825 100644
> > > --- a/include/linux/cpufreq.h
> > > +++ b/include/linux/cpufreq.h
> > > @@ -390,6 +390,12 @@ struct cpufreq_driver {
> > > */
> > > #define CPUFREQ_NO_AUTO_DYNAMIC_SWITCHING (1 << 6)
> > >
> > > +/*
> > > + * Set by drivers that want the core to automatically register the cpufreq
> > > + * driver as a thermal cooling device
>
> Add a full-stop here please.
Fixed
Thanks for the review.
> > > + */
> > > +#define CPUFREQ_AUTO_REGISTER_COOLING_DEV (1 << 7)
> > > +
> > > int cpufreq_register_driver(struct cpufreq_driver *driver_data);
> > > int cpufreq_unregister_driver(struct cpufreq_driver *driver_data);
> > >
On Thu, Jan 17, 2019 at 4:27 AM Rafael J. Wysocki <[email protected]> wrote:
>
> On Monday, January 14, 2019 5:34:53 PM CET Amit Kucheria wrote:
> > Several cpufreq drivers register themselves as thermal cooling devices.
> > Adding a pointer to struct cpufreq_policy removes the need for them to
> > store this pointer in a private data structure.
> >
> > We can then auto-register the cpufreq driver as a thermal cooling device
> > from cpufreq core code.
> >
> > Signed-off-by: Amit Kucheria <[email protected]>
> > ---
> > include/linux/cpufreq.h | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
> > index c86d6d8bdfed..7d0cf54125fa 100644
> > --- a/include/linux/cpufreq.h
> > +++ b/include/linux/cpufreq.h
> > @@ -95,6 +95,11 @@ struct cpufreq_policy {
> > struct cpufreq_frequency_table *freq_table;
> > enum cpufreq_table_sorting freq_table_sorted;
> >
> > +#ifdef CONFIG_CPU_THERMAL
> > + /* Pointer to the cooling device if used for thermal mitigation */
> > + struct thermal_cooling_device *cooldev;
> > +#endif
> > +
>
> Why here and not at the end of the struct?
Fixed
> > struct list_head policy_list;
> > struct kobject kobj;
> > struct completion kobj_unregister;
> >
>
> Also, I would suggest combining this one with patch [02/10].
Will do.
Thanks for the review.