[06/11] DT: arm64: exynos: add support for thermal trip irq-mode
diff mbox series

Message ID 20181016145647eucas1p206c068806214535da339588d77cbdfa5~eHqLYh2dR2135021350eucas1p2K@eucas1p2.samsung.com
State Superseded
Headers show
Series
  • thermal: add new flag irq-mode for trip point
Related show

Commit Message

Lukasz Luba Oct. 16, 2018, 2:56 p.m. UTC
This patch adds support for new flash which indicates
that trip point triggers irq when temperature is met.
Exynos5433 supports 8 trip point which will trigger irq.
Above that number other trip points should be registered
without 'irq-mode' flag.
That will force the thermal framework to start polling
the temperature sensor under configured conditions and
handle the trip point.

Cc: Kukjin Kim <kgene@kernel.org>
Cc: Krzysztof Kozlowski <krzk@kernel.org>
Cc: devicetree@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Signed-off-by: Lukasz Luba <l.luba@partner.samsung.com>
---
 arch/arm64/boot/dts/exynos/exynos5433-tmu.dtsi | 105 ++++++++++++++++---------
 1 file changed, 70 insertions(+), 35 deletions(-)

Comments

Krzysztof Kozlowski Oct. 16, 2018, 3:09 p.m. UTC | #1
On Tue, 16 Oct 2018 at 16:56, Lukasz Luba <l.luba@partner.samsung.com> wrote:
>

Hi Lukasz,

Thanks for patches.

I did not receive the first patches in the series. It is okay (depends
on the context) but in such case sending cover letter to all people is
quite useful. It helps to understand the entire patchset. Since I did
not get them, I really do not know:
1. Whether DTS patches are independent?
2. How this fits in the big picture of Exynos thermal?


> This patch adds support for new flash which indicates

What is "flash"?

Please wrap your lines accordingly (75 characters like in
submitting-patches.rst). Current wrapping makes it more difficult to
read.

> that trip point triggers irq when temperature is met.
> Exynos5433 supports 8 trip point which will trigger irq.

"Exynos5433 supports 8 trip points which trigger interrupt."
(or IRQ but not irq)

> Above that number other trip points should be registered
> without 'irq-mode' flag.

Why they should be registered without irq-mode?

> That will force the thermal framework to start polling
> the temperature sensor under configured conditions and
> handle the trip point.

How does it fit into existing polling mode?

