[v3,0/8] Add Mediatek thermal dirver and dtsi
mbox series

Message ID 20200103064407.19861-1-michael.kao@mediatek.com
Headers show
Series
  • Add Mediatek thermal dirver and dtsi
Related show

Message

Michael Kao Jan. 3, 2020, 6:43 a.m. UTC
This patchset supports for MT8183 chip to mtk_thermal.c.
Add thermal zone of all the thermal sensor in SoC for
another get temperatrue. They don't need to thermal throttle.
And we bind coolers for thermal zone nodes of cpu_thermal.

Rebase to kernel-5.5-rc1.

Update content:

[1/8]
Update sustainable power of cpu, tzts1~5 and tztsABB.

[7/8]
Bypass the failure that non cpu_thermal sensor is not find in thermal-zones
in dts, which is normal for mt8173, so prompt a warning here instead of
failing.

Return -EAGAIN instead of -EACCESS on the first read of sensor that
often are bogus values. This can avoid following warning on boot:

  thermal thermal_zone6: failed to read out thermal zone (-13)


This patch series base on these patches [1][2][3][4].

[1]support for reading chip ID and efuse (https://patchwork.kernel.org/patch/10902131/)
[2]arm64: dts: mt8183: Add reset-cells in infracfg (https://patchwork.kernel.org/patch/10908653/)
[3]clk: reset: Modify reset-controller driver (https://patchwork.kernel.org/patch/10908657/)
[4]PM / AVS: SVS: Introduce SVS engine (https://patchwork.kernel.org/patch/10923289/)

Matthias Kaehlcke (2):
  arm64: dts: mt8183: Configure CPU cooling
  arm64: dts: mt8183: Increase polling frequency for CPU thermal zone

Michael Kao (6):
  arm64: dts: mt8183: add thermal zone node
  arm64: dts: mt8183: add/update dynamic power coefficients
  arm64: dts: mt8183: Add #cooling-cells to CPU nodes
  thermal: mediatek: mt8183: fix bank number settings
  thermal: mediatek: add another get_temp ops for thermal sensors
  thermal: mediatek: use spinlock to protect PTPCORESEL

 arch/arm64/boot/dts/mediatek/mt8183.dtsi | 157 +++++++++++++++++++++++
 drivers/thermal/mtk_thermal.c            |  88 +++++++++++--
 2 files changed, 231 insertions(+), 14 deletions(-)

Comments

Hsin-Yi Wang Jan. 8, 2020, 9:58 a.m. UTC | #1
On Fri, Jan 3, 2020 at 2:44 PM Michael Kao <michael.kao@mediatek.com> wrote:
>
> MT8183_NUM_ZONES should be set to 1
> because MT8183 doesn't have multiple banks.
>
> Fixes: a4ffe6b52d27 ("thermal: mediatek: add support for MT8183")
> Signed-off-by: Michael Kao <michael.kao@mediatek.com>
> ---
Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
Daniel Lezcano Jan. 9, 2020, 11:31 a.m. UTC | #2
On 03/01/2020 07:44, Michael Kao wrote:
> From: "michael.kao" <michael.kao@mediatek.com>
> 
> Add thermal zone node to Mediatek MT8183 dts file.
> 
> Signed-off-by: Michael Kao <michael.kao@mediatek.com>
> ---
>  arch/arm64/boot/dts/mediatek/mt8183.dtsi | 85 ++++++++++++++++++++++++
>  1 file changed, 85 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> index 10b32471bc7b..a2793cf3d994 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> @@ -570,6 +570,88 @@
>  			status = "disabled";
>  		};
>  
> +		thermal: thermal@1100b000 {
> +			#thermal-sensor-cells = <1>;
> +			compatible = "mediatek,mt8183-thermal";
> +			reg = <0 0x1100b000 0 0x1000>;
> +			interrupts = <0 76 IRQ_TYPE_LEVEL_LOW>;

What is this interrupt for?

> +			clocks = <&infracfg CLK_INFRA_THERM>,
> +				 <&infracfg CLK_INFRA_AUXADC>;
> +			clock-names = "therm", "auxadc";
> +			resets = <&infracfg  MT8183_INFRACFG_AO_THERM_SW_RST>;
> +			mediatek,auxadc = <&auxadc>;
> +			mediatek,apmixedsys = <&apmixedsys>;
> +			mediatek,hw-reset-temp = <117000>;
> +			nvmem-cells = <&thermal_calibration>;
> +			nvmem-cell-names = "calibration-data";
> +		};
> +
> +		thermal-zones {
> +			cpu_thermal: cpu_thermal {
> +				polling-delay-passive = <1000>;
> +				polling-delay = <1000>;

[ ... ]
Matthias Brugger Jan. 10, 2020, 2:40 p.m. UTC | #3
On 03/01/2020 07:44, Michael Kao wrote:
> From: "michael.kao" <michael.kao@mediatek.com>
> 
> Add dynamic power coefficients for all cores and update those of
> CPU0 and CPU4.

No update in this patch. I suppose it need rewording.

Regards,
Matthias

> 
> Signed-off-by: Michael Kao <michael.kao@mediatek.com>
> ---
>  arch/arm64/boot/dts/mediatek/mt8183.dtsi | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> index a2793cf3d994..cfb74af260e0 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> @@ -73,6 +73,7 @@
>  			reg = <0x000>;
>  			enable-method = "psci";
>  			capacity-dmips-mhz = <741>;
> +			dynamic-power-coefficient = <84>;
>  		};
>  
>  		cpu1: cpu@1 {
> @@ -81,6 +82,7 @@
>  			reg = <0x001>;
>  			enable-method = "psci";
>  			capacity-dmips-mhz = <741>;
> +			dynamic-power-coefficient = <84>;
>  		};
>  
>  		cpu2: cpu@2 {
> @@ -89,6 +91,7 @@
>  			reg = <0x002>;
>  			enable-method = "psci";
>  			capacity-dmips-mhz = <741>;
> +			dynamic-power-coefficient = <84>;
>  		};
>  
>  		cpu3: cpu@3 {
> @@ -97,6 +100,7 @@
>  			reg = <0x003>;
>  			enable-method = "psci";
>  			capacity-dmips-mhz = <741>;
> +			dynamic-power-coefficient = <84>;
>  		};
>  
>  		cpu4: cpu@100 {
> @@ -105,6 +109,7 @@
>  			reg = <0x100>;
>  			enable-method = "psci";
>  			capacity-dmips-mhz = <1024>;
> +			dynamic-power-coefficient = <211>;
>  		};
>  
>  		cpu5: cpu@101 {
> @@ -113,6 +118,7 @@
>  			reg = <0x101>;
>  			enable-method = "psci";
>  			capacity-dmips-mhz = <1024>;
> +			dynamic-power-coefficient = <211>;
>  		};
>  
>  		cpu6: cpu@102 {
> @@ -121,6 +127,7 @@
>  			reg = <0x102>;
>  			enable-method = "psci";
>  			capacity-dmips-mhz = <1024>;
> +			dynamic-power-coefficient = <211>;
>  		};
>  
>  		cpu7: cpu@103 {
> @@ -129,6 +136,7 @@
>  			reg = <0x103>;
>  			enable-method = "psci";
>  			capacity-dmips-mhz = <1024>;
> +			dynamic-power-coefficient = <211>;
>  		};
>  	};
>  
>
Daniel Lezcano Feb. 20, 2020, 11:52 a.m. UTC | #4
On 11/02/2020 04:17, Michael Kao wrote:
> On Thu, 2020-01-09 at 12:31 +0100, Daniel Lezcano wrote:
>> On 03/01/2020 07:44, Michael Kao wrote:
>>> From: "michael.kao" <michael.kao@mediatek.com>
>>>
>>> Add thermal zone node to Mediatek MT8183 dts file.
>>>
>>> Signed-off-by: Michael Kao <michael.kao@mediatek.com>
>>> ---
>>>  arch/arm64/boot/dts/mediatek/mt8183.dtsi | 85 ++++++++++++++++++++++++
>>>  1 file changed, 85 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
>>> index 10b32471bc7b..a2793cf3d994 100644
>>> --- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
>>> +++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
>>> @@ -570,6 +570,88 @@
>>>  			status = "disabled";
>>>  		};
>>>  
>>> +		thermal: thermal@1100b000 {
>>> +			#thermal-sensor-cells = <1>;
>>> +			compatible = "mediatek,mt8183-thermal";
>>> +			reg = <0 0x1100b000 0 0x1000>;
>>> +			interrupts = <0 76 IRQ_TYPE_LEVEL_LOW>;
>>
>> What is this interrupt for?
> 
> The interrupts pin is designed in our SoC. But it is not used in our
> upstream thermal code now. There is also add the settings but not use
> for mt8173.dtsi. To align the thermal dtsi format, I follow the past
> experience to add the interrupt settings of this project first.

Assuming the interrupt can be set by the driver to fire when a specified
temperature is set, I suggest to change your driver to handle it so you
can get rid of the polling waking up the SoC every second.
Matthias Brugger Feb. 20, 2020, 8:56 p.m. UTC | #5
On 03/01/2020 07:44, Michael Kao wrote:
> From: "michael.kao" <michael.kao@mediatek.com>
> 
> Add thermal zone node to Mediatek MT8183 dts file.
> 
> Signed-off-by: Michael Kao <michael.kao@mediatek.com>
> ---
>  arch/arm64/boot/dts/mediatek/mt8183.dtsi | 85 ++++++++++++++++++++++++
>  1 file changed, 85 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> index 10b32471bc7b..a2793cf3d994 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> @@ -570,6 +570,88 @@
>  			status = "disabled";
>  		};
>  
> +		thermal: thermal@1100b000 {
> +			#thermal-sensor-cells = <1>;
> +			compatible = "mediatek,mt8183-thermal";
> +			reg = <0 0x1100b000 0 0x1000>;
> +			interrupts = <0 76 IRQ_TYPE_LEVEL_LOW>;
> +			clocks = <&infracfg CLK_INFRA_THERM>,
> +				 <&infracfg CLK_INFRA_AUXADC>;
> +			clock-names = "therm", "auxadc";
> +			resets = <&infracfg  MT8183_INFRACFG_AO_THERM_SW_RST>;
> +			mediatek,auxadc = <&auxadc>;
> +			mediatek,apmixedsys = <&apmixedsys>;
> +			mediatek,hw-reset-temp = <117000>;

Non uptream property, please delte

> +			nvmem-cells = <&thermal_calibration>;
> +			nvmem-cell-names = "calibration-data";
> +		};
> +
> +		thermal-zones {
> +			cpu_thermal: cpu_thermal {
> +				polling-delay-passive = <1000>;
> +				polling-delay = <1000>;
> +				thermal-sensors = <&thermal 0>;
> +				sustainable-power = <5000>;
> +			};
> +
> +			/* The tzts1 ~ tzts6 don't need to polling */
> +			/* The tzts1 ~ tzts6 don't need to thermal throttle */
> +
> +			tzts1: tzts1 {
> +				polling-delay-passive = <0>;
> +				polling-delay = <0>;
> +				thermal-sensors = <&thermal 1>;
> +				sustainable-power = <5000>;
> +				trips {};
> +				cooling-maps {};
> +			};
> +
> +			tzts2: tzts2 {
> +				polling-delay-passive = <0>;
> +				polling-delay = <0>;
> +				thermal-sensors = <&thermal 2>;
> +				sustainable-power = <5000>;
> +				trips {};
> +				cooling-maps {};
> +			};
> +
> +			tzts3: tzts3 {
> +				polling-delay-passive = <0>;
> +				polling-delay = <0>;
> +				thermal-sensors = <&thermal 3>;
> +				sustainable-power = <5000>;
> +				trips {};
> +				cooling-maps {};
> +			};
> +
> +			tzts4: tzts4 {
> +				polling-delay-passive = <0>;
> +				polling-delay = <0>;
> +				thermal-sensors = <&thermal 4>;
> +				sustainable-power = <5000>;
> +				trips {};
> +				cooling-maps {};
> +			};
> +
> +			tzts5: tzts5 {
> +				polling-delay-passive = <0>;
> +				polling-delay = <0>;
> +				thermal-sensors = <&thermal 5>;
> +				sustainable-power = <5000>;
> +				trips {};
> +				cooling-maps {};
> +			};
> +
> +			tztsABB: tztsABB {
> +				polling-delay-passive = <0>;
> +				polling-delay = <0>;
> +				thermal-sensors = <&thermal 6>;
> +				sustainable-power = <5000>;
> +				trips {};
> +				cooling-maps {};
> +			};
> +		};
> +
>  		audiosys: syscon@11220000 {
>  			compatible = "mediatek,mt8183-audiosys", "syscon";
>  			reg = <0 0x11220000 0 0x1000>;
> @@ -580,6 +662,9 @@
>  			compatible = "mediatek,mt8183-efuse",
>  				     "mediatek,efuse";
>  			reg = <0 0x11f10000 0 0x1000>;

New line here please.

> +			thermal_calibration: calib@180 {
> +				reg = <0x180 0xc>;
> +			};
>  		};
>  
>  		mfgcfg: syscon@13000000 {
>
Matthias Brugger Feb. 20, 2020, 8:57 p.m. UTC | #6
On 20/02/2020 12:52, Daniel Lezcano wrote:
> On 11/02/2020 04:17, Michael Kao wrote:
>> On Thu, 2020-01-09 at 12:31 +0100, Daniel Lezcano wrote:
>>> On 03/01/2020 07:44, Michael Kao wrote:
>>>> From: "michael.kao" <michael.kao@mediatek.com>
>>>>
>>>> Add thermal zone node to Mediatek MT8183 dts file.
>>>>
>>>> Signed-off-by: Michael Kao <michael.kao@mediatek.com>
>>>> ---
>>>>  arch/arm64/boot/dts/mediatek/mt8183.dtsi | 85 ++++++++++++++++++++++++
>>>>  1 file changed, 85 insertions(+)
>>>>
>>>> diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
>>>> index 10b32471bc7b..a2793cf3d994 100644
>>>> --- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
>>>> +++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
>>>> @@ -570,6 +570,88 @@
>>>>  			status = "disabled";
>>>>  		};
>>>>  
>>>> +		thermal: thermal@1100b000 {
>>>> +			#thermal-sensor-cells = <1>;
>>>> +			compatible = "mediatek,mt8183-thermal";
>>>> +			reg = <0 0x1100b000 0 0x1000>;
>>>> +			interrupts = <0 76 IRQ_TYPE_LEVEL_LOW>;
>>>
>>> What is this interrupt for?
>>
>> The interrupts pin is designed in our SoC. But it is not used in our
>> upstream thermal code now. There is also add the settings but not use
>> for mt8173.dtsi. To align the thermal dtsi format, I follow the past
>> experience to add the interrupt settings of this project first.
> 
> Assuming the interrupt can be set by the driver to fire when a specified
> temperature is set, I suggest to change your driver to handle it so you
> can get rid of the polling waking up the SoC every second.
> 

For the record the interrupt is a required property by the binding description.

Regards,
Matthias
Matthias Brugger Feb. 20, 2020, 9:06 p.m. UTC | #7
On 11/02/2020 03:05, Michael Kao wrote:
> On Fri, 2020-01-10 at 15:40 +0100, Matthias Brugger wrote:
>> I suppose it need rewording.
> 
> Hi Matthias,
> 
> This patch was resent following with the patch series,Add Mediatek
> thermal driver and dtsi.
> I have write all the changes in the cover letter.
> There is no change in this patch.
> 
> Do you mean that I need to add some word to commit message or
> change the dynamic power coefficients?
> 

Your commit message says:
"Add dynamic power coefficients for all cores and update those of CPU0 and CPU4."

But the power coefficients for CPU0-4 are not updated, but added.

I fixed the commit message and pushed to v5.6-next/dts64

Please double check that everything is correct.

Regards,
Matthias
Matthias Brugger Feb. 20, 2020, 9:59 p.m. UTC | #8
On 03/01/2020 07:44, Michael Kao wrote:
> From: "michael.kao" <michael.kao@mediatek.com>
> 
> The #cooling-cells property needs to be specified to allow a CPU
> to be used as cooling device.
> 
> Signed-off-by: Michael Kao <michael.kao@mediatek.com>

Applied to v5.6-next/dts64

Thanks

> ---
>  arch/arm64/boot/dts/mediatek/mt8183.dtsi | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> index cfb74af260e0..63378ae14a16 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> @@ -9,6 +9,7 @@
>  #include <dt-bindings/interrupt-controller/arm-gic.h>
>  #include <dt-bindings/interrupt-controller/irq.h>
>  #include "mt8183-pinfunc.h"
> +#include <dt-bindings/thermal/thermal.h>
>  
>  / {
>  	compatible = "mediatek,mt8183";
> @@ -74,6 +75,7 @@
>  			enable-method = "psci";
>  			capacity-dmips-mhz = <741>;
>  			dynamic-power-coefficient = <84>;
> +			#cooling-cells = <2>;
>  		};
>  
>  		cpu1: cpu@1 {
> @@ -83,6 +85,7 @@
>  			enable-method = "psci";
>  			capacity-dmips-mhz = <741>;
>  			dynamic-power-coefficient = <84>;
> +			#cooling-cells = <2>;
>  		};
>  
>  		cpu2: cpu@2 {
> @@ -92,6 +95,7 @@
>  			enable-method = "psci";
>  			capacity-dmips-mhz = <741>;
>  			dynamic-power-coefficient = <84>;
> +			#cooling-cells = <2>;
>  		};
>  
>  		cpu3: cpu@3 {
> @@ -101,6 +105,7 @@
>  			enable-method = "psci";
>  			capacity-dmips-mhz = <741>;
>  			dynamic-power-coefficient = <84>;
> +			#cooling-cells = <2>;
>  		};
>  
>  		cpu4: cpu@100 {
> @@ -110,6 +115,7 @@
>  			enable-method = "psci";
>  			capacity-dmips-mhz = <1024>;
>  			dynamic-power-coefficient = <211>;
> +			#cooling-cells = <2>;
>  		};
>  
>  		cpu5: cpu@101 {
> @@ -119,6 +125,7 @@
>  			enable-method = "psci";
>  			capacity-dmips-mhz = <1024>;
>  			dynamic-power-coefficient = <211>;
> +			#cooling-cells = <2>;
>  		};
>  
>  		cpu6: cpu@102 {
> @@ -128,6 +135,7 @@
>  			enable-method = "psci";
>  			capacity-dmips-mhz = <1024>;
>  			dynamic-power-coefficient = <211>;
> +			#cooling-cells = <2>;
>  		};
>  
>  		cpu7: cpu@103 {
> @@ -137,6 +145,7 @@
>  			enable-method = "psci";
>  			capacity-dmips-mhz = <1024>;
>  			dynamic-power-coefficient = <211>;
> +			#cooling-cells = <2>;
>  		};
>  	};
>  
>
Michael Kao Feb. 26, 2020, 1:58 a.m. UTC | #9
On Thu, 2020-02-20 at 21:57 +0100, Matthias Brugger wrote:
> 
> On 20/02/2020 12:52, Daniel Lezcano wrote:
> > On 11/02/2020 04:17, Michael Kao wrote:
> >> On Thu, 2020-01-09 at 12:31 +0100, Daniel Lezcano wrote:
> >>> On 03/01/2020 07:44, Michael Kao wrote:
> >>>> From: "michael.kao" <michael.kao@mediatek.com>
> >>>>
> >>>> Add thermal zone node to Mediatek MT8183 dts file.
> >>>>
> >>>> Signed-off-by: Michael Kao <michael.kao@mediatek.com>
> >>>> ---
> >>>>  arch/arm64/boot/dts/mediatek/mt8183.dtsi | 85 ++++++++++++++++++++++++
> >>>>  1 file changed, 85 insertions(+)
> >>>>
> >>>> diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> >>>> index 10b32471bc7b..a2793cf3d994 100644
> >>>> --- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> >>>> +++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> >>>> @@ -570,6 +570,88 @@
> >>>>  			status = "disabled";
> >>>>  		};
> >>>>  
> >>>> +		thermal: thermal@1100b000 {
> >>>> +			#thermal-sensor-cells = <1>;
> >>>> +			compatible = "mediatek,mt8183-thermal";
> >>>> +			reg = <0 0x1100b000 0 0x1000>;
> >>>> +			interrupts = <0 76 IRQ_TYPE_LEVEL_LOW>;
> >>>
> >>> What is this interrupt for?
> >>
> >> The interrupts pin is designed in our SoC. But it is not used in our
> >> upstream thermal code now. There is also add the settings but not use
> >> for mt8173.dtsi. To align the thermal dtsi format, I follow the past
> >> experience to add the interrupt settings of this project first.
> > 
> > Assuming the interrupt can be set by the driver to fire when a specified
> > temperature is set, I suggest to change your driver to handle it so you
> > can get rid of the polling waking up the SoC every second.
> > 
> 
> For the record the interrupt is a required property by the binding description.
> 
> Regards,
> Matthias

After checking with interrupt owner, it is not required property for
thermal.
I will remove the property of hw-reset-temp and interrupt.
And also I will add new line to fix format.
These three change will resend v4 to fix them.