* [PATCH] cpufreq: imx6q/thermal: imx: move CPU cooling device from thermal to cpufreq
@ 2017-11-15 9:23 Bastian Stender
2017-11-15 9:25 ` Viresh Kumar
2017-11-16 11:49 ` Shawn Guo
0 siblings, 2 replies; 7+ messages in thread
From: Bastian Stender @ 2017-11-15 9:23 UTC (permalink / raw)
To: Rafael J . Wysocki, Viresh Kumar
Cc: linux-pm, Zhang Rui, Eduardo Valentin, Shawn Guo, kernel,
Bastian Stender
The cooling device should be part of the i.MX cpufreq driver. So move
it there.
Use of_cpufreq_power_cooling_register to link the cooling device to the
device tree node provided.
This makes it possible to bind the cpufreq cooling device to a custom
thermal zone via a cooling-maps entry like:
cooling-maps {
map0 {
trip = <&board_alert>;
cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
};
};
Assuming a cpu node exists with label "cpu0" and #cooling-cells
property.
Signed-off-by: Bastian Stender <bst@pengutronix.de>
---
This is a rework of "thermal: imx: use cpufreq cooling of
registration method" (id:20171103164203.5805-1-bst@pengutronix.de) and a
merged version of the follow-up patches "thermal: imx: remove cooling
device" (id:20171114134829.1354-1-bst@pengutronix.de) and "cpufreq:
imx6q: add CPU as cooling device"
(id:20171114135128.6173-1-bst@pengutronix.de).
---
drivers/cpufreq/imx6q-cpufreq.c | 37 +++++++++++++++++++++++++++++++++++++
drivers/thermal/imx_thermal.c | 15 ---------------
2 files changed, 37 insertions(+), 15 deletions(-)
diff --git a/drivers/cpufreq/imx6q-cpufreq.c b/drivers/cpufreq/imx6q-cpufreq.c
index 14466a9b01c0..6c7c59766990 100644
--- a/drivers/cpufreq/imx6q-cpufreq.c
+++ b/drivers/cpufreq/imx6q-cpufreq.c
@@ -9,6 +9,7 @@
#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/of.h>
@@ -35,6 +36,7 @@ static struct clk *pll2_bus_clk;
static struct clk *secondary_sel_clk;
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 transition_latency;
@@ -169,6 +171,32 @@ static int imx6q_set_target(struct cpufreq_policy *policy, unsigned int index)
return 0;
}
+static void imx6q_cpufreq_ready(struct cpufreq_policy *policy)
+{
+ struct device_node *np = of_node_get(cpu_dev->of_node);
+ u32 capacitance = 0;
+
+ if (WARN_ON(!np))
+ return;
+
+ if (of_find_property(np, "#cooling-cells", NULL)) {
+ of_property_read_u32(np, "dynamic-power-coefficient", &capacitance);
+
+ cdev = of_cpufreq_power_cooling_register(np,
+ policy, capacitance, NULL);
+
+ if (IS_ERR(cdev)) {
+ dev_err(cpu_dev,
+ "running cpufreq without cooling device: %ld\n",
+ PTR_ERR(cdev));
+
+ cdev = NULL;
+ }
+ }
+
+ of_node_put(np);
+}
+
static int imx6q_cpufreq_init(struct cpufreq_policy *policy)
{
int ret;
@@ -180,13 +208,22 @@ static int imx6q_cpufreq_init(struct cpufreq_policy *policy)
return ret;
}
+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,
.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,
};
diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
index 4798b4b1fd77..3bdbde675698 100644
--- a/drivers/thermal/imx_thermal.c
+++ b/drivers/thermal/imx_thermal.c
@@ -91,7 +91,6 @@ static struct thermal_soc_data thermal_imx6sx_data = {
struct imx_thermal_data {
struct cpufreq_policy *policy;
struct thermal_zone_device *tz;
- struct thermal_cooling_device *cdev;
enum thermal_device_mode mode;
struct regmap *tempmon;
u32 c1, c2; /* See formula in imx_get_sensor_data() */
@@ -533,22 +532,12 @@ static int imx_thermal_probe(struct platform_device *pdev)
return -EPROBE_DEFER;
}
- data->cdev = cpufreq_cooling_register(data->policy);
- if (IS_ERR(data->cdev)) {
- ret = PTR_ERR(data->cdev);
- dev_err(&pdev->dev,
- "failed to register cpufreq cooling device: %d\n", ret);
- cpufreq_cpu_put(data->policy);
- return ret;
- }
-
data->thermal_clk = devm_clk_get(&pdev->dev, NULL);
if (IS_ERR(data->thermal_clk)) {
ret = PTR_ERR(data->thermal_clk);
if (ret != -EPROBE_DEFER)
dev_err(&pdev->dev,
"failed to get thermal clk: %d\n", ret);
- cpufreq_cooling_unregister(data->cdev);
cpufreq_cpu_put(data->policy);
return ret;
}
@@ -563,7 +552,6 @@ static int imx_thermal_probe(struct platform_device *pdev)
ret = clk_prepare_enable(data->thermal_clk);
if (ret) {
dev_err(&pdev->dev, "failed to enable thermal clk: %d\n", ret);
- cpufreq_cooling_unregister(data->cdev);
cpufreq_cpu_put(data->policy);
return ret;
}
@@ -579,7 +567,6 @@ static int imx_thermal_probe(struct platform_device *pdev)
dev_err(&pdev->dev,
"failed to register thermal zone device %d\n", ret);
clk_disable_unprepare(data->thermal_clk);
- cpufreq_cooling_unregister(data->cdev);
cpufreq_cpu_put(data->policy);
return ret;
}
@@ -608,7 +595,6 @@ static int imx_thermal_probe(struct platform_device *pdev)
dev_err(&pdev->dev, "failed to request alarm irq: %d\n", ret);
clk_disable_unprepare(data->thermal_clk);
thermal_zone_device_unregister(data->tz);
- cpufreq_cooling_unregister(data->cdev);
cpufreq_cpu_put(data->policy);
return ret;
}
@@ -630,7 +616,6 @@ static int imx_thermal_remove(struct platform_device *pdev)
clk_disable_unprepare(data->thermal_clk);
thermal_zone_device_unregister(data->tz);
- cpufreq_cooling_unregister(data->cdev);
cpufreq_cpu_put(data->policy);
return 0;
--
2.11.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH] cpufreq: imx6q/thermal: imx: move CPU cooling device from thermal to cpufreq
2017-11-15 9:23 [PATCH] cpufreq: imx6q/thermal: imx: move CPU cooling device from thermal to cpufreq Bastian Stender
@ 2017-11-15 9:25 ` Viresh Kumar
2017-11-16 11:49 ` Shawn Guo
1 sibling, 0 replies; 7+ messages in thread
From: Viresh Kumar @ 2017-11-15 9:25 UTC (permalink / raw)
To: Bastian Stender
Cc: Rafael J . Wysocki, linux-pm, Zhang Rui, Eduardo Valentin,
Shawn Guo, kernel
Please make sure you add version history and correct version number in
$Subject next time. You must have added V2 in subject here.
On 15-11-17, 10:23, Bastian Stender wrote:
> The cooling device should be part of the i.MX cpufreq driver. So move
> it there.
>
> Use of_cpufreq_power_cooling_register to link the cooling device to the
> device tree node provided.
>
> This makes it possible to bind the cpufreq cooling device to a custom
> thermal zone via a cooling-maps entry like:
>
> cooling-maps {
> map0 {
> trip = <&board_alert>;
> cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
> };
> };
>
> Assuming a cpu node exists with label "cpu0" and #cooling-cells
> property.
>
> Signed-off-by: Bastian Stender <bst@pengutronix.de>
> ---
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
--
viresh
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] cpufreq: imx6q/thermal: imx: move CPU cooling device from thermal to cpufreq
2017-11-15 9:23 [PATCH] cpufreq: imx6q/thermal: imx: move CPU cooling device from thermal to cpufreq Bastian Stender
2017-11-15 9:25 ` Viresh Kumar
@ 2017-11-16 11:49 ` Shawn Guo
2017-12-13 1:00 ` Rafael J. Wysocki
1 sibling, 1 reply; 7+ messages in thread
From: Shawn Guo @ 2017-11-16 11:49 UTC (permalink / raw)
To: Bastian Stender, Leonard Crestez
Cc: Rafael J . Wysocki, Viresh Kumar, linux-pm, Zhang Rui,
Eduardo Valentin, kernel
On Wed, Nov 15, 2017 at 10:23:32AM +0100, Bastian Stender wrote:
> The cooling device should be part of the i.MX cpufreq driver. So move
> it there.
>
> Use of_cpufreq_power_cooling_register to link the cooling device to the
> device tree node provided.
>
> This makes it possible to bind the cpufreq cooling device to a custom
> thermal zone via a cooling-maps entry like:
>
> cooling-maps {
> map0 {
> trip = <&board_alert>;
> cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
> };
> };
>
> Assuming a cpu node exists with label "cpu0" and #cooling-cells
> property.
>
> Signed-off-by: Bastian Stender <bst@pengutronix.de>
I'm fine with the patch, but I would invite Leonard to take a look.
Shawn
> ---
>
> This is a rework of "thermal: imx: use cpufreq cooling of
> registration method" (id:20171103164203.5805-1-bst@pengutronix.de) and a
> merged version of the follow-up patches "thermal: imx: remove cooling
> device" (id:20171114134829.1354-1-bst@pengutronix.de) and "cpufreq:
> imx6q: add CPU as cooling device"
> (id:20171114135128.6173-1-bst@pengutronix.de).
> ---
> drivers/cpufreq/imx6q-cpufreq.c | 37 +++++++++++++++++++++++++++++++++++++
> drivers/thermal/imx_thermal.c | 15 ---------------
> 2 files changed, 37 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/cpufreq/imx6q-cpufreq.c b/drivers/cpufreq/imx6q-cpufreq.c
> index 14466a9b01c0..6c7c59766990 100644
> --- a/drivers/cpufreq/imx6q-cpufreq.c
> +++ b/drivers/cpufreq/imx6q-cpufreq.c
> @@ -9,6 +9,7 @@
> #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/of.h>
> @@ -35,6 +36,7 @@ static struct clk *pll2_bus_clk;
> static struct clk *secondary_sel_clk;
>
> 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 transition_latency;
> @@ -169,6 +171,32 @@ static int imx6q_set_target(struct cpufreq_policy *policy, unsigned int index)
> return 0;
> }
>
> +static void imx6q_cpufreq_ready(struct cpufreq_policy *policy)
> +{
> + struct device_node *np = of_node_get(cpu_dev->of_node);
> + u32 capacitance = 0;
> +
> + if (WARN_ON(!np))
> + return;
> +
> + if (of_find_property(np, "#cooling-cells", NULL)) {
> + of_property_read_u32(np, "dynamic-power-coefficient", &capacitance);
> +
> + cdev = of_cpufreq_power_cooling_register(np,
> + policy, capacitance, NULL);
> +
> + if (IS_ERR(cdev)) {
> + dev_err(cpu_dev,
> + "running cpufreq without cooling device: %ld\n",
> + PTR_ERR(cdev));
> +
> + cdev = NULL;
> + }
> + }
> +
> + of_node_put(np);
> +}
> +
> static int imx6q_cpufreq_init(struct cpufreq_policy *policy)
> {
> int ret;
> @@ -180,13 +208,22 @@ static int imx6q_cpufreq_init(struct cpufreq_policy *policy)
> return ret;
> }
>
> +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,
> .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,
> };
> diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
> index 4798b4b1fd77..3bdbde675698 100644
> --- a/drivers/thermal/imx_thermal.c
> +++ b/drivers/thermal/imx_thermal.c
> @@ -91,7 +91,6 @@ static struct thermal_soc_data thermal_imx6sx_data = {
> struct imx_thermal_data {
> struct cpufreq_policy *policy;
> struct thermal_zone_device *tz;
> - struct thermal_cooling_device *cdev;
> enum thermal_device_mode mode;
> struct regmap *tempmon;
> u32 c1, c2; /* See formula in imx_get_sensor_data() */
> @@ -533,22 +532,12 @@ static int imx_thermal_probe(struct platform_device *pdev)
> return -EPROBE_DEFER;
> }
>
> - data->cdev = cpufreq_cooling_register(data->policy);
> - if (IS_ERR(data->cdev)) {
> - ret = PTR_ERR(data->cdev);
> - dev_err(&pdev->dev,
> - "failed to register cpufreq cooling device: %d\n", ret);
> - cpufreq_cpu_put(data->policy);
> - return ret;
> - }
> -
> data->thermal_clk = devm_clk_get(&pdev->dev, NULL);
> if (IS_ERR(data->thermal_clk)) {
> ret = PTR_ERR(data->thermal_clk);
> if (ret != -EPROBE_DEFER)
> dev_err(&pdev->dev,
> "failed to get thermal clk: %d\n", ret);
> - cpufreq_cooling_unregister(data->cdev);
> cpufreq_cpu_put(data->policy);
> return ret;
> }
> @@ -563,7 +552,6 @@ static int imx_thermal_probe(struct platform_device *pdev)
> ret = clk_prepare_enable(data->thermal_clk);
> if (ret) {
> dev_err(&pdev->dev, "failed to enable thermal clk: %d\n", ret);
> - cpufreq_cooling_unregister(data->cdev);
> cpufreq_cpu_put(data->policy);
> return ret;
> }
> @@ -579,7 +567,6 @@ static int imx_thermal_probe(struct platform_device *pdev)
> dev_err(&pdev->dev,
> "failed to register thermal zone device %d\n", ret);
> clk_disable_unprepare(data->thermal_clk);
> - cpufreq_cooling_unregister(data->cdev);
> cpufreq_cpu_put(data->policy);
> return ret;
> }
> @@ -608,7 +595,6 @@ static int imx_thermal_probe(struct platform_device *pdev)
> dev_err(&pdev->dev, "failed to request alarm irq: %d\n", ret);
> clk_disable_unprepare(data->thermal_clk);
> thermal_zone_device_unregister(data->tz);
> - cpufreq_cooling_unregister(data->cdev);
> cpufreq_cpu_put(data->policy);
> return ret;
> }
> @@ -630,7 +616,6 @@ static int imx_thermal_remove(struct platform_device *pdev)
> clk_disable_unprepare(data->thermal_clk);
>
> thermal_zone_device_unregister(data->tz);
> - cpufreq_cooling_unregister(data->cdev);
> cpufreq_cpu_put(data->policy);
>
> return 0;
> --
> 2.11.0
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] cpufreq: imx6q/thermal: imx: move CPU cooling device from thermal to cpufreq
2017-11-16 11:49 ` Shawn Guo
@ 2017-12-13 1:00 ` Rafael J. Wysocki
2017-12-13 13:41 ` Leonard Crestez
0 siblings, 1 reply; 7+ messages in thread
From: Rafael J. Wysocki @ 2017-12-13 1:00 UTC (permalink / raw)
To: Shawn Guo
Cc: Bastian Stender, Leonard Crestez, Viresh Kumar, linux-pm,
Zhang Rui, Eduardo Valentin, kernel
On Thursday, November 16, 2017 12:49:18 PM CET Shawn Guo wrote:
> On Wed, Nov 15, 2017 at 10:23:32AM +0100, Bastian Stender wrote:
> > The cooling device should be part of the i.MX cpufreq driver. So move
> > it there.
> >
> > Use of_cpufreq_power_cooling_register to link the cooling device to the
> > device tree node provided.
> >
> > This makes it possible to bind the cpufreq cooling device to a custom
> > thermal zone via a cooling-maps entry like:
> >
> > cooling-maps {
> > map0 {
> > trip = <&board_alert>;
> > cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
> > };
> > };
> >
> > Assuming a cpu node exists with label "cpu0" and #cooling-cells
> > property.
> >
> > Signed-off-by: Bastian Stender <bst@pengutronix.de>
>
> I'm fine with the patch, but I would invite Leonard to take a look.
Given no response, what should I do?
Thanks,
Rafael
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] cpufreq: imx6q/thermal: imx: move CPU cooling device from thermal to cpufreq
2017-12-13 1:00 ` Rafael J. Wysocki
@ 2017-12-13 13:41 ` Leonard Crestez
2017-12-19 10:22 ` Bastian Stender
0 siblings, 1 reply; 7+ messages in thread
From: Leonard Crestez @ 2017-12-13 13:41 UTC (permalink / raw)
To: Bastian Stender, Rafael J. Wysocki
Cc: Shawn Guo, Viresh Kumar, linux-pm, Zhang Rui, Eduardo Valentin,
kernel
On Wed, 2017-12-13 at 02:00 +0100, Rafael J. Wysocki wrote:
> On Thursday, November 16, 2017 12:49:18 PM CET Shawn Guo wrote:
> >
> > On Wed, Nov 15, 2017 at 10:23:32AM +0100, Bastian Stender wrote:
> > >
> > > The cooling device should be part of the i.MX cpufreq driver. So move
> > > it there.
> > >
> > > Use of_cpufreq_power_cooling_register to link the cooling device to the
> > > device tree node provided.
> > >
> > > This makes it possible to bind the cpufreq cooling device to a custom
> > > thermal zone via a cooling-maps entry like:
> > >
> > > cooling-maps {
> > > map0 {
> > > trip = <&board_alert>;
> > > cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
> > > };
> > > };
> > >
> > > Assuming a cpu node exists with label "cpu0" and #cooling-cells
> > > property.
> > >
> > > Signed-off-by: Bastian Stender <bst@pengutronix.de>
> > I'm fine with the patch, but I would invite Leonard to take a look.
> Given no response, what should I do?
Sorry for keeping you waiting. I don't much about the "cooling device"
infrastructure so I don't have much to add, I guess I assumed a reply
wasn't required?
Looking at the code you are removing "thermal_cooling_device *cdev;"
from imx_thermal_data but kept "cpufreq_policy *policy". Isn't that
unused now?
Also, your code only adds the cooling device if certain devicetree
properties are present. Shouldn't your patch come with imx6*.dtsi
changes, or make the capacitance value default to zero somehow?
Otherwise the effect of the patch on most boards would be to disable
cpufreq cooling.
--
Regards,
Leonard
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] cpufreq: imx6q/thermal: imx: move CPU cooling device from thermal to cpufreq
2017-12-13 13:41 ` Leonard Crestez
@ 2017-12-19 10:22 ` Bastian Stender
2017-12-19 10:30 ` Lucas Stach
0 siblings, 1 reply; 7+ messages in thread
From: Bastian Stender @ 2017-12-19 10:22 UTC (permalink / raw)
To: Leonard Crestez, Rafael J. Wysocki
Cc: linux-pm, Viresh Kumar, Eduardo Valentin, kernel, Zhang Rui,
Shawn Guo
Hi Leonard,
On 12/13/2017 02:41 PM, Leonard Crestez wrote:
> On Wed, 2017-12-13 at 02:00 +0100, Rafael J. Wysocki wrote:
>> On Thursday, November 16, 2017 12:49:18 PM CET Shawn Guo wrote:
>>>
>>> On Wed, Nov 15, 2017 at 10:23:32AM +0100, Bastian Stender wrote:
>>>>
>>>> The cooling device should be part of the i.MX cpufreq driver. So move
>>>> it there.
>>>>
>>>> Use of_cpufreq_power_cooling_register to link the cooling device to the
>>>> device tree node provided.
>>>>
>>>> This makes it possible to bind the cpufreq cooling device to a custom
>>>> thermal zone via a cooling-maps entry like:
>>>>
>>>> cooling-maps {
>>>> map0 {
>>>> trip = <&board_alert>;
>>>> cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
>>>> };
>>>> };
>>>>
>>>> Assuming a cpu node exists with label "cpu0" and #cooling-cells
>>>> property.
>>>>
>>>> Signed-off-by: Bastian Stender <bst@pengutronix.de>
>
>>> I'm fine with the patch, but I would invite Leonard to take a look.
>
>> Given no response, what should I do?
>
> Sorry for keeping you waiting. I don't much about the "cooling device"
> infrastructure so I don't have much to add, I guess I assumed a reply
> wasn't required?
>
> Looking at the code you are removing "thermal_cooling_device *cdev;"
> from imx_thermal_data but kept "cpufreq_policy *policy". Isn't that
> unused now?
Yes, you are right. Will send v2.
> Also, your code only adds the cooling device if certain devicetree
> properties are present. Shouldn't your patch come with imx6*.dtsi
> changes, or make the capacitance value default to zero somehow?
> Otherwise the effect of the patch on most boards would be to disable
> cpufreq cooling.
Good point. I guess the cpu nodes in imx6*.dtsi should get
cooling-min-level/cooling-min-level/#cooling-cells properties, right?
This would achieve the old behavior.
The capacitance value already defaults to zero if
"dynamic-power-coefficient" is not present.
Regards,
Bastian
--
Pengutronix e.K.
Industrial Linux Solutions
http://www.pengutronix.de/
Peiner Str. 6-8, 31137 Hildesheim, Germany
Amtsgericht Hildesheim, HRA 2686
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] cpufreq: imx6q/thermal: imx: move CPU cooling device from thermal to cpufreq
2017-12-19 10:22 ` Bastian Stender
@ 2017-12-19 10:30 ` Lucas Stach
0 siblings, 0 replies; 7+ messages in thread
From: Lucas Stach @ 2017-12-19 10:30 UTC (permalink / raw)
To: Bastian Stender, Leonard Crestez, Rafael J. Wysocki
Cc: linux-pm, Viresh Kumar, Eduardo Valentin, kernel, Zhang Rui,
Shawn Guo
Am Dienstag, den 19.12.2017, 11:22 +0100 schrieb Bastian Stender:
> Hi Leonard,
>
> On 12/13/2017 02:41 PM, Leonard Crestez wrote:
> > On Wed, 2017-12-13 at 02:00 +0100, Rafael J. Wysocki wrote:
> > > On Thursday, November 16, 2017 12:49:18 PM CET Shawn Guo wrote:
> > > >
> > > > On Wed, Nov 15, 2017 at 10:23:32AM +0100, Bastian Stender
> > > > wrote:
> > > > >
> > > > > The cooling device should be part of the i.MX cpufreq driver.
> > > > > So move
> > > > > it there.
> > > > >
> > > > > Use of_cpufreq_power_cooling_register to link the cooling
> > > > > device to the
> > > > > device tree node provided.
> > > > >
> > > > > This makes it possible to bind the cpufreq cooling device to
> > > > > a custom
> > > > > thermal zone via a cooling-maps entry like:
> > > > >
> > > > > cooling-maps {
> > > > > map0 {
> > > > > trip = <&board_alert>;
> > > > > cooling-device = <&cpu0
> > > > > THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
> > > > > };
> > > > > };
> > > > >
> > > > > Assuming a cpu node exists with label "cpu0" and #cooling-
> > > > > cells
> > > > > property.
> > > > >
> > > > > Signed-off-by: Bastian Stender <bst@pengutronix.de>
> > > > I'm fine with the patch, but I would invite Leonard to take a
> > > > look.
> > > Given no response, what should I do?
> >
> > Sorry for keeping you waiting. I don't much about the "cooling
> > device"
> > infrastructure so I don't have much to add, I guess I assumed a
> > reply
> > wasn't required?
> >
> > Looking at the code you are removing "thermal_cooling_device
> > *cdev;"
> > from imx_thermal_data but kept "cpufreq_policy *policy". Isn't that
> > unused now?
>
> Yes, you are right. Will send v2.
>
> > Also, your code only adds the cooling device if certain devicetree
> > properties are present. Shouldn't your patch come with imx6*.dtsi
> > changes, or make the capacitance value default to zero somehow?
> > Otherwise the effect of the patch on most boards would be to
> > disable
> > cpufreq cooling.
>
> Good point. I guess the cpu nodes in imx6*.dtsi should get
> cooling-min-level/cooling-min-level/#cooling-cells properties, right?
> This would achieve the old behavior.
Sorry, but this breaks DT stability. Your driver code must work
correctly with older DTs where those properties aren't present.
Changing driver code and dts in lockstep is not an option.
Regards,
Lucas
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-12-19 10:30 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-11-15 9:23 [PATCH] cpufreq: imx6q/thermal: imx: move CPU cooling device from thermal to cpufreq Bastian Stender
2017-11-15 9:25 ` Viresh Kumar
2017-11-16 11:49 ` Shawn Guo
2017-12-13 1:00 ` Rafael J. Wysocki
2017-12-13 13:41 ` Leonard Crestez
2017-12-19 10:22 ` Bastian Stender
2017-12-19 10:30 ` Lucas Stach
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).