2018-05-14 08:15:15

by Anson Huang

[permalink] [raw]
Subject: [PATCH 1/3] thermal: imx: remove cpufreq cooling registration

This patch removes cpufreq cooling registration in
thermal .probe function, cpufreq cooling should be
done in cpufreq driver when it is ready.

Signed-off-by: Anson Huang <[email protected]>
---
drivers/thermal/imx_thermal.c | 16 ----------------
1 file changed, 16 deletions(-)

diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
index c30dc21..8eedb97 100644
--- a/drivers/thermal/imx_thermal.c
+++ b/drivers/thermal/imx_thermal.c
@@ -9,7 +9,6 @@

#include <linux/clk.h>
#include <linux/cpufreq.h>
-#include <linux/cpu_cooling.h>
#include <linux/delay.h>
#include <linux/device.h>
#include <linux/init.h>
@@ -207,7 +206,6 @@ static struct thermal_soc_data thermal_imx7d_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_init_calib() */
@@ -729,22 +727,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;
}
@@ -759,7 +747,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;
}
@@ -775,7 +762,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;
}
@@ -811,7 +797,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;
}
@@ -831,7 +816,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.7.4



2018-05-14 08:15:18

by Anson Huang

[permalink] [raw]
Subject: [PATCH 3/3] ARM: dts: imx: add cooling-cells for cpufreq cooling device

Add #cooling-cells for i.MX6/7 SoCs for cpufreq
cooling device usage.

Signed-off-by: Anson Huang <[email protected]>
---
arch/arm/boot/dts/imx6dl.dtsi | 1 +
arch/arm/boot/dts/imx6q.dtsi | 1 +
arch/arm/boot/dts/imx6sl.dtsi | 1 +
arch/arm/boot/dts/imx6sx.dtsi | 1 +
arch/arm/boot/dts/imx6ul.dtsi | 1 +
arch/arm/boot/dts/imx7d.dtsi | 1 +
6 files changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/imx6dl.dtsi b/arch/arm/boot/dts/imx6dl.dtsi
index 558bce8..b830dfd 100644
--- a/arch/arm/boot/dts/imx6dl.dtsi
+++ b/arch/arm/boot/dts/imx6dl.dtsi
@@ -39,6 +39,7 @@
396000 1175000
>;
clock-latency = <61036>; /* two CLK32 periods */
+ #cooling-cells = <2>;
clocks = <&clks IMX6QDL_CLK_ARM>,
<&clks IMX6QDL_CLK_PLL2_PFD2_396M>,
<&clks IMX6QDL_CLK_STEP>,
diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi
index ae7b3f1..ac9f5cb 100644
--- a/arch/arm/boot/dts/imx6q.dtsi
+++ b/arch/arm/boot/dts/imx6q.dtsi
@@ -44,6 +44,7 @@
396000 1175000
>;
clock-latency = <61036>; /* two CLK32 periods */
+ #cooling-cells = <2>;
clocks = <&clks IMX6QDL_CLK_ARM>,
<&clks IMX6QDL_CLK_PLL2_PFD2_396M>,
<&clks IMX6QDL_CLK_STEP>,
diff --git a/arch/arm/boot/dts/imx6sl.dtsi b/arch/arm/boot/dts/imx6sl.dtsi
index ab6a7e2..d35d4e9 100644
--- a/arch/arm/boot/dts/imx6sl.dtsi
+++ b/arch/arm/boot/dts/imx6sl.dtsi
@@ -65,6 +65,7 @@
396000 1175000
>;
clock-latency = <61036>; /* two CLK32 periods */
+ #cooling-cells = <2>;
clocks = <&clks IMX6SL_CLK_ARM>, <&clks IMX6SL_CLK_PLL2_PFD2>,
<&clks IMX6SL_CLK_STEP>, <&clks IMX6SL_CLK_PLL1_SW>,
<&clks IMX6SL_CLK_PLL1_SYS>;
diff --git a/arch/arm/boot/dts/imx6sx.dtsi b/arch/arm/boot/dts/imx6sx.dtsi
index 49c7205..966e39a 100644
--- a/arch/arm/boot/dts/imx6sx.dtsi
+++ b/arch/arm/boot/dts/imx6sx.dtsi
@@ -83,6 +83,7 @@
198000 1175000
>;
clock-latency = <61036>; /* two CLK32 periods */
+ #cooling-cells = <2>;
clocks = <&clks IMX6SX_CLK_ARM>,
<&clks IMX6SX_CLK_PLL2_PFD2>,
<&clks IMX6SX_CLK_STEP>,
diff --git a/arch/arm/boot/dts/imx6ul.dtsi b/arch/arm/boot/dts/imx6ul.dtsi
index 1241972..14b6ac7 100644
--- a/arch/arm/boot/dts/imx6ul.dtsi
+++ b/arch/arm/boot/dts/imx6ul.dtsi
@@ -66,6 +66,7 @@
device_type = "cpu";
reg = <0>;
clock-latency = <61036>; /* two CLK32 periods */
+ #cooling-cells = <2>;
operating-points = <
/* kHz uV */
696000 1275000
diff --git a/arch/arm/boot/dts/imx7d.dtsi b/arch/arm/boot/dts/imx7d.dtsi
index 200714e..49913717 100644
--- a/arch/arm/boot/dts/imx7d.dtsi
+++ b/arch/arm/boot/dts/imx7d.dtsi
@@ -53,6 +53,7 @@
792000 975000
>;
clock-frequency = <996000000>;
+ #cooling-cells = <2>;
};

