linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1 00/10] cpufreq: Add flag to auto-register as cooling device
@ 2019-01-14 16:34 Amit Kucheria
  2019-01-14 16:34 ` [PATCH v1 07/10] cpufreq: mediatek: Use auto-registration of thermal " Amit Kucheria
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Amit Kucheria @ 2019-01-14 16:34 UTC (permalink / raw)
  To: linux-kernel
  Cc: open list:CPU FREQUENCY DRIVERS, linux-arm-msm, Sudeep Holla,
	Rafael J. Wysocki, dianders, swboyd, edubezval, mka,
	moderated list:ARM/Mediatek SoC support, viresh.kumar,
	Matthias Brugger, moderated list:ARM/Mediatek SoC support

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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH v1 07/10] cpufreq: mediatek: Use auto-registration of thermal cooling device
  2019-01-14 16:34 [PATCH v1 00/10] cpufreq: Add flag to auto-register as cooling device Amit Kucheria
@ 2019-01-14 16:34 ` Amit Kucheria
  2019-01-14 16:35 ` [PATCH v1 09/10] cpufreq: scmi: " Amit Kucheria
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 12+ messages in thread
From: Amit Kucheria @ 2019-01-14 16:34 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-pm, linux-arm-msm, Sean Wang, Rafael J. Wysocki, dianders,
	swboyd, edubezval, mka, linux-mediatek, viresh.kumar,
	Matthias Brugger, linux-arm-kernel

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 <amit.kucheria@linaro.org>
---
 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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH v1 09/10] cpufreq: scmi: Use auto-registration of thermal cooling device
  2019-01-14 16:34 [PATCH v1 00/10] cpufreq: Add flag to auto-register as cooling device Amit Kucheria
  2019-01-14 16:34 ` [PATCH v1 07/10] cpufreq: mediatek: Use auto-registration of thermal " Amit Kucheria
@ 2019-01-14 16:35 ` Amit Kucheria
  2019-01-15 10:37   ` Sudeep Holla
  2019-01-14 16:35 ` [PATCH v1 10/10] cpufreq: scpi: " Amit Kucheria
  2019-01-17  5:49 ` [PATCH v1 00/10] cpufreq: Add flag to auto-register as " Viresh Kumar
  3 siblings, 1 reply; 12+ messages in thread
From: Amit Kucheria @ 2019-01-14 16:35 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-pm, linux-arm-msm, Sudeep Holla, Rafael J. Wysocki,
	dianders, swboyd, edubezval, mka, viresh.kumar, linux-arm-kernel

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 <amit.kucheria@linaro.org>
---
 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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH v1 10/10] cpufreq: scpi: Use auto-registration of thermal cooling device
  2019-01-14 16:34 [PATCH v1 00/10] cpufreq: Add flag to auto-register as cooling device Amit Kucheria
  2019-01-14 16:34 ` [PATCH v1 07/10] cpufreq: mediatek: Use auto-registration of thermal " Amit Kucheria
  2019-01-14 16:35 ` [PATCH v1 09/10] cpufreq: scmi: " Amit Kucheria
@ 2019-01-14 16:35 ` Amit Kucheria
  2019-01-17  5:49 ` [PATCH v1 00/10] cpufreq: Add flag to auto-register as " Viresh Kumar
  3 siblings, 0 replies; 12+ messages in thread
From: Amit Kucheria @ 2019-01-14 16:35 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-pm, linux-arm-msm, Sudeep Holla, Rafael J. Wysocki,
	dianders, swboyd, edubezval, mka, viresh.kumar, linux-arm-kernel

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 <amit.kucheria@linaro.org>
---
 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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v1 09/10] cpufreq: scmi: Use auto-registration of thermal cooling device
  2019-01-14 16:35 ` [PATCH v1 09/10] cpufreq: scmi: " Amit Kucheria
@ 2019-01-15 10:37   ` Sudeep Holla
  0 siblings, 0 replies; 12+ messages in thread
From: Sudeep Holla @ 2019-01-15 10:37 UTC (permalink / raw)
  To: Amit Kucheria
  Cc: dianders, linux-pm, linux-arm-msm, Sudeep Holla,
	Rafael J. Wysocki, linux-kernel, swboyd, edubezval, mka,
	viresh.kumar, linux-arm-kernel

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 <sudeep.holla@arm.com>