>
> Cc: Kukjin Kim <kgene@kernel.org>
> Cc: Krzysztof Kozlowski <krzk@kernel.org>
> Cc: devicetree@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> Signed-off-by: Lukasz Luba <l.luba@partner.samsung.com>
> ---
>  arch/arm64/boot/dts/exynos/exynos5433-tmu.dtsi | 105 ++++++++++++++++---------
>  1 file changed, 70 insertions(+), 35 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tmu.dtsi b/arch/arm64/boot/dts/exynos/exynos5433-tmu.dtsi
> index fe3a0b1..c4330f6 100644
> --- a/arch/arm64/boot/dts/exynos/exynos5433-tmu.dtsi
> +++ b/arch/arm64/boot/dts/exynos/exynos5433-tmu.dtsi
> @@ -17,37 +17,44 @@ thermal-zones {
>                         atlas0_alert_0: atlas0-alert-0 {
>                                 temperature = <65000>;  /* millicelsius */
>                                 hysteresis = <1000>;    /* millicelsius */
> -                               type = "active";
> +                               type = "passive";

Change of active->passive looks irrelevant to this topic. Here and
everywhere else.

Best regards,
Krzysztof
Lukasz Luba Oct. 16, 2018, 3:48 p.m. UTC | #2
Hi Krzysztof,

On 10/16/2018 05:09 PM, Krzysztof Kozlowski wrote:
> On Tue, 16 Oct 2018 at 16:56, Lukasz Luba <l.luba@partner.samsung.com> wrote:
>>
> 
> Hi Lukasz,
> 
> Thanks for patches.
> 
> I did not receive the first patches in the series. It is okay (depends
> on the context) but in such case sending cover letter to all people is
> quite useful. It helps to understand the entire patchset. Since I did
> not get them, I really do not know:
> 1. Whether DTS patches are independent?
> 2. How this fits in the big picture of Exynos thermal?
I have lost your email for my 'cc' command line, my apologies.
I will add you in v2.
Here you can find the cover letter:
https://marc.info/?l=linux-pm&m=153970180314648&w=2
regarding
1. they are independent
2. It will be less confusing and more coherent with documentation
when we use 'passive' for DVFS cooling. Unfortunately, when 'passive'
is set, it registers our trip points for polling and thermal schedules
the work. I had conversation with Daniel regarding this setup,
please check these email:
https://marc.info/?l=linux-kernel&m=153924620318766&w=2
and my response:
https://marc.info/?l=linux-kernel&m=153933708316168&w=2

> 
> 
>> This patch adds support for new flash which indicates
> 
> What is "flash"?
Should be 'flag', thanks for that.
> 
> Please wrap your lines accordingly (75 characters like in
> submitting-patches.rst). Current wrapping makes it more difficult to
> read.
OK
> 
>> that trip point triggers irq when temperature is met.
>> Exynos5433 supports 8 trip point which will trigger irq.
> 
> "Exynos5433 supports 8 trip points which trigger interrupt."
> (or IRQ but not irq)
ACK
> 
>> Above that number other trip points should be registered
>> without 'irq-mode' flag.
> 
> Why they should be registered without irq-mode?
For this HW, only 4 temperatures can be monitored for which IRQ
will fire, AFAIK.
> 
>> That will force the thermal framework to start polling
>> the temperature sensor under configured conditions and
>> handle the trip point.
> 
> How does it fit into existing polling mode?
The existing polling mode is default for 'passive' which does
not have this flag, so nothing changes. The benefit for 'passive'
which are supported by HW IRQ and has this flag is lack of polling
every Xms.
In current situation we are trying to establish this behavior by a
workaround with 'active'. the 'active' in default
does not register polling and waits for IRQs, so the framework is quiet.
This naming tricks are a bit confusing for people, though.

Thank you for comments.

Regards,
Lukasz

> 
>>
>> Cc: Kukjin Kim <kgene@kernel.org>
>> Cc: Krzysztof Kozlowski <krzk@kernel.org>
>> Cc: devicetree@vger.kernel.org
>> Cc: linux-arm-kernel@lists.infradead.org
>> Signed-off-by: Lukasz Luba <l.luba@partner.samsung.com>
>> ---
>>   arch/arm64/boot/dts/exynos/exynos5433-tmu.dtsi | 105 ++++++++++++++++---------
>>   1 file changed, 70 insertions(+), 35 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tmu.dtsi b/arch/arm64/boot/dts/exynos/exynos5433-tmu.dtsi
>> index fe3a0b1..c4330f6 100644
>> --- a/arch/arm64/boot/dts/exynos/exynos5433-tmu.dtsi
>> +++ b/arch/arm64/boot/dts/exynos/exynos5433-tmu.dtsi
>> @@ -17,37 +17,44 @@ thermal-zones {
>>                          atlas0_alert_0: atlas0-alert-0 {
>>                                  temperature = <65000>;  /* millicelsius */
>>                                  hysteresis = <1000>;    /* millicelsius */
>> -                               type = "active";
>> +                               type = "passive";
> 
> Change of active->passive looks irrelevant to this topic. Here and
> everywhere else.
> 
> Best regards,
> Krzysztof
> 
>
Krzysztof Kozlowski Oct. 17, 2018, 7:04 a.m. UTC | #3
On Tue, 16 Oct 2018 at 17:48, Lukasz Luba <l.luba@partner.samsung.com> wrote:
>
> Hi Krzysztof,
>
> On 10/16/2018 05:09 PM, Krzysztof Kozlowski wrote:
> > On Tue, 16 Oct 2018 at 16:56, Lukasz Luba <l.luba@partner.samsung.com> wrote:
> >>
> >
> > Hi Lukasz,
> >
> > Thanks for patches.
> >
> > I did not receive the first patches in the series. It is okay (depends
> > on the context) but in such case sending cover letter to all people is
> > quite useful. It helps to understand the entire patchset. Since I did
> > not get them, I really do not know:
> > 1. Whether DTS patches are independent?
> > 2. How this fits in the big picture of Exynos thermal?
> I have lost your email for my 'cc' command line, my apologies.
> I will add you in v2.
> Here you can find the cover letter:
> https://marc.info/?l=linux-pm&m=153970180314648&w=2
> regarding
> 1. they are independent
> 2. It will be less confusing and more coherent with documentation
> when we use 'passive' for DVFS cooling. Unfortunately, when 'passive'
> is set, it registers our trip points for polling and thermal schedules
> the work. I had conversation with Daniel regarding this setup,
> please check these email:
> https://marc.info/?l=linux-kernel&m=153924620318766&w=2
> and my response:
> https://marc.info/?l=linux-kernel&m=153933708316168&w=2

I see it on the linux-pm, I'll reply there.

Best regards,
Krzysztof

Patch
diff mbox series

diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tmu.dtsi b/arch/arm64/boot/dts/exynos/exynos5433-tmu.dtsi
index fe3a0b1..c4330f6 100644
--- a/arch/arm64/boot/dts/exynos/exynos5433-tmu.dtsi
+++ b/arch/arm64/boot/dts/exynos/exynos5433-tmu.dtsi
@@ -17,37 +17,44 @@  thermal-zones {
 			atlas0_alert_0: atlas0-alert-0 {
 				temperature = <65000>;	/* millicelsius */
 				hysteresis = <1000>;	/* millicelsius */
-				type = "active";
+				type = "passive";
+				irq-mode;
 			};
 			atlas0_alert_1: atlas0-alert-1 {
 				temperature = <70000>;	/* millicelsius */
 				hysteresis = <1000>;	/* millicelsius */
-				type = "active";
+				type = "passive";
+				irq-mode;
 			};
 			atlas0_alert_2: atlas0-alert-2 {
 				temperature = <75000>;	/* millicelsius */
 				hysteresis = <1000>;	/* millicelsius */
-				type = "active";
+				type = "passive";
+				irq-mode;
 			};
 			atlas0_alert_3: atlas0-alert-3 {
 				temperature = <80000>;	/* millicelsius */
 				hysteresis = <1000>;	/* millicelsius */
-				type = "active";
+				type = "passive";
+				irq-mode;
 			};
 			atlas0_alert_4: atlas0-alert-4 {
 				temperature = <85000>;	/* millicelsius */
 				hysteresis = <1000>;	/* millicelsius */
-				type = "active";
+				type = "passive";
+				irq-mode;
 			};
 			atlas0_alert_5: atlas0-alert-5 {
 				temperature = <90000>;	/* millicelsius */
 				hysteresis = <1000>;	/* millicelsius */
-				type = "active";
+				type = "passive";
+				irq-mode;
 			};
 			atlas0_alert_6: atlas0-alert-6 {
 				temperature = <95000>;	/* millicelsius */
 				hysteresis = <1000>;	/* millicelsius */
-				type = "active";
+				type = "passive";
+				irq-mode;
 			};
 		};
 
@@ -98,37 +105,44 @@  thermal-zones {
 			atlas1_alert_0: atlas1-alert-0 {
 				temperature = <65000>;	/* millicelsius */
 				hysteresis = <1000>;	/* millicelsius */
-				type = "active";
+				type = "passive";
+				irq-mode;
 			};
 			atlas1_alert_1: atlas1-alert-1 {
 				temperature = <70000>;	/* millicelsius */
 				hysteresis = <1000>;	/* millicelsius */
-				type = "active";
+				type = "passive";
+				irq-mode;
 			};
 			atlas1_alert_2: atlas1-alert-2 {
 				temperature = <75000>;	/* millicelsius */
 				hysteresis = <1000>;	/* millicelsius */
-				type = "active";
+				type = "passive";
+				irq-mode;
 			};
 			atlas1_alert_3: atlas1-alert-3 {
 				temperature = <80000>;	/* millicelsius */
 				hysteresis = <1000>;	/* millicelsius */
-				type = "active";
+				type = "passive";
+				irq-mode;
 			};
 			atlas1_alert_4: atlas1-alert-4 {
 				temperature = <85000>;	/* millicelsius */
 				hysteresis = <1000>;	/* millicelsius */
-				type = "active";
+				type = "passive";
+				irq-mode;
 			};
 			atlas1_alert_5: atlas1-alert-5 {
 				temperature = <90000>;	/* millicelsius */
 				hysteresis = <1000>;	/* millicelsius */
-				type = "active";
+				type = "passive";
+				irq-mode;
 			};
 			atlas1_alert_6: atlas1-alert-6 {
 				temperature = <95000>;	/* millicelsius */
 				hysteresis = <1000>;	/* millicelsius */
-				type = "active";
+				type = "passive";
+				irq-mode;
 			};
 		};
 	};
@@ -141,37 +155,44 @@  thermal-zones {
 			g3d_alert_0: g3d-alert-0 {
 				temperature = <70000>;	/* millicelsius */
 				hysteresis = <1000>;	/* millicelsius */
-				type = "active";
+				type = "passive";
+				irq-mode;
 			};
 			g3d_alert_1: g3d-alert-1 {
 				temperature = <75000>;	/* millicelsius */
 				hysteresis = <1000>;	/* millicelsius */
-				type = "active";
+				type = "passive";
+				irq-mode;
 			};
 			g3d_alert_2: g3d-alert-2 {
 				temperature = <80000>;	/* millicelsius */
 				hysteresis = <1000>;	/* millicelsius */
-				type = "active";
+				type = "passive";
+				irq-mode;
 			};
 			g3d_alert_3: g3d-alert-3 {
 				temperature = <85000>;	/* millicelsius */
 				hysteresis = <1000>;	/* millicelsius */
-				type = "active";
+				type = "passive";
+				irq-mode;
 			};
 			g3d_alert_4: g3d-alert-4 {
 				temperature = <90000>;	/* millicelsius */
 				hysteresis = <1000>;	/* millicelsius */
-				type = "active";
+				type = "passive";
+				irq-mode;
 			};
 			g3d_alert_5: g3d-alert-5 {
 				temperature = <95000>;	/* millicelsius */
 				hysteresis = <1000>;	/* millicelsius */
-				type = "active";
+				type = "passive";
+				irq-mode;
 			};
 			g3d_alert_6: g3d-alert-6 {
 				temperature = <100000>;	/* millicelsius */
 				hysteresis = <1000>;	/* millicelsius */
-				type = "active";
+				type = "passive";
+				irq-mode;
 			};
 		};
 	};
@@ -184,37 +205,44 @@  thermal-zones {
 			apollo_alert_0: apollo-alert-0 {
 				temperature = <65000>;	/* millicelsius */
 				hysteresis = <1000>;	/* millicelsius */
-				type = "active";
+				type = "passive";
+				irq-mode;
 			};
 			apollo_alert_1: apollo-alert-1 {
 				temperature = <70000>;	/* millicelsius */
 				hysteresis = <1000>;	/* millicelsius */
-				type = "active";
+				type = "passive";
+				irq-mode;
 			};
 			apollo_alert_2: apollo-alert-2 {
 				temperature = <75000>;	/* millicelsius */
 				hysteresis = <1000>;	/* millicelsius */
-				type = "active";
+				type = "passive";
+				irq-mode;
 			};
 			apollo_alert_3: apollo-alert-3 {
 				temperature = <80000>;	/* millicelsius */
 				hysteresis = <1000>;	/* millicelsius */
-				type = "active";
+				type = "passive";
+				irq-mode;
 			};
 			apollo_alert_4: apollo-alert-4 {
 				temperature = <85000>;	/* millicelsius */
 				hysteresis = <1000>;	/* millicelsius */
-				type = "active";
+				type = "passive";
+				irq-mode;
 			};
 			apollo_alert_5: apollo-alert-5 {
 				temperature = <90000>;	/* millicelsius */
 				hysteresis = <1000>;	/* millicelsius */
-				type = "active";
+				type = "passive";
+				irq-mode;
 			};
 			apollo_alert_6: apollo-alert-6 {
 				temperature = <95000>;	/* millicelsius */
 				hysteresis = <1000>;	/* millicelsius */
-				type = "active";
+				type = "passive";
+				irq-mode;
 			};
 		};
 
@@ -255,37 +283,44 @@  thermal-zones {
 			isp_alert_0: isp-alert-0 {
 				temperature = <80000>;	/* millicelsius */
 				hysteresis = <1000>;	/* millicelsius */
-				type = "active";
+				type = "passive";
+				irq-mode;
 			};
 			isp_alert_1: isp-alert-1 {
 				temperature = <85000>;	/* millicelsius */
 				hysteresis = <1000>;	/* millicelsius */
-				type = "active";
+				type = "passive";
+				irq-mode;
 			};
 			isp_alert_2: isp-alert-2 {
 				temperature = <90000>;	/* millicelsius */
 				hysteresis = <1000>;	/* millicelsius */
-				type = "active";
+				type = "passive";
+				irq-mode;
 			};
 			isp_alert_3: isp-alert-3 {
 				temperature = <95000>;	/* millicelsius */
 				hysteresis = <1000>;	/* millicelsius */
-				type = "active";
+				type = "passive";
+				irq-mode;
 			};
 			isp_alert_4: isp-alert-4 {
 				temperature = <100000>;	/* millicelsius */
 				hysteresis = <1000>;	/* millicelsius */
-				type = "active";
+				type = "passive";
+				irq-mode;
 			};
 			isp_alert_5: isp-alert-5 {
 				temperature = <105000>;	/* millicelsius */
 				hysteresis = <1000>;	/* millicelsius */
-				type = "active";
+				type = "passive";
+				irq-mode;
 			};
 			isp_alert_6: isp-alert-6 {
 				temperature = <110000>;	/* millicelsius */
 				hysteresis = <1000>;	/* millicelsius */
-				type = "active";
+				type = "passive";
+				irq-mode;
 			};
 		};
 	};