cpu1: cpu@1 {
--
2.7.4


2018-05-14 08:16:42

by Anson Huang

[permalink] [raw]
Subject: [PATCH 2/3] cpufreq: imx6q: add cpufreq cooling device based on device tree

Register the imx6q cpufreq driver as a cooling device
based on the thermal device tree framework.

When temperature exceeds the passive trip point, cpufreq
driver will be used to throttle CPUs.

Signed-off-by: Anson Huang <[email protected]>
---
drivers/cpufreq/imx6q-cpufreq.c | 9 +++++++++
1 file changed, 9 insertions(+)

diff --git a/drivers/cpufreq/imx6q-cpufreq.c b/drivers/cpufreq/imx6q-cpufreq.c
index 83cf631..27ce84d 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>
@@ -50,6 +51,7 @@ static struct clk_bulk_data clks[] = {
};

static struct device *cpu_dev;
+struct thermal_cooling_device *cdev;
static bool free_opp;
static struct cpufreq_frequency_table *freq_table;
static unsigned int max_freq;
@@ -202,6 +204,11 @@ static int imx6q_cpufreq_init(struct cpufreq_policy *policy)
return ret;
}

+static void imx6q_cpufreq_ready(struct cpufreq_policy *policy)
+{
+ cdev = of_cpufreq_cooling_register(policy);
+}
+
static struct cpufreq_driver imx6q_cpufreq_driver = {
.flags = CPUFREQ_NEED_INITIAL_FREQ_CHECK,
.verify = cpufreq_generic_frequency_table_verify,
@@ -210,6 +217,7 @@ static struct cpufreq_driver imx6q_cpufreq_driver = {
.init = imx6q_cpufreq_init,
.name = "imx6q-cpufreq",
.attr = cpufreq_generic_attr,
+ .ready = imx6q_cpufreq_ready,
.suspend = cpufreq_generic_suspend,
};

@@ -482,6 +490,7 @@ static int imx6q_cpufreq_probe(struct platform_device *pdev)

static int imx6q_cpufreq_remove(struct platform_device *pdev)
{
+ cpufreq_cooling_unregister(cdev);
cpufreq_unregister_driver(&imx6q_cpufreq_driver);
dev_pm_opp_free_cpufreq_table(cpu_dev, &freq_table);
if (free_opp)
--
2.7.4


2018-05-14 08:36:03

by Daniel Lezcano

[permalink] [raw]
Subject: Re: [PATCH 1/3] thermal: imx: remove cpufreq cooling registration

On Mon, May 14, 2018 at 04:09:17PM +0800, Anson Huang wrote:
> This patch removes cpufreq cooling registration in
> thermal .probe function, cpufreq cooling should be
> done in cpufreq driver when it is ready.
>
> Signed-off-by: Anson Huang <[email protected]>
> ---

Acked-by: Daniel Lezcano <[email protected]>

--

<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

2018-05-14 08:39:05

by Bastian Krause

[permalink] [raw]
Subject: Re: [PATCH 1/3] thermal: imx: remove cpufreq cooling registration

Hi,

On 05/14/2018 10:09 AM, Anson Huang wrote:
> This patch removes cpufreq cooling registration in
> thermal .probe function, cpufreq cooling should be
> done in cpufreq driver when it is ready.
>
> Signed-off-by: Anson Huang <[email protected]>

It seems you are trying to achieve something similar to a patch I sent a
couple of month ago. Unfortunately I did not have the time to rework it yet:

https://patchwork.kernel.org/patch/10059085/

Some of the comments might apply here too.

Regards,
Bastian

> ---
> drivers/thermal/imx_thermal.c | 16 ----------------
> 1 file changed, 16 deletions(-)
>
> diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
> index c30dc21..8eedb97 100644
> --- a/drivers/thermal/imx_thermal.c
> +++ b/drivers/thermal/imx_thermal.c
> @@ -9,7 +9,6 @@
>
> #include <linux/clk.h>
> #include <linux/cpufreq.h>
> -#include <linux/cpu_cooling.h>
> #include <linux/delay.h>
> #include <linux/device.h>
> #include <linux/init.h>
> @@ -207,7 +206,6 @@ static struct thermal_soc_data thermal_imx7d_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_init_calib() */
> @@ -729,22 +727,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;
> }
> @@ -759,7 +747,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;
> }
> @@ -775,7 +762,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;
> }
> @@ -811,7 +797,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;
> }
> @@ -831,7 +816,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;
>