--
Regards,
Sudeep

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v1 00/10] cpufreq: Add flag to auto-register as cooling device
  2019-01-14 16:34 [PATCH v1 00/10] cpufreq: Add flag to auto-register as cooling device Amit Kucheria
                   ` (2 preceding siblings ...)
  2019-01-14 16:35 ` [PATCH v1 10/10] cpufreq: scpi: " Amit Kucheria
@ 2019-01-17  5:49 ` Viresh Kumar
  2019-01-17 10:08   ` Rafael J. Wysocki
  3 siblings, 1 reply; 12+ messages in thread
From: Viresh Kumar @ 2019-01-17  5:49 UTC (permalink / raw)
  To: Amit Kucheria
  Cc: open list:CPU FREQUENCY DRIVERS, linux-arm-msm,
	Rafael J. Wysocki, dianders, linux-kernel, edubezval, mka,
	moderated list:ARM/Mediatek SoC support, Sudeep Holla,
	Matthias Brugger, swboyd,
	moderated list:ARM/Mediatek SoC support

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v1 00/10] cpufreq: Add flag to auto-register as cooling device
  2019-01-17  5:49 ` [PATCH v1 00/10] cpufreq: Add flag to auto-register as " Viresh Kumar
@ 2019-01-17 10:08   ` Rafael J. Wysocki
  2019-01-17 10:14     ` Rafael J. Wysocki
  2019-01-17 10:21     ` Viresh Kumar
  0 siblings, 2 replies; 12+ messages in thread
From: Rafael J. Wysocki @ 2019-01-17 10:08 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: open list:CPU FREQUENCY DRIVERS, linux-arm-msm,
	Rafael J. Wysocki, Doug Anderson, Amit Kucheria,
	Linux Kernel Mailing List, Eduardo Valentin, Matthias Kaehlcke,
	moderated list:ARM/Mediatek SoC support, Sudeep Holla,
	Matthias Brugger, Stephen Boyd,
	moderated list:ARM/Mediatek SoC support

On Thu, Jan 17, 2019 at 6:49 AM Viresh Kumar <viresh.kumar@linaro.org> 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'?

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v1 00/10] cpufreq: Add flag to auto-register as cooling device
  2019-01-17 10:08   ` Rafael J. Wysocki
@ 2019-01-17 10:14     ` Rafael J. Wysocki
  2019-01-17 10:28       ` Viresh Kumar
  2019-01-17 10:21     ` Viresh Kumar
  1 sibling, 1 reply; 12+ messages in thread
From: Rafael J. Wysocki @ 2019-01-17 10:14 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: open list:CPU FREQUENCY DRIVERS, linux-arm-msm,
	Rafael J. Wysocki, Doug Anderson, Amit Kucheria,
	Linux Kernel Mailing List, Eduardo Valentin, Matthias Kaehlcke,
	moderated list:ARM/Mediatek SoC support, Sudeep Holla,
	Matthias Brugger, Stephen Boyd,
	moderated list:ARM/Mediatek SoC support

On Thu, Jan 17, 2019 at 11:08 AM Rafael J. Wysocki <rafael@kernel.org> wrote:
>
> On Thu, Jan 17, 2019 at 6:49 AM Viresh Kumar <viresh.kumar@linaro.org> 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.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v1 00/10] cpufreq: Add flag to auto-register as cooling device
  2019-01-17 10:08   ` Rafael J. Wysocki
  2019-01-17 10:14     ` Rafael J. Wysocki
@ 2019-01-17 10:21     ` Viresh Kumar
  2019-01-17 10:29       ` Rafael J. Wysocki
  1 sibling, 1 reply; 12+ messages in thread
From: Viresh Kumar @ 2019-01-17 10:21 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: open list:CPU FREQUENCY DRIVERS, linux-arm-msm,
	Rafael J. Wysocki, Doug Anderson, Amit Kucheria,
	Linux Kernel Mailing List, Eduardo Valentin, Matthias Kaehlcke,
	moderated list:ARM/Mediatek SoC support, Sudeep Holla,
	Matthias Brugger, Stephen Boyd,
	moderated list:ARM/Mediatek SoC support