--
Pengutronix e.K.
Industrial Linux Solutions
http://www.pengutronix.de/
Peiner Str. 6-8, 31137 Hildesheim, Germany
Amtsgericht Hildesheim, HRA 2686

2018-05-14 09:12:33

by Anson Huang

[permalink] [raw]
Subject: RE: [PATCH 1/3] thermal: imx: remove cpufreq cooling registration



Anson Huang
Best Regards!


> -----Original Message-----
> From: Bastian Stender [mailto:[email protected]]
> Sent: Monday, May 14, 2018 4:37 PM
> To: Anson Huang <[email protected]>; [email protected];
> [email protected]; [email protected]; Fabio Estevam
> <[email protected]>; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]
> Cc: [email protected]; [email protected]; dl-linux-imx
> <[email protected]>; [email protected];
> [email protected]
> Subject: Re: [PATCH 1/3] thermal: imx: remove cpufreq cooling registration
>
> Hi,
>
> On 05/14/2018 10:09 AM, Anson Huang wrote:
> > This patch removes cpufreq cooling registration in thermal .probe
> > function, cpufreq cooling should be done in cpufreq driver when it is
> > ready.
> >
> > Signed-off-by: Anson Huang <[email protected]>
>
> It seems you are trying to achieve something similar to a patch I sent a couple
> of month ago. Unfortunately I did not have the time to rework it yet:
>
>
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatch
> work.kernel.org%2Fpatch%2F10059085%2F&data=02%7C01%7Canson.huang
> %40nxp.com%7C179da3635cab4a14deef08d5b975ecb6%7C686ea1d3bc2b4c6f
> a92cd99c5c301635%7C0%7C0%7C636618838508978629&sdata=BcQ9tc%2BE
> CZ%2Fk4AsZFxshgmvSsPg7eRN0ASzP8LO8yBI%3D&reserved=0
>
> Some of the comments might apply here too.

Ah, I did NOT notice this thread, so how to proceed, will you continue to finish your patch?
If yes, then you can just ignore/skip my patch, thanks.

Anson.

>
> Regards,
> Bastian
>
> > ---
> > drivers/thermal/imx_thermal.c | 16 ----------------
> > 1 file changed, 16 deletions(-)
> >
> > diff --git a/drivers/thermal/imx_thermal.c
> > b/drivers/thermal/imx_thermal.c index c30dc21..8eedb97 100644
> > --- a/drivers/thermal/imx_thermal.c
> > +++ b/drivers/thermal/imx_thermal.c
> > @@ -9,7 +9,6 @@
> >
> > #include <linux/clk.h>
> > #include <linux/cpufreq.h>
> > -#include <linux/cpu_cooling.h>
> > #include <linux/delay.h>
> > #include <linux/device.h>
> > #include <linux/init.h>
> > @@ -207,7 +206,6 @@ static struct thermal_soc_data thermal_imx7d_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_init_calib() */ @@ -729,22
> > +727,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;
> > }
> > @@ -759,7 +747,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;
> > }
> > @@ -775,7 +762,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;
> > }
> > @@ -811,7 +797,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;
> > }
> > @@ -831,7 +816,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;
> >
>
> --
> Pengutronix e.K.
> Industrial Linux Solutions
> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.
> pengutronix.de%2F&data=02%7C01%7Canson.huang%40nxp.com%7C179da36
> 35cab4a14deef08d5b975ecb6%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0
> %7C0%7C636618838508978629&sdata=kHHjnbmj2kV0aPlAvULXUh%2Fm%2Fp
> gbu21luMQ6jfIUgLo%3D&reserved=0
> Peiner Str. 6-8, 31137 Hildesheim, Germany Amtsgericht Hildesheim, HRA 2686

2018-05-14 09:18:43

by Bastian Krause

[permalink] [raw]
Subject: Re: [PATCH 1/3] thermal: imx: remove cpufreq cooling registration

On 05/14/2018 11:10 AM, Anson Huang wrote:
>> -----Original Message-----
>> From: Bastian Stender [mailto:[email protected]]
>> Sent: Monday, May 14, 2018 4:37 PM
>> To: Anson Huang <[email protected]>; [email protected];
>> [email protected]; [email protected]; Fabio Estevam
>> <[email protected]>; [email protected]; [email protected];
>> [email protected]; [email protected]; [email protected];
>> [email protected]
>> Cc: [email protected]; [email protected]; dl-linux-imx
>> <[email protected]>; [email protected];
>> [email protected]
>> Subject: Re: [PATCH 1/3] thermal: imx: remove cpufreq cooling registration
>>
>> Hi,
>>
>> On 05/14/2018 10:09 AM, Anson Huang wrote:
>>> This patch removes cpufreq cooling registration in thermal .probe
>>> function, cpufreq cooling should be done in cpufreq driver when it is
>>> ready.
>>>
>>> Signed-off-by: Anson Huang <[email protected]>
>>
>> It seems you are trying to achieve something similar to a patch I sent a couple
>> of month ago. Unfortunately I did not have the time to rework it yet:
>>
>>
>> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatch
>> work.kernel.org%2Fpatch%2F10059085%2F&data=02%7C01%7Canson.huang
>> %40nxp.com%7C179da3635cab4a14deef08d5b975ecb6%7C686ea1d3bc2b4c6f
>> a92cd99c5c301635%7C0%7C0%7C636618838508978629&sdata=BcQ9tc%2BE
>> CZ%2Fk4AsZFxshgmvSsPg7eRN0ASzP8LO8yBI%3D&reserved=0
>>
>> Some of the comments might apply here too.
>
> Ah, I did NOT notice this thread, so how to proceed, will you
> continue to finish your patch? If yes, then you can just ignore/skip
> my patch, thanks.
Yes, I will revisit the patch in a couple of weeks. It would be nice to
get your feedback then.

Regards,
Bastian

--
Pengutronix e.K.
Industrial Linux Solutions
http://www.pengutronix.de/
Peiner Str. 6-8, 31137 Hildesheim, Germany
Amtsgericht Hildesheim, HRA 2686