On 17-01-19, 11:08, Rafael J. Wysocki wrote:
> On Thu, Jan 17, 2019 at 6:49 AM Viresh Kumar <viresh.kumar@linaro.org> 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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v1 00/10] cpufreq: Add flag to auto-register as cooling device
  2019-01-17 10:14     ` Rafael J. Wysocki
@ 2019-01-17 10:28       ` Viresh Kumar
  2019-01-17 10:31         ` Rafael J. Wysocki
  0 siblings, 1 reply; 12+ messages in thread
From: Viresh Kumar @ 2019-01-17 10:28 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: open list:CPU FREQUENCY DRIVERS, linux-arm-msm,
	Rafael J. Wysocki, Doug Anderson, Amit Kucheria,
	Linux Kernel Mailing List, Eduardo Valentin, Matthias Kaehlcke,
	moderated list:ARM/Mediatek SoC support, Sudeep Holla,
	Matthias Brugger, Stephen Boyd,
	moderated list:ARM/Mediatek SoC support

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v1 00/10] cpufreq: Add flag to auto-register as cooling device
  2019-01-17 10:21     ` Viresh Kumar
@ 2019-01-17 10:29       ` Rafael J. Wysocki
  0 siblings, 0 replies; 12+ messages in thread
From: Rafael J. Wysocki @ 2019-01-17 10:29 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Doug Anderson, Rafael J. Wysocki, linux-arm-msm,
	open list:CPU FREQUENCY DRIVERS, Rafael J. Wysocki,
	Linux Kernel Mailing List, Amit Kucheria, Stephen Boyd,
	Eduardo Valentin, Matthias Kaehlcke,
	moderated list:ARM/Mediatek SoC support, Sudeep Holla,
	Matthias Brugger, moderated list:ARM/Mediatek SoC support

On Thu, Jan 17, 2019 at 11:21 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> On 17-01-19, 11:08, Rafael J. Wysocki wrote:
> > On Thu, Jan 17, 2019 at 6:49 AM Viresh Kumar <viresh.kumar@linaro.org> 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.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v1 00/10] cpufreq: Add flag to auto-register as cooling device
  2019-01-17 10:28       ` Viresh Kumar
@ 2019-01-17 10:31         ` Rafael J. Wysocki
  0 siblings, 0 replies; 12+ messages in thread
From: Rafael J. Wysocki @ 2019-01-17 10:31 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Doug Anderson, Rafael J. Wysocki, linux-arm-msm,
	open list:CPU FREQUENCY DRIVERS, Rafael J. Wysocki,
	Linux Kernel Mailing List, Amit Kucheria, Stephen Boyd,
	Eduardo Valentin, Matthias Kaehlcke,
	moderated list:ARM/Mediatek SoC support, Sudeep Holla,
	Matthias Brugger, moderated list:ARM/Mediatek SoC support

On Thu, Jan 17, 2019 at 11:28 AM Viresh Kumar <viresh.kumar@linaro.org> 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.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

end of thread, other threads:[~2019-01-17 10:31 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-14 16:34 [PATCH v1 00/10] cpufreq: Add flag to auto-register as cooling device Amit Kucheria
2019-01-14 16:34 ` [PATCH v1 07/10] cpufreq: mediatek: Use auto-registration of thermal " Amit Kucheria
2019-01-14 16:35 ` [PATCH v1 09/10] cpufreq: scmi: " Amit Kucheria
2019-01-15 10:37   ` Sudeep Holla
2019-01-14 16:35 ` [PATCH v1 10/10] cpufreq: scpi: " Amit Kucheria
2019-01-17  5:49 ` [PATCH v1 00/10] cpufreq: Add flag to auto-register as " Viresh Kumar
2019-01-17 10:08   ` Rafael J. Wysocki
2019-01-17 10:14     ` Rafael J. Wysocki
2019-01-17 10:28       ` Viresh Kumar
2019-01-17 10:31         ` Rafael J. Wysocki
2019-01-17 10:21     ` Viresh Kumar
2019-01-17 10:29       ` Rafael J. Wysocki

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).